Re: [PATCH] bindings: python: fix out-of-source build

[Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>]

On Mon, 16 Jan 2023 at 19:15, <jf@xxxxxxxxxxxxxxxxxxx> wrote:
>
> From: Joerg Faschingbauer <jf@xxxxxxxxxxxxxxxxxxx>
>
> Makefile.am delegates the build of the python extension to its
> setup.py file, which references the extension .c files relative to the
> source dir. This makes it impossible to build in a directory that is
> different from the source directory (for example, for PC and ARM but
> from the same source).
>

Thanks for sparing me the surprise of finding that out when writing
the bitbake recipe. Great catch!

A couple nits below:

> * Modify Makefile.am to pass automake's $(srcdir) into setup.py via
>   GPIOD_SRCDIR environment variable.
> * Modify setup.py to pick up .c files relative from that directory.

Can you leave your Sign-off here?

> ---
>  bindings/python/Makefile.am |  6 ++++--
>  bindings/python/setup.py    | 23 ++++++++++++++---------
>  2 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/bindings/python/Makefile.am b/bindings/python/Makefile.am
> index 3212a8f..6c2f99b 100644
> --- a/bindings/python/Makefile.am
> +++ b/bindings/python/Makefile.am
> @@ -12,13 +12,15 @@ endif
>  all-local:
>         GPIOD_VERSION_STRING=$(VERSION_STR) \
>         GPIOD_WITH_TESTS=$(BUILD_TESTS) \
> -       $(PYTHON) setup.py build_ext --inplace \
> +       GPIOD_SRCDIR=$(srcdir) \
> +       $(PYTHON) $(srcdir)/setup.py build_ext --inplace \
>                 --include-dirs=$(top_srcdir)/include/:$(top_srcdir)/tests/gpiosim/ \
>                 --library-dirs=$(top_builddir)/lib/.libs/:$(top_srcdir)/tests/gpiosim/.libs/
>
>  install-exec-local:
>         GPIOD_WITH_TESTS= \
> -       $(PYTHON) setup.py install --prefix=$(prefix)
> +       GPIOD_SRCDIR=$(srcdir) \
> +       $(PYTHON) $(srcdir)/setup.py install --prefix=$(prefix)
>
>  SUBDIRS = gpiod
>
> diff --git a/bindings/python/setup.py b/bindings/python/setup.py
> index a951069..3ab01e1 100644
> --- a/bindings/python/setup.py
> +++ b/bindings/python/setup.py
> @@ -2,17 +2,22 @@
>  # SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <brgl@xxxxxxxx>
>
>  from os import environ
> +import os.path

Maybe `from os import environ, path`?

>  from setuptools import setup, Extension, find_packages
>
> +srcdir = environ.get('GPIOD_SRCDIR', '.')
> +def src(path):
> +    return os.path.join(srcdir, path)
> +
>  gpiod_ext = Extension(
>      "gpiod._ext",
>      sources=[
> -        "gpiod/ext/chip.c",
> -        "gpiod/ext/common.c",
> -        "gpiod/ext/line-config.c",
> -        "gpiod/ext/line-settings.c",
> -        "gpiod/ext/module.c",
> -        "gpiod/ext/request.c",
> +        src("gpiod/ext/chip.c"),
> +        src("gpiod/ext/common.c"),
> +        src("gpiod/ext/line-config.c"),
> +        src("gpiod/ext/line-settings.c"),
> +        src("gpiod/ext/module.c"),
> +        src("gpiod/ext/request.c"),
>      ],
>      define_macros=[("_GNU_SOURCE", "1")],
>      libraries=["gpiod"],
> @@ -21,7 +26,7 @@ gpiod_ext = Extension(
>
>  gpiosim_ext = Extension(
>      "tests.gpiosim._ext",
> -    sources=["tests/gpiosim/ext.c"],
> +    sources=[src("tests/gpiosim/ext.c")],
>      define_macros=[("_GNU_SOURCE", "1")],
>      libraries=["gpiosim"],
>      extra_compile_args=["-Wall", "-Wextra"],
> @@ -29,7 +34,7 @@ gpiosim_ext = Extension(
>
>  procname_ext = Extension(
>      "tests.procname._ext",
> -    sources=["tests/procname/ext.c"],
> +    sources=[src("tests/procname/ext.c")],
>      define_macros=[("_GNU_SOURCE", "1")],
>      extra_compile_args=["-Wall", "-Wextra"],
>  )
> @@ -39,7 +44,7 @@ if "GPIOD_WITH_TESTS" in environ and environ["GPIOD_WITH_TESTS"] == "1":
>      extensions.append(gpiosim_ext)
>      extensions.append(procname_ext)
>
> -with open("gpiod/version.py", "r") as fd:
> +with open(src("gpiod/version.py"), "r") as fd:
>      exec(fd.read())
>
>  setup(
> --
> 2.38.1
>

Looks good otherwise!

Bart