Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pylibfdt/meson.build: fix Python library being rebuilt during install #136

Closed
wants to merge 2 commits into from

Conversation

blmaier
Copy link
Contributor

@blmaier blmaier commented Jun 23, 2024

User @sharkcz noted that when the '--quiet' flag is removed from ./setup.py, the following can be seen from the ./setup.py install stage.

  Running custom install script 'dtc/g/pylibfdt/../setup.py --top-builddir \
    dtc/g/redhat-linux-build install --prefix=/usr/local --root=$DESTDIR'
  running install
  ...
  building '_libfdt' extension
  swigging dtc/g/pylibfdt/../pylibfdt/libfdt.i to \
    dtc/g/pylibfdt/../pylibfdt/libfdt_wrap.c
  swig -python -Idtc/g/pylibfdt/../libfdt -o \
    dtc/g/pylibfdt/../pylibfdt/libfdt_wrap.c dtc/g/pylibfdt/../pylibfdt/libfdt.i
  gcc -fno-strict-overflow -Wsign-compare -DDYNAMIC_ANNOTATIONS_ENABLED=1 \
    -DNDEBUG -fexceptions -fexceptions -fexceptions -fPIC -DPY_SSIZE_T_CLEAN \
    -Idtc/g/pylibfdt/../libfdt -I/usr/include/python3.12 -c \
    dtc/g/pylibfdt/../pylibfdt/libfdt_wrap.c -o \
    build/temp.linux-ppc64le-cpython-312dtc/g/pylibfdt/../pylibfdt/libfdt_wrap.o
  creating build/lib.linux-ppc64le-cpython-312
  gcc -shared build/temp.linux-ppc64le-cpython-312dtc/g/pylibfdt/../pylibfdt/libfdt_wrap.o \
    -Ldtc/g/redhat-linux-build/libfdt -L/usr/lib64 -lfdt -o \
    build/lib.linux-ppc64le-cpython-312/_libfdt.cpython-312-powerpc64le-linux-gnu.so
  copying dtc/g/pylibfdt/../pylibfdt/libfdt.py -> build/lib.linux-ppc64le-cpython-312

Meaning the python library is getting recompiled during the meson install phase. This causes build issues as Meson does not set the compiler and linker flags during the install phase.

The reason the library is getting rebuilt is during the normal build with "build_ext", the --build-lib flag gets passed which changes the default output build directory. But there is no equivalent option for the "install" command. Install instead looks in the default directory "./build" and so does not find the previously built library.

Since we can't fix the "install" command, drop the --build-lib flag. This causes setup.py to compile the libraries at
<meson-build>/build/lib.linux-x86_64-cpython-312/. We must also then fix run_tests.sh to find the library build directory as it's machine-dependent.

Fixes: #135

@dgibson
Copy link
Owner

dgibson commented Jun 24, 2024

@blmaier thanks for the patch. Unfortunately, something still seems to be missing. It appears the logic you added to find the library during the tests only works on some distros when making meson builds: as you can see the github actions for alpine and fedora are failing, and likewise a local meson build fails the tests for me, because it's unable to import the libfdt module.

User @sharkcz noted that when the '--quiet' flag is removed from
./setup.py, the following can be seen from the `./setup.py install`
stage.

  Running custom install script 'dtc/g/pylibfdt/../setup.py --top-builddir \
    dtc/g/redhat-linux-build install --prefix=/usr/local --root=$DESTDIR'
  running install
  ...
  building '_libfdt' extension
  swigging dtc/g/pylibfdt/../pylibfdt/libfdt.i to \
    dtc/g/pylibfdt/../pylibfdt/libfdt_wrap.c
  swig -python -Idtc/g/pylibfdt/../libfdt -o \
    dtc/g/pylibfdt/../pylibfdt/libfdt_wrap.c dtc/g/pylibfdt/../pylibfdt/libfdt.i
  gcc -fno-strict-overflow -Wsign-compare -DDYNAMIC_ANNOTATIONS_ENABLED=1 \
    -DNDEBUG -fexceptions -fexceptions -fexceptions -fPIC -DPY_SSIZE_T_CLEAN \
    -Idtc/g/pylibfdt/../libfdt -I/usr/include/python3.12 -c \
    dtc/g/pylibfdt/../pylibfdt/libfdt_wrap.c -o \
    build/temp.linux-ppc64le-cpython-312dtc/g/pylibfdt/../pylibfdt/libfdt_wrap.o
  creating build/lib.linux-ppc64le-cpython-312
  gcc -shared build/temp.linux-ppc64le-cpython-312dtc/g/pylibfdt/../pylibfdt/libfdt_wrap.o \
    -Ldtc/g/redhat-linux-build/libfdt -L/usr/lib64 -lfdt -o \
    build/lib.linux-ppc64le-cpython-312/_libfdt.cpython-312-powerpc64le-linux-gnu.so
  copying dtc/g/pylibfdt/../pylibfdt/libfdt.py -> build/lib.linux-ppc64le-cpython-312

Meaning the python library is getting recompiled during the `meson
install` phase. This causes build issues as Meson does not set the
compiler and linker flags during the install phase.

The reason the library is getting rebuilt is during the normal build
with "build_ext", the `--build-lib` flag gets passed which changes the
default output build directory. But there is no equivalent option for
the "install" command. Install instead looks in the default directory
"./build" and so does not find the previously built library.

Since we can't fix the "install" command, drop the --build-lib flag.
This causes setup.py to compile the libraries at
`<meson-build>/build/lib.linux-x86_64-cpython-312/`. We must also then
fix run_tests.sh to find the library build directory as it's
machine-dependent.

Fixes: dgibson#135
Signed-off-by: Brandon Maier <[email protected]>
@blmaier blmaier force-pushed the python-extension-rebuild branch from 56e0e88 to 6572aca Compare June 24, 2024 02:42
@blmaier
Copy link
Contributor Author

blmaier commented Jun 24, 2024

@dgibson weird I can't reproduce this locally, but I was able to fix it with the github workflow.

There does seem to be a clang64 test failing under Windows. Though it appears this is failing on 'main' so that is likely unrelated 😃

@dgibson
Copy link
Owner

dgibson commented Jun 24, 2024

@blmaier thanks for the update

@dgibson weird I can't reproduce this locally, but I was able to fix it with the github workflow.

Huh, odd.

There does seem to be a clang64 test failing under Windows. Though it appears this is failing on 'main' so that is likely unrelated 😃

Yes, that's been around for a while, the reason is not obvious to me but seems to be something with the environment rather than a bug in dtc itself. Don't really have the Windows knowledge to fix it.

@@ -6,7 +6,7 @@ pylibfdt = custom_target(
input: 'libfdt.i',
depends: libfdt,
output: '_libfdt.so',
command: [setup_py, 'build_ext', '--build-lib=' + meson.current_build_dir()],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you remove --build-lib here, but it's still used in the Makefile build. Is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, I forgot to check the Makefiles. It has the same issue with rebuilding between a make all -> make install. Removing --build-lib fixes it there too. Pushed a second commit to fix Makefile.

…nstall

The Python module gets built during the 'make install' command even if
'make all' was run prior for build.

This is the same issue as described in the previous commit
"pylibfdt/meson.build: fix Python library being rebuilt during install".
Remove the '--build-lib' flag so setuptools can find the build module.

Signed-off-by: Brandon Maier <[email protected]>
@dgibson
Copy link
Owner

dgibson commented Jun 26, 2024

Merged, thanks.

@dgibson dgibson closed this Jun 26, 2024
@sharkcz
Copy link
Contributor

sharkcz commented Jun 27, 2024

for the record, it resolves the issue for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

python extension rebuilt during meson's install phase
3 participants