Merge lp:~cjwatson/launchpad-buildd/git-path-tags into lp:launchpad-buildd

[Colin Watson]

Allow checking out a git tag rather than a branch.

[William Grant (wgrant)]

review: Approve (code)

[lp:~cjwatson/launchpad-buildd/git-path-tags]

330. By Colin Watson on 2018-06-05

Merge trunk.

=== modified file 'debian/changelog'
--- debian/changelog	2018-06-05 02:04:39 +0000
+++ debian/changelog	2018-06-05 02:07:44 +0000
@@ -9,6 +9,7 @@
     VM that is guaranteed to be torn down after the build.
   * 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).
 
  -- Colin Watson <[e-mail]>  Tue, 08 May 2018 10:36:22 +0100
 
 
=== modified file 'lpbuildd/target/build_snap.py'
--- lpbuildd/target/build_snap.py	2018-06-05 02:04:39 +0000
+++ lpbuildd/target/build_snap.py	2018-06-05 02:07:44 +0000
@@ -150,7 +150,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-06-05 02:04:39 +0000
+++ lpbuildd/target/tests/test_build_snap.py	2018-06-05 02:07:44 +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-03-14 14:04:22 +0000
+++ lpbuildd/target/vcs.py	2018-06-05 02:07:44 +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"