| Message ID | 20231025093649.25222-1-william.vinnicombe@raspberrypi.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi William, (CC'ing Tomi) Thank you for the patch. On Wed, Oct 25, 2023 at 10:36:49AM +0100, William Vinnicombe via libcamera-devel wrote: > The existing meson.build file installs the bindings to an architecture > specific libdir (eg /usr/local/lib/aarch64-linux-gnu/), which is not > picked up by default python which only looks in the non architecture > specific libdir (eg /usr/local/lib/python3.11/). It also will always > build using the system python, rather than building using the same > python as meson is using. This prevents a user being able to build the > bindings for a different version of python, without changing their > system python to that version. > > Modify the build process to use the meson Python module to build the > python bindings targets, so it installs them to the correct directories > for python, and builds them for the version of python that meson is > running with. > > Signed-off-by: William Vinnicombe <william.vinnicombe@raspberrypi.com> > --- > src/py/libcamera/meson.build | 23 ++++++++++++----------- > 1 file changed, 12 insertions(+), 11 deletions(-) > > diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build > index f58c7198..e9e3f915 100644 > --- a/src/py/libcamera/meson.build > +++ b/src/py/libcamera/meson.build > @@ -1,6 +1,8 @@ > # SPDX-License-Identifier: CC0-1.0 > > -py3_dep = dependency('python3', required : get_option('pycamera')) > + > +py = import('python').find_installation('python3', required : get_option('pycamera')) > +py3_dep = py.dependency(required : get_option('pycamera')) Does this still work when cross-compiling ? I recall we tried the meson python module, and it had problems with cross-compilation. Tomi may know more. > > if not py3_dep.found() > pycamera_enabled = false > @@ -78,15 +80,12 @@ pycamera_args = [ > '-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT', > ] > > -destdir = get_option('libdir') / ('python' + py3_dep.version()) / 'site-packages' / 'libcamera' > - > -pycamera = shared_module('_libcamera', > - pycamera_sources, > - install : true, > - install_dir : destdir, > - name_prefix : '', > - dependencies : pycamera_deps, > - cpp_args : pycamera_args) > +pycamera = py.extension_module('_libcamera', > + pycamera_sources, > + install : true, > + subdir : 'libcamera', > + dependencies : pycamera_deps, > + cpp_args : pycamera_args) > > # Create symlinks from the build dir to the source dir so that we can use the > # Python module directly from the build dir. > @@ -99,7 +98,9 @@ run_command('ln', '-fsrT', meson.current_source_dir() / 'utils', > meson.current_build_dir() / 'utils', > check : true) > > -install_data(['__init__.py'], install_dir : destdir) > +py.install_sources(['__init__.py'], > + subdir : 'libcamera', > + pure : false) > > # \todo Generate stubs when building. See https://peps.python.org/pep-0484/#stub-files > # Note: Depends on pybind11-stubgen. To generate pylibcamera stubs:
On 25/10/2023 14:04, Laurent Pinchart wrote: > Hi William, > > (CC'ing Tomi) > > Thank you for the patch. > > On Wed, Oct 25, 2023 at 10:36:49AM +0100, William Vinnicombe via libcamera-devel wrote: >> The existing meson.build file installs the bindings to an architecture >> specific libdir (eg /usr/local/lib/aarch64-linux-gnu/), which is not >> picked up by default python which only looks in the non architecture >> specific libdir (eg /usr/local/lib/python3.11/). It also will always >> build using the system python, rather than building using the same >> python as meson is using. This prevents a user being able to build the >> bindings for a different version of python, without changing their >> system python to that version. >> >> Modify the build process to use the meson Python module to build the >> python bindings targets, so it installs them to the correct directories >> for python, and builds them for the version of python that meson is >> running with. >> >> Signed-off-by: William Vinnicombe <william.vinnicombe@raspberrypi.com> >> --- >> src/py/libcamera/meson.build | 23 ++++++++++++----------- >> 1 file changed, 12 insertions(+), 11 deletions(-) >> >> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build >> index f58c7198..e9e3f915 100644 >> --- a/src/py/libcamera/meson.build >> +++ b/src/py/libcamera/meson.build >> @@ -1,6 +1,8 @@ >> # SPDX-License-Identifier: CC0-1.0 >> >> -py3_dep = dependency('python3', required : get_option('pycamera')) >> + >> +py = import('python').find_installation('python3', required : get_option('pycamera')) >> +py3_dep = py.dependency(required : get_option('pycamera')) > > Does this still work when cross-compiling ? I recall we tried the meson > python module, and it had problems with cross-compilation. Tomi may know > more. Right. When cross-compiling, you don't want that "[the build process] builds them for the version of python that meson is running with", as meson is running (for me) in x86 environment, but the target is arm. I'm not sure if there's been any work on fixing all that in the meson side, but the last time I talked to them they didn't have any clear ideas how to fix it. Perhaps this can be solved by manually dealing with all the python details in the meson.build file, and adding a meson option so that the user can tell which python to use. Tomi
Hi both, Thanks for your comments - I hadn't considered cross compilation, but from looking into it I can see that this change would cause the problems you've mentioned. Would it be an acceptable solution to wrap these changes within "if meson.is_cross_build() else ..." so that cross compilation uses the previous code still, but native compilation uses the meson python module? If so, I'll submit a v2 with those changes. Thanks, William On Thu, 26 Oct 2023 at 06:39, Tomi Valkeinen < tomi.valkeinen@ideasonboard.com> wrote: > On 25/10/2023 14:04, Laurent Pinchart wrote: > > Hi William, > > > > (CC'ing Tomi) > > > > Thank you for the patch. > > > > On Wed, Oct 25, 2023 at 10:36:49AM +0100, William Vinnicombe via > libcamera-devel wrote: > >> The existing meson.build file installs the bindings to an architecture > >> specific libdir (eg /usr/local/lib/aarch64-linux-gnu/), which is not > >> picked up by default python which only looks in the non architecture > >> specific libdir (eg /usr/local/lib/python3.11/). It also will always > >> build using the system python, rather than building using the same > >> python as meson is using. This prevents a user being able to build the > >> bindings for a different version of python, without changing their > >> system python to that version. > >> > >> Modify the build process to use the meson Python module to build the > >> python bindings targets, so it installs them to the correct directories > >> for python, and builds them for the version of python that meson is > >> running with. > >> > >> Signed-off-by: William Vinnicombe <william.vinnicombe@raspberrypi.com> > >> --- > >> src/py/libcamera/meson.build | 23 ++++++++++++----------- > >> 1 file changed, 12 insertions(+), 11 deletions(-) > >> > >> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build > >> index f58c7198..e9e3f915 100644 > >> --- a/src/py/libcamera/meson.build > >> +++ b/src/py/libcamera/meson.build > >> @@ -1,6 +1,8 @@ > >> # SPDX-License-Identifier: CC0-1.0 > >> > >> -py3_dep = dependency('python3', required : get_option('pycamera')) > >> + > >> +py = import('python').find_installation('python3', required : > get_option('pycamera')) > >> +py3_dep = py.dependency(required : get_option('pycamera')) > > > > Does this still work when cross-compiling ? I recall we tried the meson > > python module, and it had problems with cross-compilation. Tomi may know > > more. > > Right. When cross-compiling, you don't want that "[the build process] > builds them for the version of python that meson is running with", as > meson is running (for me) in x86 environment, but the target is arm. > > I'm not sure if there's been any work on fixing all that in the meson > side, but the last time I talked to them they didn't have any clear > ideas how to fix it. > > Perhaps this can be solved by manually dealing with all the python > details in the meson.build file, and adding a meson option so that the > user can tell which python to use. > > Tomi > >
On 26/10/2023 13:15, William Vinnicombe wrote: > Hi both, > > Thanks for your comments - I hadn't considered cross compilation, but > from looking into it I can see that this change would cause the problems > you've mentioned. Would it be an acceptable solution to wrap these > changes within "if meson.is_cross_build() else ..." so that cross > compilation uses the previous code still, but native compilation uses > the meson python module? If so, I'll submit a v2 with those changes. I don't see why not, presuming it works =). Do you have a cross-compilation environment (e.g. buildroot) for testing? Tomi > > Thanks, > > William > > On Thu, 26 Oct 2023 at 06:39, Tomi Valkeinen > <tomi.valkeinen@ideasonboard.com > <mailto:tomi.valkeinen@ideasonboard.com>> wrote: > > On 25/10/2023 14:04, Laurent Pinchart wrote: > > Hi William, > > > > (CC'ing Tomi) > > > > Thank you for the patch. > > > > On Wed, Oct 25, 2023 at 10:36:49AM +0100, William Vinnicombe via > libcamera-devel wrote: > >> The existing meson.build file installs the bindings to an > architecture > >> specific libdir (eg /usr/local/lib/aarch64-linux-gnu/), which is not > >> picked up by default python which only looks in the non architecture > >> specific libdir (eg /usr/local/lib/python3.11/). It also will always > >> build using the system python, rather than building using the same > >> python as meson is using. This prevents a user being able to > build the > >> bindings for a different version of python, without changing their > >> system python to that version. > >> > >> Modify the build process to use the meson Python module to build the > >> python bindings targets, so it installs them to the correct > directories > >> for python, and builds them for the version of python that meson is > >> running with. > >> > >> Signed-off-by: William Vinnicombe > <william.vinnicombe@raspberrypi.com > <mailto:william.vinnicombe@raspberrypi.com>> > >> --- > >> src/py/libcamera/meson.build | 23 ++++++++++++----------- > >> 1 file changed, 12 insertions(+), 11 deletions(-) > >> > >> diff --git a/src/py/libcamera/meson.build > b/src/py/libcamera/meson.build > >> index f58c7198..e9e3f915 100644 > >> --- a/src/py/libcamera/meson.build > >> +++ b/src/py/libcamera/meson.build > >> @@ -1,6 +1,8 @@ > >> # SPDX-License-Identifier: CC0-1.0 > >> > >> -py3_dep = dependency('python3', required : get_option('pycamera')) > >> + > >> +py = import('python').find_installation('python3', required : > get_option('pycamera')) > >> +py3_dep = py.dependency(required : get_option('pycamera')) > > > > Does this still work when cross-compiling ? I recall we tried the > meson > > python module, and it had problems with cross-compilation. Tomi > may know > > more. > > Right. When cross-compiling, you don't want that "[the build process] > builds them for the version of python that meson is running with", as > meson is running (for me) in x86 environment, but the target is arm. > > I'm not sure if there's been any work on fixing all that in the meson > side, but the last time I talked to them they didn't have any clear > ideas how to fix it. > > Perhaps this can be solved by manually dealing with all the python > details in the meson.build file, and adding a meson option so that the > user can tell which python to use. > > Tomi >
On Thu, 26 Oct 2023 at 11:18, Tomi Valkeinen < tomi.valkeinen@ideasonboard.com> wrote: > On 26/10/2023 13:15, William Vinnicombe wrote: > > Hi both, > > > > Thanks for your comments - I hadn't considered cross compilation, but > > from looking into it I can see that this change would cause the problems > > you've mentioned. Would it be an acceptable solution to wrap these > > changes within "if meson.is_cross_build() else ..." so that cross > > compilation uses the previous code still, but native compilation uses > > the meson python module? If so, I'll submit a v2 with those changes. > > I don't see why not, presuming it works =). Do you have a > cross-compilation environment (e.g. buildroot) for testing? > I have tested my v2 with buildroot and it seems to be working as expected, so I'll push that to the mailing list now. Although probably worth someone else testing it too, as I don't have much experience with cross-compiling. > > Tomi > > > > > Thanks, > > > > William > > > > On Thu, 26 Oct 2023 at 06:39, Tomi Valkeinen > > <tomi.valkeinen@ideasonboard.com > > <mailto:tomi.valkeinen@ideasonboard.com>> wrote: > > > > On 25/10/2023 14:04, Laurent Pinchart wrote: > > > Hi William, > > > > > > (CC'ing Tomi) > > > > > > Thank you for the patch. > > > > > > On Wed, Oct 25, 2023 at 10:36:49AM +0100, William Vinnicombe via > > libcamera-devel wrote: > > >> The existing meson.build file installs the bindings to an > > architecture > > >> specific libdir (eg /usr/local/lib/aarch64-linux-gnu/), which is > not > > >> picked up by default python which only looks in the non > architecture > > >> specific libdir (eg /usr/local/lib/python3.11/). It also will > always > > >> build using the system python, rather than building using the > same > > >> python as meson is using. This prevents a user being able to > > build the > > >> bindings for a different version of python, without changing > their > > >> system python to that version. > > >> > > >> Modify the build process to use the meson Python module to build > the > > >> python bindings targets, so it installs them to the correct > > directories > > >> for python, and builds them for the version of python that meson > is > > >> running with. > > >> > > >> Signed-off-by: William Vinnicombe > > <william.vinnicombe@raspberrypi.com > > <mailto:william.vinnicombe@raspberrypi.com>> > > >> --- > > >> src/py/libcamera/meson.build | 23 ++++++++++++----------- > > >> 1 file changed, 12 insertions(+), 11 deletions(-) > > >> > > >> diff --git a/src/py/libcamera/meson.build > > b/src/py/libcamera/meson.build > > >> index f58c7198..e9e3f915 100644 > > >> --- a/src/py/libcamera/meson.build > > >> +++ b/src/py/libcamera/meson.build > > >> @@ -1,6 +1,8 @@ > > >> # SPDX-License-Identifier: CC0-1.0 > > >> > > >> -py3_dep = dependency('python3', required : > get_option('pycamera')) > > >> + > > >> +py = import('python').find_installation('python3', required : > > get_option('pycamera')) > > >> +py3_dep = py.dependency(required : get_option('pycamera')) > > > > > > Does this still work when cross-compiling ? I recall we tried the > > meson > > > python module, and it had problems with cross-compilation. Tomi > > may know > > > more. > > > > Right. When cross-compiling, you don't want that "[the build process] > > builds them for the version of python that meson is running with", as > > meson is running (for me) in x86 environment, but the target is arm. > > > > I'm not sure if there's been any work on fixing all that in the meson > > side, but the last time I talked to them they didn't have any clear > > ideas how to fix it. > > > > Perhaps this can be solved by manually dealing with all the python > > details in the meson.build file, and adding a meson option so that > the > > user can tell which python to use. > > > > Tomi > > > >
diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build index f58c7198..e9e3f915 100644 --- a/src/py/libcamera/meson.build +++ b/src/py/libcamera/meson.build @@ -1,6 +1,8 @@ # SPDX-License-Identifier: CC0-1.0 -py3_dep = dependency('python3', required : get_option('pycamera')) + +py = import('python').find_installation('python3', required : get_option('pycamera')) +py3_dep = py.dependency(required : get_option('pycamera')) if not py3_dep.found() pycamera_enabled = false @@ -78,15 +80,12 @@ pycamera_args = [ '-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT', ] -destdir = get_option('libdir') / ('python' + py3_dep.version()) / 'site-packages' / 'libcamera' - -pycamera = shared_module('_libcamera', - pycamera_sources, - install : true, - install_dir : destdir, - name_prefix : '', - dependencies : pycamera_deps, - cpp_args : pycamera_args) +pycamera = py.extension_module('_libcamera', + pycamera_sources, + install : true, + subdir : 'libcamera', + dependencies : pycamera_deps, + cpp_args : pycamera_args) # Create symlinks from the build dir to the source dir so that we can use the # Python module directly from the build dir. @@ -99,7 +98,9 @@ run_command('ln', '-fsrT', meson.current_source_dir() / 'utils', meson.current_build_dir() / 'utils', check : true) -install_data(['__init__.py'], install_dir : destdir) +py.install_sources(['__init__.py'], + subdir : 'libcamera', + pure : false) # \todo Generate stubs when building. See https://peps.python.org/pep-0484/#stub-files # Note: Depends on pybind11-stubgen. To generate pylibcamera stubs:
The existing meson.build file installs the bindings to an architecture specific libdir (eg /usr/local/lib/aarch64-linux-gnu/), which is not picked up by default python which only looks in the non architecture specific libdir (eg /usr/local/lib/python3.11/). It also will always build using the system python, rather than building using the same python as meson is using. This prevents a user being able to build the bindings for a different version of python, without changing their system python to that version. Modify the build process to use the meson Python module to build the python bindings targets, so it installs them to the correct directories for python, and builds them for the version of python that meson is running with. Signed-off-by: William Vinnicombe <william.vinnicombe@raspberrypi.com> --- src/py/libcamera/meson.build | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)