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.

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
  • Loading branch information
martinpitt committed Aug 26, 2023
1 parent fb673e0 commit c857e53
Show file tree
Hide file tree
Showing 11 changed files with 29 additions and 42 deletions.
3 changes: 2 additions & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ 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
# testing with multiple architectures/compilers/valgrind applies to the C bridge; pybridge has its own pytest scenario
DISTCHECK_CONFIGURE_FLAGS = --enable-prefix-only --enable-old-bridge

# 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
7 changes: 0 additions & 7 deletions pkg/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,6 @@ $(DIST_STAMP): $(srcdir)/package-lock.json $(PKG_INPUTS)

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
-include $(wildcard pkg/Makefile.qunit*)
endif

INSTALL_DATA_LOCAL_TARGETS += install-bundles
install-bundles:
cd $(srcdir)/dist; find */* -type f -exec install -D -m 644 '{}' '$(abspath $(DESTDIR)$(datadir))/cockpit/{}' \;
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 stable release; also, the old Python breaks some features
%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 @@ -57,7 +57,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 @@ -68,14 +68,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 c857e53

Please sign in to comment.