[libcamera-devel] Added alternative to meson install command, if it fails with --user argument
diff mbox series

Message ID 20210322074056.26330-1-vedantparanjape160201@gmail.com
State Superseded
Headers show
Series
  • [libcamera-devel] Added alternative to meson install command, if it fails with --user argument
Related show

Commit Message

Vedant Paranjape March 22, 2021, 7:40 a.m. UTC 
Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
---
 README.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Sebastian Fricke March 22, 2021, 8:11 a.m. UTC | #1 
Hey Vedant,

Thank you for the patch.

I believe this patch would benefit from a description, that makes clear
why this change is needed. The current version feels like you've hit a
quite specific problem and you provide a quite generic solution to it.

On 22.03.2021 13:10, Vedant Paranjape wrote:
>Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
>---
> README.rst | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/README.rst b/README.rst
>index 1427c714..0bfd39d9 100644
>--- a/README.rst
>+++ b/README.rst
>@@ -56,7 +56,9 @@ Meson Build system: [required]
>
>             pip3 install --user meson
>             pip3 install --user --upgrade meson
>-
>+
>+        If this fails, retry with `pip3 install meson`
>+

This feels a little too broad as there are multiple reasons for this
command to fail. Can you show the error message that you encountered?

> for the libcamera core: [required]
>         python3-yaml python3-ply python3-jinja2
>
>-- 
>2.25.1

Greetings,
Sebastian

>
>_______________________________________________
>libcamera-devel mailing list
>libcamera-devel@lists.libcamera.org
>https://lists.libcamera.org/listinfo/libcamera-devel
Vedant Paranjape March 22, 2021, 8:18 a.m. UTC | #2 
Sure, will do that. Should I add the error message in commit message or the
readme?

On Mon, 22 Mar, 2021, 13:41 Sebastian Fricke, <sebastian.fricke@posteo.net>
wrote:

> Hey Vedant,
>
> Thank you for the patch.
>
> I believe this patch would benefit from a description, that makes clear
> why this change is needed. The current version feels like you've hit a
> quite specific problem and you provide a quite generic solution to it.
>
> On 22.03.2021 13:10, Vedant Paranjape wrote:
> >Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
> >---
> > README.rst | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> >diff --git a/README.rst b/README.rst
> >index 1427c714..0bfd39d9 100644
> >--- a/README.rst
> >+++ b/README.rst
> >@@ -56,7 +56,9 @@ Meson Build system: [required]
> >
> >             pip3 install --user meson
> >             pip3 install --user --upgrade meson
> >-
> >+
> >+        If this fails, retry with `pip3 install meson`
> >+
>
> This feels a little too broad as there are multiple reasons for this
> command to fail. Can you show the error message that you encountered?
>
> > for the libcamera core: [required]
> >         python3-yaml python3-ply python3-jinja2
> >
> >--
> >2.25.1
>
> Greetings,
> Sebastian
>
> >
> >_______________________________________________
> >libcamera-devel mailing list
> >libcamera-devel@lists.libcamera.org
> >https://lists.libcamera.org/listinfo/libcamera-devel
>
Sebastian Fricke March 22, 2021, 8:28 a.m. UTC | #3 
Hey Vedant,

On 22.03.2021 13:48, Vedant Paranjape wrote:
>Sure, will do that. Should I add the error message in commit message or the
>readme?

I think you should place the error message into the commit message so
that it becomes clear which problem is tackled by this patch. But if it
is a very long error message, I would write a short version into the
commit message and the long version into this mail-series.

>
>On Mon, 22 Mar, 2021, 13:41 Sebastian Fricke, <sebastian.fricke@posteo.net>
>wrote:
>
>> Hey Vedant,
>>
>> Thank you for the patch.
>>
>> I believe this patch would benefit from a description, that makes clear
>> why this change is needed. The current version feels like you've hit a
>> quite specific problem and you provide a quite generic solution to it.
>>
>> On 22.03.2021 13:10, Vedant Paranjape wrote:
>> >Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
>> >---
>> > README.rst | 4 +++-
>> > 1 file changed, 3 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/README.rst b/README.rst
>> >index 1427c714..0bfd39d9 100644
>> >--- a/README.rst
>> >+++ b/README.rst
>> >@@ -56,7 +56,9 @@ Meson Build system: [required]
>> >
>> >             pip3 install --user meson
>> >             pip3 install --user --upgrade meson
>> >-
>> >+
>> >+        If this fails, retry with `pip3 install meson`
>> >+
>>
>> This feels a little too broad as there are multiple reasons for this
>> command to fail. Can you show the error message that you encountered?
>>
>> > for the libcamera core: [required]
>> >         python3-yaml python3-ply python3-jinja2
>> >
>> >--
>> >2.25.1
>>
>> Greetings,
>> Sebastian
>>
>> >
>> >_______________________________________________
>> >libcamera-devel mailing list
>> >libcamera-devel@lists.libcamera.org
>> >https://lists.libcamera.org/listinfo/libcamera-devel
>>
Jacopo Mondi March 22, 2021, 8:33 a.m. UTC | #4 
Hello

One nit: please keep the subject line (as close as possible) to the 75
columns limit. I know sometimes it's hard, but in your case a simpler

libcamera: Add alternative meson install command

would be enough.

On Mon, Mar 22, 2021 at 01:48:41PM +0530, Vedant Paranjape wrote:
> Sure, will do that. Should I add the error message in commit message or the
> readme?

I feel this is documenting an issue not related to libcamera but to
the user setup. If installing --user fails, one should go and look why
in general pip3 fails to install in the user directory, that's not
specific to libcamera. Or have I missed something trivial ?

Thanks
  j

