[PATCH v3 0/4] Add meson build system

[PATCH v3 0/4] Add meson build system

[marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA]

From: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Hi

This series adds support for meson build system. The main reason for this,
beside using a more modern and simpler build system, is to enable subproject()
support in QEMU.

v3:
 - remove workaround for meson#2992 which is now unnecessary
 - add description to meson options
 - pass NO_YAML & NO_PYTHON down to run_tests.sh
 - commit comment tweaks
 - rebased

v2:
 - various misc improvements after David Gibson v1 review
 - add various meson_options.txt build options
 - add editorconfig patch

Marc-André Lureau (4):
  pylibfdt: allow build out of tree
  pylibfdt: fix build lib location
  build-sys: add meson build
  travis: test meson build

 .travis.yml                |  16 +++++
 libfdt/meson.build         |  50 ++++++++++++++
 meson.build                | 127 ++++++++++++++++++++++++++++++++++++
 meson_options.txt          |  10 +++
 pylibfdt/Makefile.pylibfdt |   4 +-
 pylibfdt/meson.build       |  13 ++++
 pylibfdt/setup.py          |  25 ++++---
 tests/meson.build          | 130 +++++++++++++++++++++++++++++++++++++
 version_gen.h.in           |   1 +
 9 files changed, 366 insertions(+), 10 deletions(-)
 create mode 100644 libfdt/meson.build
 create mode 100644 meson.build
 create mode 100644 meson_options.txt
 create mode 100644 pylibfdt/meson.build
 create mode 100644 tests/meson.build
 create mode 100644 version_gen.h.in

-- 
2.26.2

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

[marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA]

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
+    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'],
 )
-- 
2.26.2

[PATCH v3 2/4] pylibfdt: fix build lib location

[marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA]

From: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

setup.py build_ext is run from top_srcdir with Makefile.

../pylibfdt will produce output files in parent directory.

Note that setup.py install will rebuild it with the default 'build'
directory. There doesn't seem to be a way to override that.

Signed-off-by: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 pylibfdt/Makefile.pylibfdt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
index 32ae1c5..1b5f236 100644
--- a/pylibfdt/Makefile.pylibfdt
+++ b/pylibfdt/Makefile.pylibfdt
@@ -18,7 +18,7 @@ endif
 
 $(PYMODULE): $(PYLIBFDT_srcs) $(LIBFDT_archive) $(SETUP) $(VERSION_FILE)
 	@$(VECHO) PYMOD $@
-	$(PYTHON) $(SETUP) $(SETUPFLAGS) build_ext --build-lib=../$(PYLIBFDT_dir)
+	$(PYTHON) $(SETUP) $(SETUPFLAGS) build_ext --build-lib=$(PYLIBFDT_dir)
 
 install_pylibfdt: $(PYMODULE)
 	@$(VECHO) INSTALL-PYLIB
-- 
2.26.2

[PATCH v3 3/4] build-sys: add meson build

[marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA]

From: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

The meson build system allows projects to "vendor" dtc easily, thanks to
subproject(). QEMU has recently switched to meson, and adding meson
support to dtc will help to handle the QEMU submodule.

