[PATCH 0/2] More pylibfdt setup.py rework

[PATCH 0/2] More pylibfdt setup.py rework

[Rob Herring]

This series is further work to get libfdt to 'just work' as a python 
package dependency. It's worse than I originally realized when my setup 
broke due to recently added libfdt functions.

The first patch is a fix to my previous rework. The 2nd is a RFC.

Rob

Rob Herring (2):
  pylibfdt: fix swig build in install
  pylibfdt: Compile and build libfdt directly into shim library

 MANIFEST.in |  5 +----
 setup.py    | 49 ++++++++++++++++++++++++-------------------------
 2 files changed, 25 insertions(+), 29 deletions(-)

-- 
2.32.0

[PATCH 1/2] pylibfdt: fix swig build in install

[Rob Herring]

A 'pip install' is silently broken unless the tree is dirty and contains
pylibfdt/libfdt.py. The problem is a known issue[1] with SWIG and
setuptools where the 'build_py' stage needing module.py runs before
the 'build_ext' stage which generates it. The work-around is to override
'build_py' to run 'build_ext' first.

[1] https://stackoverflow.com/questions/50239473/building-a-module-with-setuptools-and-swig

Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 MANIFEST.in | 1 -
 setup.py    | 8 ++++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/MANIFEST.in b/MANIFEST.in
index d9fb71b77a65..6e7244d195e6 100644
--- a/MANIFEST.in
+++ b/MANIFEST.in
@@ -6,7 +6,6 @@ include GPL
 include BSD-2-Clause
 include setup.py
 include pylibfdt/libfdt.i
