Merge lp:~cjwatson/launchpad/git-path-HEAD into lp:launchpad

[Colin Watson]

Allow configuring a snap to build from the current branch of a Git repository rather than explicitly naming a branch.

This is a better default for build.snapcraft.io than hardcoding master.

This must not be landed until https://code.launchpad.net/~cjwatson/launchpad-buildd/snap-default-branch/+merge/323604 has been deployed to production.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2016-12-02 12:04:11 +0000
+++ lib/lp/code/configure.zcml	2017-05-05 12:17:30 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -896,6 +896,11 @@
         permission="launchpad.View"
         interface="lp.code.interfaces.gitref.IGitRef" />
   </class>
+  <class class="lp.code.model.gitref.GitRefDefault">
+    <require
+        permission="launchpad.View"
+        interface="lp.code.interfaces.gitref.IGitRef" />
+  </class>
   <class class="lp.code.model.gitref.GitRefFrozen">
     <require
         permission="launchpad.View"

=== modified file 'lib/lp/code/model/gitref.py'
--- lib/lp/code/model/gitref.py	2016-12-05 14:46:40 +0000
+++ lib/lp/code/model/gitref.py	2017-05-05 12:17:30 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -536,8 +536,72 @@
             commit_message=commit_message, review_requests=review_requests)
 
 