meson rules are arguably simpler to write and maintain than
the hand-crafted/custom Makefile. meson support various backends, and
default build options (including coverage, sanitizer, debug/release
etc, see: https://mesonbuild.com/Builtin-options.html)

Compare to the Makefiles, the same build targets should be built and
installed and the same tests should be run ("meson test" can be provided
extra test arguments for running the equivalent of checkm/checkv).

There is no support EXTRAVERSION/LOCAL_VERSION/CONFIG_LOCALVERSION,
instead the version is simply set with project(), and vcs_tag() is
used for git/dirty version reporting (This is most common and is
hopefully enough. If necessary, configure-time options could be added
for extra versioning.).

libfdt shared library is build following regular naming conventions:
instead of libfdt.so.1 -> libfdt-1.6.0.so (with current build-sys),
libfdt.so.1 -> libfdt.so.1.6.0. I am not sure why the current build
system use an uncommon naming pattern. I also included a libfdt.pc
pkg-config file, as convenience.

Both Linux native build and mingw cross-build pass. CI pass. Tests are
only run on native build.

The current Makefiles are left in-tree, and make/check still work.
Eventually, the Makefiles could be marked as deprecated, to start a
transition period and avoid having to maintain 2 build systems in the
near future.

(run_tests.sh could eventually be replaced by the meson test runner,
which would have several advantages in term of flexibility/features,
but this is left for another day)

Signed-off-by: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 libfdt/meson.build   |  50 +++++++++++++++++
 meson.build          | 127 ++++++++++++++++++++++++++++++++++++++++++
 meson_options.txt    |  10 ++++
 pylibfdt/meson.build |  13 +++++
 tests/meson.build    | 130 +++++++++++++++++++++++++++++++++++++++++++
 version_gen.h.in     |   1 +
 6 files changed, 331 insertions(+)
 create mode 100644 libfdt/meson.build
 create mode 100644 meson.build
 create mode 100644 meson_options.txt
 create mode 100644 pylibfdt/meson.build
 create mode 100644 tests/meson.build
 create mode 100644 version_gen.h.in

diff --git a/libfdt/meson.build b/libfdt/meson.build
new file mode 100644
index 0000000..0307ffb
--- /dev/null
+++ b/libfdt/meson.build
@@ -0,0 +1,50 @@
+version_script = '-Wl,--version-script=@0@'.format(meson.current_source_dir() / 'version.lds')
+if not cc.has_link_argument(version_script)
+  version_script = []
+endif
+
+sources = files(
+  'fdt.c',
+  'fdt_addresses.c',
+  'fdt_check.c',
+  'fdt_empty_tree.c',
+  'fdt_overlay.c',
+  'fdt_ro.c',
+  'fdt_rw.c',
+  'fdt_strerror.c',
+  'fdt_sw.c',
+  'fdt_wip.c',
+)
+
+libfdt = library(
+  'fdt', sources,
+  version: '1.6.0',
+  link_args: ['-Wl,--no-undefined', version_script],
+  link_depends: 'version.lds',
+  install: true,
+)
+
+libfdt_inc = include_directories('.')
+
+libfdt_dep = declare_dependency(
+  include_directories: libfdt_inc,
+  link_with: libfdt,
+)
+
+install_headers(
+  files(
+    'fdt.h',
+    'libfdt.h',
+    'libfdt_env.h',
+  )
+)
+
+pkgconfig = import('pkgconfig')
+
+pkgconfig.generate(
+  libraries: libfdt,
+  version: meson.project_version(),
+  filebase: 'libfdt',
+  name: 'libfdt',
+  description: 'Flat Device Tree manipulation',
+)
diff --git a/meson.build b/meson.build
new file mode 100644
index 0000000..2c65104
--- /dev/null
+++ b/meson.build
@@ -0,0 +1,127 @@
+project('dtc', 'c',
+  version: '1.6.0',
+  license: ['GPL2+', 'BSD-2'],
+  default_options: 'werror=true',
+)
+
+cc = meson.get_compiler('c')
+
+add_project_arguments(cc.get_supported_arguments([
+  '-Wall',
+  '-Wpointer-arith',
+  '-Wcast-qual',
+  '-Wnested-externs',
+  '-Wstrict-prototypes',
+  '-Wmissing-prototypes',
+  '-Wredundant-decls',
+  '-Wshadow'
+]),language: 'c')
+
+if host_machine.system() == 'windows'
+  add_project_arguments(
+    '-D__USE_MINGW_ANSI_STDIO=1',
+    language: 'c'
+  )
+endif
+
+add_project_arguments(
+  '-DFDT_ASSUME_MASK=' + get_option('assume-mask').to_string(),
+  language: 'c'
+)
+
+yamltree = 'yamltree.c'
+yaml = dependency('yaml-0.1', required: get_option('yaml'))
+if not yaml.found()
+  add_project_arguments('-DNO_YAML', language: 'c')
+  yamltree = []
+endif
+
+valgrind = dependency('valgrind', required: get_option('valgrind'))
+if not valgrind.found()
+  add_project_arguments('-DNO_VALGRIND', language: 'c')
+endif
+
+py = import('python')
+py = py.find_installation(required: get_option('python'))
+swig = find_program('swig', required: get_option('python'))
+
+version_gen_h = vcs_tag(
+  input: 'version_gen.h.in',
+  output: 'version_gen.h',
+)
+
+subdir('libfdt')
+
+if get_option('tools')
+  flex = find_program('flex', required: true)
+  bison = find_program('bison', required: true)
+
+  util_dep = declare_dependency(
+    sources: ['util.c', version_gen_h],
+    include_directories: '.',
+    dependencies: libfdt_dep
+  )
+
+  lgen = generator(
+    flex,
+    output: '@PLAINNAME@.lex.c',
+    arguments: ['-o', '@OUTPUT@', '@INPUT@'],
+  )
+
+  pgen = generator(
+    bison,
+    output: ['@BASENAME@.tab.c', '@BASENAME@.tab.h'],
+    arguments: ['@INPUT@', '--defines=@OUTPUT1@', '--output=@OUTPUT0@'],
+  )
+
+  if cc.check_header('fnmatch.h')
+    executable(
+      'convert-dtsv0',
+      [
+        lgen.process('convert-dtsv0-lexer.l'),
+        'srcpos.c',
+      ],
+      dependencies: util_dep,
+      install: true,
+    )
+  endif
+
+  executable(
+    'dtc',
+    [
+      lgen.process('dtc-lexer.l'),
+      pgen.process('dtc-parser.y'),
+      'checks.c',
+      'data.c',
+      'dtc.c',
+      'flattree.c',
+      'fstree.c',
+      'livetree.c',
+      'srcpos.c',
+      'treesource.c',
+      yamltree,
+    ],
+    dependencies: [util_dep, yaml],
+    install: true,
+  )
+
+  foreach e: ['fdtdump', 'fdtget', 'fdtput', 'fdtoverlay']
+    executable(e, files(e + '.c'), dependencies: util_dep, install: true)
+  endforeach
+
+  install_data(
+    'dtdiff',
+    install_dir: get_option('prefix') / get_option('bindir'),
+    install_mode: 'rwxr-xr-x',
+  )
+endif
+
+if not meson.is_cross_build()
+  if py.found() and swig.found()
+    subdir('pylibfdt')
+  endif
+
+  if get_option('tools')
+    subdir('tests')
+  endif
+endif
diff --git a/meson_options.txt b/meson_options.txt
new file mode 100644
index 0000000..ea59c28
--- /dev/null
+++ b/meson_options.txt
@@ -0,0 +1,10 @@
+option('tools', type: 'boolean', value: true,
+       description: 'Build tools')
+option('assume-mask', type: 'integer', value: 0,
+       description: 'Control the assumptions made (e.g. risking security issues) in the code.')
+option('yaml', type: 'feature', value: 'auto',
+       description: 'YAML support')
+option('valgrind', type: 'feature', value: 'auto',
+       description: 'Valgrind support')
+option('python', type: 'feature', value: 'auto',
+       description: 'Build pylibfdt Python library')
diff --git a/pylibfdt/meson.build b/pylibfdt/meson.build
new file mode 100644
index 0000000..088f249
--- /dev/null
+++ b/pylibfdt/meson.build
@@ -0,0 +1,13 @@
+setup_py = find_program('setup.py')
+setup_py = [setup_py.path(), '--quiet', '--top-builddir', meson.current_build_dir() / '..']
+
+custom_target(
+  'pylibfdt',
+  input: 'libfdt.i',
+  output: '_libfdt.so',
+  depends: version_gen_h,
+  command: [setup_py, 'build_ext', '--build-lib=' + meson.current_build_dir()],
+  build_by_default: true,
+)
+
+meson.add_install_script(setup_py, 'install', '--prefix=' + get_option('prefix'), '--root=$DESTDIR')
diff --git a/tests/meson.build b/tests/meson.build
new file mode 100644
index 0000000..8976dc1
--- /dev/null
+++ b/tests/meson.build
@@ -0,0 +1,130 @@
+trees = static_library('trees', files('trees.S'), c_args: '-D__ASSEMBLY__',
+                       include_directories: libfdt_inc)
+
+dumptrees = executable('dumptrees', files('dumptrees.c'),
+                       link_with: trees, dependencies: libfdt_dep)
+
+dumptrees_dtb = custom_target(
+  'dumptrees',
+  command: [dumptrees, meson.current_build_dir()],
+  output: [
+    'test_tree1.dtb',
+    'bad_node_char.dtb',
+    'bad_node_format.dtb',
+    'bad_prop_char.dtb',
+    'ovf_size_strings.dtb',
+    'truncated_property.dtb',
+    'truncated_string.dtb',
+    'truncated_memrsv.dtb',
+  ]
+)
+
+testutil_dep = declare_dependency(sources: ['testutils.c'], link_with: trees)
+
+tests = [
+  'add_subnode_with_nops',
+  'addr_size_cells',
+  'addr_size_cells2',
+  'appendprop1',
+  'appendprop2',
+  'appendprop_addrrange',
+  'boot-cpuid',
+  'char_literal',
+  'check_full',
+  'check_header',
+  'check_path',
+  'del_node',
+  'del_property',
+  'dtb_reverse',
+  'dtbs_equal_ordered',
+  'dtbs_equal_unordered',
+  'extra-terminating-null',
+  'find_property',
+  'fs_tree1',
+  'get_alias',
+  'get_mem_rsv',
+  'get_name',
+  'get_path',
+  'get_phandle',
+  'get_prop_offset',
+  'getprop',
+  'incbin',
+  'integer-expressions',
+  'mangle-layout',
+  'move_and_save',
+  'node_check_compatible',
+  'node_offset_by_compatible',
+  'node_offset_by_phandle',
+  'node_offset_by_prop_value',
+  'nop_node',
+  'nop_property',
+  'nopulate',
+  'notfound',
+  'open_pack',
+  'overlay',
+  'overlay_bad_fixup',
+  'parent_offset',
+  'path-references',
+  'path_offset',
+  'path_offset_aliases',
+  'phandle_format',
+  'property_iterate',
+  'propname_escapes',
+  'references',
+  'root_node',
+  'rw_oom',
+  'rw_tree1',
+  'set_name',
+  'setprop',
+  'setprop_inplace',
+  'sized_cells',
+  'string_escapes',
+  'stringlist',
+  'subnode_iterate',
+  'subnode_offset',
+  'supernode_atdepth_offset',
+  'sw_states',
+  'sw_tree1',
+  'utilfdt_test',
+]
+
+tests += [
+  'truncated_memrsv',
+  'truncated_property',
+  'truncated_string',
+]
+
+dl = cc.find_library('dl', required: false)
+if dl.found()
+  tests += [
+    'asm_tree_dump',
+    'value-labels',
+  ]
+endif
+
+foreach t: tests
+  executable(t, files(t + '.c'), dependencies: [testutil_dep, util_dep, libfdt_dep, dl])
+endforeach
+
+run_tests = find_program('run_tests.sh')
+
+
+env = [
+  'PYTHON=' + py.path(),
+  'PYTHONPATH=' + meson.source_root() / 'pylibfdt',
+]
+
+if not py.found()
+  env += 'NO_PYTHON=1'
+endif
+if not yaml.found()
+  env += 'NO_YAML=1'
+endif
+
+test(
+  'run-test',
+  run_tests,
+  workdir: meson.current_build_dir(),
+  depends: dumptrees_dtb,
+  env: env,
+)
diff --git a/version_gen.h.in b/version_gen.h.in
new file mode 100644
index 0000000..7771abb
--- /dev/null
+++ b/version_gen.h.in
@@ -0,0 +1 @@
+#define DTC_VERSION "DTC @VCS_TAG@"
-- 
2.26.2

[PATCH v3 4/4] travis: test meson build

[marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA]

From: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Signed-off-by: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 .travis.yml | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index a5163de..e2d74a4 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -6,6 +6,12 @@ env:
   # COVERITY_SCAN_TOKEN (dgibson/dtc)
   - secure: "vlHvXe618//IM9LQaKzqsrUbjs7ng0L9UCST4kJbJnFQDXvVe5JiSmJGd4ef7mm0NUv5bMRl2W3xCiu6BYAu/NvU3tMNHoLG+JgCJs0+wLJXbWOwji/NmH7olqgJG+CmpaCMXjARF6+nrTnBYHJL6cYyf4KVoV4B0I/hLUW91+s="
 
+before_install:
+  - '[ $TRAVIS_CPU_ARCH = amd64 ] && sudo add-apt-repository -y "deb http://archive.ubuntu.com/ubuntu xenial-backports main universe" || sudo add-apt-repository -y "deb http://ports.ubuntu.com xenial-backports main universe"'
+  - sudo apt-get -q update
+  - sudo apt-get -t xenial-backports install -y python3 python3-setuptools python3-pip ninja-build
+  - sudo pip3 install meson
+
 matrix:
   include:
     - addons:
@@ -25,12 +31,16 @@ matrix:
       script:
         - make
         - make check && make checkm
+        - meson build
+        - ninja -C build test
 
     # Check it builds properly without optional packages:
     #     python, valgrind, libyaml
     - script:
         - make
         - make check
+        - meson build
+        - ninja -C build test
 
     - arch: arm64
       addons:
@@ -42,6 +52,8 @@ matrix:
       script:
         - make
         - make check checkm
+        - meson build
+        - ninja -C build test
 
     - arch: ppc64le
       addons:
@@ -52,6 +64,8 @@ matrix:
       script:
         - make
         - make check
+        - meson build
+        - ninja -C build test
 
     - arch: s390x
       addons:
@@ -63,3 +77,5 @@ matrix:
       script:
         - make
         - make check checkm
+        - meson build
+        - ninja -C build test
-- 
2.26.2

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

[David Gibson]

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

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.

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

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

Re: [PATCH v3 2/4] pylibfdt: fix build lib location

[David Gibson]

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

On Tue, Sep 15, 2020 at 11:27:03PM +0400, marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org wrote:
> From: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> 
> setup.py build_ext is run from top_srcdir with Makefile.
> 
> ../pylibfdt will produce output files in parent directory.

Uh.. can you elaborate a bit on what exactly ends up where, and why
the new behaviour you're setting up is better.

> Note that setup.py install will rebuild it with the default 'build'
> directory. There doesn't seem to be a way to override that.

That rings a bell.  ISTR it being a real PITA.

> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

> ---
>  pylibfdt/Makefile.pylibfdt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
> index 32ae1c5..1b5f236 100644
> --- a/pylibfdt/Makefile.pylibfdt
> +++ b/pylibfdt/Makefile.pylibfdt
> @@ -18,7 +18,7 @@ endif
>  
>  $(PYMODULE): $(PYLIBFDT_srcs) $(LIBFDT_archive) $(SETUP) $(VERSION_FILE)
>  	@$(VECHO) PYMOD $@
> -	$(PYTHON) $(SETUP) $(SETUPFLAGS) build_ext --build-lib=../$(PYLIBFDT_dir)
> +	$(PYTHON) $(SETUP) $(SETUPFLAGS) build_ext --build-lib=$(PYLIBFDT_dir)
>  
>  install_pylibfdt: $(PYMODULE)
>  	@$(VECHO) INSTALL-PYLIB

-- 
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 v3 2/4] pylibfdt: fix build lib location

