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

[Mitch Burton]

[Mitch Burton (mitchburton)]

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.

[Andreas Hasenack (ahasenack)]

This conflicts with https://code.launchpad.net/~bryanfraschetti/ubuntu/+source/landscape-client/+git/landscape-client/+merge/483097

[Mitch Burton (mitchburton)]

I've included the changes from https://code.launchpad.net/~bryanfraschetti/ubuntu/+source/landscape-client/+git/landscape-client/+merge/483097 in this MR to resolve the conflict.

[Andreas Hasenack (ahasenack)]

I rejected https://code.launchpad.net/~bryanfraschetti/ubuntu/+source/landscape-client/+git/landscape-client/+merge/483097 then

[Andreas Hasenack (ahasenack)]

review: Needs Fixing

[Mitch Burton (mitchburton)]

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

[Andreas Hasenack (ahasenack)]

+1

review: Approve

[Andreas Hasenack (ahasenack)]

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

[Mitch Burton (mitchburton)]

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.

[Andreas Hasenack (ahasenack)]

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

[Mitch Burton (mitchburton)]

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

[Andreas Hasenack (ahasenack)]

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

[Andreas Hasenack (ahasenack)]

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

+1

review: Approve

[Andreas Hasenack (ahasenack)]

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

review: Needs Fixing

[Mitch Burton (mitchburton)]

Ah my mistake. Will rectify.

[Andreas Hasenack (ahasenack)]

\o/

[Andreas Hasenack (ahasenack)]

+1

review: Approve

[Andreas Hasenack (ahasenack)]

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

