Re: [PATCH v3 1/4] pylibfdt: allow build out of tree

["Marc-André Lureau" <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

Hi

On Mon, Sep 21, 2020 at 10:29 AM David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>
> On Tue, Sep 15, 2020 at 11:27:02PM +0400, marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org wrote:
> > From: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >
> > With meson, we have to support out-of-tree build. Fix path lookup.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  pylibfdt/Makefile.pylibfdt |  2 +-
> >  pylibfdt/setup.py          | 25 +++++++++++++++++--------
> >  2 files changed, 18 insertions(+), 9 deletions(-)
> >
> > diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
> > index 6866a0b..32ae1c5 100644
> > --- a/pylibfdt/Makefile.pylibfdt
> > +++ b/pylibfdt/Makefile.pylibfdt
> > @@ -10,7 +10,7 @@ PYLIBFDT_CLEANDIRS_L = build __pycache__
> >  PYLIBFDT_CLEANDIRS = $(PYLIBFDT_CLEANDIRS_L:%=$(PYLIBFDT_dir)/%)
> >
> >  SETUP = $(PYLIBFDT_dir)/setup.py
> > -SETUPFLAGS =
> > +SETUPFLAGS = --top-builddir .
> >
> >  ifndef V
> >  SETUPFLAGS += --quiet
> > diff --git a/pylibfdt/setup.py b/pylibfdt/setup.py
> > index 53f2bef..f8ec924 100755
> > --- a/pylibfdt/setup.py
> > +++ b/pylibfdt/setup.py
> > @@ -19,23 +19,31 @@ import sys
> >  VERSION_PATTERN = '^#define DTC_VERSION "DTC ([^"]*)"$'
> >
> >
> > +def get_top_builddir():
> > +    assert '--top-builddir' in sys.argv
>
> I see you've added the option to the Makefile, but if this were
> invoked manually, I'm not sure that you want to just die if the
> --top-builddir option isn't included.
>
> Even if you do, I think you want a more meaningful error than an
> assertion failure.

The assertion is quite explicit:

Traceback (most recent call last):
  File "pylibfdt/setup.py", line 30, in <module>
    top_builddir = get_top_builddir()
  File "pylibfdt/setup.py", line 23, in get_top_builddir
    assert '--top-builddir' in sys.argv
AssertionError

Having extra arguments to setuptools is tricky:
https://stackoverflow.com/questions/677577/distutils-how-to-pass-a-user-defined-parameter-to-setup-py

Since it's a programmer error, that you are not suppose to run into
because you'd use make or ninja, it seems enough to me. But if you
insist, I can update to print a more friendly message somehow.
>
> > +    index = sys.argv.index('--top-builddir')
> > +    sys.argv.pop(index)
> > +    return sys.argv.pop(index)
> > +
> > +
> > +srcdir = os.path.dirname(os.path.abspath(sys.argv[0]))
> > +top_builddir = get_top_builddir()
> > +
> > +
> >  def get_version():
> > -    version_file = "../version_gen.h"
> > +    version_file = os.path.join(top_builddir, 'version_gen.h')
> >      f = open(version_file, 'rt')
> >      m = re.match(VERSION_PATTERN, f.readline())
> >      return m.group(1)
> >
> >
> > -setupdir = os.path.dirname(os.path.abspath(sys.argv[0]))
> > -os.chdir(setupdir)
> > -
> >  libfdt_module = Extension(
> >      '_libfdt',
> > -    sources=['libfdt.i'],
> > -    include_dirs=['../libfdt'],
> > +    sources=[os.path.join(srcdir, 'libfdt.i')],
> > +    include_dirs=[os.path.join(srcdir, '../libfdt')],
> >      libraries=['fdt'],
> > -    library_dirs=['../libfdt'],
> > -    swig_opts=['-I../libfdt'],
> > +    library_dirs=[os.path.join(top_builddir, 'libfdt')],
> > +    swig_opts=['-I' + os.path.join(srcdir, '../libfdt')],
> >  )
> >
> >  setup(
> > @@ -44,5 +52,6 @@ setup(
> >      author='Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>',
> >      description='Python binding for libfdt',
> >      ext_modules=[libfdt_module],
> > +    package_dir={'': srcdir},
> >      py_modules=['libfdt'],
> >  )
>
> --
> 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