>
> On Mon, 22 Mar, 2021, 13:41 Sebastian Fricke, <sebastian.fricke@posteo.net>
> wrote:
>
> > Hey Vedant,
> >
> > Thank you for the patch.
> >
> > I believe this patch would benefit from a description, that makes clear
> > why this change is needed. The current version feels like you've hit a
> > quite specific problem and you provide a quite generic solution to it.
> >
> > On 22.03.2021 13:10, Vedant Paranjape wrote:
> > >Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
> > >---
> > > README.rst | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > >diff --git a/README.rst b/README.rst
> > >index 1427c714..0bfd39d9 100644
> > >--- a/README.rst
> > >+++ b/README.rst
> > >@@ -56,7 +56,9 @@ Meson Build system: [required]
> > >
> > >             pip3 install --user meson
> > >             pip3 install --user --upgrade meson
> > >-
> > >+
> > >+        If this fails, retry with `pip3 install meson`
> > >+
> >
> > This feels a little too broad as there are multiple reasons for this
> > command to fail. Can you show the error message that you encountered?
> >
> > > for the libcamera core: [required]
> > >         python3-yaml python3-ply python3-jinja2
> > >
> > >--
> > >2.25.1
> >
> > Greetings,
> > Sebastian
> >
> > >
> > >_______________________________________________
> > >libcamera-devel mailing list
> > >libcamera-devel@lists.libcamera.org
> > >https://lists.libcamera.org/listinfo/libcamera-devel
> >
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Vedant Paranjape March 22, 2021, 8:35 a.m. UTC | #5 
Ok, I will do that.

On Mon, 22 Mar, 2021, 14:03 Jacopo Mondi, <jacopo@jmondi.org> wrote:

> Hello
>
> One nit: please keep the subject line (as close as possible) to the 75
> columns limit. I know sometimes it's hard, but in your case a simpler
>
> libcamera: Add alternative meson install command
>
> would be enough.
>
> On Mon, Mar 22, 2021 at 01:48:41PM +0530, Vedant Paranjape wrote:
> > Sure, will do that. Should I add the error message in commit message or
> the
> > readme?
>
> I feel this is documenting an issue not related to libcamera but to
> the user setup. If installing --user fails, one should go and look why
> in general pip3 fails to install in the user directory, that's not
> specific to libcamera. Or have I missed something trivial ?
>
> Thanks
>   j
>
> >
> > On Mon, 22 Mar, 2021, 13:41 Sebastian Fricke, <
> sebastian.fricke@posteo.net>
> > wrote:
> >
> > > Hey Vedant,
> > >
> > > Thank you for the patch.
> > >
> > > I believe this patch would benefit from a description, that makes clear
> > > why this change is needed. The current version feels like you've hit a
> > > quite specific problem and you provide a quite generic solution to it.
> > >
> > > On 22.03.2021 13:10, Vedant Paranjape wrote:
> > > >Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
> > > >---
> > > > README.rst | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > >diff --git a/README.rst b/README.rst
> > > >index 1427c714..0bfd39d9 100644
> > > >--- a/README.rst
> > > >+++ b/README.rst
> > > >@@ -56,7 +56,9 @@ Meson Build system: [required]
> > > >
> > > >             pip3 install --user meson
> > > >             pip3 install --user --upgrade meson
> > > >-
> > > >+
> > > >+        If this fails, retry with `pip3 install meson`
> > > >+
> > >
> > > This feels a little too broad as there are multiple reasons for this
> > > command to fail. Can you show the error message that you encountered?
> > >
> > > > for the libcamera core: [required]
> > > >         python3-yaml python3-ply python3-jinja2
> > > >
> > > >--
> > > >2.25.1
> > >
> > > Greetings,
> > > Sebastian
> > >
> > > >
> > > >_______________________________________________
> > > >libcamera-devel mailing list
> > > >libcamera-devel@lists.libcamera.org
> > > >https://lists.libcamera.org/listinfo/libcamera-devel
> > >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
>
Paul Elder March 22, 2021, 9:29 a.m. UTC | #6 
Hello Vedant,

Thank you for the patch.

As Jacopo pointed out, the subject should be more concise. You're also
missing a changelog.

On Mon, Mar 22, 2021 at 01:10:56PM +0530, Vedant Paranjape wrote:
> Signed-off-by: Vedant Paranjape <vedantparanjape160201@gmail.com>
> ---
>  README.rst | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/README.rst b/README.rst
> index 1427c714..0bfd39d9 100644
> --- a/README.rst
> +++ b/README.rst
> @@ -56,7 +56,9 @@ Meson Build system: [required]
>  
>              pip3 install --user meson
>              pip3 install --user --upgrade meson
> -
> +        
> +        If this fails, retry with `pip3 install meson`
> +        

I don't think this is the right place, and I don't think there's enough
information.

This is the dependencies section, so I think your supplemental
information should go before "Dependencies", and maybe after the code
that's after "To fetch the sources, build and install".

iirc the issue was about a potential version mismatch beteween the
version that the root user uses, and the version that your user uses. So
you should outline this issue, and then provide a solution to it. Maybe
recommending not to use --user if meson is already installed
system-wide?


Paul

>  for the libcamera core: [required]
>          python3-yaml python3-ply python3-jinja2
>  
> -- 
> 2.25.1

Patch
diff mbox series 

diff --git a/README.rst b/README.rst
index 1427c714..0bfd39d9 100644
--- a/README.rst
+++ b/README.rst
@@ -56,7 +56,9 @@  Meson Build system: [required]
 
             pip3 install --user meson
             pip3 install --user --upgrade meson
-
+        
+        If this fails, retry with `pip3 install meson`
+        
 for the libcamera core: [required]
         python3-yaml python3-ply python3-jinja2