[Marc-André Lureau]

Hi

On Mon, Sep 21, 2020 at 10:46 AM David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>
> On Tue, Sep 15, 2020 at 11:27:03PM +0400, marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org wrote:
> > From: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >
> > setup.py build_ext is run from top_srcdir with Makefile.
> >
> > ../pylibfdt will produce output files in parent directory.
>
> Uh.. can you elaborate a bit on what exactly ends up where, and why
> the new behaviour you're setting up is better.


 ~/src/dtc $  make
...
~/src/dtc $  ls ../pylibfdt
_libfdt.cpython-38-x86_64-linux-gnu.so


It "escapes" the source directory

>
> > Note that setup.py install will rebuild it with the default 'build'
> > directory. There doesn't seem to be a way to override that.
> That rings a bell.  ISTR it being a real PITA.
>

It's a minor inconvenience imho. You just end up with an extra build/
directory during install.

> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>
> > ---
> >  pylibfdt/Makefile.pylibfdt | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
> > index 32ae1c5..1b5f236 100644
> > --- a/pylibfdt/Makefile.pylibfdt
> > +++ b/pylibfdt/Makefile.pylibfdt
> > @@ -18,7 +18,7 @@ endif
> >
> >  $(PYMODULE): $(PYLIBFDT_srcs) $(LIBFDT_archive) $(SETUP) $(VERSION_FILE)
> >       @$(VECHO) PYMOD $@
> > -     $(PYTHON) $(SETUP) $(SETUPFLAGS) build_ext --build-lib=../$(PYLIBFDT_dir)
> > +     $(PYTHON) $(SETUP) $(SETUPFLAGS) build_ext --build-lib=$(PYLIBFDT_dir)
> >
> >  install_pylibfdt: $(PYMODULE)
> >       @$(VECHO) INSTALL-PYLIB
>
> --
> 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

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

