Merge ~mitchburton/ubuntu/+source/landscape-client:plucky-fix-build into ubuntu/+source/landscape-client:ubuntu/devel

Proposed by Mitch Burton
Status: Merged
Merged at revision: 8992f1bea43124d019dfec59ef75fdb3b11cb308
Proposed branch: ~mitchburton/ubuntu/+source/landscape-client:plucky-fix-build
Merge into: ubuntu/+source/landscape-client:ubuntu/devel
Diff against target: 369 lines (+312/-3)
7 files modified
debian/changelog (+18/-0)
debian/patches/2087852-feat-manage-ubuntu-sources-glob.patch (+188/-0)
debian/patches/allow-http-proxy-in-tests.patch (+60/-0)
debian/patches/series (+3/-0)
debian/patches/unittest-makeSuite-deprecation.patch (+40/-0)
debian/rules (+2/-2)
debian/tests/control (+1/-1)
Reviewer Review Type Date Requested Status
Andreas Hasenack Approve
Ubuntu Sponsors Pending
Review via email: mp+483505@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Mitch Burton (mitchburton) wrote :

Successful build can be seen in this PPA: https://launchpad.net/~mitchburton/+archive/ubuntu/landscape-client-ppa/+packages, version 24.12-0ubuntu2~ppa2.

I've had to remove the ownership change to root for landscape-sysinfo.wrapper and apt-update. Since these are both 755 it shouldn't impact the ability to execute them, but I am curious if anyone knows why this root ownership change now produces build errors.

Revision history for this message
Andreas Hasenack (ahasenack) wrote :
Revision history for this message
Mitch Burton (mitchburton) wrote :
Revision history for this message
Andreas Hasenack (ahasenack) wrote :
Revision history for this message
Andreas Hasenack (ahasenack) :
review: Needs Fixing
Revision history for this message
Andreas Hasenack (ahasenack) :
Revision history for this message
Andreas Hasenack (ahasenack) :
Revision history for this message
Mitch Burton (mitchburton) wrote :

I believe I've addressed all the diff comments left by ahasenack:

  - Created bug LP: #2106263 for the FTBFS and referenced it in the changelog and commits
  - Updated the headers on the first patch to include the Bug-Ubuntu reference to same
  - Reformatted the changelog entry for multiple authors

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

+1

review: Approve
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Actually, hold that thought. I triggered the autopkgtests in your ppa, and got a failure[1]:

558s [ERROR]
558s Traceback (most recent call last):
558s File "/tmp/autopkgtest.IgNtA7/build.1IR/src/landscape/client/tests/test_watchdog.py", line 580, in setUp
558s super().setUp()
558s File "/tmp/autopkgtest.IgNtA7/build.1IR/src/landscape/client/tests/helpers.py", line 74, in setUp
558s result = testing.HelperTestCase.setUp(self)
558s File "/tmp/autopkgtest.IgNtA7/build.1IR/src/landscape/lib/testing.py", line 49, in setUp
558s result = helper.set_up(self)
558s File "/tmp/autopkgtest.IgNtA7/build.1IR/src/landscape/client/tests/helpers.py", line 148, in set_up
558s test_case.broker_service = FakeBrokerService(config)
558s File "/tmp/autopkgtest.IgNtA7/build.1IR/src/landscape/client/broker/service.py", line 54, in __init__
558s super().__init__(config)
558s File "/tmp/autopkgtest.IgNtA7/build.1IR/src/landscape/client/service.py", line 36, in __init__
558s self.persist = get_versioned_persist(self)
558s File "/tmp/autopkgtest.IgNtA7/build.1IR/src/landscape/client/deployment.py", line 255, in get_versioned_persist
558s persist.save(service.persist_filename)
558s File "/tmp/autopkgtest.IgNtA7/build.1IR/src/landscape/lib/persist.py", line 179, in save
558s shutil.chown(dirname, user=self._user, group=self._group)
558s File "/usr/lib/python3.13/shutil.py", line 1427, in chown
558s raise LookupError("no such user: {!r}".format(user))
558s builtins.LookupError: no such user: 'landscape'

