build: Make Python bridge the default #19246
Conversation
martinpitt
force-pushed
the
pybridge-default
branch
2 times, most recently
from
0006cee
to
a7c9885
Compare
martinpitt
added
the
no-test
martinpitt
force-pushed
the
pybridge-default
branch
from
a7c9885
to
c857e53
Compare
martinpitt
removed
the
no-test
martinpitt
force-pushed
the
pybridge-default
branch
from
c857e53
to
5d786ab
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
martinpitt
added
the
no-test
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
martinpitt
force-pushed
the
pybridge-default
branch
from
5d786ab
to
c39668b
Compare
|
@allisonkarlitskaya Still no-test, but the unit tests work. Would you mind doing another round of review before I re-enable the full impact of integration tests? Danke! |
martinpitt
removed
the
no-test
martinpitt
force-pushed
the
pybridge-default
branch
from
c39668b
to
ba30d58
Compare
martinpitt
added
the
.github-changes
| @@ -13,6 +13,9 @@ WORKDIR /home/builder | |||
|
|
|||
| ENV LANG=C.UTF-8 | |||
|
|
|||
| # HACK: unbreak distcheck on Debian: https://bugs.debian.org/1035546 | |||
| ENV DEB_PYTHON_INSTALL_LAYOUT=deb | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
If I've wrapped my head around this situation properly then it also means that something like this, for example:
./configure
make
sudo make install
would result in Cockpit getting installed into /usr/local/local/ on Debian ...
There was a problem hiding this comment.
The crux of the issue is that although sysconfig has a check for situations where the prefix is not the same as the base prefix (ie: venvs) it doesn't consider pip's --prefix flag when applying this logic...
There was a problem hiding this comment.
notes for myself:
- check the
sudo make installbehaviour from above - possibly: toss away
pip installand install the wheel ourselves; create single-line wrappers for cockpit-{bridge,askpass,beiboot} in our tree and install via scripts directory; use#!/usr/bin/env python3hashbangs
There was a problem hiding this comment.
Indeed a make install DESTDIR=/tmp/i in Debian creates /tmp/i/usr/local/local/lib/python3.11, and /tmp/i/usr/local/local/bin/cockpit-{bridge,askpass,beiboot} as we suspected.
I tried to convert to python3-installer:
--- src/Makefile.am
+++ src/Makefile.am
@@ -26,7 +26,7 @@ install-python:
@# wheel-based installation with .dist-info.
@# This needs to work on RHEL8 up through modern Fedora, offline, with
@# system packages available to the build.
- python3 -m pip install --no-index --force-reinstall --root='$(DESTDIR)/' --prefix='$(prefix)' \
+ python3 -m installer --destdir='$(DESTDIR)/' --prefix='$(prefix)' \
"$$(python3 '$(srcdir)'/src/build_backend.py --wheel '$(srcdir)' tmp/wheel)"
mkdir -p $(DESTDIR)$(libexecdir)
mv -t $(DESTDIR)$(libexecdir) $(DESTDIR)$(bindir)/cockpit-askpass $(DESTDIR)$(bindir)/cockpit-beibootand it has the same problem. That's unsurprising, as the offending patch is in Debian's cpython, not in pip.
|
@allisonkarlitskaya AFAICS we need to pick one of these:
My proposal would be 2. (i.e. this PR as-is) and I'll do a follow-up for 4. (which will drop the hack). Or if you prefer, 4. first and then this PR without the _LAYOUT hack. WDYT? |
|
I'm fine with 2 as well, but what about the broken |
|
Yeah, it'll fail on "cockpit-askpass not found", but what can you do.. We ship stable and backported cockpit packages in the distro, and even provide a mechanism to build debs from upstream. So far nobody has complained, and I'd only like to go that far in trying to work around this. |
This has to go through SSH authentication and remote page loading, which occasionally takes more than 15s on loaded CI infrastructure.
This simplifies the hack: We no longer need the temporary symlink dance, but let the upstream build system put the Python module directly into the distro target path that `dh_python` will use anyway.
Fixes distcheck when configuring for with the Python bridge. This also needs to remove tmp/wheel/, as otherwise it counts as a leftover file in the source tree with `make distcheck`.
Replace --enable-pybridge with --enable-old-bridge, and flip the logic. That way, it will slowly disappear as old distro releases become unsupported. This also means that builders from upstream now get the Python bridge by default. The distcheck scenarios now apply to the Python bridge. Add a `$DEB_PYTHON_INSTALL_LAYOUT` hack to work around https://bugs.debian.org/1035546 to unbreak the installation of the generated wrapper binaries, as by default they'd go into prefix/usr/local/bin on Debian. Add a new distcheck scenario for the C bridge, to ensure that we don't break that. https://issues.redhat.com/browse/COCKPIT-1037
martinpitt
force-pushed
the
pybridge-default
branch
from
ba30d58
to
b82c60f
Compare
github-actions
bot
removed
the
.github-changes
martinpitt
added
the
.github-changes
| # HACK: Debian's pip breaks --prefix: https://bugs.debian.org/1035546 with | ||
| # default install layout | ||
| override_dh_auto_install: | ||
| DEB_PYTHON_INSTALL_LAYOUT=deb dh_auto_install |
There was a problem hiding this comment.
This looks good, thanks.
Replace --enable-pybridge with --enable-old-bridge, and flip the logic. That way, it will slowly disappear as old distro releases become unsupported. This also means that builders from upstream now get the Python bridge by default.
Keep the unit tests running with the C bridge for the time being, to exercise it with multiple architectures/compilers/valgrind.
https://issues.redhat.com/browse/COCKPIT-1037