| Message ID | 1578660103-29974-1-git-send-email-madhavan.krishnan@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Madhavan, Thank you for the patch. On Fri, Jan 10, 2020 at 01:41:43PM +0100, madhavan.krishnan@linaro.org wrote: > From: Madhavan Krishnan <madhavan.krishnan@linaro.org> > > importing python module can provide the exact path > which can be used in desktop build as well as from > any build system > --- > include/libcamera/meson.build | 4 +++- > src/libcamera/meson.build | 4 +++- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > index 99abf06..83525e2 100644 > --- a/include/libcamera/meson.build > +++ b/include/libcamera/meson.build > @@ -21,13 +21,15 @@ include_dir = join_paths(libcamera_include_dir, 'libcamera') > install_headers(libcamera_api, > subdir : include_dir) > > +python_mod = import('python3').find_python() > + Strictly speaking, the return value of find_python() isn't a module but an external_program object. I would thus rename python_mod to python. Furthermore, as python is used in multiple directories, I think we should get the python installation in the top-level meson.build file instead of duplicating here and in src/libcamera/meson.build. You can do so right after the add_project_link_arguments() call there. Finally, the python3 module is deprecated and replaced with the python module in meson 0.46. We're currently based on meson 0.41, so we should keep using the python3 module, but we will soon bump the minimum required version to 0.47. I will then update this to use the new python module. Not change is required now. The rest of the patch looks good to me. Could you submit a v2 with the above modifications ? > gen_controls = files('../../src/libcamera/gen-controls.py') > > control_ids_h = custom_target('control_ids_h', > input : files('../../src/libcamera/control_ids.yaml', 'control_ids.h.in'), > output : 'control_ids.h', > depend_files : gen_controls, > - command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'], > + command : [python_mod, gen_controls, '-o', '@OUTPUT@', '@INPUT@'], > install : true, > install_dir : join_paths('include', include_dir)) > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index c4f965b..243935b 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -65,13 +65,15 @@ if libudev.found() > ]) > endif > > +python_mod = import('python3').find_python() > + > gen_controls = files('gen-controls.py') > > control_ids_cpp = custom_target('control_ids_cpp', > input : files('control_ids.yaml', 'control_ids.cpp.in'), > output : 'control_ids.cpp', > depend_files : gen_controls, > - command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@']) > + command : [python_mod, gen_controls, '-o', '@OUTPUT@', '@INPUT@']) > > libcamera_sources += control_ids_cpp > libcamera_sources += control_ids_h
Le ven. 10 janv. 2020 08 h 05, <madhavan.krishnan@linaro.org> a écrit : > From: Madhavan Krishnan <madhavan.krishnan@linaro.org> > > importing python module can provide the exact path > which can be used in desktop build as well as from > any build system > Are you certain this is the right solution ? There is absolutely no other meson project using this hack to call python script. I notice the shebang isn't pythonic (not using env), could be that. I just don't believe such hack is required. It's definately not in meson style. --- > include/libcamera/meson.build | 4 +++- > src/libcamera/meson.build | 4 +++- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > index 99abf06..83525e2 100644 > --- a/include/libcamera/meson.build > +++ b/include/libcamera/meson.build > @@ -21,13 +21,15 @@ include_dir = join_paths(libcamera_include_dir, > 'libcamera') > install_headers(libcamera_api, > subdir : include_dir) > > +python_mod = import('python3').find_python() > + > gen_controls = files('../../src/libcamera/gen-controls.py') > > control_ids_h = custom_target('control_ids_h', > input : > files('../../src/libcamera/control_ids.yaml', 'control_ids.h.in'), > output : 'control_ids.h', > depend_files : gen_controls, > - command : [gen_controls, '-o', '@OUTPUT@', > '@INPUT@'], > + command : [python_mod, gen_controls, '-o', > '@OUTPUT@', '@INPUT@'], > install : true, > install_dir : join_paths('include', > include_dir)) > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index c4f965b..243935b 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -65,13 +65,15 @@ if libudev.found() > ]) > endif > > +python_mod = import('python3').find_python() > + > gen_controls = files('gen-controls.py') > > control_ids_cpp = custom_target('control_ids_cpp', > input : files('control_ids.yaml', ' > control_ids.cpp.in'), > output : 'control_ids.cpp', > depend_files : gen_controls, > - command : [gen_controls, '-o', '@OUTPUT@', > '@INPUT@']) > + command : [python_mod, gen_controls, > '-o', '@OUTPUT@', '@INPUT@']) > > libcamera_sources += control_ids_cpp > libcamera_sources += control_ids_h > -- > 2.7.4 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel >
Hi Nicolas, On Sat, Jan 11, 2020 at 08:44:17AM -0500, Nicolas Dufresne wrote: > Le ven. 10 janv. 2020 08 h 05, <madhavan.krishnan@linaro.org> a écrit : > > From: Madhavan Krishnan <madhavan.krishnan@linaro.org> > > > > importing python module can provide the exact path > > which can be used in desktop build as well as from > > any build system > > Are you certain this is the right solution ? There is absolutely no other meson > project using this hack to call python script. I notice the shebang isn't > pythonic (not using env), could be that. I just don't believe such hack is > required. It's definately not in meson style. That's a good point. Madhavan, would the following patch fix your problem ? Nicolas, any objection against the Suggested-by line below ? commit 169165b9b00849d30774f2f1cffbdf49b934a717 Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Date: Sat Jan 11 22:01:41 2020 +0200 libcamera: gen-controls.py: Don't hardcode path to python interpreter The gen-controls.py script hardcodes the path to the python interpreter to /usr/bin/python3 in the first line of the script. This hardcodes usage of the host python3, even when building in cross-compilation environments that may ship their own version of python. Fix it by setting the interpreter to '/usr/bin/env python3'. Reported-by: Madhavan Krishnan <madhavan.krishnan@linaro.org> Suggested-by: Nicolas Dufresne <nicolas@ndufresne.ca> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> diff --git a/src/libcamera/gen-controls.py b/src/libcamera/gen-controls.py index 940386cc68c8..2e5ac5c81cca 100755 --- a/src/libcamera/gen-controls.py +++ b/src/libcamera/gen-controls.py @@ -1,4 +1,4 @@ -#!/usr/bin/python3 +#!/usr/bin/env python3 # SPDX-License-Identifier: GPL-2.0-or-later # Copyright (C) 2019, Google Inc. # > > --- > > include/libcamera/meson.build | 4 +++- > > src/libcamera/meson.build | 4 +++- > > 2 files changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build > > index 99abf06..83525e2 100644 > > --- a/include/libcamera/meson.build > > +++ b/include/libcamera/meson.build > > @@ -21,13 +21,15 @@ include_dir = join_paths(libcamera_include_dir, > > 'libcamera') > > install_headers(libcamera_api, > > subdir : include_dir) > > > > +python_mod = import('python3').find_python() > > + > > gen_controls = files('../../src/libcamera/gen-controls.py') > > > > control_ids_h = custom_target('control_ids_h', > > input : files('../../src/libcamera/ > > control_ids.yaml', 'control_ids.h.in'), > > output : 'control_ids.h', > > depend_files : gen_controls, > > - command : [gen_controls, '-o', '@OUTPUT@', > > '@INPUT@'], > > + command : [python_mod, gen_controls, '-o', > > '@OUTPUT@', '@INPUT@'], > > install : true, > > install_dir : join_paths('include', > > include_dir)) > > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > index c4f965b..243935b 100644 > > --- a/src/libcamera/meson.build > > +++ b/src/libcamera/meson.build > > @@ -65,13 +65,15 @@ if libudev.found() > > ]) > > endif > > > > +python_mod = import('python3').find_python() > > + > > gen_controls = files('gen-controls.py') > > > > control_ids_cpp = custom_target('control_ids_cpp', > > input : files('control_ids.yaml', ' > > control_ids.cpp.in'), > > output : 'control_ids.cpp', > > depend_files : gen_controls, > > - command : [gen_controls, '-o', '@OUTPUT@', > > '@INPUT@']) > > + command : [python_mod, gen_controls, '-o', > > '@OUTPUT@', '@INPUT@']) > > > > libcamera_sources += control_ids_cpp > > libcamera_sources += control_ids_h
Hi Laurent, The issue I was facing get fixed with the following changes mentioned by you. -#!/usr/bin/python3 +#!/usr/bin/env python3 Thanks, for the update and immediate response. And we will be using this patch for our build system. Best Regards Madhavan K ---------- Forwarded message --------- > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Date: Sun, 12 Jan 2020 at 01:37 > Subject: Re: [libcamera-devel] [PATCH] meson: import python3 to use also > from sysroot > To: Nicolas Dufresne <nicolas@ndufresne.ca> > Cc: <madhavan.krishnan@linaro.org>, <libcamera-devel@lists.libcamera.org> > > > Hi Nicolas, > > On Sat, Jan 11, 2020 at 08:44:17AM -0500, Nicolas Dufresne wrote: > > Le ven. 10 janv. 2020 08 h 05, <madhavan.krishnan@linaro.org> a écrit : > > > From: Madhavan Krishnan <madhavan.krishnan@linaro.org> > > > > > > importing python module can provide the exact path > > > which can be used in desktop build as well as from > > > any build system > > > > Are you certain this is the right solution ? There is absolutely no > other meson > > project using this hack to call python script. I notice the shebang isn't > > pythonic (not using env), could be that. I just don't believe such hack > is > > required. It's definately not in meson style. > > That's a good point. Madhavan, would the following patch fix your > problem ? Nicolas, any objection against the Suggested-by line below ? > > commit 169165b9b00849d30774f2f1cffbdf49b934a717 > Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Date: Sat Jan 11 22:01:41 2020 +0200 > > libcamera: gen-controls.py: Don't hardcode path to python interpreter > > The gen-controls.py script hardcodes the path to the python interpreter > to /usr/bin/python3 in the first line of the script. This hardcodes > usage of the host python3, even when building in cross-compilation > environments that may ship their own version of python. Fix it by > setting the interpreter to '/usr/bin/env python3'. > > Reported-by: Madhavan Krishnan <madhavan.krishnan@linaro.org> > Suggested-by: Nicolas Dufresne <nicolas@ndufresne.ca> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > diff --git a/src/libcamera/gen-controls.py b/src/libcamera/gen-controls.py > index 940386cc68c8..2e5ac5c81cca 100755 > --- a/src/libcamera/gen-controls.py > +++ b/src/libcamera/gen-controls.py > @@ -1,4 +1,4 @@ > -#!/usr/bin/python3 > +#!/usr/bin/env python3 > # SPDX-License-Identifier: GPL-2.0-or-later > # Copyright (C) 2019, Google Inc. > # > > > > --- > > > include/libcamera/meson.build | 4 +++- > > > src/libcamera/meson.build | 4 +++- > > > 2 files changed, 6 insertions(+), 2 deletions(-) > > > > > > diff --git a/include/libcamera/meson.build > b/include/libcamera/meson.build > > > index 99abf06..83525e2 100644 > > > --- a/include/libcamera/meson.build > > > +++ b/include/libcamera/meson.build > > > @@ -21,13 +21,15 @@ include_dir = join_paths(libcamera_include_dir, > > > 'libcamera') > > > install_headers(libcamera_api, > > > subdir : include_dir) > > > > > > +python_mod = import('python3').find_python() > > > + > > > gen_controls = files('../../src/libcamera/gen-controls.py') > > > > > > control_ids_h = custom_target('control_ids_h', > > > input : files('../../src/libcamera/ > > > control_ids.yaml', 'control_ids.h.in'), > > > output : 'control_ids.h', > > > depend_files : gen_controls, > > > - command : [gen_controls, '-o', '@OUTPUT@ > ', > > > '@INPUT@'], > > > + command : [python_mod, gen_controls, > '-o', > > > '@OUTPUT@', '@INPUT@'], > > > install : true, > > > install_dir : join_paths('include', > > > include_dir)) > > > > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > > index c4f965b..243935b 100644 > > > --- a/src/libcamera/meson.build > > > +++ b/src/libcamera/meson.build > > > @@ -65,13 +65,15 @@ if libudev.found() > > > ]) > > > endif > > > > > > +python_mod = import('python3').find_python() > > > + > > > gen_controls = files('gen-controls.py') > > > > > > control_ids_cpp = custom_target('control_ids_cpp', > > > input : files('control_ids.yaml', ' > > > control_ids.cpp.in'), > > > output : 'control_ids.cpp', > > > depend_files : gen_controls, > > > - command : [gen_controls, '-o', > '@OUTPUT@', > > > '@INPUT@']) > > > + command : [python_mod, gen_controls, > '-o', > > > '@OUTPUT@', '@INPUT@']) > > > > > > libcamera_sources += control_ids_cpp > > > libcamera_sources += control_ids_h > > -- > Regards, > > Laurent Pinchart >
Hi Madhavan, On Tue, Jan 14, 2020 at 04:00:36PM +0530, Madhavan Krishnan wrote: > Hi Laurent, > > The issue I was facing get fixed with the following changes mentioned by you. > > -#!/usr/bin/python3 > +#!/usr/bin/env python3 > > Thanks, for the update and immediate response. And we will be using this patch > for our build system. You're welcome. Can I add your Tested-by: line to the patch ?
Le sam. 11 janv. 2020 15 h 07, Laurent Pinchart < laurent.pinchart@ideasonboard.com> a écrit : > Hi Nicolas, > > On Sat, Jan 11, 2020 at 08:44:17AM -0500, Nicolas Dufresne wrote: > > Le ven. 10 janv. 2020 08 h 05, <madhavan.krishnan@linaro.org> a écrit : > > > From: Madhavan Krishnan <madhavan.krishnan@linaro.org> > > > > > > importing python module can provide the exact path > > > which can be used in desktop build as well as from > > > any build system > > > > Are you certain this is the right solution ? There is absolutely no > other meson > > project using this hack to call python script. I notice the shebang isn't > > pythonic (not using env), could be that. I just don't believe such hack > is > > required. It's definately not in meson style. > > That's a good point. Madhavan, would the following patch fix your > problem ? Nicolas, any objection against the Suggested-by line below ? > Good with me. Glad I could help. > commit 169165b9b00849d30774f2f1cffbdf49b934a717 > Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Date: Sat Jan 11 22:01:41 2020 +0200 > > libcamera: gen-controls.py: Don't hardcode path to python interpreter > > The gen-controls.py script hardcodes the path to the python interpreter > to /usr/bin/python3 in the first line of the script. This hardcodes > usage of the host python3, even when building in cross-compilation > environments that may ship their own version of python. Fix it by > setting the interpreter to '/usr/bin/env python3'. > > Reported-by: Madhavan Krishnan <madhavan.krishnan@linaro.org> > Suggested-by: Nicolas Dufresne <nicolas@ndufresne.ca> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > diff --git a/src/libcamera/gen-controls.py b/src/libcamera/gen-controls.py > index 940386cc68c8..2e5ac5c81cca 100755 > --- a/src/libcamera/gen-controls.py > +++ b/src/libcamera/gen-controls.py > @@ -1,4 +1,4 @@ > -#!/usr/bin/python3 > +#!/usr/bin/env python3 > # SPDX-License-Identifier: GPL-2.0-or-later > # Copyright (C) 2019, Google Inc. > # > > > > --- > > > include/libcamera/meson.build | 4 +++- > > > src/libcamera/meson.build | 4 +++- > > > 2 files changed, 6 insertions(+), 2 deletions(-) > > > > > > diff --git a/include/libcamera/meson.build > b/include/libcamera/meson.build > > > index 99abf06..83525e2 100644 > > > --- a/include/libcamera/meson.build > > > +++ b/include/libcamera/meson.build > > > @@ -21,13 +21,15 @@ include_dir = join_paths(libcamera_include_dir, > > > 'libcamera') > > > install_headers(libcamera_api, > > > subdir : include_dir) > > > > > > +python_mod = import('python3').find_python() > > > + > > > gen_controls = files('../../src/libcamera/gen-controls.py') > > > > > > control_ids_h = custom_target('control_ids_h', > > > input : files('../../src/libcamera/ > > > control_ids.yaml', 'control_ids.h.in'), > > > output : 'control_ids.h', > > > depend_files : gen_controls, > > > - command : [gen_controls, '-o', '@OUTPUT@ > ', > > > '@INPUT@'], > > > + command : [python_mod, gen_controls, > '-o', > > > '@OUTPUT@', '@INPUT@'], > > > install : true, > > > install_dir : join_paths('include', > > > include_dir)) > > > > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > > index c4f965b..243935b 100644 > > > --- a/src/libcamera/meson.build > > > +++ b/src/libcamera/meson.build > > > @@ -65,13 +65,15 @@ if libudev.found() > > > ]) > > > endif > > > > > > +python_mod = import('python3').find_python() > > > + > > > gen_controls = files('gen-controls.py') > > > > > > control_ids_cpp = custom_target('control_ids_cpp', > > > input : files('control_ids.yaml', ' > > > control_ids.cpp.in'), > > > output : 'control_ids.cpp', > > > depend_files : gen_controls, > > > - command : [gen_controls, '-o', > '@OUTPUT@', > > > '@INPUT@']) > > > + command : [python_mod, gen_controls, > '-o', > > > '@OUTPUT@', '@INPUT@']) > > > > > > libcamera_sources += control_ids_cpp > > > libcamera_sources += control_ids_h > > -- > Regards, > > Laurent Pinchart >
diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build index 99abf06..83525e2 100644 --- a/include/libcamera/meson.build +++ b/include/libcamera/meson.build @@ -21,13 +21,15 @@ include_dir = join_paths(libcamera_include_dir, 'libcamera') install_headers(libcamera_api, subdir : include_dir) +python_mod = import('python3').find_python() + gen_controls = files('../../src/libcamera/gen-controls.py') control_ids_h = custom_target('control_ids_h', input : files('../../src/libcamera/control_ids.yaml', 'control_ids.h.in'), output : 'control_ids.h', depend_files : gen_controls, - command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@'], + command : [python_mod, gen_controls, '-o', '@OUTPUT@', '@INPUT@'], install : true, install_dir : join_paths('include', include_dir)) diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index c4f965b..243935b 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -65,13 +65,15 @@ if libudev.found() ]) endif +python_mod = import('python3').find_python() + gen_controls = files('gen-controls.py') control_ids_cpp = custom_target('control_ids_cpp', input : files('control_ids.yaml', 'control_ids.cpp.in'), output : 'control_ids.cpp', depend_files : gen_controls, - command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@']) + command : [python_mod, gen_controls, '-o', '@OUTPUT@', '@INPUT@']) libcamera_sources += control_ids_cpp libcamera_sources += control_ids_h
From: Madhavan Krishnan <madhavan.krishnan@linaro.org> importing python module can provide the exact path which can be used in desktop build as well as from any build system --- include/libcamera/meson.build | 4 +++- src/libcamera/meson.build | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-)