launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/git-path-tags into lp:launchpad-buildd with lp:~cjwatson/launchpad-buildd/refactor-vcs as a prerequisite.

Commit message:
Allow checking out a git tag rather than a branch.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1687078 in launchpad-buildd: "LP unable to build snap from a git tag"
  https://bugs.launchpad.net/launchpad-buildd/+bug/1687078

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/git-path-tags/+merge/344995
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/git-path-tags into lp:launchpad-buildd.

=== modified file 'debian/changelog'
--- debian/changelog	2018-05-03 00:35:21 +0000
+++ debian/changelog	2018-05-03 00:35:21 +0000
@@ -28,6 +28,7 @@
 =======
   * Refactor VCS operations from lpbuildd.target.build_snap out to a module
     that can be used by other targets.
+  * Allow checking out a git tag rather than a branch (LP: #1687078).
 >>>>>>> MERGE-SOURCE
 
  -- Colin Watson <cjwatson@xxxxxxxxxx>  Fri, 23 Mar 2018 07:53:24 +0000

=== modified file 'lpbuildd/target/build_snap.py'
--- lpbuildd/target/build_snap.py	2018-05-03 00:35:21 +0000
+++ lpbuildd/target/build_snap.py	2018-05-03 00:35:21 +0000
@@ -162,7 +162,9 @@
                 self.args.git_path
                 if self.args.git_path is not None else "HEAD")
             status["revision_id"] = self.run_build_command(
-                ["git", "rev-parse", rev],
+                # The ^{} suffix copes with tags: we want to peel them
+                # recursively until we get an actual commit.
+                ["git", "rev-parse", rev + "^{}"],
                 cwd=os.path.join("/build", self.args.name),
                 get_output=True).rstrip("\n")
         self.save_status(status)

=== modified file 'lpbuildd/target/tests/test_build_snap.py'
--- lpbuildd/target/tests/test_build_snap.py	2018-05-03 00:35:21 +0000
+++ lpbuildd/target/tests/test_build_snap.py	2018-05-03 00:35:21 +0000
@@ -207,7 +207,7 @@
                 ["git", "submodule", "update", "--init", "--recursive"],
                 cwd="/build/test-snap"),
             RanBuildCommand(
-                ["git", "rev-parse", "HEAD"],
+                ["git", "rev-parse", "HEAD^{}"],
                 cwd="/build/test-snap", get_output=True),
             ]))
         status_path = os.path.join(build_snap.backend.build_path, "status")
@@ -232,7 +232,33 @@
                 ["git", "submodule", "update", "--init", "--recursive"],
                 cwd="/build/test-snap"),
             RanBuildCommand(
-                ["git", "rev-parse", "next"],
+                ["git", "rev-parse", "next^{}"],
+                cwd="/build/test-snap", get_output=True),
+            ]))
+        status_path = os.path.join(build_snap.backend.build_path, "status")
+        with open(status_path) as status:
+            self.assertEqual({"revision_id": "0" * 40}, json.load(status))
+
+    def test_repo_git_with_tag_path(self):
+        args = [
+            "buildsnap",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--git-repository", "lp:foo", "--git-path", "refs/tags/1.0",
+            "test-snap",
+            ]
+        build_snap = parse_args(args=args).operation
+        build_snap.backend.build_path = self.useFixture(TempDir()).path
+        build_snap.backend.run = FakeRevisionID("0" * 40)
+        build_snap.repo()
+        self.assertThat(build_snap.backend.run.calls, MatchesListwise([
+            RanBuildCommand(
+                ["git", "clone", "-b", "1.0", "lp:foo", "test-snap"],
+                cwd="/build"),
+            RanBuildCommand(
+                ["git", "submodule", "update", "--init", "--recursive"],
+                cwd="/build/test-snap"),
+            RanBuildCommand(
+                ["git", "rev-parse", "refs/tags/1.0^{}"],
                 cwd="/build/test-snap", get_output=True),
             ]))
         status_path = os.path.join(build_snap.backend.build_path, "status")
@@ -263,7 +289,7 @@
                 ["git", "submodule", "update", "--init", "--recursive"],
                 cwd="/build/test-snap", **env),
             RanBuildCommand(
-                ["git", "rev-parse", "HEAD"],
+                ["git", "rev-parse", "HEAD^{}"],
                 cwd="/build/test-snap", get_output=True),
             ]))
         status_path = os.path.join(build_snap.backend.build_path, "status")

=== modified file 'lpbuildd/target/vcs.py'
--- lpbuildd/target/vcs.py	2018-05-03 00:35:21 +0000
+++ lpbuildd/target/vcs.py	2018-05-03 00:35:21 +0000
@@ -75,7 +75,19 @@
             if quiet:
                 cmd.append("-q")
             if self.args.git_path is not None:
-                cmd.extend(["-b", self.args.git_path])
+                git_path = self.args.git_path
+                # "git clone -b" is a bit odd: it takes either branches or
+                # tags, but they must be in their short form, i.e. "master"
+                # rather than "refs/heads/master" and "1.0" rather than
+                # "refs/tags/1.0".  There's thus room for ambiguity if a
+                # repository has a branch and a tag with the same name (the
+                # branch will win), but using tags in the first place is
+                # pretty rare here and a name collision is rarer still.
+                # Launchpad shortens branch names before sending them to us,
+                # but not tag names.
+                if git_path.startswith("refs/tags/"):
+                    git_path = git_path[len("refs/tags/"):]
+                cmd.extend(["-b", git_path])
             cmd.extend([self.args.git_repository, name])
             if not self.ssl_verify:
                 env["GIT_SSL_NO_VERIFY"] = "1"