[libcamera-devel,v2] py: libcamera: Improve python binding installation

19186 diff mbox series

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. For cross-compiling, still use the previous method to
build the bindings, as the host machine version of python should be
used instead.

Signed-off-by: William Vinnicombe <william.vinnicombe@raspberrypi.com>
---
 src/py/libcamera/meson.build | 60 +++++++++++++++++++++++++++---------
 1 file changed, 45 insertions(+), 15 deletions(-)

Could I request some reviews on this V2 of my python patch? It should now
work with cross-compilation, as it still uses the old method for building
the bindings when cross-compiling.

Thanks,
William

On Thu, 26 Oct 2023 at 13:52, William Vinnicombe <
william.vinnicombe@raspberrypi.com> 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. For cross-compiling, still use the previous method to
> build the bindings, as the host machine version of python should be
> used instead.
>
> Signed-off-by: William Vinnicombe <william.vinnicombe@raspberrypi.com>
> ---
>  src/py/libcamera/meson.build | 60 +++++++++++++++++++++++++++---------
>  1 file changed, 45 insertions(+), 15 deletions(-)
>
> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
> index f58c7198..128793aa 100644
> --- a/src/py/libcamera/meson.build
> +++ b/src/py/libcamera/meson.build
> @@ -1,10 +1,25 @@
>  # SPDX-License-Identifier: CC0-1.0
>
> -py3_dep = dependency('python3', required : get_option('pycamera'))
> -
> -if not py3_dep.found()
> -    pycamera_enabled = false
> -    subdir_done()
> +if meson.is_cross_build()
> +    py3_dep = dependency('python3', required : get_option('pycamera'))
> +
> +    if not py3_dep.found()
> +        pycamera_enabled = false
> +        subdir_done()
> +    endif
> +else
> +    py = import('python').find_installation('python3', required :
> get_option('pycamera'))
> +
> +    if not py.found()
> +        pycamera_enabled = false
> +        subdir_done()
> +    else
> +        py3_dep = py.dependency(required : get_option('pycamera'))
> +        if not py3_dep.found()
> +            pycamera_enabled = false
> +            subdir_done()
> +        endif
> +    endif
>  endif
>
>  pybind11_dep = dependency('pybind11', required : get_option('pycamera'))
> @@ -78,15 +93,24 @@ 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)
> +if meson.is_cross_build()
> +    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)
> +else
> +    pycamera = py.extension_module('_libcamera',
> +                                   pycamera_sources,
> +                                   install : true,
> +                                   subdir : 'libcamera',
> +                                   dependencies : pycamera_deps,
> +                                   cpp_args : pycamera_args)
> +endif
>
>  # 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 +123,13 @@ run_command('ln', '-fsrT', meson.current_source_dir()
> / 'utils',
>              meson.current_build_dir() / 'utils',
>              check : true)
>
> -install_data(['__init__.py'], install_dir : destdir)
> +if meson.is_cross_build()
> +    install_data(['__init__.py'], install_dir : destdir)
> +else
> +    py.install_sources(['__init__.py'],
> +                       subdir : 'libcamera',
> +                       pure : false)
> +endif
>
>  # \todo Generate stubs when building. See
> https://peps.python.org/pep-0484/#stub-files
>  # Note: Depends on pybind11-stubgen. To generate pylibcamera stubs:
> --
> 2.39.2
>
>

Hi,

On 26/10/2023 15:50, 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

For me, in buildroot, it would be "/usr/lib/python3.11/site-packages", 
so no arch part. On my PC, though, I get 
"/usr/lib/x86_64-linux-gnu/python3.10/site-packages/libcamera", which is 
not in the python search path. However, even if I did drop the arch 
part, it wouldn't be right for my PC as the python system search paths 
don't contain "site-packages", but "dist-packages": 
"/usr/lib/python3.10/dist-packages". Although there _is_ "site-packages" 
in the path, but for the user specific pip path: 
"/home/tomba/.local/lib/python3.10/site-packages"...

> 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

Well... I don't know which one of those is better or worse =). And it's 
not system python (if that means the build system), it's the target python.

I don't think it's a good idea to always use the same python as meson is 
using, and for cross-compiling that's totally wrong, of course. It 
should be perfectly fine for meson to always use the normal system 
python, while targeting some specific python version.

In your version, shouldn't the import('python').find_installation() call 
be configurable, so that the user would tell meson where to look for the 
python installation? The docs say find_installation() can take e.g. a 
path to the python.

> bindings for a different version of python, without changing their
> system python to that version.

How do you change the python version with your patch?

> 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. For cross-compiling, still use the previous method to
> build the bindings, as the host machine version of python should be
> used instead.

To clarify, if using your version (i.e. the "official" meson version) 
for finding python, the build goes totally bonkers as meson starts 
mixing up the build system's and target's libraries. I think it was less 
bonkers earlier, but now it looks to be totally broken (probably related 
to a meson bug Laurent found recently).

In any case, this patch keeps the cross-compilation working for me, and 
a native build on my PC works too. So it's ugly, but, afaics, works.

You could add some comments to the meson.build file, to explain why all 
this is being done.

  Tomi

> Signed-off-by: William Vinnicombe <william.vinnicombe@raspberrypi.com>
> ---
>   src/py/libcamera/meson.build | 60 +++++++++++++++++++++++++++---------
>   1 file changed, 45 insertions(+), 15 deletions(-)
> 
> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
> index f58c7198..128793aa 100644
> --- a/src/py/libcamera/meson.build
> +++ b/src/py/libcamera/meson.build
> @@ -1,10 +1,25 @@
>   # SPDX-License-Identifier: CC0-1.0
>   
> -py3_dep = dependency('python3', required : get_option('pycamera'))
> -
> -if not py3_dep.found()
> -    pycamera_enabled = false
> -    subdir_done()
> +if meson.is_cross_build()
> +    py3_dep = dependency('python3', required : get_option('pycamera'))
> +
> +    if not py3_dep.found()
> +        pycamera_enabled = false
> +        subdir_done()
> +    endif
> +else
> +    py = import('python').find_installation('python3', required : get_option('pycamera'))
> +
> +    if not py.found()
> +        pycamera_enabled = false
> +        subdir_done()
> +    else
> +        py3_dep = py.dependency(required : get_option('pycamera'))
> +        if not py3_dep.found()
> +            pycamera_enabled = false
> +            subdir_done()
> +        endif
> +    endif
>   endif
>   
>   pybind11_dep = dependency('pybind11', required : get_option('pycamera'))
> @@ -78,15 +93,24 @@ 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)
> +if meson.is_cross_build()
> +    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)
> +else
> +    pycamera = py.extension_module('_libcamera',
> +                                   pycamera_sources,
> +                                   install : true,
> +                                   subdir : 'libcamera',
> +                                   dependencies : pycamera_deps,
> +                                   cpp_args : pycamera_args)
> +endif
>   
>   # 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 +123,13 @@ run_command('ln', '-fsrT', meson.current_source_dir() / 'utils',
>               meson.current_build_dir() / 'utils',
>               check : true)
>   
> -install_data(['__init__.py'], install_dir : destdir)
> +if meson.is_cross_build()
> +    install_data(['__init__.py'], install_dir : destdir)
> +else
> +    py.install_sources(['__init__.py'],
> +                       subdir : 'libcamera',
> +                       pure : false)
> +endif
>   
>   # \todo Generate stubs when building. See https://peps.python.org/pep-0484/#stub-files
>   # Note: Depends on pybind11-stubgen. To generate pylibcamera stubs: