Merge lp:~ubuntu-cdimage/ubuntu-cdimage/focal into lp:ubuntu-cdimage

[Iain Lane]

Various fixes to make cdimage work on focal (ancientminister, new cdimage host). I tried to retain compatibility with xenial (nusakan) - the changes have been tested there in isolation but I've not actually pulled an updated cdimage to test them end-to-end.

Mainly launchpadlib, gpg and bittorrent-related changes.

I could split this up into logical MPs if you want (but bzr...).

[Iain Lane (laney)]

This and https://code.launchpad.net/~laney/debian-cd/focal/+merge/386316 depend on each other.

[Iain Lane (laney)]

Make that https://code.launchpad.net/~laney/debian-cd/focal/+merge/386318

[Steve Langasek (vorlon)]

when you say they depend on each other, do you mean that they need to land together in order to not break builds on nusakan?

[Steve Langasek (vorlon)]

review: Needs Fixing

[Iain Lane (laney)]

Nah, that's not the best wording. It's more that they both need to land to work on focal.

[Steve Langasek (vorlon)]

ok, lgtm then

review: Approve

[lp:~ubuntu-cdimage/ubuntu-cdimage/focal]

1879. By Iain Lane on 2020-06-29

sign: Drop privkeydir in _signing_command(), it's unused there

=== modified file 'lib/cdimage/launchpad.py'
--- lib/cdimage/launchpad.py	2019-01-11 17:45:45 +0000
+++ lib/cdimage/launchpad.py	2020-06-29 11:27:35 +0000
@@ -148,7 +148,20 @@
 
 
 def login(instance):
-    return Launchpad.login_with("ubuntu-cdimage", instance, version="devel")
+    try:
+        import launchpadlib.credentials
+        store = launchpadlib.credentials.KeyringCredentialStore(None,
+                                                                fallback=True)
+        return Launchpad.login_with("ubuntu-cdimage",
+                                    instance,
+                                    version="devel",
+                                    credential_store=store)
+    except TypeError:
+        # prior to focal (1.10.13), the fallback= kwarg didn't exist and
+        # Launchpadlib worked under sudo
+        return Launchpad.login_with("ubuntu-cdimage",
+                                    instance,
+                                    version="devel")
 
 
 class _LaunchpadCache:
 
=== modified file 'lib/cdimage/sign.py'
--- lib/cdimage/sign.py	2015-11-11 20:06:02 +0000
+++ lib/cdimage/sign.py	2020-06-29 11:27:35 +0000
@@ -23,30 +23,30 @@
 
 
 def _gnupg_files(config):
+    gpgdir = config["GNUPG_DIR"]
     gpgconf = os.path.join(config["GNUPG_DIR"], "gpg.conf")
     secring = os.path.join(config["GNUPG_DIR"], "secring.gpg")
+    privkeydir = os.path.join(config["GNUPG_DIR"], "private-keys-v1.d")
     pubring = os.path.join(config["GNUPG_DIR"], "pubring.gpg")
     trustdb = os.path.join(config["GNUPG_DIR"], "trustdb.gpg")
-    return gpgconf, secring, pubring, trustdb
+    return gpgdir, gpgconf, secring, privkeydir, pubring, trustdb
 
 
 def can_sign(config):
-    gpgconf, secring, pubring, trustdb = _gnupg_files(config)
-    if (not os.path.exists(secring) or not os.path.exists(pubring) or
-            not os.path.exists(trustdb) or not config["SIGNING_KEYID"]):
+    _, _, secring, privkeydir, pubring, trustdb = _gnupg_files(config)
+    if (not (os.path.exists(privkeydir) or os.path.exists(secring)) or
+        not os.path.exists(pubring) or not os.path.exists(trustdb) or
+       not config["SIGNING_KEYID"]):
         logger.warning("No keys found; not signing images.")
         return False
     return True
 
 
 def _signing_command(config):
