Merge ~mitchburton/ubuntu/+source/landscape-client:2087852-fix-apt-source-file-management-oracular into ubuntu/+source/landscape-client:ubuntu/oracular-devel

[Mitch Burton]

[Julian Andres Klode (juliank)]

I'm going to reject all the further landscape-client SRU merges. Please sort out the situation with the SRU and the proposed changes for the next SRU with a dedicated sponsor before coming back to the general sponsorship queue, as the situation is too complex to unravel here.

review: Disapprove

[Mitch Burton (mitchburton)]

Hi juliank (and other sponsors).

I'm Sorry for clogging up the queue.

After chatting with ahasenack about it, I think approving this MP will begin to unclog things. It's a change on top of the change for #2087852 that failed verification for oracular, which is what is blocking our other SRU MPs.

Similarly, there's another MP for noble, which is in the same situation: https://code.launchpad.net/~mitchburton/ubuntu/+source/landscape-client/+git/landscape-client/+merge/486291

If it's easier, ahasenack has offered to remove the failed packages from proposed.

[Julian Andres Klode (juliank)]

review: Approve

[Julian Andres Klode (juliank)]

Oof I did not notice earlier but

Mitch Burton <mitch@mitch-ThinkPad-T14-Gen-5>

please use a real email!

And update the bug to explain what went wrong the first time and how it's being fixed now. Ta!

review: Needs Fixing

4bbbae0... by Mitch Burton on 2025-06-13

release for oracular

[Mitch Burton (mitchburton)]

Fixed the email. I'll do the same for the other MPs and sort out the bug description.

Unmerged commits

4bbbae0... by Mitch Burton on 2025-06-13

release for oracular

b3c2cc2... by Mitch Burton on 2025-06-10