[Marc-André Lureau]

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

Re: [PATCH v3 2/4] pylibfdt: fix build lib location

[David Gibson]

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

On Mon, Sep 21, 2020 at 12:09:16PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Sep 21, 2020 at 10:46 AM David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> >
> > On Tue, Sep 15, 2020 at 11:27:03PM +0400, marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org wrote:
> > > From: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > >
> > > setup.py build_ext is run from top_srcdir with Makefile.
> > >
> > > ../pylibfdt will produce output files in parent directory.
> >
> > Uh.. can you elaborate a bit on what exactly ends up where, and why
> > the new behaviour you're setting up is better.
> 
> 
>  ~/src/dtc $  make
> ...
> ~/src/dtc $  ls ../pylibfdt
> _libfdt.cpython-38-x86_64-linux-gnu.so
> 
> 
> It "escapes" the source directory

Sorry, still not clear to me.  Is that the old behaviour or the new?
How does it differ from the other one?

> > > Note that setup.py install will rebuild it with the default 'build'
> > > directory. There doesn't seem to be a way to override that.
> > That rings a bell.  ISTR it being a real PITA.
> 
> It's a minor inconvenience imho. You just end up with an extra build/
> directory during install.
> 
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >
> > > ---
> > >  pylibfdt/Makefile.pylibfdt | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
> > > index 32ae1c5..1b5f236 100644
> > > --- a/pylibfdt/Makefile.pylibfdt
> > > +++ b/pylibfdt/Makefile.pylibfdt
> > > @@ -18,7 +18,7 @@ endif
> > >
> > >  $(PYMODULE): $(PYLIBFDT_srcs) $(LIBFDT_archive) $(SETUP) $(VERSION_FILE)
> > >       @$(VECHO) PYMOD $@
> > > -     $(PYTHON) $(SETUP) $(SETUPFLAGS) build_ext --build-lib=../$(PYLIBFDT_dir)
> > > +     $(PYTHON) $(SETUP) $(SETUPFLAGS) build_ext --build-lib=$(PYLIBFDT_dir)
> > >
> > >  install_pylibfdt: $(PYMODULE)
> > >       @$(VECHO) INSTALL-PYLIB
> >
> 