-    gpgconf, secring, pubring, trustdb = _gnupg_files(config)
+    gpgdir, gpgconf, _, _, _, _ = _gnupg_files(config)
     cmd = [
         "gpg", "--options", gpgconf,
-        "--no-default-keyring",
-        "--secret-keyring", secring,
-        "--keyring", pubring,
-        "--trustdb-name", trustdb,
+        "--homedir", gpgdir,
         "--no-options", "--batch", "--no-tty",
         "--armour", "--detach-sign",
         # FBB75451 and EFE21092 have different digest preferences.  GnuPG
 
=== modified file 'lib/cdimage/tests/test_checksums.py'
--- lib/cdimage/tests/test_checksums.py	2020-06-17 00:21:41 +0000
+++ lib/cdimage/tests/test_checksums.py	2020-06-29 11:27:35 +0000
@@ -88,7 +88,8 @@
         checksum_file = ChecksumFile(
             self.config, self.temp_dir, "SHA256SUMS", hashlib.sha256)
         self.assertEqual(
-            hashlib.sha256(data).hexdigest(), checksum_file.checksum(entry_path))
+            hashlib.sha256(data).hexdigest(),
+            checksum_file.checksum(entry_path))
 
     def test_add(self):
         entry_path = os.path.join(self.temp_dir, "entry")
 
=== modified file 'lib/cdimage/tests/test_sign.py'
--- lib/cdimage/tests/test_sign.py	2015-11-11 20:06:02 +0000
+++ lib/cdimage/tests/test_sign.py	2020-06-29 11:27:35 +0000
@@ -34,9 +34,12 @@
     def test_gnupg_files(self):
         config = Config(read=False)
         config["GNUPG_DIR"] = "/path"
-        gpgconf, secring, pubring, trustdb = _gnupg_files(config)
+        gpgdir, gpgconf, secring, privkeydir, pubring, trustdb = \
+            _gnupg_files(config)
+        self.assertEqual("/path", gpgdir)
         self.assertEqual("/path/gpg.conf", gpgconf)
         self.assertEqual("/path/secring.gpg", secring)
+        self.assertEqual("/path/private-keys-v1.d", privkeydir)
         self.assertEqual("/path/pubring.gpg", pubring)
         self.assertEqual("/path/trustdb.gpg", trustdb)
 
