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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



Hi

On Mon, Sep 28, 2020 at 12:03 PM David Gibson
<david@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Sep 21, 2020 at 12:22:06PM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > On Mon, Sep 21, 2020 at 10:29 AM David Gibson
> > <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > On Tue, Sep 15, 2020 at 11:27:02PM +0400, marcandre.lureau@xxxxxxxxxx wrote:
> > > > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> > > >
> > > > With meson, we have to support out-of-tree build. Fix path lookup.
> > > >
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> > > > ---
> > > >  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.
>
> Hm.  Invoking this only via make or ninja would be normal for someone
> building dtc as a unit.  But, I'm not so familiar with the Python
> ecosystem, so I wonder if someone specifically looking at the Python
> bindings would expect to be able to invoke setup.py directly.

Maybe, so if the error message about missing '--top-builddir' is not
explicit enough, I can try to make the error a bit more human
friendly.

But as you can see from the stackoverflow question, it's not uncommon
to want to have extra arguments to setup.py, and there are various
solutions offered for that.

>
> > 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@xxxxxxxxxxxx>',
> > > >      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





[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux