Skip to content

Commit

Permalink
build: Make Python bridge the default
Browse files Browse the repository at this point in the history
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 new
distcheck scenario for the C bridge, to ensure that we don't break that.

https://issues.redhat.com/browse/COCKPIT-1037
  • Loading branch information
martinpitt committed Sep 4, 2023
1 parent c56e6fe commit c39668b
Show file tree
Hide file tree
Showing 12 changed files with 33 additions and 36 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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' }
Expand Down Expand Up @@ -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 }}'
2 changes: 1 addition & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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::
Expand Down
14 changes: 7 additions & 7 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions containers/flatpak/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
1 change: 0 additions & 1 deletion containers/flatpak/prepare
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ def create_manifest(
'buildsystem': 'autotools',
'config-opts': [
'--enable-cockpit-client',
'--enable-pybridge',
'--disable-polkit',
'--disable-ssh',
'--disable-pcp',
Expand Down
2 changes: 1 addition & 1 deletion pkg/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/bridge/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/ws/Makefile-ws.am
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
3 changes: 1 addition & 2 deletions tools/arch/PKGBUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
19 changes: 8 additions & 11 deletions tools/cockpit.spec
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down Expand Up @@ -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
Expand All @@ -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 \
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions tools/debian/rules
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -48,7 +48,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*
Expand All @@ -59,14 +59,14 @@ endif

execute_after_dh_install-indep:
# avoid dh_missing failure
ifeq ($(C_BRIDGE),)
ifeq ($(OLD_BRIDGE),)
rm -r debian/tmp/usr/lib/python*
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

0 comments on commit c39668b

Please sign in to comment.