@@ -47,10 +50,7 @@
         command = _signing_command(config)
         self.assertEqual([
             "gpg", "--options", "/path/gpg.conf",
-            "--no-default-keyring",
-            "--secret-keyring", "/path/secring.gpg",
-            "--keyring", "/path/pubring.gpg",
-            "--trustdb-name", "/path/trustdb.gpg",
+            "--homedir", "/path",
             "--no-options", "--batch", "--no-tty",
             "--armour", "--detach-sign",
             "--digest-algo", "SHA512",
@@ -64,10 +64,7 @@
         command = _signing_command(config)
         self.assertEqual([
             "gpg", "--options", "/path/gpg.conf",
-            "--no-default-keyring",
-            "--secret-keyring", "/path/secring.gpg",
-            "--keyring", "/path/pubring.gpg",
-            "--trustdb-name", "/path/trustdb.gpg",
+            "--homedir", "/path",
             "--no-options", "--batch", "--no-tty",
             "--armour", "--detach-sign",
             "--digest-algo", "SHA512",
@@ -97,7 +94,8 @@
         config = Config(read=False)
         config["GNUPG_DIR"] = self.use_temp_dir()
         config["SIGNING_KEYID"] = "01234567"
-        gpgconf, secring, pubring, trustdb = _gnupg_files(config)
+        gpgdir, gpgconf, secring, privkeydir, pubring, trustdb = \
+            _gnupg_files(config)
         sign_path = os.path.join(self.temp_dir, "to-sign")
         for path in gpgconf, secring, pubring, trustdb, sign_path:
             touch(path)
@@ -106,10 +104,7 @@
         self.assertLogEqual([])
         expected_command = [
             "gpg", "--options", gpgconf,
-            "--no-default-keyring",
-            "--secret-keyring", secring,
-            "--keyring", pubring,
-            "--trustdb-name", trustdb,
+            "--homedir", config["GNUPG_DIR"],
             "--no-options", "--batch", "--no-tty",
             "--armour", "--detach-sign",
             "--digest-algo", "SHA512",
@@ -127,13 +122,15 @@
         config = Config(read=False)
         config["GNUPG_DIR"] = self.use_temp_dir()
         config["SIGNING_KEYID"] = "01234567"
-        gpgconf, secring, pubring, trustdb = _gnupg_files(config)
+        gpgdir, gpgconf, secring, privkeydir, pubring, trustdb = \
+            _gnupg_files(config)
         sign_path = os.path.join(self.temp_dir, "to-sign")
         for path in gpgconf, secring, pubring, trustdb, sign_path:
             touch(path)
         touch("%s.gpg" % sign_path)
         self.capture_logging()
-        self.assertRaises(
-            subprocess.CalledProcessError, sign_cdimage, config, sign_path)
+        with self.assertRaises(subprocess.CalledProcessError):
+            sign_cdimage(config, sign_path)
+            mock_check_call.assert_called()
         self.assertLogEqual([])
         self.assertFalse(os.path.exists("%s.gpg" % sign_path))
 
=== modified file 'lib/cdimage/tests/test_tree.py'
--- lib/cdimage/tests/test_tree.py	2020-06-19 00:57:47 +0000
+++ lib/cdimage/tests/test_tree.py	2020-06-29 11:27:35 +0000
@@ -2293,8 +2293,8 @@
             "preinstalled-netbook"))
 
 
-def call_btmakemetafile_zsyncmake(command, *args, **kwargs):
-    if command[0] == "btmakemetafile":
+def call_mktorrent_zsyncmake(command, *args, **kwargs):
+    if command[0] == "mktorrent":
         touch("%s.torrent" % command[-1])
     elif command[0] == "zsyncmake":
         for i in range(1, len(command)):
@@ -2396,7 +2396,7 @@
         self.assertLogEqual(
             ["Creating torrent for %s ..." % path for path in paths])
         command_base = [
-            "btmakemetafile", "https://torrent.ubuntu.com/announce",
+            "mktorrent", "-a", "https://torrent.ubuntu.com/announce",
             "--comment", "Ubuntu CD cdimage.ubuntu.com",
         ]
         mock_check_call.assert_has_calls([
@@ -2417,7 +2417,7 @@
             self.get_publisher(official="named").publish_release_prefixes())
 
     @mock.patch("cdimage.osextras.find_on_path", return_value=True)
-    @mock.patch("subprocess.call", side_effect=call_btmakemetafile_zsyncmake)
+    @mock.patch("subprocess.call", side_effect=call_mktorrent_zsyncmake)
     def test_publish_release_arch_ubuntu_desktop_named(self, mock_call, *args):
         self.config["PROJECT"] = "ubuntu"
         self.config["CAPPROJECT"] = "Ubuntu"
@@ -2461,7 +2461,7 @@
                 "%s.iso" % target_base,
             ]),
             mock.call([
-                "btmakemetafile", mock.ANY,
+                "mktorrent", mock.ANY, mock.ANY,
                 "--comment", "Ubuntu CD cdimage.ubuntu.com",
                 "%s.iso" % target_base,
             ], stdout=mock.ANY),
@@ -2479,7 +2479,7 @@
             os.stat("%s.iso.torrent" % torrent_base))
 
     @mock.patch("cdimage.osextras.find_on_path", return_value=True)
-    @mock.patch("subprocess.call", side_effect=call_btmakemetafile_zsyncmake)
+    @mock.patch("subprocess.call", side_effect=call_mktorrent_zsyncmake)
     def test_publish_release_arch_ubuntu_desktop_no(self, mock_call, *args):
         self.config["PROJECT"] = "ubuntu"
         self.config["CAPPROJECT"] = "Ubuntu"
@@ -2512,7 +2512,7 @@
         self.assertFalse(os.path.islink("%s.iso" % target_base))
         self.assertFalse(os.path.islink("%s.manifest" % target_base))
         mock_call.assert_called_once_with([
-            "btmakemetafile", mock.ANY,
+            "mktorrent", mock.ANY, mock.ANY,
             "--comment", "Ubuntu CD cdimage.ubuntu.com",
             "%s.iso" % target_base,
         ], stdout=mock.ANY)
@@ -2527,7 +2527,7 @@
             os.stat("%s.iso.torrent" % torrent_base))
 
     @mock.patch("cdimage.osextras.find_on_path", return_value=True)
