Re: [libgpiod][PATCH v3 1/1] bindings: python: optionally include module in sdist

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Fri, Oct 13, 2023 at 1:28 PM Phil Howard <phil@xxxxxxxxxxxxx> wrote:
>
> Build libgpiod into Python module for build_ext or bdist_wheel.
>
> Include libgpiod source in sdist so that the Python module
> can be built from source by end users, even with a missing
> or mismatched system libgpiod.
>
> Add optional environment variable "LINK_SYSTEM_LIBGPIOD=1" to
> generate a module via build_ext or bdist_wheel that links
> against system libgpiod.
>
> Update build to pass "GPIOD_VERSION_STR" as an environment
> variable when calling setup.py. This is saved to
> "gpiod-version-str.txt" and included in the sdist for
> standalone builds.
>
> The old make/make install behaviour is preserved by
> supplying "LINK_SYSTEM_LIBGPIOD=1" and an additional sdist
> package is built and saved into dist/ for upload to pypi.
>
> Signed-off-by: Phil Howard <phil@xxxxxxxxxxxxx>
> ---
>  bindings/python/MANIFEST.in |   5 ++
>  bindings/python/Makefile.am |   5 ++
>  bindings/python/setup.py    | 107 ++++++++++++++++++++++++++++++------
>  3 files changed, 101 insertions(+), 16 deletions(-)
>
> diff --git a/bindings/python/MANIFEST.in b/bindings/python/MANIFEST.in
> index c7124d4..acf9391 100644
> --- a/bindings/python/MANIFEST.in
> +++ b/bindings/python/MANIFEST.in
> @@ -2,6 +2,7 @@
>  # SPDX-FileCopyrightText: 2023 Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
>
>  include setup.py
> +include gpiod-version-str.txt
>
>  recursive-include gpiod *.py
>  recursive-include tests *.py
> @@ -11,3 +12,7 @@ recursive-include gpiod/ext *.h
>
>  recursive-include tests/gpiosim *.c
>  recursive-include tests/procname *.c
> +
> +recursive-include lib *.c
> +recursive-include lib *.h
> +recursive-include include *.h
> diff --git a/bindings/python/Makefile.am b/bindings/python/Makefile.am
> index 079ceb1..fda8f94 100644
> --- a/bindings/python/Makefile.am
> +++ b/bindings/python/Makefile.am
> @@ -12,10 +12,15 @@ BUILD_TESTS = 1
>  endif
>
>  all-local:
> +       GPIOD_VERSION_STR=$(VERSION_STR) \
>         GPIOD_WITH_TESTS=$(BUILD_TESTS) \
> +       LINK_SYSTEM_LIBGPIOD=1 \
>         $(PYTHON) 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/
> +       GPIOD_VERSION_STR=$(VERSION_STR) \
> +       $(PYTHON) setup.py sdist
> +
>
>  install-exec-local:
>         GPIOD_WITH_TESTS= \
> diff --git a/bindings/python/setup.py b/bindings/python/setup.py
> index df10e18..878c38d 100644
> --- a/bindings/python/setup.py
> +++ b/bindings/python/setup.py
> @@ -1,10 +1,43 @@
>  # SPDX-License-Identifier: GPL-2.0-or-later
>  # SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <brgl@xxxxxxxx>
>
> -from os import environ, path
> +from os import environ, path, unlink
>  from setuptools import setup, Extension, find_packages
>  from setuptools.command.build_ext import build_ext as orig_build_ext
> -from shutil import rmtree
> +from setuptools.command.sdist import sdist as orig_sdist
> +from shutil import rmtree, copytree
> +
> +
> +def get_gpiod_version_str():
> +    try:
> +        return environ["GPIOD_VERSION_STR"]

Would it be possible - in order to keep the default behavior really
backward compatible - to make this optional, and if it wasn't passed
then we don't allow built-in libgpiod packaging?

This way, the yocto recipe wouldn't require any changes at all, which
would be preferable.

Bart

> +    except KeyError:
> +        return open("gpiod-version-str.txt", "r").read()
> +
> +
> +def copy_libgpiod_files(func):
> +    """
> +    In order to include the lib and include directories in the sdist
> +    we must temporarily copy them up into the python bindings directory.
> +
> +    If "./lib" exists we are building from an sdist package and will not
> +    try to copy the files again.
> +    """
> +
> +    def wrapper(self):
> +        copy_src = not path.exists("./lib")
> +        if copy_src:
> +            gpiod_version_str = get_gpiod_version_str()
> +            open("gpiod-version-str.txt", "w").write(gpiod_version_str)
> +            copytree("../../lib", "./lib")
> +            copytree("../../include", "./include")
> +        func(self)
> +        if copy_src:
> +            unlink("gpiod-version-str.txt")
> +            rmtree("./lib")
> +            rmtree("./include")
> +
> +    return wrapper
>
>
>  class build_ext(orig_build_ext):
> @@ -14,24 +47,69 @@ class build_ext(orig_build_ext):
>      were built (and possibly copied to the source directory if inplace is set).
>      """
>
> +    @copy_libgpiod_files
>      def run(self):
>          super().run()
>          rmtree(path.join(self.build_lib, "tests"), ignore_errors=True)
>
>
> +class sdist(orig_sdist):
> +    """
> +    Wrap sdist so that we can copy the lib and include files into . where
> +    MANIFEST.in will include them in the source package.
> +    """
> +
> +    @copy_libgpiod_files
> +    def run(self):
> +        super().run()
> +
> +
> +with open("gpiod/version.py", "r") as fd:
> +    exec(fd.read())
> +
> +sources = [
> +    # gpiod Python bindings
> +    "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",
> +]
> +
> +if environ.get("LINK_SYSTEM_LIBGPIOD") == "1":
> +    libraries = ["gpiod"]
> +    include_dirs = ["gpiod"]
> +else:
> +    sources += [
> +        # gpiod library
> +        "lib/chip.c",
> +        "lib/chip-info.c",
> +        "lib/edge-event.c",
> +        "lib/info-event.c",
> +        "lib/internal.c",
> +        "lib/line-config.c",
> +        "lib/line-info.c",
> +        "lib/line-request.c",
> +        "lib/line-settings.c",
> +        "lib/misc.c",
> +        "lib/request-config.c",
> +    ]
> +    libraries = []
> +    include_dirs = ["include", "lib", "gpiod/ext"]
> +
> +
>  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",
> -    ],
> +    libraries=libraries,
> +    sources=sources,
>      define_macros=[("_GNU_SOURCE", "1")],
> -    libraries=["gpiod"],
> -    extra_compile_args=["-Wall", "-Wextra"],
> +    include_dirs=include_dirs,
> +    extra_compile_args=[
> +        "-Wall",
> +        "-Wextra",
> +        '-DGPIOD_VERSION_STR="{}"'.format(get_gpiod_version_str()),
> +    ],
>  )
>
>  gpiosim_ext = Extension(
> @@ -54,15 +132,12 @@ if environ.get("GPIOD_WITH_TESTS") == "1":
>      extensions.append(gpiosim_ext)
>      extensions.append(procname_ext)
>
> -with open("gpiod/version.py", "r") as fd:
> -    exec(fd.read())
> -
>  setup(
>      name="libgpiod",
>      packages=find_packages(exclude=["tests", "tests.*"]),
>      python_requires=">=3.9.0",
>      ext_modules=extensions,
> -    cmdclass={"build_ext": build_ext},
> +    cmdclass={"build_ext": build_ext, "sdist": sdist},
>      version=__version__,
>      author="Bartosz Golaszewski",
>      author_email="brgl@xxxxxxxx",
> --
> 2.34.1
>