launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+469997@xxxxxxxxxxxxxxxxxx>]

Jürgen Gmach has proposed merging ~jugmac00/launchpad-buildd:fix-branch-scalability-issues into launchpad-buildd:master.

Commit message:
Fix branch scalability issues for OCI builds

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #2067047 in launchpad-buildd: "OCI recipe builds failing due to branch scalability issue"
  https://bugs.launchpad.net/launchpad-buildd/+bug/2067047

For more details, see:
https://code.launchpad.net/~jugmac00/launchpad-buildd/+git/launchpad-buildd/+merge/469997
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/launchpad-buildd:fix-branch-scalability-issues into launchpad-buildd:master.

diff --git a/debian/changelog b/debian/changelog
index cc5ceb1..136911c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+launchpad-buildd (240) UNRELEASED; urgency=medium
+
+  * Prefer `--single-branch` over `--no-single-branch` when cloning
+    repositories to avoid branch scalability issues. Fixes LP:#2067047
+
+ -- Jürgen Gmach <juergen.gmach@xxxxxxxxxxxxx>  Wed, 24 Jul 2024 08:22:42 +0200
+
 launchpad-buildd (239) focal; urgency=medium
 
   * Restart snapd after certificate installation and proxy initialization.
diff --git a/lpbuildd/target/build_oci.py b/lpbuildd/target/build_oci.py
index 1a52854..0ac32ab 100644
--- a/lpbuildd/target/build_oci.py
+++ b/lpbuildd/target/build_oci.py
@@ -92,7 +92,10 @@ class BuildOCI(
         logger.info("Running repo phase...")
         env = self.build_proxy_environment(proxy_url=self.args.proxy_url)
         self.vcs_fetch(
-            self.args.name, cwd="/home/buildd", env=env, git_shallow_clone=True
+            self.args.name,
+            cwd="/home/buildd",
+            env=env,
+            git_shallow_clone_with_single_branch=True,
         )
 
     def build(self):
diff --git a/lpbuildd/target/tests/test_build_oci.py b/lpbuildd/target/tests/test_build_oci.py
index 116d343..1fb098f 100644
--- a/lpbuildd/target/tests/test_build_oci.py
+++ b/lpbuildd/target/tests/test_build_oci.py
@@ -271,7 +271,9 @@ class TestBuildOCI(TestCase):
                             "-n",
                             "--depth",
                             "1",
-                            "--no-single-branch",
+                            "-b",
+                            "HEAD",
+                            "--single-branch",
                             "lp:foo",
                             "test-image",
                         ],
@@ -323,7 +325,9 @@ class TestBuildOCI(TestCase):
                             "-n",
                             "--depth",
                             "1",
-                            "--no-single-branch",
+                            "-b",
+                            "next",
+                            "--single-branch",
                             "lp:foo",
                             "test-image",
                         ],
@@ -375,7 +379,9 @@ class TestBuildOCI(TestCase):
                             "-n",
                             "--depth",
                             "1",
-                            "--no-single-branch",
+                            "-b",
+                            "refs/tags/1.0",
+                            "--single-branch",
                             "lp:foo",
                             "test-image",
                         ],
@@ -433,7 +439,9 @@ class TestBuildOCI(TestCase):
                             "-n",
                             "--depth",
                             "1",
-                            "--no-single-branch",
+                            "-b",
+                            "HEAD",
+                            "--single-branch",
                             "lp:foo",
                             "test-image",
                         ],
diff --git a/lpbuildd/target/vcs.py b/lpbuildd/target/vcs.py
index 6df9a1a..b5bc949 100644
--- a/lpbuildd/target/vcs.py
+++ b/lpbuildd/target/vcs.py
@@ -59,13 +59,21 @@ class VCSOperationMixin(StatusOperationMixin):
             return ["git"]
 
     def vcs_fetch(
-        self, name, cwd, env=None, quiet=False, git_shallow_clone=False
+        self,
+        name,
+        cwd,
+        env=None,
+        quiet=False,
+        git_shallow_clone_with_single_branch=False,
     ):
         full_env = OrderedDict()
         full_env["LANG"] = "C.UTF-8"
         full_env["SHELL"] = "/bin/sh"
         if env:
             full_env.update(env)
+        # XXX: jugmac00 2024-07-24: this method could be refactored to make it
+        # more clear that we both handle the bzr and the git case
+        # or even better, we should have separate classes to handle git and bzr
         if self.args.branch is not None:
             cmd = ["bzr", "branch"]
             if quiet:
@@ -78,17 +86,17 @@ class VCSOperationMixin(StatusOperationMixin):
             cmd = ["git", "clone", "-n"]
             if quiet:
                 cmd.append("-q")
-            if git_shallow_clone:
-                cmd.extend(["--depth", "1", "--no-single-branch"])
+            git_path = self.args.git_path
+            if self.args.git_path is None:
+                git_path = "HEAD"
+            if git_shallow_clone_with_single_branch:
+                cmd.extend(["--depth", "1", "-b", git_path, "--single-branch"])
             cmd.extend([self.args.git_repository, name])
             if not self.ssl_verify:
                 env["GIT_SSL_NO_VERIFY"] = "1"
         self.backend.run(cmd, cwd=cwd, env=full_env)
         if self.args.git_repository is not None:
             repository = os.path.join(cwd, name)
-            git_path = self.args.git_path
-            if self.args.git_path is None:
-                git_path = "HEAD"
             self.backend.run(
                 ["git", "checkout", "-q", git_path],
                 cwd=repository,