-- 
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 v3 2/4] pylibfdt: fix build lib location

[Marc-André Lureau]

Hi

On Fri, Sep 25, 2020 at 2:40 PM David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>
> On Mon, Sep 21, 2020 at 12:09:16PM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > On Mon, Sep 21, 2020 at 10:46 AM David Gibson
> > <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > >
> > > On Tue, Sep 15, 2020 at 11:27:03PM +0400, marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org wrote:
> > > > From: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > >
> > > > setup.py build_ext is run from top_srcdir with Makefile.
> > > >
> > > > ../pylibfdt will produce output files in parent directory.
> > >
> > > Uh.. can you elaborate a bit on what exactly ends up where, and why
> > > the new behaviour you're setting up is better.
> >
> >
> >  ~/src/dtc $  make
> > ...
> > ~/src/dtc $  ls ../pylibfdt
> > _libfdt.cpython-38-x86_64-linux-gnu.so
> >
> >
> > It "escapes" the source directory
>
> Sorry, still not clear to me.  Is that the old behaviour or the new?

It's the current/old behaviour.

> How does it differ from the other one?

What I propose will put it again under $(curdir)/pylibfdt instead.

Apparently this was a regression from commit
1e4a0928f3b3b827824222572e551a60935607e3.

Since setup.py is invoked from top-level Makefile location, you
shouldn't have added ../ I guess.

