build: Make Python bridge the default

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

.github/workflows/unit-tests-refresh.yml

@@ -36,6 +36,10 @@ jobs:
         timeout-minutes: 15
         run: containers/unit-tests/start --verbose --env=CC=clang --image-tag=i386 --make distcheck
 
+      - name: Run amd64 gcc distcheck test for C bridge
+        timeout-minutes: 15
+        run: containers/unit-tests/start --verbose --env=CC=gcc --env=EXTRA_DISTCHECK_CONFIGURE_FLAGS=--enable-old-bridge --image-tag=latest --make distcheck
+
       - name: Run pytest-cov test
         timeout-minutes: 15
         run: containers/unit-tests/start --verbose --env=CC=gcc --image-tag=latest --make pytest-cov

.github/workflows/unit-tests.yml

@@ -9,8 +9,11 @@ jobs:
         startarg:
           # avoid check-memory on i386, it has literally thousands of uninteresting/wrong errors
           - { make: 'check-memory', cc: 'gcc', tag: 'latest' }
+          # with default Python bridge
           - { make: 'distcheck', cc: 'clang', tag: 'latest' }
           - { make: 'distcheck', cc: 'gcc', tag: 'i386' }
+          # with old C bridge
+          - { make: 'distcheck', cc: 'gcc', distcheck_flags: '--enable-old-bridge', tag: 'latest' }
           # this runs static code checks, unlike distcheck
           - { make: 'check', cc: 'gcc', tag: 'latest' }
           - { make: 'pytest-cov', cc: 'gcc', tag: 'latest' }
@@ -43,5 +46,6 @@ jobs:
               --env=FORCE_COLOR=1 \
               --env=CC='${{ matrix.startarg.cc }}' \
               --env=CFLAGS='-O2 -gdwarf-4' \
+              --env=EXTRA_DISTCHECK_CONFIGURE_FLAGS='${{ matrix.startarg.distcheck_flags }}' \
               --image-tag='${{ matrix.startarg.tag }}' \
               --make '${{ matrix.startarg.make }}'

Makefile.am

@@ -67,7 +67,7 @@ dist-hook: $(distdir)/tools/debian/copyright
 $(distdir)/tools/debian/copyright: $(DIST_STAMP)
 	$(AM_V_GEN) NODE_ENV=$(NODE_ENV) $(srcdir)/tools/build-debian-copyright > $@
 
-DISTCHECK_CONFIGURE_FLAGS = --enable-prefix-only
+DISTCHECK_CONFIGURE_FLAGS = --enable-prefix-only $(EXTRA_DISTCHECK_CONFIGURE_FLAGS)
 
 # Validate our AppStream metadata
 distcheck-hook::

configure.ac

