launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21530
[Merge] lp:~cjwatson/launchpad/git-path-HEAD into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/git-path-HEAD into lp:launchpad.
Commit message:
Allow configuring a snap to build from the current branch of a Git repository rather than explicitly naming a branch.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1688224 in Launchpad itself: "Allow building snap from current default branch of repository"
https://bugs.launchpad.net/launchpad/+bug/1688224
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-path-HEAD/+merge/323660
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.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-path-HEAD into lp:launchpad.
=== 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)
Follow ups