diff --git a/debian/changelog b/debian/changelog
index cd69d49..461698e 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,21 @@
+landscape-client (24.12-0ubuntu2) plucky; urgency=medium
+
+  [ Bryan Fraschetti ]
+  * d/p/2087852-feat-manage-ubuntu-sources-glob.patch: include DEB822 formatted sources
+    when managing apt sources by regex matching .sources files (LP: #2087852)
+
+  [ Mitch Burton ]
+  * Fix FTBFS (LP: #2106263)
+    - d/p/unittest-makeSuite-deprecation.patch: fix tests on python versions
+      where unittest.makeSuite has been removed
+    - d/rules: don't make root:root owner of installed executables
+    - d/tests/control: set environment variables for autopkgtest user and
+      build environment
+    - d/p/allow-http-proxy-in-tests.patch: fix tests by making them
+      insensitive to http/https proxy environment variables
+
+ -- Mitch Burton <[e-mail]>  Wed, 26 Mar 2025 17:47:28 -0700
+
 landscape-client (24.12-0ubuntu1) plucky; urgency=medium
 
   * 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
new file mode 100644
index 0000000..7dd82a5
--- /dev/null
+++ b/debian/patches/2087852-feat-manage-ubuntu-sources-glob.patch
@@ -0,0 +1,188 @@
+Subject: feat: add support for .sources when applying repository profiles
+
+ This is the backport of an upstream commit which added support to manage
+ ubuntu sources in DEB822 format (.sources files) alongside the one-liner
+ format (.list files). The bug reported is outlined in LP: #2087852.
+
+Origin: upstream, https://github.com/canonical/landscape-client/commit/556f87c2819b218029ba0a13e2b2ceddfbec3e6b
+Bug-Ubuntu: https://bugs.launchpad.net/landscape-client/+bug/2087852
+Last-Update: 2025-03-31
+
+diff --git a/landscape/client/manager/aptsources.py b/landscape/client/manager/aptsources.py
+index 179fa723..785fa215 100644
+--- a/landscape/client/manager/aptsources.py
++++ b/landscape/client/manager/aptsources.py
+@@ -27,6 +27,11 @@ class AptSources(ManagerPlugin):
+     SOURCES_LIST_D = "/etc/apt/sources.list.d"
+     TRUSTED_GPG_D = "/etc/apt/trusted.gpg.d"
+
++    """
++    Valid file patterns for one-line and Deb822-style sources, respectively.
++    """
++    SOURCES_LIST_D_FILE_PATTERNS = ["*.list", "*.sources"]
++
+     def register(self, registry):
+         super().register(registry)
+         registry.register_message(
+@@ -140,10 +145,14 @@ def _handle_sources(self, ignored, sources):
+             "manage_sources_list_d",
+             True,
+         )
++
+         if manage_sources_list_d not in FALSE_VALUES:
+-            filenames = glob.glob(os.path.join(self.SOURCES_LIST_D, "*.list"))
+-            for filename in filenames:
+-                shutil.move(filename, f"{filename}.save")
++            for pattern in self.SOURCES_LIST_D_FILE_PATTERNS:
++                filenames = glob.glob(
++                    os.path.join(self.SOURCES_LIST_D, pattern)
++                )
++                for filename in filenames:
++                    shutil.move(filename, f"{filename}.save")
+
+         for source in sources:
+             filename = os.path.join(
+diff --git a/landscape/client/manager/tests/test_aptsources.py b/landscape/client/manager/tests/test_aptsources.py
+index aa648be6..c17aac44 100644
+--- a/landscape/client/manager/tests/test_aptsources.py
++++ b/landscape/client/manager/tests/test_aptsources.py
+@@ -260,20 +260,28 @@ def buggy_source_handler(*args):
+
+     def test_renames_sources_list_d(self):
+         """
+-        The sources files in sources.list.d are renamed to .save when a message
+-        is received if config says to manage them, which is the default.
++        The sources files (.list, .sources) in sources.list.d
++        are renamed to .save when a message is received
++        if config says to manage them, which is the default.
+         """
++        FILE_1_LIST = os.path.join(
++            self.sourceslist.SOURCES_LIST_D, "file1.list"
++        )
++
++        FILE_2_SOURCES = os.path.join(
++            self.sourceslist.SOURCES_LIST_D, "file2.sources"
++        )
+         with open(
+-            os.path.join(self.sourceslist.SOURCES_LIST_D, "file1.list"),
++            FILE_1_LIST,
+             "w",
+-        ) as sources1:
+-            sources1.write("ok\n")
++        ) as source1:
++            source1.write("ok\n")
+
+         with open(
+-            os.path.join(self.sourceslist.SOURCES_LIST_D, "file2.list.save"),
++            FILE_2_SOURCES,
+             "w",
+-        ) as sources2:
+-            sources2.write("ok\n")
++        ) as source2:
++            source2.write("ok\n")
+
+         self.manager.dispatch_message(
+             {
+@@ -285,45 +293,42 @@ def test_renames_sources_list_d(self):
+         )
+
+         self.assertFalse(
+-            os.path.exists(
+-                os.path.join(self.sourceslist.SOURCES_LIST_D, "file1.list"),
+-            ),
++            os.path.exists(FILE_1_LIST),
+         )
+
++        self.assertFalse(os.path.exists(FILE_2_SOURCES))
++
+         self.assertTrue(
+-            os.path.exists(
+-                os.path.join(
+-                    self.sourceslist.SOURCES_LIST_D,
+-                    "file1.list.save",
+-                ),
+-            ),
++            os.path.exists(f"{FILE_1_LIST}.save"),
+         )
+
+         self.assertTrue(
+-            os.path.exists(
+-                os.path.join(
+-                    self.sourceslist.SOURCES_LIST_D,
+-                    "file2.list.save",
+-                ),
+-            ),
++            os.path.exists(f"{FILE_2_SOURCES}.save"),
+         )
+
+     def test_does_not_rename_sources_list_d(self):
+         """
+-        The sources files in sources.list.d are not renamed to .save when a
+-        message is received if config says not to manage them.
++        The sources files (.list, .sources) in sources.list.d
++        are not renamed to .save when a message is received
++        if config says not to manage them
+         """
++        FILE_3_LIST = os.path.join(
++            self.sourceslist.SOURCES_LIST_D, "file3.list"
++        )
++
++        FILE_4_SOURCES = os.path.join(
++            self.sourceslist.SOURCES_LIST_D, "file4.sources"
++        )
+         with open(
+-            os.path.join(self.sourceslist.SOURCES_LIST_D, "file1.list"),
++            FILE_3_LIST,
+             "w",
+-        ) as sources1:
+-            sources1.write("ok\n")
+-
++        ) as source3:
++            source3.write("ok\n")
+         with open(
+-            os.path.join(self.sourceslist.SOURCES_LIST_D, "file2.list.save"),
++            FILE_4_SOURCES,
+             "w",
+-        ) as sources2:
+-            sources2.write("ok\n")
++        ) as source4:
++            source4.write("ok\n")
+
+         self.manager.config.manage_sources_list_d = False
+         self.manager.dispatch_message(
+@@ -336,27 +341,19 @@ def test_does_not_rename_sources_list_d(self):
+         )
+
+         self.assertTrue(
+-            os.path.exists(
+-                os.path.join(self.sourceslist.SOURCES_LIST_D, "file1.list"),
+-            ),
++            os.path.exists(FILE_3_LIST),
++        )
++
++        self.assertTrue(
++            os.path.exists(FILE_4_SOURCES),
+         )
+
+         self.assertFalse(
+-            os.path.exists(
+-                os.path.join(
+-                    self.sourceslist.SOURCES_LIST_D,
+-                    "file1.list.save",
+-                ),
+-            ),
++            os.path.exists(f"{FILE_3_LIST}.save"),
+         )
+
+-        self.assertTrue(
+-            os.path.exists(
+-                os.path.join(
+-                    self.sourceslist.SOURCES_LIST_D,
+-                    "file2.list.save",
+-                ),
+-            ),
++        self.assertFalse(
++            os.path.exists(f"{FILE_4_SOURCES}.save"),
+         )
+
+     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
new file mode 100644
index 0000000..3fc6473
--- /dev/null
+++ b/debian/patches/allow-http-proxy-in-tests.patch
@@ -0,0 +1,60 @@
+Description: Fix config tests; make them insensitive to http(s) proxy environment variables
+Author: Mitch Burton <[e-mail]>
+Origin: upstream
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/landscape-client/+bug/2106263
+Applied-Upstream: https://github.com/canonical/landscape-client/commit/e79f605e3c69fc5b9b529a9997e4e89a6728066f
+Last-Update: 2025-04-07
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+Index: landscape-client/landscape/client/tests/test_configuration.py
+===================================================================
+--- landscape-client.orig/landscape/client/tests/test_configuration.py
++++ landscape-client/landscape/client/tests/test_configuration.py
+@@ -756,6 +756,7 @@ class ConfigurationFunctionsTest(Landsca
+         )
+ 
+     @mock.patch("landscape.client.configuration.ServiceConfig")
++    @mock.patch("os.environ", new={})
+     def test_silent_setup(self, mock_serviceconfig):
+         """
+         Only command-line options are used in silent mode.
+@@ -775,6 +776,7 @@ url = https://landscape.canonical.com/me
+         )
+ 
+     @mock.patch("landscape.client.configuration.ServiceConfig")
++    @mock.patch("os.environ", {})
+     def test_silent_setup_no_register(self, mock_serviceconfig):
+         """
+         Called with command line options to write a config file but no
+@@ -846,6 +848,7 @@ url = https://landscape.canonical.com/me
+         )
+ 
+     @mock.patch("landscape.client.configuration.ServiceConfig")
++    @mock.patch("os.environ", {})
+     def test_silent_setup_unicode_computer_title(self, mock_serviceconfig):
+         """
+         Setup accepts a non-ascii computer title and registration is
+@@ -880,6 +883,7 @@ url = https://landscape.canonical.com/me
+ 
+     @mock.patch("landscape.client.configuration.input")
+     @mock.patch("landscape.client.configuration.ServiceConfig")
++    @mock.patch("os.environ", new={})
+     def test_silent_script_users_imply_script_execution_plugin(
+         self,
+         mock_serviceconfig,
+@@ -951,6 +955,7 @@ bus = session
+         mock_serviceconfig.set_start_on_boot.assert_called_once_with(True)
+ 
+     @mock.patch("landscape.client.configuration.ServiceConfig")
++    @mock.patch("os.environ", new={})
+     def test_silent_setup_with_ping_url(self, mock_serviceconfig):
+         mock_serviceconfig.restart_landscape.return_value = True
+         filename = self.makeFile(
+@@ -1814,6 +1819,7 @@ registration_key = shared-secret
+         )
+ 
+     @mock.patch("landscape.client.configuration.ServiceConfig")
++    @mock.patch("os.environ", new={})
+     def test_import_from_file_may_reset_old_options(self, mock_serviceconfig):
+         """
+         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 @@
 fix-landscape-client-manpage.patch