-    @mock.patch("subprocess.call", side_effect=call_btmakemetafile_zsyncmake)
+    @mock.patch("subprocess.call", side_effect=call_mktorrent_zsyncmake)
     def test_publish_release_kubuntu_desktop_named(self, mock_call, *args):
         self.config["PROJECT"] = "kubuntu"
         self.config["CAPPROJECT"] = "Kubuntu"
@@ -2685,10 +2685,9 @@
         publisher.make_torrents(
             os.path.join(self.temp_dir, "dir"), "ubuntu-13.04")
         command_base = [
-            "btmakemetafile", "https://torrent.ubuntu.com/announce",
-            "--announce_list",
-            ("https://torrent.ubuntu.com/announce|"
-                "https://ipv6.torrent.ubuntu.com/announce"),
+            "mktorrent",
+            "-a", "https://torrent.ubuntu.com/announce",
+            "-a", "https://ipv6.torrent.ubuntu.com/announce",
             "--comment", "Ubuntu CD releases.ubuntu.com",
         ]
         mock_check_call.assert_has_calls([
@@ -2708,7 +2707,7 @@
             self.get_publisher().publish_release_prefixes())
 
     @mock.patch("cdimage.osextras.find_on_path", return_value=True)
-    @mock.patch("subprocess.call", side_effect=call_btmakemetafile_zsyncmake)
+    @mock.patch("subprocess.call", side_effect=call_mktorrent_zsyncmake)
     def test_publish_release_arch_ubuntu_desktop_yes(self, mock_call, *args):
         self.config["PROJECT"] = "ubuntu"
         self.config["CAPPROJECT"] = "Ubuntu"
@@ -2766,8 +2765,9 @@
                 "%s.iso" % pool_base,
             ]),
             mock.call([
-                "btmakemetafile", mock.ANY,
-                "--announce_list", mock.ANY,
+                "mktorrent",
+                "-a", mock.ANY,
+                "-a", mock.ANY,
                 "--comment", "Ubuntu CD releases.ubuntu.com",
                 "%s.iso" % target_base,
             ], stdout=mock.ANY),
@@ -2785,7 +2785,7 @@
             os.stat("%s.iso.torrent" % torrent_base))
 
     @mock.patch("cdimage.osextras.find_on_path", return_value=True)
-    @mock.patch("subprocess.call", side_effect=call_btmakemetafile_zsyncmake)
+    @mock.patch("subprocess.call", side_effect=call_mktorrent_zsyncmake)
     def test_publish_release_arch_ubuntu_desktop_poolonly(self, mock_call,
                                                           *args):
         self.config["PROJECT"] = "ubuntu"
@@ -2823,7 +2823,7 @@
         ])
 
     @mock.patch("cdimage.osextras.find_on_path", return_value=True)
-    @mock.patch("subprocess.call", side_effect=call_btmakemetafile_zsyncmake)
+    @mock.patch("subprocess.call", side_effect=call_mktorrent_zsyncmake)
     def test_publish_release_kubuntu_desktop_yes(self, mock_call, *args):
         self.config["PROJECT"] = "kubuntu"
         self.config["CAPPROJECT"] = "Kubuntu"
 
=== modified file 'lib/cdimage/tree.py'
--- lib/cdimage/tree.py	2020-06-19 00:57:47 +0000
+++ lib/cdimage/tree.py	2020-06-29 11:27:35 +0000
@@ -2971,15 +2971,9 @@
         if not self.dry_run:
             logger.info("Creating torrent for %s ..." % path)
         osextras.unlink_force("%s.torrent" % path)
-        command = ["btmakemetafile", self.torrent_tracker]
+        command = ["mktorrent", "-a", self.torrent_tracker]
         if isinstance(self.tree, SimpleReleaseTree):
-            # N.B.: Only the bittornado version of btmakemetafile has
-            # the --announce_list flag.
-            command.extend([
-                "--announce_list",
-                "%s|%s" % (
-                    self.torrent_tracker, self.ipv6_torrent_tracker),
-            ])
+            command.extend(["-a", self.ipv6_torrent_tracker])
         command.extend([
             "--comment",
             "%s CD %s" % (self.config.capproject, self.tree.site_name),