@@ -76,11 +76,11 @@ AC_ARG_ENABLE(ssh, AS_HELP_STRING([--disable-ssh], [Disable cockpit-ssh build an
 AM_CONDITIONAL(WITH_COCKPIT_SSH, test "$enable_ssh" != "no")
 AC_MSG_RESULT(${enable_ssh:=yes})
 
-# --enable-pybridge
-AC_MSG_CHECKING([whether to install the python cockpit-bridge])
-AC_ARG_ENABLE(pybridge, AS_HELP_STRING([--enable-pybridge], [Install python cockpit-bridge]))
-AM_CONDITIONAL(WITH_PYBRIDGE, test "$enable_pybridge" = "yes")
-AC_MSG_RESULT(${enable_pybridge:=no})
+# --enable-old-bridge
+AC_MSG_CHECKING([whether to install the old C cockpit-bridge])
+AC_ARG_ENABLE(old_bridge, AS_HELP_STRING([--enable-old-bridge], [Install old C cockpit-bridge]))
+AM_CONDITIONAL(WITH_OLD_BRIDGE, test "$enable_old_bridge" = "yes")
+AC_MSG_RESULT(${enable_old_bridge:=no})
 
 
 # pkg-config
@@ -354,8 +354,8 @@ AC_ARG_ENABLE([cockpit-client],
 AC_MSG_RESULT($enable_cockpit_client)
 AM_CONDITIONAL([ENABLE_COCKPIT_CLIENT], [test "$enable_cockpit_client" = "yes"])
 
-if test "$enable_cockpit_client" = "yes" && test "$enable_pybridge" = "no"; then
-        AC_MSG_ERROR([--enable-cockpit-client requires --enable-pybridge])
+if test "$enable_cockpit_client" = "yes" && test "$enable_old_bridge" = "yes"; then
+        AC_MSG_ERROR([--enable-cockpit-client conflicts with --enable-old-bridge])
 fi
 
 # Debug

containers/flatpak/Makefile.am

@@ -11,12 +11,10 @@ INSTALL_FLATPAK_TARGETS = \
 	install-dist_scalableiconDATA \
 	install-dist_symboliciconDATA \
 	install-cockpit-client-symlink \
+	install-python \
+	install-bundles \
 	$(NULL)
 
-if WITH_PYBRIDGE
-INSTALL_FLATPAK_TARGETS += install-python install-bundles
-endif
-
 install-for-flatpak: $(INSTALL_FLATPAK_TARGETS)
 	appstream-util validate --nonet src/client/org.cockpit_project.CockpitClient.metainfo.xml
 	if test -s "${DOWNSTREAM_RELEASES_XML}"; then \

containers/flatpak/prepare

@@ -101,7 +101,6 @@ def create_manifest(
                 'buildsystem': 'autotools',
                 'config-opts': [
                     '--enable-cockpit-client',
-                    '--enable-pybridge',
                     '--disable-polkit',
                     '--disable-ssh',
                     '--disable-pcp',

containers/unit-tests/Dockerfile

@@ -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
+
 VOLUME /source
 
 COPY entrypoint /

pkg/Makefile.am

@@ -44,7 +44,7 @@ EXTRA_DIST += build.js files.js package.json package-lock.json
 # This is how the qunit tests get included.  We need to prevent automake from
 # seeing them during ./autogen.sh, but need make to find them at compile time.
 # We don't run them in the pybridge case since they're part of `pytest`.
-if !WITH_PYBRIDGE
+if WITH_OLD_BRIDGE
 -include $(wildcard pkg/Makefile.qunit*)
 endif
 

src/Makefile.am

@@ -20,7 +20,7 @@ pytest-cov: $(BUILT_SOURCES) $(DIST_STAMP) $(MANIFESTS)
 	$(MAKE) test-server
 	cd '$(srcdir)' && abs_builddir='$(abs_builddir)' pytest --cov
 
-if WITH_PYBRIDGE
+if !WITH_OLD_BRIDGE
 INSTALL_DATA_LOCAL_TARGETS += install-python
 install-python:
 	@# wheel-based installation with .dist-info.

src/bridge/Makefile.am

@@ -38,7 +38,7 @@ libcockpit_metrics_a_SOURCES = \
 	src/bridge/cockpitsamples.h \
 	$(NULL)
 
-if !WITH_PYBRIDGE
+if WITH_OLD_BRIDGE
 
 # -----------------------------------------------------------------------------
 # libcockpit-bridge.a: code used in cockpit-bridge and its tests

src/ws/Makefile-ws.am

@@ -132,7 +132,7 @@ test_kerberos_CPPFLAGS = $(libcockpit_ws_a_CPPFLAGS) $(TEST_CPP)
 test_kerberos_LDADD = $(libcockpit_ws_a_LIBS) $(TEST_LIBS) $(krb5_LIBS)
 test_kerberos_SOURCES = src/ws/test-kerberos.c
 
-if !WITH_PYBRIDGE
+if WITH_OLD_BRIDGE
 
 # These are -ws tests but they involve invoking ./cockpit-bridge.
 

tools/arch/PKGBUILD

@@ -41,8 +41,7 @@ build() {
     --disable-dependency-tracking \
     --disable-silent-rules \
     --with-cockpit-user=cockpit-ws \
-    --with-cockpit-ws-instance-user=cockpit-wsinstance \
-    --enable-pybridge
+    --with-cockpit-ws-instance-user=cockpit-wsinstance
 
   make all
 }

tools/cockpit.spec

@@ -53,12 +53,9 @@ Version:        0
 Release:        1%{?dist}
 Source0:        https://github.com/cockpit-project/cockpit/releases/download/%{version}/cockpit-%{version}.tar.xz
 
-%if 0%{?fedora} >= 38 || 0%{?rhel} >= 9
-%define cockpit_enable_python 1
-%endif
-
-%if !%{defined cockpit_enable_python}
-%define cockpit_enable_python 0
+# Don't change the bridge in the RHEL 8; the old SSH breaks some features, see @todoPybridgeRHEL8
+%if 0%{?rhel} == 8
+%define enable_old_bridge 1
 %endif
 
 # in RHEL 8 the source package is duplicated: cockpit (building basic packages like cockpit-{bridge,system})
@@ -170,7 +167,7 @@ Suggests: cockpit-selinux
 Requires: subscription-manager-cockpit
 %endif
 
-%if %{cockpit_enable_python}
+%if 0%{?enable_old_bridge} == 0
 BuildRequires:  python3-devel
 BuildRequires:  python3-pip
 %if 0%{?rhel} == 0
@@ -196,8 +193,8 @@ BuildRequires:  python3-tox-current-env
     --docdir=%_defaultdocdir/%{name} \
 %endif
     --with-pamdir='%{pamdir}' \
-%if %{cockpit_enable_python}
-    --enable-pybridge \
+%if 0%{?enable_old_bridge}
+    --enable-old-bridge \
 %endif
 %if 0%{?build_basic} == 0
     --disable-ssh \
@@ -208,7 +205,7 @@ BuildRequires:  python3-tox-current-env
 %check
 make -j$(nproc) check
 
-%if %{cockpit_enable_python} && 0%{?rhel} == 0
+%if 0%{?enable_old_bridge} == 0 && 0%{?rhel} == 0
 %tox
 %endif
 
@@ -374,7 +371,7 @@ system on behalf of the web based user interface.
 %doc %{_mandir}/man1/cockpit-bridge.1.gz
 %{_bindir}/cockpit-bridge
 %{_libexecdir}/cockpit-askpass
-%if %{cockpit_enable_python}
+%if 0%{?enable_old_bridge} == 0
 %{python3_sitelib}/%{name}*
 %{_libexecdir}/cockpit-beiboot
 %endif

tools/debian/rules

@@ -3,9 +3,9 @@
 DEB_HOST_MULTIARCH ?= $(shell dpkg-architecture -qDEB_HOST_MULTIARCH)
 
 # Keep the older C bridge on stable releases
-C_BRIDGE = $(filter $(shell . /etc/os-release; echo $${VERSION_ID:-unstable}),11 12 22.04)
-ifeq ($(C_BRIDGE),)
-CONFIG_OPTIONS += --enable-pybridge
+OLD_BRIDGE = $(filter $(shell . /etc/os-release; echo $${VERSION_ID:-unstable}),11 12 22.04)
+ifneq ($(OLD_BRIDGE),)
+CONFIG_OPTIONS += --enable-old-bridge
 endif
 
 # riscv is an emulated architecture for now, and too slow to run expensive unit tests
@@ -53,7 +53,7 @@ override_dh_install:
 	rm debian/tmp/usr/share/metainfo/org.cockpit-project.cockpit-selinux.metainfo.xml
 
 	dh_install -Xusr/src/debug
-ifeq ($(C_BRIDGE),)
+ifeq ($(OLD_BRIDGE),)
 	# we don't need this, it contains full build paths and breaks reproducibility
 	rm -r debian/tmp/usr/lib/python*/*-packages/*.dist-info
 	dh_install -p cockpit-bridge debian/tmp/usr/lib/python*
@@ -64,14 +64,14 @@ endif
 
 execute_after_dh_install-indep:
 	# avoid dh_missing failure
-ifeq ($(C_BRIDGE),)
+ifeq ($(OLD_BRIDGE),)
 	rm -r debian/tmp/usr/lib/python* debian/tmp/usr/lib/cockpit/cockpit-beiboot
 endif
 
 # run pytests *after* installation, so that we can make sure that we installed the right files
 execute_after_dh_install-arch:
 ifeq (, $(findstring nocheck, $(DEB_BUILD_OPTIONS)))
-ifeq ($(C_BRIDGE),)
+ifeq ($(OLD_BRIDGE),)
 	pytest -vv -k 'not linter and not test_descriptions' -opythonpath=$$(ls -d debian/cockpit-bridge/usr/lib/python3*/dist-packages)
 endif
 endif