launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21262
[Merge] lp:~cjwatson/launchpad/snap-git-url into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-git-url into lp:launchpad with lp:~cjwatson/launchpad/git-ref-remote as a prerequisite.
Commit message:
Allow building snaps from an external Git repository.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-git-url/+merge/312348
The UI for this works to the extent that it displays something approximately reasonable and lets you edit such snaps, but the only reasonable way to create them is via the API [1] since the +new-snap view is based on Branch or GitRef. That's OK for the moment since the immediate audience for this is the separate build.snapcraft.io service.
https://code.launchpad.net/~cjwatson/launchpad-buildd/snap-proxy-allow-repo/+merge/312138 will need to be deployed before dispatching builds based on external repositories will work.
[1] Technically, you can create one based on some LP-hosted repository and then edit it into the shape you want. I don't count that as a reasonable way.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-git-url into lp:launchpad.
=== modified file 'lib/lp/snappy/browser/snap.py'
--- lib/lp/snappy/browser/snap.py 2016-11-07 18:14:32 +0000
+++ lib/lp/snappy/browser/snap.py 2016-12-02 13:18:34 +0000
@@ -700,7 +700,7 @@
custom_widget('store_distro_series', LaunchpadRadioWidget)
custom_widget('store_channels', LabeledMultiCheckBoxWidget)
custom_widget('vcs', LaunchpadRadioWidget)
- custom_widget('git_ref', GitRefWidget)
+ custom_widget('git_ref', GitRefWidget, allow_external=True)
custom_widget('auto_build_archive', SnapArchiveWidget)
custom_widget('auto_build_pocket', LaunchpadDropdownWidget)
=== modified file 'lib/lp/snappy/browser/tests/test_snap.py'
--- lib/lp/snappy/browser/tests/test_snap.py 2016-11-07 18:14:32 +0000
+++ lib/lp/snappy/browser/tests/test_snap.py 2016-12-02 13:18:34 +0000
@@ -724,6 +724,29 @@
"name.",
extract_text(find_tags_by_class(browser.contents, "message")[1]))
+ def test_edit_snap_git_url(self):
+ series = self.factory.makeUbuntuDistroSeries()
+ with admin_logged_in():
+ snappy_series = self.factory.makeSnappySeries(
+ usable_distro_series=[series])
+ old_ref = self.factory.makeGitRefRemote()
+ new_ref = self.factory.makeGitRefRemote()
+ new_repository_url = new_ref.repository_url
+ new_path = new_ref.path
+ snap = self.factory.makeSnap(
+ registrant=self.person, owner=self.person, distroseries=series,
+ git_ref=old_ref, store_series=snappy_series)
+ browser = self.getViewBrowser(snap, user=self.person)
+ browser.getLink("Edit snap package").click()
+ browser.getControl("Git repository").value = new_repository_url
+ browser.getControl("Git branch").value = new_path
+ browser.getControl("Update snap package").click()
+ login_person(self.person)
+ content = find_main_content(browser.contents)
+ self.assertThat(
+ "Source:\n%s\nEdit snap package" % new_ref.display_name,
+ MatchesTagText(content, "source"))
+
def setUpDistroSeries(self):
"""Set up a distroseries with some available processors."""
distroseries = self.factory.makeUbuntuDistroSeries()
@@ -1259,6 +1282,33 @@
Primary Archive for Ubuntu Linux
""", self.getMainText(build.snap))
+ def test_index_git_url(self):
+ ref = self.factory.makeGitRefRemote(
+ repository_url=u"https://git.example.org/foo";,
+ path=u"refs/heads/master")
+ snap = self.makeSnap(git_ref=ref)
+ build = self.makeBuild(
+ snap=snap, status=BuildStatus.FULLYBUILT,
+ duration=timedelta(minutes=30))
+ self.assertTextMatchesExpressionIgnoreWhitespace("""\
+ Snap packages snap-name
+ .*
+ Snap package information
+ Owner: Test Person
+ Distribution series: Ubuntu Shiny
+ Source: https://git.example.org/foo master
+ Build schedule: \(\?\)
+ Built on request
+ Source archive for automatic builds:
+ Pocket for automatic builds:
+ Builds of this snap package are not automatically uploaded to
+ the store.
+ Latest builds
+ Status When complete Architecture Archive
+ Successfully built 30 minutes ago i386
+ Primary Archive for Ubuntu Linux
+ """, self.getMainText(build.snap))
+
def test_index_success_with_buildlog(self):
# The build log is shown if it is there.
build = self.makeBuild(
=== modified file 'lib/lp/snappy/browser/tests/test_snaplisting.py'
--- lib/lp/snappy/browser/tests/test_snaplisting.py 2016-09-07 11:12:58 +0000
+++ lib/lp/snappy/browser/tests/test_snaplisting.py 2016-12-02 13:18:34 +0000
@@ -11,6 +11,7 @@
from lp.code.tests.helpers import GitHostingFixture
from lp.services.database.constants import (
ONE_DAY_AGO,
+ SEVEN_DAYS_AGO,
UTC_NOW,
)
from lp.services.webapp import canonical_url
@@ -107,16 +108,22 @@
owner = self.factory.makePerson(displayname="Snap Owner")
self.factory.makeSnap(
registrant=owner, owner=owner, branch=self.factory.makeAnyBranch(),
+ date_created=SEVEN_DAYS_AGO)
+ [ref] = self.factory.makeGitRefs()
+ self.factory.makeSnap(
+ registrant=owner, owner=owner, git_ref=ref,
date_created=ONE_DAY_AGO)
- [ref] = self.factory.makeGitRefs()
+ remote_ref = self.factory.makeGitRefRemote()
self.factory.makeSnap(
- registrant=owner, owner=owner, git_ref=ref, date_created=UTC_NOW)
+ registrant=owner, owner=owner, git_ref=remote_ref,
+ date_created=UTC_NOW)
text = self.getMainText(owner, "+snaps")
self.assertTextMatchesExpressionIgnoreWhitespace("""
Snap packages for Snap Owner
- Name Source Registered
- snap-name.* ~.*:.* .*
- snap-name.* lp:.* .*""", text)
+ Name Source Registered
+ snap-name.* http://.* path-.* .*
+ snap-name.* ~.*:.* .*
+ snap-name.* lp:.* .*""", text)
def test_project_snap_listing(self):
# We can see snap packages for a project.
=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py 2016-11-22 02:13:11 +0000
+++ lib/lp/snappy/interfaces/snap.py 2016-12-02 13:18:34 +0000
@@ -85,6 +85,7 @@
from lp.services.fields import (
PersonChoice,
PublicPersonChoice,
+ URIField,
)
from lp.services.webhooks.interfaces import IWebhookTarget
from lp.snappy.interfaces.snappyseries import (
@@ -380,6 +381,18 @@
"A Git repository with a branch containing a snapcraft.yaml "
"recipe at the top level.")))
+ git_repository_url = exported(URIField(
+ title=_("Git repository URL"), required=False, readonly=True,
+ description=_(
+ "The URL of a Git repository with a branch containing a "
+ "snapcraft.yaml recipe at the top level."),
+ allowed_schemes=["git", "http", "https"],
+ allow_userinfo=True,
+ allow_port=True,
+ allow_query=False,
+ allow_fragment=False,
+ trailing_slash=False))
+
git_path = exported(TextLine(
title=_("Git branch path"), required=False, readonly=True,
description=_(
@@ -503,11 +516,13 @@
@export_factory_operation(
ISnap, [
"owner", "distro_series", "name", "description", "branch",
- "git_ref", "auto_build", "auto_build_archive", "auto_build_pocket",
+ "git_repository", "git_repository_url", "git_path", "git_ref",
+ "auto_build", "auto_build_archive", "auto_build_pocket",
"private"])
@operation_for_version("devel")
def new(registrant, owner, distro_series, name, description=None,
- branch=None, git_ref=None, auto_build=False,
+ branch=None, git_repository=None, git_repository_url=None,
+ git_path=None, git_ref=None, auto_build=False,
auto_build_archive=None, auto_build_pocket=None,
require_virtualized=True, processors=None, date_created=None,
private=False, store_upload=False, store_series=None,
@@ -577,6 +592,23 @@
:raises BadSnapSearchContext: if the context is not understood.
"""
+ @operation_parameters(url=TextLine(title=_("The URL to search for.")))
+ @call_with(visible_by_user=REQUEST_USER)
+ @operation_returns_collection_of(ISnap)
+ @export_read_operation()
+ @operation_for_version("devel")
+ def findByURL(url, visible_by_user=None):
+ """Return all snap packages that build from the given URL.
+
+ This currently only works for packages that build directly from a
+ URL, rather than being linked to a Bazaar branch or Git repository
+ hosted in Launchpad.
+
+ :param url: A URL.
+ :param visible_by_user: If not None, only return packages visible by
+ this user.
+ """
+
def preloadDataForSnaps(snaps, user):
"""Load the data related to a list of snap packages."""
=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py 2016-11-22 02:13:11 +0000
+++ lib/lp/snappy/model/snap.py 2016-12-02 13:18:34 +0000
@@ -17,6 +17,7 @@
Desc,
LeftJoin,
Not,
+ Or,
)
from storm.locals import (
Bool,
@@ -54,7 +55,10 @@
IAllGitRepositories,
IGitCollection,
)
-from lp.code.interfaces.gitref import IGitRef
+from lp.code.interfaces.gitref import (
+ IGitRef,
+ IGitRefRemoteSet,
+ )
from lp.code.interfaces.gitrepository import IGitRepository
from lp.code.model.branch import Branch
from lp.code.model.branchcollection import GenericBranchCollection
@@ -71,6 +75,7 @@
IHasOwner,
IPersonRoles,
)
+from lp.registry.model.teammembership import TeamParticipation
from lp.services.config import config
from lp.services.database.bulk import load_related
from lp.services.database.constants import (
@@ -115,6 +120,7 @@
from lp.snappy.interfaces.snappyseries import ISnappyDistroSeriesSet
from lp.snappy.model.snapbuild import SnapBuild
from lp.soyuz.interfaces.archive import ArchiveDisabled
+from lp.soyuz.interfaces.buildrecords import IncompatibleArguments
from lp.soyuz.model.archive import (
Archive,
get_enabled_archive_filter,
@@ -163,6 +169,8 @@
git_repository_id = Int(name='git_repository', allow_none=True)
git_repository = Reference(git_repository_id, 'GitRepository.id')
+ git_repository_url = Unicode(name='git_repository_url', allow_none=True)
+
git_path = Unicode(name='git_path', allow_none=True)
auto_build = Bool(name='auto_build', allow_none=False)
@@ -226,6 +234,9 @@
"""See `ISnap`."""
if self.git_repository is not None:
return self.git_repository.getRefByPath(self.git_path)
+ elif self.git_repository_url is not None:
+ return getUtility(IGitRefRemoteSet).new(
+ self.git_repository_url, self.git_path)
else:
return None
@@ -234,9 +245,11 @@
"""See `ISnap`."""
if value is not None:
self.git_repository = value.repository
+ self.git_repository_url = value.repository_url
self.git_path = value.path
else:
self.git_repository = None
+ self.git_repository_url = None
self.git_path = None
@property
@@ -549,7 +562,8 @@
"""See `ISnapSet`."""
def new(self, registrant, owner, distro_series, name, description=None,
- branch=None, git_ref=None, auto_build=False,
+ branch=None, git_repository=None, git_repository_url=None,
+ git_path=None, git_ref=None, auto_build=False,
auto_build_archive=None, auto_build_pocket=None,
require_virtualized=True, processors=None, date_created=DEFAULT,
private=False, store_upload=False, store_series=None,
@@ -565,6 +579,21 @@
"%s cannot create snap packages owned by %s." %
(registrant.displayname, owner.displayname))
+ if sum([git_repository is not None, git_repository_url is not None,
+ git_ref is not None]) > 1:
+ raise IncompatibleArguments(
+ "You cannot specify more than one of 'git_repository', "
+ "'git_repository_url', and 'git_ref'.")
+ if ((git_repository is None and git_repository_url is None) !=
+ (git_path is None)):
+ raise IncompatibleArguments(
+ "You must specify both or neither of "
+ "'git_repository'/'git_repository_url' and 'git_path'.")
+ if git_repository is not None:
+ git_ref = git_repository.getRefByPath(git_path)
+ elif git_repository_url is not None:
+ git_ref = getUtility(IGitRefRemoteSet).new(
+ git_repository_url, git_path)
if branch is None and git_ref is None:
raise NoSourceForSnap
if self.exists(owner, name):
@@ -603,8 +632,8 @@
return True
# Public snaps with private sources are not allowed.
- source_ref = branch or git_ref
- if source_ref.information_type in PRIVATE_INFORMATION_TYPES:
+ source = branch or git_ref
+ if source.information_type in PRIVATE_INFORMATION_TYPES:
return False
# Public snaps owned by private teams are not allowed.
@@ -653,8 +682,12 @@
return owned.union(packaged)
bzr_collection = removeSecurityProxy(getUtility(IAllBranches))
+ bzr_snaps = _getSnaps(bzr_collection)
git_collection = removeSecurityProxy(getUtility(IAllGitRepositories))
- return _getSnaps(bzr_collection).union(_getSnaps(git_collection))
+ git_snaps = _getSnaps(git_collection)
+ git_url_snaps = IStore(Snap).find(
+ Snap, Snap.owner == person, Snap.git_repository_url != None)
+ return bzr_snaps.union(git_snaps).union(git_url_snaps)
def findByProject(self, project, visible_by_user=None):
"""See `ISnapSet`."""
@@ -705,6 +738,28 @@
snaps.order_by(Desc(Snap.date_last_modified))
return snaps
+ def findByURL(self, url, visible_by_user=None):
+ """See `ISnapSet`."""
+ clauses = [Snap.git_repository_url == url]
+ # XXX cjwatson 2016-11-25: This is in principle a poor query, but we
+ # don't yet have the access grant infrastructure to do better, and
+ # in any case since we're querying for a single URL the numbers
+ # involved should be very small.
+ if visible_by_user is None:
+ visibility_clause = Snap.private == False
+ else:
+ roles = IPersonRoles(visible_by_user)
+ if roles.in_admin or roles.in_commercial_admin:
+ visibility_clause = True
+ else:
+ visibility_clause = Or(
+ Snap.private == False,
+ And(
+ TeamParticipation.person == visible_by_user,
+ TeamParticipation.teamID == Snap.owner_id))
+ clauses.append(visibility_clause)
+ return IStore(Snap).find(Snap, *clauses).config(distinct=True)
+
def preloadDataForSnaps(self, snaps, user=None):
"""See `ISnapSet`."""
snaps = [removeSecurityProxy(snap) for snap in snaps]
=== modified file 'lib/lp/snappy/model/snapbuildbehaviour.py'
--- lib/lp/snappy/model/snapbuildbehaviour.py 2016-09-21 02:50:41 +0000
+++ lib/lp/snappy/model/snapbuildbehaviour.py 2016-12-02 13:18:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""An `IBuildFarmJobBehaviour` for `SnapBuild`.
@@ -105,7 +105,11 @@
if build.snap.branch is not None:
args["branch"] = build.snap.branch.bzr_identity
elif build.snap.git_ref is not None:
- args["git_repository"] = build.snap.git_repository.git_https_url
+ if build.snap.git_ref.repository_url is not None:
+ args["git_repository"] = build.snap.git_ref.repository_url
+ else:
+ args["git_repository"] = (
+ build.snap.git_repository.git_https_url)
# "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.
=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py 2016-11-22 02:13:11 +0000
+++ lib/lp/snappy/tests/test_snap.py 2016-12-02 13:18:34 +0000
@@ -87,7 +87,10 @@
DoesNotSnapshot,
HasQueryCount,
)
-from lp.testing.pages import webservice_for_person
+from lp.testing.pages import (
+ LaunchpadWebServiceCaller,
+ webservice_for_person,
+ )
class TestSnapFeatureFlag(TestCaseWithFactory):
@@ -591,6 +594,18 @@
self.assertTrue(snap.require_virtualized)
self.assertFalse(snap.private)
+ def test_creation_git_url(self):
+ # A Snap can be backed directly by a URL for an external Git
+ # repository, rather than a Git repository hosted in Launchpad.
+ ref = self.factory.makeGitRefRemote()
+ components = self.makeSnapComponents(git_ref=ref)
+ snap = getUtility(ISnapSet).new(**components)
+ self.assertIsNone(snap.branch)
+ self.assertIsNone(snap.git_repository)
+ self.assertEqual(ref.repository_url, snap.git_repository_url)
+ self.assertEqual(ref.path, snap.git_path)
+ self.assertEqual(ref, snap.git_ref)
+
def test_private_snap_for_public_sources(self):
# Creating private snaps for public sources is allowed.
[ref] = self.factory.makeGitRefs()
@@ -803,6 +818,20 @@
BadSnapSearchContext, snap_set.findByContext,
self.factory.makeDistribution())
+ def test_findByURL(self):
+ # ISnapSet.findByURL returns visible Snaps with the given URL.
+ urls = [u"https://git.example.org/foo";, u"https://git.example.org/bar";]
+ snaps = []
+ for url in urls:
+ snaps.append(self.factory.makeSnap(
+ git_ref=self.factory.makeGitRefRemote(repository_url=url)))
+ snaps.append(
+ self.factory.makeSnap(branch=self.factory.makeAnyBranch()))
+ snaps.append(
+ self.factory.makeSnap(git_ref=self.factory.makeGitRefs()[0]))
+ self.assertContentEqual(
+ [snaps[0]], getUtility(ISnapSet).findByURL(urls[0]))
+
def test__findStaleSnaps(self):
# Stale; not built automatically.
self.factory.makeSnap(is_stale=True)
@@ -1252,6 +1281,52 @@
"No such snap package with this owner: 'nonexistent'.",
response.body)
+ def test_findByURL(self):
+ # lp.snaps.findByURL returns visible Snaps with the given URL.
+ persons = [self.factory.makePerson(), self.factory.makePerson()]
+ urls = [u"https://git.example.org/foo";, u"https://git.example.org/bar";]
+ snaps = []
+ for url in urls:
+ for person in persons:
+ for private in (False, True):
+ ref = self.factory.makeGitRefRemote(repository_url=url)
+ snaps.append(self.factory.makeSnap(
+ registrant=person, git_ref=ref, private=private))
+ with admin_logged_in():
+ ws_snaps = [
+ self.webservice.getAbsoluteUrl(api_url(snap))
+ for snap in snaps]
+ commercial_admin = (
+ getUtility(ILaunchpadCelebrities).commercial_admin.teamowner)
+ logout()
+ # Anonymous requests can only see public snaps.
+ anon_webservice = LaunchpadWebServiceCaller("test", "")
+ response = anon_webservice.named_get(
+ "/+snaps", "findByURL", url=urls[0], api_version="devel")
+ self.assertEqual(200, response.status)
+ self.assertContentEqual(
+ [ws_snaps[0], ws_snaps[2]],
+ [entry["self_link"] for entry in response.jsonBody()["entries"]])
+ # persons[0] can see both public snaps with this URL, as well as
+ # their own private snap.
+ webservice = webservice_for_person(
+ persons[0], permission=OAuthPermission.READ_PRIVATE)
+ response = webservice.named_get(
+ "/+snaps", "findByURL", url=urls[0], api_version="devel")
+ self.assertEqual(200, response.status)
+ self.assertContentEqual(
+ ws_snaps[:3],
+ [entry["self_link"] for entry in response.jsonBody()["entries"]])
+ # Admins can see all snaps with this URL.
+ commercial_admin_webservice = webservice_for_person(
+ commercial_admin, permission=OAuthPermission.READ_PRIVATE)
+ response = commercial_admin_webservice.named_get(
+ "/+snaps", "findByURL", url=urls[0], api_version="devel")
+ self.assertEqual(200, response.status)
+ self.assertContentEqual(
+ ws_snaps[:4],
+ [entry["self_link"] for entry in response.jsonBody()["entries"]])
+
def setProcessors(self, user, snap, names):
ws = webservice_for_person(
user, permission=OAuthPermission.WRITE_PUBLIC)
=== modified file 'lib/lp/snappy/tests/test_snapbuildbehaviour.py'
--- lib/lp/snappy/tests/test_snapbuildbehaviour.py 2016-06-28 21:10:18 +0000
+++ lib/lp/snappy/tests/test_snapbuildbehaviour.py 2016-12-02 13:18:34 +0000
@@ -11,13 +11,13 @@
import fixtures
from mock import (
+ Mock,
patch,
- Mock,
)
-import transaction
from testtools import ExpectedException
from testtools.deferredruntest import AsynchronousDeferredRunTest
from testtools.matchers import IsInstance
+import transaction
from twisted.internet import defer
from twisted.trial.unittest import TestCase as TrialTestCase
from zope.component import getUtility
@@ -244,6 +244,28 @@
}, 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.
+ url = u"https://git.example.org/foo";
+ ref = self.factory.makeGitRefRemote(
+ repository_url=url, path=u"refs/heads/master")
+ 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,
+ "git_path": "master",
+ "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)
=== modified file 'lib/lp/testing/matchers.py'
--- lib/lp/testing/matchers.py 2015-10-13 16:58:20 +0000
+++ lib/lp/testing/matchers.py 2016-12-02 13:18:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -395,7 +395,8 @@
self.soup_content = soup_content
def get_details(self):
- return {'content': self.soup_content}
+ content = unicode(self.soup_content).encode('utf8')
+ return {'content': Content(UTF8_TEXT, lambda: [content])}
class MissingElement(SoupMismatch):
Follow ups
