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

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Thu, Oct 12, 2023 at 11:02 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.
>
> Signed-off-by: Phil Howard <phil@xxxxxxxxxxxxx>
> ---
>  bindings/python/MANIFEST.in |  4 ++
>  bindings/python/setup.py    | 95 +++++++++++++++++++++++++++++++------
>  2 files changed, 84 insertions(+), 15 deletions(-)
>
> diff --git a/bindings/python/MANIFEST.in b/bindings/python/MANIFEST.in
> index c7124d4..eff8977 100644
> --- a/bindings/python/MANIFEST.in
> +++ b/bindings/python/MANIFEST.in
> @@ -11,3 +11,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/setup.py b/bindings/python/setup.py
> index fd674aa..e3b571c 100644
> --- a/bindings/python/setup.py
> +++ b/bindings/python/setup.py
> @@ -4,7 +4,30 @@
>  from os import environ, path
>  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 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:
> +            copytree("../../lib", "./lib")
> +            copytree("../../include", "./include")
> +        func(self)
> +        if copy_src:
> +            rmtree("./lib")
> +            rmtree("./include")
> +
> +    return wrapper
>
>
>  class build_ext(orig_build_ext):
> @@ -14,24 +37,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(__version__),

This is incorrect. The version we want to pass here is the version of
libgpiod, not the version of python bindings which is now decoupled
from the former.

I'm not sure how to correctly do it. We can read the contents of
configure.ac for one and get it from there. Maybe you have a better
idea.

Bart

> +    ],
>  )
>
>  gpiosim_ext = Extension(
> @@ -54,15 +122,12 @@ 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:
> -    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
>