>
> > > > Note that setup.py install will rebuild it with the default 'build'
> > > > directory. There doesn't seem to be a way to override that.
> > > That rings a bell.  ISTR it being a real PITA.
> >
> > It's a minor inconvenience imho. You just end up with an extra build/
> > directory during install.
> >
> > > >
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > >
> > > > ---
> > > >  pylibfdt/Makefile.pylibfdt | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
> > > > index 32ae1c5..1b5f236 100644
> > > > --- a/pylibfdt/Makefile.pylibfdt
> > > > +++ b/pylibfdt/Makefile.pylibfdt
> > > > @@ -18,7 +18,7 @@ endif
> > > >
> > > >  $(PYMODULE): $(PYLIBFDT_srcs) $(LIBFDT_archive) $(SETUP) $(VERSION_FILE)
> > > >       @$(VECHO) PYMOD $@
> > > > -     $(PYTHON) $(SETUP) $(SETUPFLAGS) build_ext --build-lib=../$(PYLIBFDT_dir)
> > > > +     $(PYTHON) $(SETUP) $(SETUPFLAGS) build_ext --build-lib=$(PYLIBFDT_dir)
> > > >
> > > >  install_pylibfdt: $(PYMODULE)
> > > >       @$(VECHO) INSTALL-PYLIB
> > >
> >
>
> --
> 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

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