+unittest-makeSuite-deprecation.patch
+2087852-feat-manage-ubuntu-sources-glob.patch
+allow-http-proxy-in-tests.patch
diff --git a/debian/patches/unittest-makeSuite-deprecation.patch b/debian/patches/unittest-makeSuite-deprecation.patch
new file mode 100644
index 0000000..e06382e
--- /dev/null
+++ b/debian/patches/unittest-makeSuite-deprecation.patch
@@ -0,0 +1,40 @@
+Description: Fix tests on python versions where unittest.makeSuite has been removed
+Author: Mitch Burton <[e-mail]>
+Origin: upstream
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/landscape-client/+bug/2106263
+Applied-Upstream: https://github.com/canonical/landscape-client/commit/e3f051ad6a33029845f9f11b17649299004598a7
+Last-Update: 2025-03-26
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+Index: landscape-client/landscape/lib/tests/test_sequenceranges.py
+===================================================================
+--- landscape-client.orig/landscape/lib/tests/test_sequenceranges.py
++++ landscape-client/landscape/lib/tests/test_sequenceranges.py
+@@ -297,11 +297,21 @@ class RemoveFromRangesTest(unittest.Test
+ def test_suite():
+     return unittest.TestSuite(
+         (
+-            unittest.makeSuite(SequenceToRangesTest),
+-            unittest.makeSuite(RangesToSequenceTest),
+-            unittest.makeSuite(SequenceRangesTest),
+-            unittest.makeSuite(FindRangesIndexTest),
+-            unittest.makeSuite(AddToRangesTest),
+-            unittest.makeSuite(RemoveFromRangesTest),
++            unittest.defaultTestLoader.loadTestsFromTestCase(
++                SequenceToRangesTest
++            ),
++            unittest.defaultTestLoader.loadTestsFromTestCase(
++                RangesToSequenceTest
++            ),
++            unittest.defaultTestLoader.loadTestsFromTestCase(
++                SequenceRangesTest
++            ),
++            unittest.defaultTestLoader.loadTestsFromTestCase(
++                FindRangesIndexTest
++            ),
++            unittest.defaultTestLoader.loadTestsFromTestCase(AddToRangesTest),
++            unittest.defaultTestLoader.loadTestsFromTestCase(
++                RemoveFromRangesTest
++            ),
+         ),
+     )
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:
 override_dh_auto_install:
 	dh_auto_install
 	make -C apt-update
-	install -D -o root -g root -m 755 debian/landscape-sysinfo.wrapper $(CURDIR)/$(root_dir)$(SHAREDIR)/landscape-sysinfo.wrapper
-	install -D -o root -g root -m 755 apt-update/apt-update $(CURDIR)/$(root_dir)$(LIBDIR)/apt-update
+	install -D -m 755 debian/landscape-sysinfo.wrapper $(CURDIR)/$(root_dir)$(SHAREDIR)/landscape-sysinfo.wrapper
+	install -D -m 755 apt-update/apt-update $(CURDIR)/$(root_dir)$(LIBDIR)/apt-update
 
 override_dh_installsystemd:
 	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 @@
 Tests: smoke
 Restrictions: allow-stderr, needs-sudo, superficial
 
-Test-Command: HOME=$(mktemp -d) make check
+Test-Command: HOME=$(mktemp -d) LANDSCAPE_CLIENT_USER=$(whoami) LANDSCAPE_CLIENT_BUILDING=1 make check
 Depends: @builddeps@
 Restrictions: allow-stderr
 Features: test-name=unit