-@implementer(IGitRef)
-class GitRefFrozen(GitRefMixin):
+class GitRefDatabaseBackedMixin(GitRefMixin):
+    """A mixin for virtual Git references backed by a database record."""
+
+    @property
+    def _non_database_attributes(self):
+        """A sequence of attributes not backed by the database."""
+        raise NotImplementedError()
+
+    @property
+    def _self_in_database(self):
+        """Return the equivalent database-backed record of self."""
+        raise NotImplementedError()
+
+    def __getattr__(self, name):
+        return getattr(self._self_in_database, name)
+
+    def __setattr__(self, name, value):
+        if name in self._non_database_attributes:
+            self.__dict__[name] = value
+        else:
+            setattr(self._self_in_database, name, value)
+
+    def __eq__(self, other):
+        return (
+            self.repository == other.repository and
+            self.path == other.path and
+            self.commit_sha1 == other.commit_sha1)
+
+    def __ne__(self, other):
+        return not self == other
+
+    def __hash__(self):
+        return hash(self.repository) ^ hash(self.path) ^ hash(self.commit_sha1)
+
+
+@implementer(IGitRef)
+class GitRefDefault(GitRefDatabaseBackedMixin):
+    """A reference to the default branch in a Git repository.
+
+    This always refers to whatever the default branch currently is, even if
+    it changes later.
+    """
+
+    def __init__(self, repository):
+        self.repository_id = repository.id
+        self.repository = repository
+        self.path = u"HEAD"
+
+    _non_database_attributes = ("repository_id", "repository", "path")
+
+    @property
+    def _self_in_database(self):
+        """See `GitRefDatabaseBackedMixin`."""
+        path = self.repository.default_branch
+        if path is None:
+            raise NotFoundError("Repository '%s' has no default branch")
+        ref = IStore(GitRef).get(GitRef, (self.repository_id, path))
+        if ref is None:
+            raise NotFoundError(
+                "Repository '%s' has default branch '%s', but there is no "
+                "such reference" % (self.repository, path))
+        return ref
+
+
+@implementer(IGitRef)
+class GitRefFrozen(GitRefDatabaseBackedMixin):
     """A frozen Git reference.
 
     This is like a GitRef, but is frozen at a particular commit, even if the
@@ -554,9 +618,12 @@
         self.path = path
         self.commit_sha1 = commit_sha1
 
+    _non_database_attributes = (
+        "repository_id", "repository", "path", "commit_sha1")
+
     @property
     def _self_in_database(self):
-        """Return the equivalent database-backed record of self."""
+        """See `GitRefDatabaseBackedMixin`."""
         ref = IStore(GitRef).get(GitRef, (self.repository_id, self.path))
         if ref is None:
             raise NotFoundError(
@@ -564,27 +631,6 @@
                 "'%s'" % (self.repository, self.path))
         return ref
 
-    def __getattr__(self, name):
-        return getattr(self._self_in_database, name)
-
-    def __setattr__(self, name, value):
-        if name in ("repository_id", "repository", "path", "commit_sha1"):
-            self.__dict__[name] = value
-        else:
-            setattr(self._self_in_database, name, value)
-
-    def __eq__(self, other):
-        return (
-            self.repository == other.repository and
-            self.path == other.path and
-            self.commit_sha1 == other.commit_sha1)
-
-    def __ne__(self, other):
-        return not self == other
-
-    def __hash__(self):
-        return hash(self.repository) ^ hash(self.path) ^ hash(self.commit_sha1)
-
 
 @implementer(IGitRef)
 @provider(IGitRefRemoteSet)

=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2017-02-10 12:52:07 +0000
+++ lib/lp/code/model/gitrepository.py	2017-05-05 12:17:30 +0000
@@ -107,7 +107,10 @@
 from lp.code.interfaces.revision import IRevisionSet
 from lp.code.mail.branch import send_git_repository_modified_notifications
 from lp.code.model.branchmergeproposal import BranchMergeProposal
-from lp.code.model.gitref import GitRef
+from lp.code.model.gitref import (
+    GitRef,
+    GitRefDefault,
+    )
 from lp.code.model.gitsubscription import GitSubscription
 from lp.registry.enums import PersonVisibility
 from lp.registry.errors import CannotChangeInformationType
@@ -515,6 +518,8 @@
                 self.getInternalPath(), default_branch=ref.path)
 
     def getRefByPath(self, path):
+        if path == u"HEAD":
+            return GitRefDefault(self)
         paths = [path]
         if not path.startswith(u"refs/heads/"):
             paths.append(u"refs/heads/%s" % path)

=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py	2017-02-10 12:52:07 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py	2017-05-05 12:17:30 +0000
@@ -1182,6 +1182,20 @@
         self.assertEqual(ref, ref.repository.getRefByPath(u"master"))
         self.assertIsNone(ref.repository.getRefByPath(u"other"))
 
+    def test_getRefByPath_HEAD(self):
+        # The special ref path "HEAD" always refers to the current default
+        # branch.
+        [ref] = self.factory.makeGitRefs(paths=[u"refs/heads/master"])
+        ref_HEAD = ref.repository.getRefByPath(u"HEAD")
+        self.assertEqual(ref.repository, ref_HEAD.repository)
+        self.assertEqual(u"HEAD", ref_HEAD.path)
+        self.assertRaises(NotFoundError, getattr, ref_HEAD, "commit_sha1")
+        removeSecurityProxy(ref.repository)._default_branch = (
+            u"refs/heads/missing")
+        self.assertRaises(NotFoundError, getattr, ref_HEAD, "commit_sha1")
+        removeSecurityProxy(ref.repository)._default_branch = ref.path
+        self.assertEqual(ref.commit_sha1, ref_HEAD.commit_sha1)
+
     def test_planRefChanges(self):
         # planRefChanges copes with planning changes to refs in a repository
         # where some refs have been created, some deleted, and some changed.
@@ -2700,6 +2714,7 @@
         repository_db = self.factory.makeGitRepository()
         ref_dbs = self.factory.makeGitRefs(
             repository=repository_db, paths=[u"refs/heads/a", u"refs/heads/b"])
+        removeSecurityProxy(repository_db)._default_branch = u"refs/heads/a"
         repository_url = api_url(repository_db)
         ref_urls = [api_url(ref_db) for ref_db in ref_dbs]
         webservice = webservice_for_person(
@@ -2710,6 +2725,7 @@
                 ("refs/heads/a", ref_urls[0]),
                 ("b", ref_urls[1]),
                 ("refs/heads/b", ref_urls[1]),
+                ("HEAD", "%s/+ref/HEAD" % repository_url),
                 ):
             response = webservice.named_get(
                 repository_url, "getRefByPath", path=path)

=== modified file 'lib/lp/snappy/model/snapbuildbehaviour.py'
--- lib/lp/snappy/model/snapbuildbehaviour.py	2016-12-05 23:11:57 +0000
+++ lib/lp/snappy/model/snapbuildbehaviour.py	2017-05-05 12:17:30 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """An `IBuildFarmJobBehaviour` for `SnapBuild`.
@@ -114,7 +114,8 @@
             # "git clone -b" doesn't accept full ref names.  If this becomes
             # a problem then we could change launchpad-buildd to do "git
             # clone" followed by "git checkout" instead.