[David Gibson]

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

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

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.

> 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

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

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

[Marc-André Lureau]

Hi

On Mon, Sep 28, 2020 at 12:03 PM David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> 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-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.
>
> 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-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

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

[David Gibson]

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

On Tue, Sep 29, 2020 at 03:46:16PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Sep 28, 2020 at 12:03 PM David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> 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-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > > >
> > > > On Tue, Sep 15, 2020 at 11:27:02PM +0400, marcandre.lureau@redhat.com 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.
> >
> > 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.

It's not the fact it's an extra argument that concerns me, only the
fact that it is a *required* extra argument.

-- 
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 v3 1/4] pylibfdt: allow build out of tree

[Marc-André Lureau]

Hi

On Tue, Sep 29, 2020 at 6:42 PM David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>
> On Tue, Sep 29, 2020 at 03:46:16PM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > On Mon, Sep 28, 2020 at 12:03 PM David Gibson
> > <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> 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-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > > > >
> > > > > On Tue, Sep 15, 2020 at 11:27:02PM +0400, marcandre.lureau@redhat.com 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@redhat.com>
> > > > > > ---
> > > > > >  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.
>
> It's not the fact it's an extra argument that concerns me, only the
> fact that it is a *required* extra argument.

I can try to make it default to the current directory, but it will
probably have to handle errors elsewhere then in more archaic ways.

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

[David Gibson]

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

On Tue, Sep 29, 2020 at 07:37:22PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Sep 29, 2020 at 6:42 PM David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> >
> > On Tue, Sep 29, 2020 at 03:46:16PM +0400, Marc-André Lureau wrote:
> > > Hi
> > >
> > > On Mon, Sep 28, 2020 at 12:03 PM David Gibson
> > > <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> 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-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > > > > >
> > > > > > On Tue, Sep 15, 2020 at 11:27:02PM +0400, marcandre.lureau@redhat.com 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@redhat.com>
> > > > > > > ---
> > > > > > >  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.
> >
> > It's not the fact it's an extra argument that concerns me, only the
> > fact that it is a *required* extra argument.
> 
> I can try to make it default to the current directory, but it will
> probably have to handle errors elsewhere then in more archaic ways.

Have a look, please.  If it proves really hard, I'll reconsider.

-- 
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 v3 2/4] pylibfdt: fix build lib location

[David Gibson]

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

On Fri, Sep 25, 2020 at 05:21:54PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Sep 25, 2020 at 2:40 PM David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> >
> > On Mon, Sep 21, 2020 at 12:09:16PM +0400, Marc-André Lureau wrote:
> > > Hi
> > >
> > > On Mon, Sep 21, 2020 at 10:46 AM David Gibson
> > > <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > > >
> > > > On Tue, Sep 15, 2020 at 11:27:03PM +0400, marcandre.lureau@redhat.com wrote:
> > > > > From: Marc-André Lureau <marcandre.lureau-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > > >
> > > > > setup.py build_ext is run from top_srcdir with Makefile.
> > > > >
> > > > > ../pylibfdt will produce output files in parent directory.
> > > >
> > > > Uh.. can you elaborate a bit on what exactly ends up where, and why
> > > > the new behaviour you're setting up is better.
> > >
> > >
> > >  ~/src/dtc $  make
> > > ...
> > > ~/src/dtc $  ls ../pylibfdt
> > > _libfdt.cpython-38-x86_64-linux-gnu.so
> > >
> > >
> > > It "escapes" the source directory
> >
> > Sorry, still not clear to me.  Is that the old behaviour or the new?
> 
> It's the current/old behaviour.
> 
> > How does it differ from the other one?
> 
> What I propose will put it again under $(curdir)/pylibfdt instead.
> 
> Apparently this was a regression from commit
> 1e4a0928f3b3b827824222572e551a60935607e3.
> 
> Since setup.py is invoked from top-level Makefile location, you
> shouldn't have added ../ I guess.

Ah!  Now I get it.  Can you expand the commit message bit to make that
clearer.

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