Merge lp:~cjwatson/launchpad/snap-git-url into lp:launchpad

[Colin Watson]

Allow building snaps from an external Git repository.

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.

[William Grant (wgrant)]

review: Approve (code)

=== 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-05 19:02:57 +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-05 19:02:57 +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-05 19:02:57 +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-05 19:02:57 +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,
@@ -540,7 +555,7 @@
 
         :param person: An `IPerson`.
         :param visible_by_user: If not None, only return packages visible by
-            this user.
+            this user; otherwise, only return publicly-visible packages.
         """
 
     def findByProject(project, visible_by_user=None):
@@ -548,7 +563,7 @@
 
         :param project: An `IProduct`.
         :param visible_by_user: If not None, only return packages visible by
-            this user.
+            this user; otherwise, only return publicly-visible packages.
         """
 
     def findByBranch(branch):
@@ -571,12 +586,29 @@
         :param context: An `IPerson`, `IProduct, `IBranch`,
             `IGitRepository`, or `IGitRef`.
         :param visible_by_user: If not None, only return packages visible by
-            this user.
+            this user; otherwise, only return publicly-visible packages.
         :param order_by_date: If True, order packages by descending
             modification date.
         :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; otherwise, only return publicly-visible packages.
+        """
+
     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-05 19:02:57 +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-05 19:02:57 +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-05 19:02:57 +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-05 19:02:57 +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-05 19:02:57 +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):