-include pylibfdt/*.py
 include libfdt/libfdt.h
 include libfdt/fdt.h
 include libfdt/libfdt_env.h
diff --git a/setup.py b/setup.py
index 029aa6182221..a8e54a361512 100755
--- a/setup.py
+++ b/setup.py
@@ -11,6 +11,8 @@ Written by Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
 """
 
 from setuptools import setup, Extension
+from setuptools.command.build_py import build_py as _build_py
+
 import os
 import re
 import sys
@@ -40,11 +42,17 @@ libfdt_module = Extension(
     swig_opts=['-I' + os.path.join(srcdir, 'libfdt')],
 )
 
+class build_py(_build_py):
+    def run(self):
+        self.run_command("build_ext")
+        return super().run()
+
 setup(
     name='libfdt',
     use_scm_version={
         "root": srcdir,
     },
+    cmdclass = {'build_py' : build_py},
     setup_requires = ['setuptools_scm'],
     author='Simon Glass',
     author_email='sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org',
-- 
2.32.0

Re: [PATCH 1/2] pylibfdt: fix swig build in install

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2167 bytes --]

On Thu, Feb 03, 2022 at 12:04:07PM -0600, Rob Herring wrote:
> A 'pip install' is silently broken unless the tree is dirty and contains
> pylibfdt/libfdt.py. The problem is a known issue[1] with SWIG and
> setuptools where the 'build_py' stage needing module.py runs before
> the 'build_ext' stage which generates it. The work-around is to override
> 'build_py' to run 'build_ext' first.
> 
> [1] https://stackoverflow.com/questions/50239473/building-a-module-with-setuptools-and-swig
> 
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Applied, thanks.

> ---
>  MANIFEST.in | 1 -
>  setup.py    | 8 ++++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/MANIFEST.in b/MANIFEST.in
> index d9fb71b77a65..6e7244d195e6 100644
> --- a/MANIFEST.in
> +++ b/MANIFEST.in
> @@ -6,7 +6,6 @@ include GPL
>  include BSD-2-Clause
>  include setup.py
>  include pylibfdt/libfdt.i
> -include pylibfdt/*.py
>  include libfdt/libfdt.h
>  include libfdt/fdt.h
>  include libfdt/libfdt_env.h
> diff --git a/setup.py b/setup.py
> index 029aa6182221..a8e54a361512 100755
> --- a/setup.py
> +++ b/setup.py
> @@ -11,6 +11,8 @@ Written by Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>  """
>  
>  from setuptools import setup, Extension
> +from setuptools.command.build_py import build_py as _build_py
> +
>  import os
>  import re
>  import sys
> @@ -40,11 +42,17 @@ libfdt_module = Extension(
>      swig_opts=['-I' + os.path.join(srcdir, 'libfdt')],
>  )
>  
> +class build_py(_build_py):
> +    def run(self):
> +        self.run_command("build_ext")
> +        return super().run()
> +
>  setup(
>      name='libfdt',
>      use_scm_version={
>          "root": srcdir,
>      },
> +    cmdclass = {'build_py' : build_py},
>      setup_requires = ['setuptools_scm'],
>      author='Simon Glass',
>      author_email='sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org',

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/2] pylibfdt: Compile and build libfdt directly into shim library

[Rob Herring]

The interactions between pylibfdt setup.py and the host and build system
is fragile. setup.py can be called directly or via pip, tox, pytest, etc.
Building the SWIG shim library needs the libfdt headers and library .so
file. These can be located in the source tree, the OOT build directory,
or several locations in the host filesystem if installed.

Furthermore, the SWIG shim library is tightly coupled to the version of
libfdt it is built against. Specifically, all functions defined in the
libfdt.h header used for the build must resolve at runtime whether they
are used or not. IOW, the installed libfdt must be the same version (or
newer?) than what pylibfdt was built against.

The typical way to solve this problem would be to allow user provided
library and include directories, but this doesn't seem to work too well
with setup.py. While setup.py sub-commands can take such options, it
doesn't work with implicit commands (e.g. an 'install' triggers 'build')
or pip.

The simplest solution to all this is just build libfdt into the shim
library. This avoids any possibility of version mismatches. The python
setuptools already knows how to compile C files in extensions, we just need
to list the files.

Cc: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Hector Oron <zumbi-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
Cc: Peter Robinson <pbrobinson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Natanael Copa <ncopa-Tvuechaw34bCfDggNXIi3w@public.gmane.org>
Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
RFC because this leaves the meson integration broken and I suspect the
distro folks (Cc'ed here) won't really like duplicating libfdt. Note that
the 'shim' alone is about 3x the size of libfdt.

IMO, the meson wrapper should just be removed. Python users know how to
run setup.py or pip. Why add a layer of indirection?
---
 MANIFEST.in |  4 +---
 setup.py    | 43 +++++++++++++++++--------------------------
 2 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/MANIFEST.in b/MANIFEST.in
index 6e7244d195e6..45243f7f0057 100644
--- a/MANIFEST.in
+++ b/MANIFEST.in
@@ -6,9 +6,7 @@ include GPL
 include BSD-2-Clause
 include setup.py
 include pylibfdt/libfdt.i
-include libfdt/libfdt.h
-include libfdt/fdt.h
-include libfdt/libfdt_env.h
+include libfdt/*
 include tests/test_pylibfdt.py
 include tests/test_tree1.dts
 include tests/test_props.dts
diff --git a/setup.py b/setup.py
index a8e54a361512..ba8ff556c44d 100755
--- a/setup.py
+++ b/setup.py
@@ -13,33 +13,26 @@ Written by Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
 from setuptools import setup, Extension
 from setuptools.command.build_py import build_py as _build_py
 
-import os
-import re
-import sys
-
-srcdir = os.path.dirname(__file__)
-
-with open(os.path.join(srcdir, "README"), "r") as fh:
+with open("README", "r") as fh:
     long_description = fh.read()
 
-def get_top_builddir():
-    if '--top-builddir' in sys.argv:
-        index = sys.argv.index('--top-builddir')
-        sys.argv.pop(index)
-        return sys.argv.pop(index)
-    else:
-        return srcdir
-
-top_builddir = get_top_builddir()
-
 libfdt_module = Extension(
     '_libfdt',
-    sources=[os.path.join(srcdir, 'pylibfdt/libfdt.i')],
+    sources=['pylibfdt/libfdt.i',
+             'libfdt/fdt.c',
+             'libfdt/fdt_ro.c',
+             'libfdt/fdt_rw.c',
+             'libfdt/fdt_sw.c',
+             'libfdt/fdt_wip.c',
+             'libfdt/fdt_addresses.c',
+             'libfdt/fdt_check.c',
+             'libfdt/fdt_empty_tree.c',
+             'libfdt/fdt_overlay.c',
+             'libfdt/fdt_strerror.c',
+             ],
     define_macros=[('PY_SSIZE_T_CLEAN', None)],
-    include_dirs=[os.path.join(srcdir, 'libfdt')],
-    libraries=['fdt'],
-    library_dirs=[os.path.join(top_builddir, 'libfdt')],
-    swig_opts=['-I' + os.path.join(srcdir, 'libfdt')],
+    include_dirs=['libfdt'],
+    swig_opts=['-Ilibfdt'],
 )
 
 class build_py(_build_py):
@@ -49,16 +42,14 @@ class build_py(_build_py):
 
 setup(
     name='libfdt',
-    use_scm_version={
-        "root": srcdir,
-    },
+    use_scm_version=True,
     cmdclass = {'build_py' : build_py},
     setup_requires = ['setuptools_scm'],
     author='Simon Glass',
     author_email='sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org',
     description='Python binding for libfdt',
     ext_modules=[libfdt_module],
-    package_dir={'': os.path.join(srcdir, 'pylibfdt')},
+    package_dir={'': 'pylibfdt'},
     py_modules=['libfdt'],
 
     long_description=long_description,
-- 
2.32.0

Re: [PATCH 2/2] pylibfdt: Compile and build libfdt directly into shim library

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3392 bytes --]

On Thu, Feb 03, 2022 at 12:04:08PM -0600, Rob Herring wrote:
> The interactions between pylibfdt setup.py and the host and build system
> is fragile. setup.py can be called directly or via pip, tox, pytest, etc.
> Building the SWIG shim library needs the libfdt headers and library .so
> file. These can be located in the source tree, the OOT build directory,
> or several locations in the host filesystem if installed.

Right.  I do believe that if feasible we should only support building
from the main dtc tree, and therefore only using the files from that
tree, not anywhere else on the host system.

> Furthermore, the SWIG shim library is tightly coupled to the version of
> libfdt it is built against. Specifically, all functions defined in the
> libfdt.h header used for the build must resolve at runtime whether they
> are used or not. IOW, the installed libfdt must be the same version (or
> newer?) than what pylibfdt was built against.

I believe "or newer" should be safe, at least assuming we don't badly
screw up the symbol versioning.  That's a pretty significant
difference from "must be the same version".

> The typical way to solve this problem would be to allow user provided
> library and include directories, but this doesn't seem to work too well
> with setup.py. While setup.py sub-commands can take such options, it
> doesn't work with implicit commands (e.g. an 'install' triggers 'build')
> or pip.
> 
> The simplest solution to all this is just build libfdt into the shim
> library. This avoids any possibility of version mismatches. The python
> setuptools already knows how to compile C files in extensions, we just need
> to list the files.

Urgh.  I don't love having what's essentially a different way of
building the code than the existing make or meson stuff.

> Cc: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Cc: Hector Oron <zumbi-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
> Cc: Peter Robinson <pbrobinson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Natanael Copa <ncopa-Tvuechaw34bCfDggNXIi3w@public.gmane.org>
> Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> ---
> RFC because this leaves the meson integration broken and I suspect the
> distro folks (Cc'ed here) won't really like duplicating libfdt. Note that
> the 'shim' alone is about 3x the size of libfdt.

No, they probably won't.  Note that the primary reason that distro
folks like to re-use shared libraries as much as possible isn't about
size, it's about ease of deploying updates (including for security).

Then again, they're probably having to get used to the harder case
these days, due to Go's static linking fetish.

> IMO, the meson wrapper should just be removed. Python users know how to
> run setup.py or pip. Why add a layer of indirection?

I'm not exactly sure what you're considering the "meson wrapper".
Note that from my point of view anything which means that doing a full
build and test from scratch would require more than a single make or
meson command is pretty much unacceptable.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] pylibfdt: Compile and build libfdt directly into shim library

[Rob Herring]

On Tue, Feb 8, 2022 at 12:01 AM David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>
> On Thu, Feb 03, 2022 at 12:04:08PM -0600, Rob Herring wrote:
> > The interactions between pylibfdt setup.py and the host and build system
> > is fragile. setup.py can be called directly or via pip, tox, pytest, etc.
> > Building the SWIG shim library needs the libfdt headers and library .so
> > file. These can be located in the source tree, the OOT build directory,
> > or several locations in the host filesystem if installed.
>
> Right.  I do believe that if feasible we should only support building
> from the main dtc tree, and therefore only using the files from that
> tree, not anywhere else on the host system.
>
> > Furthermore, the SWIG shim library is tightly coupled to the version of
> > libfdt it is built against. Specifically, all functions defined in the
> > libfdt.h header used for the build must resolve at runtime whether they
> > are used or not. IOW, the installed libfdt must be the same version (or
> > newer?) than what pylibfdt was built against.
>
> I believe "or newer" should be safe, at least assuming we don't badly
> screw up the symbol versioning.  That's a pretty significant
> difference from "must be the same version".

The problem is I think the more common scenario is the installed
version being older as the distro version is likely older.

> > The typical way to solve this problem would be to allow user provided
> > library and include directories, but this doesn't seem to work too well
> > with setup.py. While setup.py sub-commands can take such options, it
> > doesn't work with implicit commands (e.g. an 'install' triggers 'build')
> > or pip.
> >
> > The simplest solution to all this is just build libfdt into the shim
> > library. This avoids any possibility of version mismatches. The python
> > setuptools already knows how to compile C files in extensions, we just need
> > to list the files.
>
> Urgh.  I don't love having what's essentially a different way of
> building the code than the existing make or meson stuff.

There's not really any avoiding it. setuptools is doing some portion
of the build no matter what. Currently, that's just the wrapper code.
This patch adds building libfdt using that same infrastructure.

> > Cc: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > Cc: Hector Oron <zumbi-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
> > Cc: Peter Robinson <pbrobinson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Cc: Natanael Copa <ncopa-Tvuechaw34bCfDggNXIi3w@public.gmane.org>
> > Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > ---
> > RFC because this leaves the meson integration broken and I suspect the
> > distro folks (Cc'ed here) won't really like duplicating libfdt. Note that
> > the 'shim' alone is about 3x the size of libfdt.
>
> No, they probably won't.  Note that the primary reason that distro
> folks like to re-use shared libraries as much as possible isn't about
> size, it's about ease of deploying updates (including for security).

No comments from anyone else, so I take that as agreement with this patch. :(

> Then again, they're probably having to get used to the harder case
> these days, due to Go's static linking fetish.
>
> > IMO, the meson wrapper should just be removed. Python users know how to
> > run setup.py or pip. Why add a layer of indirection?
>
> I'm not exactly sure what you're considering the "meson wrapper".

Calling meson/make to run setup.py.

> Note that from my point of view anything which means that doing a full
> build and test from scratch would require more than a single make or
> meson command is pretty much unacceptable.

Those aren't mutually exclusive. I assume you agree supporting the
'normal' python tools and usage is worthwhile and required?

With this patch plus my test changes, it would be 2 commands:

make check
pytest (or 'tox' if you want to do python version matrix testing)

or 3:
meson build/
meson test -C build/
pytest


The primary issue here is that the python tools are designed to run
from the setup.py directory and meson wants something different. The
current support to use the meson build directory is incomplete. After
a 'meson compile', the tree is dirty with:

pylibfdt/libfdt.py
pylibfdt/libfdt_wrap.c

And then after 'meson test' we have:

        pylibfdt/__pycache__/
        pylibfdt/libfdt.py
        pylibfdt/libfdt_wrap.c
        tests/bad-chosen.dts.test.dtb
        tests/bad-dma-ranges.dts.test.dtb
        tests/bad-empty-ranges.dts.test.dtb
        tests/bad-gpio.dts.test.dtb
        tests/bad-graph.dts.test.dtb
        tests/bad-interrupt-cells.dts.test.dtb
        tests/bad-interrupt-controller.dts.test.dtb
        tests/bad-interrupt-map-mask.dts.test.dtb
        tests/bad-interrupt-map-parent.dts.test.dtb
        tests/bad-interrupt-map.dts.test.dtb
        tests/bad-name-property.dts.test.dtb
        tests/bad-ncells.dts.test.dtb
        tests/bad-phandle-cells.dts.test.dtb
        tests/bad-reg-ranges.dts.test.dtb
        tests/bad-string-props.dts.test.dtb
        tests/default-addr-size.dts.test.dtb
        tests/dup-nodename.dts.test.dtb
        tests/dup-phandle.dts.test.dtb
        tests/dup-propname.dts.test.dtb
        tests/good-gpio.dts.test.dtb
        tests/minusone-phandle.dts.test.dtb
        tests/obsolete-chosen-interrupt-controller.dts.test.dtb
        tests/pci-bridge-bad1.dts.test.dtb
        tests/pci-bridge-bad2.dts.test.dtb
        tests/pci-bridge-ok.dts.test.dtb
        tests/reg-ranges-root.dts.test.dtb
        tests/reg-without-unit-addr.dts.test.dtb
        tests/unit-addr-leading-0s.dts.test.dtb
        tests/unit-addr-leading-0x.dts.test.dtb
        tests/unit-addr-simple-bus-compatible.dts.test.dtb
        tests/unit-addr-simple-bus-reg-mismatch.dts.test.dtb
        tests/unit-addr-unique.dts.test.dtb
        tests/unit-addr-without-reg.dts.test.dtb
        tests/zero-phandle.dts.test.dtb

(This was a tree before all my recent pylibfdt changes just to make
sure it wasn't something I broke)

While you'd think it would be trivial to move these to a build dir,
I've spent days on this and haven't come up with a clean way of doing
that. Even if we did, then the next python tool you want to integrate
in is still broken (pip, pytest, tox, etc. for example). I'm sure
continuing down this path is asking for more pain. I'm not a python
expert, but in my limited experience in python projects it's easier to
conform to the python way than burning cycles trying to do something
different.

I'm fine keeping a meson wrapper if that's really a requirement, but
it's got to give up the notion of the python portions working OOT.

Rob

Re: [PATCH 2/2] pylibfdt: Compile and build libfdt directly into shim library

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 8971 bytes --]

On Tue, Feb 08, 2022 at 10:00:36AM -0600, Rob Herring wrote:
> On Tue, Feb 8, 2022 at 12:01 AM David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> >
> > On Thu, Feb 03, 2022 at 12:04:08PM -0600, Rob Herring wrote:
> > > The interactions between pylibfdt setup.py and the host and build system
> > > is fragile. setup.py can be called directly or via pip, tox, pytest, etc.
> > > Building the SWIG shim library needs the libfdt headers and library .so
> > > file. These can be located in the source tree, the OOT build directory,
> > > or several locations in the host filesystem if installed.
> >
> > Right.  I do believe that if feasible we should only support building
> > from the main dtc tree, and therefore only using the files from that
> > tree, not anywhere else on the host system.
> >
> > > Furthermore, the SWIG shim library is tightly coupled to the version of
> > > libfdt it is built against. Specifically, all functions defined in the
> > > libfdt.h header used for the build must resolve at runtime whether they
> > > are used or not. IOW, the installed libfdt must be the same version (or
> > > newer?) than what pylibfdt was built against.
> >
> > I believe "or newer" should be safe, at least assuming we don't badly
> > screw up the symbol versioning.  That's a pretty significant
> > difference from "must be the same version".
> 
> The problem is I think the more common scenario is the installed
> version being older as the distro version is likely older.

I suppose that would be the case if you're trying to install the
python stuff alone.  I guess this is the pip case?  AFAICT it wouldn't
arise if installing the python stuff from distro packages, nor if
installing everything, including the python packages, from the dtc
source tree.

> > > The typical way to solve this problem would be to allow user provided
> > > library and include directories, but this doesn't seem to work too well
> > > with setup.py. While setup.py sub-commands can take such options, it
> > > doesn't work with implicit commands (e.g. an 'install' triggers 'build')
> > > or pip.
> > >
> > > The simplest solution to all this is just build libfdt into the shim
> > > library. This avoids any possibility of version mismatches. The python
> > > setuptools already knows how to compile C files in extensions, we just need
> > > to list the files.
> >
> > Urgh.  I don't love having what's essentially a different way of
> > building the code than the existing make or meson stuff.
> 
> There's not really any avoiding it. setuptools is doing some portion
> of the build no matter what. Currently, that's just the wrapper code.
> This patch adds building libfdt using that same infrastructure.

That's not quite the same thing.  At the moment the wrapper code is
only ever built via setuptools.  With the proposed change the same
code is being routinely built by two different build systems.
Well... three I guess, while we have both make and meson support.

> > > Cc: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > > Cc: Hector Oron <zumbi-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
> > > Cc: Peter Robinson <pbrobinson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > Cc: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > Cc: Natanael Copa <ncopa-Tvuechaw34bCfDggNXIi3w@public.gmane.org>
> > > Signed-off-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > > ---
> > > RFC because this leaves the meson integration broken and I suspect the
> > > distro folks (Cc'ed here) won't really like duplicating libfdt. Note that
> > > the 'shim' alone is about 3x the size of libfdt.
> >
> > No, they probably won't.  Note that the primary reason that distro
> > folks like to re-use shared libraries as much as possible isn't about
> > size, it's about ease of deploying updates (including for security).
> 
> No comments from anyone else, so I take that as agreement with this patch. :(

Um.. I don't follow.

> > Then again, they're probably having to get used to the harder case
> > these days, due to Go's static linking fetish.
> >
> > > IMO, the meson wrapper should just be removed. Python users know how to
> > > run setup.py or pip. Why add a layer of indirection?
> >
> > I'm not exactly sure what you're considering the "meson wrapper".
> 
> Calling meson/make to run setup.py.
> 
> > Note that from my point of view anything which means that doing a full
> > build and test from scratch would require more than a single make or
> > meson command is pretty much unacceptable.
> 
> Those aren't mutually exclusive.

I'm confused.  You say these aren't mutually exclusive, then give an
example that does exactly what I said I didn't want.

> I assume you agree supporting the
> 'normal' python tools and usage is worthwhile and required?

That's certainly a goal, yes.

> With this patch plus my test changes, it would be 2 commands:
> 
> make check
> pytest (or 'tox' if you want to do python version matrix testing)

Yeah, that's what I don't want.  I want *one* command to do
everything.  dtc, libfdt, pylibfdt, the extra tools, everything.  I
guess two (setup, then build) with meson - I already don't love that
about meson.

The fact that you already point out two options (pytest or tox) for
the extra commands underlines the problem: it now requires python
specific knowledge to know how to do a full build/test.

> or 3:
> meson build/
> meson test -C build/
> pytest
> 
> 
> The primary issue here is that the python tools are designed to run
> from the setup.py directory and meson wants something different. The
> current support to use the meson build directory is incomplete. After
> a 'meson compile', the tree is dirty with:
> 
> pylibfdt/libfdt.py
> pylibfdt/libfdt_wrap.c

Yeah, that's certainly breaking the meson model.

> And then after 'meson test' we have:
> 
>         pylibfdt/__pycache__/
>         pylibfdt/libfdt.py
>         pylibfdt/libfdt_wrap.c
>         tests/bad-chosen.dts.test.dtb
>         tests/bad-dma-ranges.dts.test.dtb
>         tests/bad-empty-ranges.dts.test.dtb
>         tests/bad-gpio.dts.test.dtb
>         tests/bad-graph.dts.test.dtb
>         tests/bad-interrupt-cells.dts.test.dtb
>         tests/bad-interrupt-controller.dts.test.dtb
>         tests/bad-interrupt-map-mask.dts.test.dtb
>         tests/bad-interrupt-map-parent.dts.test.dtb
>         tests/bad-interrupt-map.dts.test.dtb
>         tests/bad-name-property.dts.test.dtb
>         tests/bad-ncells.dts.test.dtb
>         tests/bad-phandle-cells.dts.test.dtb
>         tests/bad-reg-ranges.dts.test.dtb
>         tests/bad-string-props.dts.test.dtb
>         tests/default-addr-size.dts.test.dtb
>         tests/dup-nodename.dts.test.dtb
>         tests/dup-phandle.dts.test.dtb
>         tests/dup-propname.dts.test.dtb
>         tests/good-gpio.dts.test.dtb
>         tests/minusone-phandle.dts.test.dtb
>         tests/obsolete-chosen-interrupt-controller.dts.test.dtb
>         tests/pci-bridge-bad1.dts.test.dtb
>         tests/pci-bridge-bad2.dts.test.dtb
>         tests/pci-bridge-ok.dts.test.dtb
>         tests/reg-ranges-root.dts.test.dtb
>         tests/reg-without-unit-addr.dts.test.dtb
>         tests/unit-addr-leading-0s.dts.test.dtb
>         tests/unit-addr-leading-0x.dts.test.dtb
>         tests/unit-addr-simple-bus-compatible.dts.test.dtb
>         tests/unit-addr-simple-bus-reg-mismatch.dts.test.dtb
>         tests/unit-addr-unique.dts.test.dtb
>         tests/unit-addr-without-reg.dts.test.dtb
>         tests/zero-phandle.dts.test.dtb
> 
> (This was a tree before all my recent pylibfdt changes just to make
> sure it wasn't something I broke)
> 
> While you'd think it would be trivial to move these to a build dir,
> I've spent days on this and haven't come up with a clean way of doing
> that. Even if we did, then the next python tool you want to integrate
> in is still broken (pip, pytest, tox, etc. for example). I'm sure
> continuing down this path is asking for more pain. I'm not a python
> expert, but in my limited experience in python projects it's easier to
> conform to the python way than burning cycles trying to do something
> different.

Could we fake it the other way around: have the meson stuff copy all
the sources into meson's build dir, then run the Python stuff locally
within the build dir.  Still ugly, but less bad than simply breaking
meson's base assumptions.

> I'm fine keeping a meson wrapper if that's really a requirement, but
> it's got to give up the notion of the python portions working OOT.
> 
> Rob
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]