-            args["git_path"] = build.snap.git_ref.name
+            if build.snap.git_path != u"HEAD":
+                args["git_path"] = build.snap.git_ref.name
         else:
             raise CannotBuild(
                 "Source branch/repository for ~%s/%s has been deleted." %

=== modified file 'lib/lp/snappy/tests/test_snapbuildbehaviour.py'
--- lib/lp/snappy/tests/test_snapbuildbehaviour.py	2017-01-27 12:44:41 +0000
+++ lib/lp/snappy/tests/test_snapbuildbehaviour.py	2017-05-05 12:17:30 +0000
@@ -22,6 +22,7 @@
 from twisted.internet import defer
 from twisted.trial.unittest import TestCase as TrialTestCase
 from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.builder import CannotBuild
@@ -245,6 +246,26 @@
             }, args)
 
     @defer.inlineCallbacks
+    def test_extraBuildArgs_git_HEAD(self):
+        # _extraBuildArgs returns appropriate arguments if asked to build a
+        # job for the default branch in a Launchpad-hosted Git repository.
+        [ref] = self.factory.makeGitRefs()
+        removeSecurityProxy(ref.repository)._default_branch = ref.path
+        job = self.makeJob(git_ref=ref.repository.getRefByPath(u"HEAD"))
+        expected_archives = get_sources_list_for_building(
+            job.build, job.build.distro_arch_series, None)
+        args = yield job._extraBuildArgs()
+        self.assertEqual({
+            "archive_private": False,
+            "archives": expected_archives,
+            "arch_tag": "i386",
+            "git_repository": ref.repository.git_https_url,
+            "name": u"test-snap",
+            "proxy_url": self.proxy_url,
+            "revocation_endpoint": self.revocation_endpoint,
+            }, args)
+
+    @defer.inlineCallbacks
     def test_extraBuildArgs_git_url(self):
         # _extraBuildArgs returns appropriate arguments if asked to build a
         # job for a Git branch backed by a URL for an external repository.
@@ -267,6 +288,26 @@
             }, args)
 
     @defer.inlineCallbacks
+    def test_extraBuildArgs_git_url_HEAD(self):
+        # _extraBuildArgs returns appropriate arguments if asked to build a
+        # job for the default branch in an external Git repository.
+        url = u"https://git.example.org/foo"
+        ref = self.factory.makeGitRefRemote(repository_url=url, path=u"HEAD")
+        job = self.makeJob(git_ref=ref)
+        expected_archives = get_sources_list_for_building(
+            job.build, job.build.distro_arch_series, None)
+        args = yield job._extraBuildArgs()
+        self.assertEqual({
+            "archive_private": False,
+            "archives": expected_archives,
+            "arch_tag": "i386",
+            "git_repository": url,
+            "name": u"test-snap",
+            "proxy_url": self.proxy_url,
+            "revocation_endpoint": self.revocation_endpoint,
+            }, args)
+
+    @defer.inlineCallbacks
     def test_extraBuildArgs_proxy_url_set(self):
         job = self.makeJob()
         build_request = yield job.composeBuildRequest(None)