I remember this having been fixed already, is that ppa old? Just to be sure, I'll create a new one and run the tests there. It will be a while, though.

1. https://autopkgtest.ubuntu.com/results/autopkgtest-plucky-mitchburton-landscape-client-ppa/plucky/amd64/l/landscape-client/20250404_202328_93494@/log.gz

review: Needs Information
Revision history for this message
Mitch Burton (mitchburton) wrote (last edit ):

I've made changes that should hopefully resolve the autopkgtest issues. I've also made a new upload to my PPA of this version (~ppa4). My autopkgtest testbed has the landscape user, so that's likely why I didn't see this issue previously.

I've also updated LP: #2106263 to include the autopkgtest build failure.

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

autopkgtests are still failing[1]. Multiple tests show this error pattern with the proxy variables:

626s landscape.client.tests.test_configuration.ConfigurationFunctionsTest.test_silent_setup
626s ===============================================================================
626s [FAIL]
626s Traceback (most recent call last):
626s File "/usr/lib/python3.13/unittest/mock.py", line 1426, in patched
626s return func(*newargs, **newkeywargs)
626s File "/tmp/autopkgtest.62rtX1/build.DIQ/src/landscape/client/tests/test_configuration.py", line 785, in test_silent_setup_no_register
626s self.assertConfigEqual(
626s File "/tmp/autopkgtest.62rtX1/build.DIQ/src/landscape/lib/testing.py", line 192, in assertConfigEqual
626s self.assertEqual(
626s File "/usr/lib/python3/dist-packages/twisted/trial/_synctest.py", line 444, in assertEqual
626s super().assertEqual(first, second, msg)
626s File "/usr/lib/python3.13/unittest/case.py", line 907, in assertEqual
626s assertion_func(first, second, msg=msg)
626s File "/usr/lib/python3.13/unittest/case.py", line 1206, in assertDictEqual
626s self.fail(self._formatMessage(msg, standardMsg))
626s twisted.trial.unittest.FailTest: {'url': 'https://landscape.canonical.com/me[138 chars]128'} != {'data_path': '/tmp/tmpsrhj5v8p/client', 'u[49 chars]tem'}
626s {'data_path': '/tmp/tmpsrhj5v8p/client',
626s - 'http_proxy': 'http://squid.internal:3128',
626s - 'https_proxy': 'http://squid.internal:3128',
626s 'url': 'https://landscape.canonical.com/message-system'}

1. https://autopkgtest.ubuntu.com/results/autopkgtest-plucky-mitchburton-landscape-client-ppa/plucky/amd64/l/landscape-client/20250407_093831_f2105@/log.gz

review: Needs Fixing
Revision history for this message
Mitch Burton (mitchburton) wrote :

I've added a fix for the http/https proxy variable issue based on https://github.com/canonical/landscape-client/commit/e79f605e3c69fc5b9b529a9997e4e89a6728066f

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

As soon as your ppa finishes publishing the new builds, I'll trigger the autopkgtests there.

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Tests are good. There is a failure on ppc64el that looks like it's just flaky (connection errors).

+1

review: Approve
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Actually, you dropped the d/t/control change from d/changelog :(

review: Needs Fixing
Revision history for this message
Mitch Burton (mitchburton) wrote :

Ah my mistake. Will rectify.

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

\o/

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

+1

review: Approve
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Sponsored:

Uploading landscape-client_24.12-0ubuntu2.dsc
Uploading landscape-client_24.12-0ubuntu2.debian.tar.xz
Uploading landscape-client_24.12-0ubuntu2_source.buildinfo
Uploading landscape-client_24.12-0ubuntu2_source.changes

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index cd69d49..461698e 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,21 @@
1landscape-client (24.12-0ubuntu2) plucky; urgency=medium
2
3 [ Bryan Fraschetti ]
4 * d/p/2087852-feat-manage-ubuntu-sources-glob.patch: include DEB822 formatted sources
5 when managing apt sources by regex matching .sources files (LP: #2087852)
6
7 [ Mitch Burton ]
8 * Fix FTBFS (LP: #2106263)
9 - d/p/unittest-makeSuite-deprecation.patch: fix tests on python versions
10 where unittest.makeSuite has been removed
11 - d/rules: don't make root:root owner of installed executables
12 - d/tests/control: set environment variables for autopkgtest user and
13 build environment
14 - d/p/allow-http-proxy-in-tests.patch: fix tests by making them
15 insensitive to http/https proxy environment variables
16
17 -- Mitch Burton <mitch.burton@canonical.com> Wed, 26 Mar 2025 17:47:28 -0700
18
1landscape-client (24.12-0ubuntu1) plucky; urgency=medium19landscape-client (24.12-0ubuntu1) plucky; urgency=medium
220
3 * New upstream release 24.1221 * New upstream release 24.12
diff --git a/debian/patches/2087852-feat-manage-ubuntu-sources-glob.patch b/debian/patches/2087852-feat-manage-ubuntu-sources-glob.patch
4new file mode 10064422new file mode 100644
index 0000000..7dd82a5
--- /dev/null
+++ b/debian/patches/2087852-feat-manage-ubuntu-sources-glob.patch
@@ -0,0 +1,188 @@
1Subject: feat: add support for .sources when applying repository profiles
2
3 This is the backport of an upstream commit which added support to manage
4 ubuntu sources in DEB822 format (.sources files) alongside the one-liner
5 format (.list files). The bug reported is outlined in LP: #2087852.
6
7Origin: upstream, https://github.com/canonical/landscape-client/commit/556f87c2819b218029ba0a13e2b2ceddfbec3e6b
8Bug-Ubuntu: https://bugs.launchpad.net/landscape-client/+bug/2087852
9Last-Update: 2025-03-31
10
11diff --git a/landscape/client/manager/aptsources.py b/landscape/client/manager/aptsources.py
12index 179fa723..785fa215 100644
13--- a/landscape/client/manager/aptsources.py
14+++ b/landscape/client/manager/aptsources.py
15@@ -27,6 +27,11 @@ class AptSources(ManagerPlugin):
16 SOURCES_LIST_D = "/etc/apt/sources.list.d"
17 TRUSTED_GPG_D = "/etc/apt/trusted.gpg.d"
18
19+ """
20+ Valid file patterns for one-line and Deb822-style sources, respectively.
21+ """
22+ SOURCES_LIST_D_FILE_PATTERNS = ["*.list", "*.sources"]
23+
24 def register(self, registry):
25 super().register(registry)
26 registry.register_message(
27@@ -140,10 +145,14 @@ def _handle_sources(self, ignored, sources):
28 "manage_sources_list_d",
29 True,
30 )
31+
32 if manage_sources_list_d not in FALSE_VALUES:
33- filenames = glob.glob(os.path.join(self.SOURCES_LIST_D, "*.list"))
34- for filename in filenames:
35- shutil.move(filename, f"{filename}.save")
36+ for pattern in self.SOURCES_LIST_D_FILE_PATTERNS:
37+ filenames = glob.glob(
38+ os.path.join(self.SOURCES_LIST_D, pattern)
39+ )
40+ for filename in filenames:
41+ shutil.move(filename, f"{filename}.save")
42
43 for source in sources:
44 filename = os.path.join(
45diff --git a/landscape/client/manager/tests/test_aptsources.py b/landscape/client/manager/tests/test_aptsources.py
46index aa648be6..c17aac44 100644
47--- a/landscape/client/manager/tests/test_aptsources.py
48+++ b/landscape/client/manager/tests/test_aptsources.py
49@@ -260,20 +260,28 @@ def buggy_source_handler(*args):
50
51 def test_renames_sources_list_d(self):
52 """
53- The sources files in sources.list.d are renamed to .save when a message
54- is received if config says to manage them, which is the default.
55+ The sources files (.list, .sources) in sources.list.d
56+ are renamed to .save when a message is received
57+ if config says to manage them, which is the default.
58 """
59+ FILE_1_LIST = os.path.join(
60+ self.sourceslist.SOURCES_LIST_D, "file1.list"
61+ )
62+
63+ FILE_2_SOURCES = os.path.join(
64+ self.sourceslist.SOURCES_LIST_D, "file2.sources"
65+ )
66 with open(
67- os.path.join(self.sourceslist.SOURCES_LIST_D, "file1.list"),
68+ FILE_1_LIST,
69 "w",
70- ) as sources1:
71- sources1.write("ok\n")
72+ ) as source1:
73+ source1.write("ok\n")
74
75 with open(
76- os.path.join(self.sourceslist.SOURCES_LIST_D, "file2.list.save"),
77+ FILE_2_SOURCES,
78 "w",
79- ) as sources2:
80- sources2.write("ok\n")
81+ ) as source2:
82+ source2.write("ok\n")
83
84 self.manager.dispatch_message(
85 {
86@@ -285,45 +293,42 @@ def test_renames_sources_list_d(self):
87 )
88
89 self.assertFalse(
90- os.path.exists(
91- os.path.join(self.sourceslist.SOURCES_LIST_D, "file1.list"),
92- ),
93+ os.path.exists(FILE_1_LIST),
94 )
95
96+ self.assertFalse(os.path.exists(FILE_2_SOURCES))
97+
98 self.assertTrue(
99- os.path.exists(
100- os.path.join(
101- self.sourceslist.SOURCES_LIST_D,
102- "file1.list.save",
103- ),
104- ),
105+ os.path.exists(f"{FILE_1_LIST}.save"),
106 )
107
108 self.assertTrue(
109- os.path.exists(
110- os.path.join(
111- self.sourceslist.SOURCES_LIST_D,
112- "file2.list.save",
113- ),
114- ),
115+ os.path.exists(f"{FILE_2_SOURCES}.save"),
116 )
117
118 def test_does_not_rename_sources_list_d(self):
119 """
120- The sources files in sources.list.d are not renamed to .save when a
121- message is received if config says not to manage them.
122+ The sources files (.list, .sources) in sources.list.d
123+ are not renamed to .save when a message is received
124+ if config says not to manage them
125 """
126+ FILE_3_LIST = os.path.join(
127+ self.sourceslist.SOURCES_LIST_D, "file3.list"
128+ )
129+
130+ FILE_4_SOURCES = os.path.join(
131+ self.sourceslist.SOURCES_LIST_D, "file4.sources"
132+ )
133 with open(
134- os.path.join(self.sourceslist.SOURCES_LIST_D, "file1.list"),
135+ FILE_3_LIST,
136 "w",
137- ) as sources1:
138- sources1.write("ok\n")
139-
140+ ) as source3:
141+ source3.write("ok\n")
142 with open(
143- os.path.join(self.sourceslist.SOURCES_LIST_D, "file2.list.save"),
144+ FILE_4_SOURCES,
145 "w",
146- ) as sources2:
147- sources2.write("ok\n")
148+ ) as source4:
149+ source4.write("ok\n")
150
151 self.manager.config.manage_sources_list_d = False
152 self.manager.dispatch_message(
153@@ -336,27 +341,19 @@ def test_does_not_rename_sources_list_d(self):
154 )
155
156 self.assertTrue(
157- os.path.exists(
158- os.path.join(self.sourceslist.SOURCES_LIST_D, "file1.list"),
159- ),
160+ os.path.exists(FILE_3_LIST),
161+ )
162+
163+ self.assertTrue(
164+ os.path.exists(FILE_4_SOURCES),
165 )
166
167 self.assertFalse(
168- os.path.exists(
169- os.path.join(
170- self.sourceslist.SOURCES_LIST_D,
171- "file1.list.save",
172- ),
173- ),
174+ os.path.exists(f"{FILE_3_LIST}.save"),
175 )
176
177- self.assertTrue(
178- os.path.exists(
179- os.path.join(
180- self.sourceslist.SOURCES_LIST_D,
181- "file2.list.save",
182- ),
183- ),
184+ self.assertFalse(
185+ os.path.exists(f"{FILE_4_SOURCES}.save"),
186 )
187
188 def test_create_landscape_sources(self):
diff --git a/debian/patches/allow-http-proxy-in-tests.patch b/debian/patches/allow-http-proxy-in-tests.patch
0new file mode 100644189new file mode 100644
index 0000000..3fc6473
--- /dev/null
+++ b/debian/patches/allow-http-proxy-in-tests.patch
@@ -0,0 +1,60 @@
1Description: Fix config tests; make them insensitive to http(s) proxy environment variables
2Author: Mitch Burton <mitch.burton@canonical.com>
3Origin: upstream
4Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/landscape-client/+bug/2106263
5Applied-Upstream: https://github.com/canonical/landscape-client/commit/e79f605e3c69fc5b9b529a9997e4e89a6728066f
6Last-Update: 2025-04-07
7---
8This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
9Index: landscape-client/landscape/client/tests/test_configuration.py
10===================================================================
11--- landscape-client.orig/landscape/client/tests/test_configuration.py
12+++ landscape-client/landscape/client/tests/test_configuration.py
13@@ -756,6 +756,7 @@ class ConfigurationFunctionsTest(Landsca
14 )
15
16 @mock.patch("landscape.client.configuration.ServiceConfig")
17+ @mock.patch("os.environ", new={})
18 def test_silent_setup(self, mock_serviceconfig):
19 """
20 Only command-line options are used in silent mode.
21@@ -775,6 +776,7 @@ url = https://landscape.canonical.com/me
22 )
23
24 @mock.patch("landscape.client.configuration.ServiceConfig")
25+ @mock.patch("os.environ", {})
26 def test_silent_setup_no_register(self, mock_serviceconfig):
27 """
28 Called with command line options to write a config file but no
29@@ -846,6 +848,7 @@ url = https://landscape.canonical.com/me
30 )
31
32 @mock.patch("landscape.client.configuration.ServiceConfig")
33+ @mock.patch("os.environ", {})
34 def test_silent_setup_unicode_computer_title(self, mock_serviceconfig):
35 """
36 Setup accepts a non-ascii computer title and registration is
37@@ -880,6 +883,7 @@ url = https://landscape.canonical.com/me
38
39 @mock.patch("landscape.client.configuration.input")
40 @mock.patch("landscape.client.configuration.ServiceConfig")
41+ @mock.patch("os.environ", new={})
42 def test_silent_script_users_imply_script_execution_plugin(
43 self,
44 mock_serviceconfig,
45@@ -951,6 +955,7 @@ bus = session
46 mock_serviceconfig.set_start_on_boot.assert_called_once_with(True)
47
48 @mock.patch("landscape.client.configuration.ServiceConfig")
49+ @mock.patch("os.environ", new={})
50 def test_silent_setup_with_ping_url(self, mock_serviceconfig):
51 mock_serviceconfig.restart_landscape.return_value = True
52 filename = self.makeFile(
53@@ -1814,6 +1819,7 @@ registration_key = shared-secret
54 )
55
56 @mock.patch("landscape.client.configuration.ServiceConfig")
57+ @mock.patch("os.environ", new={})
58 def test_import_from_file_may_reset_old_options(self, mock_serviceconfig):
59 """
60 This test ensures that setting an empty option in an imported
diff --git a/debian/patches/series b/debian/patches/series
index 051c373..62e9fa0 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1,4 @@
1fix-landscape-client-manpage.patch1fix-landscape-client-manpage.patch
2unittest-makeSuite-deprecation.patch
32087852-feat-manage-ubuntu-sources-glob.patch
4allow-http-proxy-in-tests.patch
diff --git a/debian/patches/unittest-makeSuite-deprecation.patch b/debian/patches/unittest-makeSuite-deprecation.patch
2new file mode 1006445new file mode 100644
index 0000000..e06382e
--- /dev/null
+++ b/debian/patches/unittest-makeSuite-deprecation.patch
@@ -0,0 +1,40 @@
1Description: Fix tests on python versions where unittest.makeSuite has been removed
2Author: Mitch Burton <mitch.burton@canonical.com>
3Origin: upstream
4Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/landscape-client/+bug/2106263
5Applied-Upstream: https://github.com/canonical/landscape-client/commit/e3f051ad6a33029845f9f11b17649299004598a7
6Last-Update: 2025-03-26
7---
8This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
9Index: landscape-client/landscape/lib/tests/test_sequenceranges.py
10===================================================================
11--- landscape-client.orig/landscape/lib/tests/test_sequenceranges.py
12+++ landscape-client/landscape/lib/tests/test_sequenceranges.py
13@@ -297,11 +297,21 @@ class RemoveFromRangesTest(unittest.Test
14 def test_suite():
15 return unittest.TestSuite(
16 (
17- unittest.makeSuite(SequenceToRangesTest),
18- unittest.makeSuite(RangesToSequenceTest),
19- unittest.makeSuite(SequenceRangesTest),
20- unittest.makeSuite(FindRangesIndexTest),
21- unittest.makeSuite(AddToRangesTest),
22- unittest.makeSuite(RemoveFromRangesTest),
23+ unittest.defaultTestLoader.loadTestsFromTestCase(
24+ SequenceToRangesTest
25+ ),
26+ unittest.defaultTestLoader.loadTestsFromTestCase(
27+ RangesToSequenceTest
28+ ),
29+ unittest.defaultTestLoader.loadTestsFromTestCase(
30+ SequenceRangesTest
31+ ),
32+ unittest.defaultTestLoader.loadTestsFromTestCase(
33+ FindRangesIndexTest
34+ ),
35+ unittest.defaultTestLoader.loadTestsFromTestCase(AddToRangesTest),
36+ unittest.defaultTestLoader.loadTestsFromTestCase(
37+ RemoveFromRangesTest
38+ ),
39 ),
40 )
diff --git a/debian/rules b/debian/rules
index 3fcf3ae..267a32a 100755
--- a/debian/rules
+++ b/debian/rules
@@ -19,8 +19,8 @@ override_dh_auto_build:
19override_dh_auto_install:19override_dh_auto_install:
20 dh_auto_install20 dh_auto_install
21 make -C apt-update21 make -C apt-update
22 install -D -o root -g root -m 755 debian/landscape-sysinfo.wrapper $(CURDIR)/$(root_dir)$(SHAREDIR)/landscape-sysinfo.wrapper22 install -D -m 755 debian/landscape-sysinfo.wrapper $(CURDIR)/$(root_dir)$(SHAREDIR)/landscape-sysinfo.wrapper
23 install -D -o root -g root -m 755 apt-update/apt-update $(CURDIR)/$(root_dir)$(LIBDIR)/apt-update23 install -D -m 755 apt-update/apt-update $(CURDIR)/$(root_dir)$(LIBDIR)/apt-update
2424
25override_dh_installsystemd:25override_dh_installsystemd:
26 dh_installsystemd26 dh_installsystemd
diff --git a/debian/tests/control b/debian/tests/control
index 082632c..0f6d748 100644
--- a/debian/tests/control
+++ b/debian/tests/control
@@ -1,7 +1,7 @@
1Tests: smoke1Tests: smoke
2Restrictions: allow-stderr, needs-sudo, superficial2Restrictions: allow-stderr, needs-sudo, superficial
33
4Test-Command: HOME=$(mktemp -d) make check4Test-Command: HOME=$(mktemp -d) LANDSCAPE_CLIENT_USER=$(whoami) LANDSCAPE_CLIENT_BUILDING=1 make check
5Depends: @builddeps@5Depends: @builddeps@
6Restrictions: allow-stderr6Restrictions: allow-stderr
7Features: test-name=unit7Features: test-name=unit

Subscribers

People subscribed via source and target branches