Merge ~athos-ribeiro/ubuntu/+source/squid:halt-build-on-test-failures-kinetic into ubuntu/+source/squid:ubuntu/kinetic-devel

Proposed by Athos Ribeiro
Status: Merged
Approved by: git-ubuntu bot
Approved revision: not available
Merged at revision: 4dbcd05f140564b189e6d0789ba52d90e0d98fc7
Proposed branch: ~athos-ribeiro/ubuntu/+source/squid:halt-build-on-test-failures-kinetic
Merge into: ubuntu/+source/squid:ubuntu/kinetic-devel
Diff against target: 122 lines (+35/-33)
5 files modified
debian/changelog (+16/-0)
debian/patches/series (+0/-1)
debian/rules (+15/-2)
debian/tests/upstream-test-suite (+4/-0)
dev/null (+0/-30)
Reviewer Review Type Date Requested Status
git-ubuntu bot Approve
Bryce Harrington (community) Approve
Canonical Server Reporter Pending
Review via email: mp+438831@code.launchpad.net

This proposal supersedes a proposal from 2023-02-03.

Description of the change

I am re-submitting these kinetic changes to include vorlon's suggestions raised during the SRU review in LP: #2004050.

The relevant changes are
- Disabling specific compiler errors instead of disabling LTO in s390x
- Removing patch instead of commenting it out in d/p/series to ease SRU review process.

As it is being discussed in [1], squid builds will not halt upon upstream unit test failures during dh_auto_test. This could hinder the squid MRE test plan process [2].

This MP proposes a patch set to force squid builds to fail upon test failures. It also includes additional changes to ensure build time and autopkgtests are also passing for those upstream tests.

This is the kinetic SRU for the change applied in lunar in [3]

[1] https://lists.ubuntu.com/archives/ubuntu-release/2023-January/005522.html
[2] https://wiki.ubuntu.com/SquidUpdates
PPA: https://launchpad.net/~athos-ribeiro/+archive/ubuntu/squid-halt-build-on-test-failures/+packages
[3] https://code.launchpad.net/~athos-ribeiro/ubuntu/+source/squid/+git/squid/+merge/436437

To post a comment you must log in.
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote : Posted in a previous version of this proposal

Note that I already have

- verified that the s390x LTO change is needed here; and
- tested a build without removing the 0003 patch, which did lead to a build failure due to failing tests, as expected.

Revision history for this message
Bryce Harrington (bryce) wrote : Posted in a previous version of this proposal

Should this be targeting the applied branch? Or am I confused?

With this targeting kinetic, should version number be 5.6-1ubuntu3.1?

review: Needs Fixing
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote : Posted in a previous version of this proposal

Thanks, Bryce!

I fixed the target branch and updated the version string.

Revision history for this message
Bryce Harrington (bryce) wrote : Posted in a previous version of this proposal

Thanks, looks good now!

review: Approve
Revision history for this message
git-ubuntu bot (git-ubuntu-bot) wrote : Posted in a previous version of this proposal

Approvers: athos-ribeiro, bryce
Uploaders: athos-ribeiro, bryce
MP auto-approved

review: Approve
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote : Posted in a previous version of this proposal

Uploaded:

Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading squid_5.6-1ubuntu3.1.dsc: done.
  Uploading squid_5.6-1ubuntu3.1.debian.tar.xz: done.
  Uploading squid_5.6-1ubuntu3.1_source.buildinfo: done.
  Uploading squid_5.6-1ubuntu3.1_source.changes: done.
Successfully uploaded packages.

Revision history for this message
Bryce Harrington (bryce) wrote :

The changes themselves LGTM, but the changelog could benefit from some additional clarifications (inline below). With those changes applied, this LGTM.

review: Approve
Revision history for this message
git-ubuntu bot (git-ubuntu-bot) wrote :

Approvers: athos-ribeiro, bryce
Uploaders: athos-ribeiro, bryce
MP auto-approved

review: Approve
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

Thanks, Bryce!

I updated the changelog (and commit message) as requested; added more information to the SRU bug to describe all the needed changes listed in the changelog; removed the accidental blank line being added to d/rules; and will check if debian would benefit from the s390x specific d/rules bit (that is a workaround for LTO issues).

Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

Uploaded

Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading squid_5.6-1ubuntu3.1.dsc: done.
  Uploading squid_5.6-1ubuntu3.1.debian.tar.xz: done.
  Uploading squid_5.6-1ubuntu3.1_source.buildinfo: done.
  Uploading squid_5.6-1ubuntu3.1_source.changes: done.
