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

[Phil Howard <phil@xxxxxxxxxxxxx>]

On Fri, 13 Oct 2023 at 10:22, Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
>
> 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.

Since configure.ac wont be available in the sdist package we'll need
to copy the version number somewhere.

Since we should probably be plumbing this into autoconf anyway I
think this var should be passed in the same way as
GPIOD_WITH_TESTS, then setup.py can grab it from environ with
a failover to loading from a packaged "gpiod-version-str.txt".

A little out of my wheelhouse, but I'll take a shot.

>
> 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
> >



-- 
Philip Howard
Technology & Lifestyle Writer
gadgetoid.com

Gadgetoid gadg-et-oid [gaj-it-oid]

                                     -adjective

1. having the characteristics or form of a gadget; resembling a
mechanical contrivance or device.