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

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

[Phil Howard]

This changeset vendors the gpiod library into the Python package.

Why?

So that setup.py can produce an sdist that is installable irrespective of the
availability or version of a distro-supplied libgpiod.

This prevents a libgpiod pypi package install balking because the distro
libgpiod is outdated or otherwise incompatible. This happens when attempting to
install the current libgpiod from pypi onto - for example - the Debian Bookworm
based Raspberry Pi OS.

The availability of a distro agnostic package also ensures that libgpiod can be
installed via pypi into an isolated virtual environment, safely specified as a
dependency for Python packages and allows Python developers to target the newest
API version irrespective of their distro supplied libgpiod.

This is essential, since a venv is now widely *required* for user Python
projects due to recommendations in pep-688 - https://peps.python.org/pep-0668/

For Raspberry Pi this sdist can also be converted into a precompiled wheel by
piwheels [1] which is, by default, added to Raspberry Pi OS as a pip index.

How?

If "LINK_SYSTEM_LIBGPIOD=1" is not specified then the gpiod._ext C Extension is
amended to include all of the C sources for gpiod, so it can be built as a
standalone module without depending upon a shared distro library.

python/bindings/Makefile.am has been modified so that this variable is included
for the "build_ext --inplace" build, preserving the old, linked behaviour.

The gpiod sources are included by copying the "lib" and "include" directories
up to the parent module, and updating "MANIFEST.in" to include the source files
when an sdist is built. Additionally the gpiod version string is included in
"libgpiod-version-str.txt" so that it is available for source builds.

Additionally bindings/python/Makefile.am has been extended so that "make"
will now produce a .tar.gz source distribution in bindings/python/dist which is
suitable for uploading to pypi and can be built and installed by any user with
python3-dev installed.

[1] - https://www.piwheels.org/


Phil Howard (1):
  bindings: python: optionally include module in sdist

 bindings/python/MANIFEST.in |   5 ++
 bindings/python/Makefile.am |   5 ++
 bindings/python/setup.py    | 107 ++++++++++++++++++++++++++++++------
 3 files changed, 101 insertions(+), 16 deletions(-)

-- 
2.34.1

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

[Phil Howard]

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@gadgetoid.com>
---
 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@linaro.org>
 
 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@bgdev.pl>
 
-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"]
+    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@bgdev.pl",
-- 
2.34.1

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

[Bartosz Golaszewski]

On Fri, Oct 13, 2023 at 1:28 PM Phil Howard <phil@gadgetoid.com> 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@gadgetoid.com>
> ---
>  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@linaro.org>
>
>  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@bgdev.pl>
>
> -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@bgdev.pl",
> --
> 2.34.1
>

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

[Phil Howard]

On Mon, 16 Oct 2023 at 14:10, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Fri, Oct 13, 2023 at 1:28 PM Phil Howard <phil@gadgetoid.com> 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@gadgetoid.com>
> > ---
> >  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@linaro.org>
> >
> >  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@bgdev.pl>
> >
> > -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?

Seems reasonable. I'll change it.

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

Ah for some reason I'd thought yocto would just use automake.

> 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@bgdev.pl",
> > --
> > 2.34.1
> >

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

[Bartosz Golaszewski]

On Mon, Oct 16, 2023 at 3:45 PM Phil Howard <phil@gadgetoid.com> wrote:
>
> On Mon, 16 Oct 2023 at 14:10, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > On Fri, Oct 13, 2023 at 1:28 PM Phil Howard <phil@gadgetoid.com> 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@gadgetoid.com>
> > > ---
> > >  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@linaro.org>
> > >
> > >  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@bgdev.pl>
> > >
> > > -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?
>
> Seems reasonable. I'll change it.
>
> > This way, the yocto recipe wouldn't require any changes at all, which
> > would be preferable.
>
> Ah for some reason I'd thought yocto would just use automake.
>

It actually uses pypi for the python bindings. They were factored out
of the main recipe and put into meta-python.

Bart

> > 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@bgdev.pl",
> > > --
> > > 2.34.1
> > >

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

[Bartosz Golaszewski]

On Mon, Oct 16, 2023 at 3:45 PM Phil Howard <phil@gadgetoid.com> wrote:
>
> On Mon, 16 Oct 2023 at 14:10, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > On Fri, Oct 13, 2023 at 1:28 PM Phil Howard <phil@gadgetoid.com> 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@gadgetoid.com>
> > > ---
> > >  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@linaro.org>
> > >
> > >  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@bgdev.pl>
> > >
> > > -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?
>
> Seems reasonable. I'll change it.

Maybe add some warning too at setup.py sdist invokation? Like:

GPIOD_VERSION_STR not set, stand-alone build disabled

Or something similar.

Bart

>
> > This way, the yocto recipe wouldn't require any changes at all, which
> > would be preferable.
>
> Ah for some reason I'd thought yocto would just use automake.
>
> > 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@bgdev.pl",
> > > --
> > > 2.34.1
> > >