d/p/fix-apt-source-file-management.patch: restore .list and .sources
files when a repo profile is disassociated (LP: #2087852)

diff --git a/debian/changelog b/debian/changelog
index 7fa9a76..5b98f7b 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+landscape-client (24.08-0ubuntu2.3) oracular; urgency=medium
+
+  * d/p/fix-apt-source-file-management.patch: restore .list and .sources files
+    when a repo profile is disassociated (LP: #2087852)
+
+ -- Mitch Burton <[e-mail]>  Fri, 13 Jun 2025 09:37:09 -0700
+
 landscape-client (24.08-0ubuntu2.2) oracular; urgency=medium
 
   * d/p/2087852-feat-manage-ubuntu-sources-glob.patch: include DEB822
diff --git a/debian/patches/fix-apt-source-file-management.patch b/debian/patches/fix-apt-source-file-management.patch
new file mode 100644
index 0000000..271d47c
--- /dev/null
+++ b/debian/patches/fix-apt-source-file-management.patch
@@ -0,0 +1,204 @@
+Description: restore .list and .sources files when a repo profile is disassociated.
+Author: Mitch Burton <[e-mail]>
+Origin: backport, https://github.com/canonical/landscape-client/commit/c9900d7a31eb6d5822bd2a66d0a6bfa3e926a7a6
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/landscape-client/+bug/2087852
+Applied-Upstream: 25.04
+Last-Update: 2025-06-10
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+Index: landscape-client/landscape/client/manager/aptsources.py
+===================================================================
+--- landscape-client.orig/landscape/client/manager/aptsources.py	2025-06-10 16:24:19.394390602 -0700
++++ landscape-client/landscape/client/manager/aptsources.py	2025-06-10 16:26:16.701119913 -0700
+@@ -114,6 +114,12 @@
+ 
+         saved_sources = f"{self.SOURCES_LIST}.save"
+ 
++        manage_sources_list_d = getattr(
++            self.registry.config,
++            "manage_sources_list_d",
++            True,
++        )
++
+         if sources:
+             fd, path = tempfile.mkstemp()
+             os.close(fd)
+@@ -135,24 +141,34 @@
+                 original_stat.st_uid,
+                 original_stat.st_gid,
+             )
++
++            if manage_sources_list_d not in FALSE_VALUES:
++                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")
+         else:
+             # Re-instate original sources
+             if os.path.isfile(saved_sources):
+                 shutil.move(saved_sources, self.SOURCES_LIST)
+ 
+-        manage_sources_list_d = getattr(
+-            self.registry.config,
+-            "manage_sources_list_d",
+-            True,
+-        )
++            if manage_sources_list_d not in FALSE_VALUES:
++                for pattern in self.SOURCES_LIST_D_FILE_PATTERNS:
++                    filenames = glob.glob(
++                        os.path.join(self.SOURCES_LIST_D, f"{pattern}.save")
++                    )
++                    for filename in filenames:
++                        restored_filename = filename.removesuffix(".save")
++                        shutil.move(filename, restored_filename)
++            # Delete Landscape source files prefixed with `landscape-`
++            landscape_source_filenames = glob.glob(
++                os.path.join(self.SOURCES_LIST_D, "landscape-*.list")
++            )
+ 
+-        if manage_sources_list_d not in FALSE_VALUES:
+-            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_filename in landscape_source_filenames:
++                os.remove(source_filename)
+ 
+         for source in sources:
+             filename = os.path.join(
+Index: landscape-client/landscape/client/manager/tests/test_aptsources.py
+===================================================================
+--- landscape-client.orig/landscape/client/manager/tests/test_aptsources.py	2025-06-10 16:24:19.394390602 -0700
++++ landscape-client/landscape/client/manager/tests/test_aptsources.py	2025-06-10 16:27:21.474385821 -0700
+@@ -165,6 +165,109 @@
+         with open(self.sourceslist.SOURCES_LIST) as sources:
+             self.assertEqual("original content\n", sources.read())
+ 
++    def test_restore_sources_list_d(self):
++        """
++        When getting a repository message without sources, AptSources
++        restores the previous files in sources.list.d.
++        """
++        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(
++            FILE_1_LIST,
++            "w",
++        ) as source1:
++            source1.write("ok\n")
++        with open(
++            FILE_2_SOURCES,
++            "w",
++        ) as source2:
++            source2.write("ok\n")
++        self.manager.dispatch_message(
++            {
++                "type": "apt-sources-replace",
++                "sources": [{"name": "bla", "content": b""}],
++                "gpg-keys": [],
++                "operation-id": 1,
++            },
++        )
++        self.assertFalse(
++            os.path.exists(FILE_1_LIST),
++        )
++        self.assertFalse(os.path.exists(FILE_2_SOURCES))
++        self.assertTrue(
++            os.path.exists(f"{FILE_1_LIST}.save"),
++        )
++        self.assertTrue(
++            os.path.exists(f"{FILE_2_SOURCES}.save"),
++        )
++        self.manager.dispatch_message(
++            {
++                "type": "apt-sources-replace",
++                "sources": [],
++                "gpg-keys": [],
++                "operation-id": 2,
++            },
++        )
++        self.assertTrue(
++            os.path.exists(FILE_1_LIST),
++        )
++        self.assertTrue(os.path.exists(FILE_2_SOURCES))
++        self.assertFalse(
++            os.path.exists(f"{FILE_1_LIST}.save"),
++        )
++        self.assertFalse(
++            os.path.exists(f"{FILE_2_SOURCES}.save"),
++        )
++    def test_restore_sources_list_d_removes_old_profile_files(self):
++        """
++        When getting a repository message without sources, old
++        source files in `/etc/apt/sources.list.d` prefixed with
++        `landscape-` will be removed.
++        """
++        first_source_name = "ginger"
++        self.manager.dispatch_message(
++            {
++                "type": "apt-sources-replace",
++                "sources": [{"name": first_source_name, "content": b""}],
++                "gpg-keys": [],
++                "operation-id": 1,
++            },
++        )
++        first_sources_path = os.path.join(
++            self.sourceslist.SOURCES_LIST_D,
++            f"landscape-{first_source_name}.list",
++        )
++        self.assertTrue(os.path.exists(first_sources_path))
++        second_source_name = "ace rothstein"
++        self.manager.dispatch_message(
++            {
++                "type": "apt-sources-replace",
++                "sources": [{"name": second_source_name, "content": b""}],
++                "gpg-keys": [],
++                "operation-id": 2,
++            },
++        )
++        second_sources_path = os.path.join(
++            self.sourceslist.SOURCES_LIST_D,
++            f"landscape-{second_source_name}.list",
++        )
++        self.assertTrue(os.path.exists(f"{first_sources_path}.save"))
++        self.assertTrue(os.path.exists(second_sources_path))
++        self.manager.dispatch_message(
++            {
++                "type": "apt-sources-replace",
++                "sources": [],
++                "gpg-keys": [],
++                "operation-id": 3,
++            },
++        )
++        self.assertFalse(os.path.exists(first_sources_path))
++        self.assertFalse(os.path.exists(second_sources_path))
++
+     def test_sources_list_permissions(self):
+         """
+         When getting a repository message, L{AptSources} keeps sources.list
+@@ -286,12 +389,16 @@
+         self.manager.dispatch_message(
+             {
+                 "type": "apt-sources-replace",
+-                "sources": [],
++                "sources": [{"name": "bla", "content": b""}],
+                 "gpg-keys": [],
+                 "operation-id": 1,
+             },
+         )
+ 
++        self.assertTrue(
++            os.path.exists(self.sourceslist.SOURCES_LIST),
++        )
++
+         self.assertFalse(
+             os.path.exists(FILE_1_LIST),
+         )
diff --git a/debian/patches/series b/debian/patches/series
index ac651f9..8383cb0 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,2 +1,3 @@
 fix-landscape-client-manpage.patch
 2087852-feat-manage-ubuntu-sources-glob.patch
+fix-apt-source-file-management.patch