Successfully uploaded packages.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 9bd5a20..396cc68 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,19 @@
6+squid (5.6-1ubuntu3.1) kinetic; urgency=medium
7+
8+ * Make builds fail when upstream test suite fails (LP: #2004050):
9+ - d/p/0003-installed-binary-for-debian-ci.patch: dropped. Do not rely on
10+ installed binaries for build time tests since this would lead to test
11+ failures.
12+ - d/rules: halt build upon test failures.
13+ - d/rules: do not include additional configuration files during
14+ build time tests. This would lead to test failures due to missing
15+ paths.
16+ - d/t/upstream-test-suite: use installed squid binary for
17+ autopkgtest config file checks.
18+ - d/rules: disable LTO related compilation errors for s390x builds.
19+
20+ -- Athos Ribeiro <athos.ribeiro@canonical.com> Mon, 30 Jan 2023 20:12:50 -0300
21+
22 squid (5.6-1ubuntu3) kinetic; urgency=medium
23
24 * SECURITY UPDATE: Exposure of Sensitive Information in Cache Manager
25diff --git a/debian/patches/0003-installed-binary-for-debian-ci.patch b/debian/patches/0003-installed-binary-for-debian-ci.patch
26deleted file mode 100644
27index fb1ce64..0000000
28--- a/debian/patches/0003-installed-binary-for-debian-ci.patch
29+++ /dev/null
30@@ -1,30 +0,0 @@
31-From: Amos Jeffries <amosjeffries@squid-cache.org>
32-Date: Sat, 21 Jul 2018 21:07:00 +1300
33-Subject: Use installed squid binary for Debian CI testing
34-
35-Index: pkg-squid/test-suite/Makefile.am
36-===================================================================
37---- pkg-squid.orig/test-suite/Makefile.am
38-+++ pkg-squid/test-suite/Makefile.am
39-@@ -150,7 +150,7 @@ VirtualDeleteOperator_SOURCES = VirtualD
40- squid-conf-tests: $(top_builddir)/src/squid.conf.default $(srcdir)/squidconf/*
41- @failed=0; cfglist="$?"; rm -f $@ || $(TRUE); \
42- for cfg in $$cfglist ; do \
43-- $(top_builddir)/src/squid -k parse -f $$cfg || \
44-+ /usr/sbin/squid -k parse -f $$cfg || \
45- { echo "FAIL: squid.conf test: $$cfg" | \
46- sed s%$(top_builddir)/src/%% | \
47- sed s%$(srcdir)/squidconf/%% ; \
48-Index: pkg-squid/test-suite/Makefile.in
49-===================================================================
50---- pkg-squid.orig/test-suite/Makefile.in
51-+++ pkg-squid/test-suite/Makefile.in
52-@@ -1477,7 +1477,7 @@ STUB.h: $(top_srcdir)/src/tests/STUB.h
53- squid-conf-tests: $(top_builddir)/src/squid.conf.default $(srcdir)/squidconf/*
54- @failed=0; cfglist="$?"; rm -f $@ || $(TRUE); \
55- for cfg in $$cfglist ; do \
56-- $(top_builddir)/src/squid -k parse -f $$cfg || \
57-+ /usr/sbin/squid -k parse -f $$cfg || \
58- { echo "FAIL: squid.conf test: $$cfg" | \
59- sed s%$(top_builddir)/src/%% | \
60- sed s%$(srcdir)/squidconf/%% ; \
61diff --git a/debian/patches/series b/debian/patches/series
62index 3c055ad..8c2318e 100644
63--- a/debian/patches/series
64+++ b/debian/patches/series
65@@ -1,6 +1,5 @@
66 0001-Default-configuration-file-for-debian.patch
67 0002-Change-default-file-locations-for-debian.patch
68-0003-installed-binary-for-debian-ci.patch
69 0005-Use-RuntimeDirectory-to-create-run-squid.patch
70 0006-Fix-build-against-OpenSSL-3-0.patch
71 90-cf.data.ubuntu.patch
72diff --git a/debian/rules b/debian/rules
73index 85d55f3..aae9452 100755
74--- a/debian/rules
75+++ b/debian/rules
76@@ -4,6 +4,11 @@ export DEB_BUILD_MAINT_OPTIONS = hardening=+all
77 export DEB_CFLAGS_MAINT_APPEND = -Wno-error=deprecated-declarations
78 export DEB_CXXFLAGS_MAINT_APPEND = -Wno-error=deprecated-declarations
79
80+ifeq ($(DEB_HOST_ARCH), s390x)
81+ DEB_CFLAGS_MAINT_APPEND += -Wno-error=stringop-overread
82+ DEB_CXXFLAGS_MAINT_APPEND += -Wno-error=stringop-overread
83+endif
84+
85 ifneq (,$(filter $(DEB_HOST_ARCH), armel m68k mips mipsel powerpc powerpcspe sh4))
86 DEB_LDFLAGS_MAINT_APPEND += -latomic
87 endif
88@@ -89,9 +94,17 @@ override_dh_auto_build:
89 dh_auto_build
90 cd debian/build-openssl && dh_auto_build
91
92+execute_before_dh_auto_test:
93+ # Do not include additional configuration files during tests. This would lead to failures due to missing paths.
94+ sed -i 's|^\(include /etc/squid/conf\.d/\*\.conf\)|# \1|' src/squid.conf.default debian/build-openssl/src/squid.conf.default
95+
96 override_dh_auto_test:
97- -dh_auto_test
98- -cd debian/build-openssl && dh_auto_test
99+ dh_auto_test
100+ cd debian/build-openssl && dh_auto_test
101+
102+execute_after_dh_auto_test:
103+ # Restore configuration file to its previous state.
104+ sed -i 's|^# \(include /etc/squid/conf\.d/\*\.conf\)|\1|' src/squid.conf.default debian/build-openssl/src/squid.conf.default
105
106 override_dh_auto_install:
107 dh_auto_install
108diff --git a/debian/tests/upstream-test-suite b/debian/tests/upstream-test-suite
109index b6c9495..d503788 100644
110--- a/debian/tests/upstream-test-suite
111+++ b/debian/tests/upstream-test-suite
112@@ -5,6 +5,10 @@ export DEB_CFLAGS_MAINT_APPEND="-Wno-error=deprecated-declarations"
113 export DEB_CXXFLAGS_MAINT_APPEND="-Wno-error=deprecated-declarations"
114
115 dpkg-source --before-build `pwd`
116+
117+# Use installed squid binary
118+sed -i 's|\$(top_builddir)/src/squid -k parse|/usr/sbin/squid -k parse|' test-suite/Makefile.am test-suite/Makefile.in
119+
120 dh_update_autotools_config
121 dh_autoreconf
122 dh_auto_configure -- ${DEB_CONFIGURE_EXTRA_FLAGS} --with-gnutls

Subscribers

People subscribed via source and target branches