launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22634
[Merge] lp:~cjwatson/launchpad/git-ref-remote-blob into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/git-ref-remote-blob into lp:launchpad with lp:~cjwatson/launchpad/snap-request-builds as a prerequisite.
Commit message:
Support fetching snapcraft.yaml from external repositories hosted on GitHub.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1770400 in Launchpad itself: "Support snapcraft architectures keyword"
https://bugs.launchpad.net/launchpad/+bug/1770400
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-ref-remote-blob/+merge/348089
This is undeniably a hack, but it's pretty well-contained and it solves the problem at hand (making it viable for build.snapcraft.io to use Snap.requestBuilds) in a pretty lightweight way. A general solution would have many more moving parts.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-ref-remote-blob into lp:launchpad.
=== modified file 'lib/lp/code/interfaces/gitref.py'
--- lib/lp/code/interfaces/gitref.py 2017-11-06 09:32:45 +0000
+++ lib/lp/code/interfaces/gitref.py 2018-06-15 17:15:14 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Git reference ("ref") interfaces."""
@@ -382,6 +382,13 @@
has_commits = Attribute("Whether this reference has any commits.")
+ def getBlob(filename):
+ """Get a blob by file name from this reference.
+
+ :param filename: Relative path of a file in the repository.
+ :return: A binary string with the blob content.
+ """
+
class IGitRefBatchNavigator(ITableBatchNavigator):
pass
=== modified file 'lib/lp/code/model/githosting.py'
--- lib/lp/code/model/githosting.py 2018-03-31 16:02:54 +0000
+++ lib/lp/code/model/githosting.py 2018-06-15 17:15:14 +0000
@@ -8,6 +8,7 @@
'GitHostingClient',
]
+import base64
import json
import sys
from urllib import quote
@@ -223,7 +224,7 @@
raise GitRepositoryScanFault(
"Failed to get file from Git repository: %s" % unicode(e))
try:
- blob = response["data"].decode("base64")
+ blob = base64.b64decode(response["data"].encode("UTF-8"))
if len(blob) != response["size"]:
raise GitRepositoryScanFault(
"Unexpected size (%s vs %s)" % (
=== modified file 'lib/lp/code/model/gitref.py'
--- lib/lp/code/model/gitref.py 2017-11-06 09:32:45 +0000
+++ lib/lp/code/model/gitref.py 2018-06-15 17:15:14 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -12,11 +12,16 @@
from datetime import datetime
from functools import partial
import json
-from urllib import quote_plus
+import re
+from urllib import (
+ quote,
+ quote_plus,
+ )
from urlparse import urlsplit
from lazr.lifecycle.event import ObjectCreatedEvent
import pytz
+import requests
from storm.locals import (
DateTime,
Int,
@@ -42,6 +47,8 @@
)
from lp.code.errors import (
BranchMergeProposalExists,
+ GitRepositoryBlobNotFound,
+ GitRepositoryScanFault,
InvalidBranchMergeProposal,
)
from lp.code.event.branchmergeproposal import (
@@ -69,6 +76,7 @@
from lp.services.database.stormbase import StormBase
from lp.services.features import getFeatureFlag
from lp.services.memcache.interfaces import IMemcacheClient
+from lp.services.timeout import urlfetch
from lp.services.webapp.interfaces import ILaunchBag
@@ -387,6 +395,10 @@
commit["sha1"])
return commits
+ def getBlob(self, filename):
+ """See `IGitRef`."""
+ return self.repository.getBlob(filename, rev=self.path)
+
@property
def recipes(self):
"""See `IHasRecipes`."""
@@ -725,6 +737,41 @@
"""See `IGitRef`."""
return []
+ def getBlob(self, filename):
+ """See `IGitRef`."""
+ # In general, we can't easily get hold of a blob from a remote
+ # repository without cloning the whole thing. In future we might
+ # dispatch a build job or a code import or something like that to do
+ # so. For now, we just special-case some providers where we know
+ # how to fetch a blob on its own.
+ url = urlsplit(self.repository_url)
+ if (url.hostname == "github.com" and
+ len(url.path.strip("/").split("/")) == 2):
+ repo_path = url.path.strip("/")
+ if repo_path.endswith(".git"):
+ repo_path = repo_path[:len(".git")]
+ try:
+ response = urlfetch(
+ "https://raw.githubusercontent.com/%s/%s/%s"; % (
+ repo_path,
+ # GitHub supports either branch or tag names here,
+ # but both must be shortened.
+ quote(re.sub(r"^refs/(?:heads|tags)/", "", self.path)),
+ quote(filename)),
+ use_proxy=True)
+ except requests.RequestException as e:
+ if e.response.status_code == requests.codes.NOT_FOUND:
+ raise GitRepositoryBlobNotFound(
+ self.repository_url, filename, rev=self.path)
+ else:
+ raise GitRepositoryScanFault(
+ "Failed to get file from Git repository at %s: %s" %
+ (self.repository_url, str(e)))
+ return response.content
+ raise GitRepositoryScanFault(
+ "Cannot fetch blob from external Git repository at %s" %
+ self.repository_url)
+
@property
def recipes(self):
"""See `IHasRecipes`."""
=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py 2017-11-24 17:22:34 +0000
+++ lib/lp/code/model/gitrepository.py 2018-06-15 17:15:14 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -1082,7 +1082,8 @@
def getBlob(self, filename, rev=None):
"""See `IGitRepository`."""
hosting_client = getUtility(IGitHostingClient)
- return hosting_client.getBlob(self.getInternalPath(), filename, rev)
+ return hosting_client.getBlob(
+ self.getInternalPath(), filename, rev=rev)
def getDiff(self, old, new):
"""See `IGitRepository`."""
=== modified file 'lib/lp/code/model/tests/test_gitref.py'
--- lib/lp/code/model/tests/test_gitref.py 2017-10-04 01:29:35 +0000
+++ lib/lp/code/model/tests/test_gitref.py 2018-06-15 17:15:14 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for Git references."""
@@ -15,6 +15,7 @@
import json
import pytz
+import responses
from testtools.matchers import (
ContainsDict,
EndsWith,
@@ -29,7 +30,11 @@
from lp.app.enums import InformationType
from lp.app.interfaces.informationtype import IInformationType
from lp.app.interfaces.launchpad import IPrivacy
-from lp.code.errors import InvalidBranchMergeProposal
+from lp.code.errors import (
+ GitRepositoryBlobNotFound,
+ GitRepositoryScanFault,
+ InvalidBranchMergeProposal,
+ )
from lp.code.tests.helpers import GitHostingFixture
from lp.services.features.testing import FeatureFixture
from lp.services.memcache.interfaces import IMemcacheClient
@@ -271,6 +276,83 @@
]))
+class TestGitRefGetBlob(TestCaseWithFactory):
+ """Tests for retrieving blobs from a Git reference."""
+
+ layer = LaunchpadFunctionalLayer
+
+ def test_local(self):
+ [ref] = self.factory.makeGitRefs()
+ hosting_fixture = self.useFixture(GitHostingFixture(blob=b"foo"))
+ self.assertEqual(b"foo", ref.getBlob("dir/file"))
+ self.assertEqual(
+ [((ref.repository.getInternalPath(), "dir/file"),
+ {"rev": ref.path})],
+ hosting_fixture.getBlob.calls)
+
+ @responses.activate
+ def test_remote_github_branch(self):
+ ref = self.factory.makeGitRefRemote(
+ repository_url="https://github.com/owner/name";,
+ path="refs/heads/path")
+ responses.add(
+ "GET",
+ "https://raw.githubusercontent.com/owner/name/path/dir/file";,
+ body=b"foo")
+ self.assertEqual(b"foo", ref.getBlob("dir/file"))
+
+ @responses.activate
+ def test_remote_github_tag(self):
+ ref = self.factory.makeGitRefRemote(
+ repository_url="https://github.com/owner/name";,
+ path="refs/tags/1.0")
+ responses.add(
+ "GET",
+ "https://raw.githubusercontent.com/owner/name/1.0/dir/file";,
+ body=b"foo")
+ self.assertEqual(b"foo", ref.getBlob("dir/file"))
+
+ @responses.activate
+ def test_remote_github_HEAD(self):
+ ref = self.factory.makeGitRefRemote(
+ repository_url="https://github.com/owner/name";, path="HEAD")
+ responses.add(
+ "GET",
+ "https://raw.githubusercontent.com/owner/name/HEAD/dir/file";,
+ body=b"foo")
+ self.assertEqual(b"foo", ref.getBlob("dir/file"))
+
+ @responses.activate
+ def test_remote_github_404(self):
+ ref = self.factory.makeGitRefRemote(
+ repository_url="https://github.com/owner/name";, path="HEAD")
+ responses.add(
+ "GET",
+ "https://raw.githubusercontent.com/owner/name/HEAD/dir/file";,
+ status=404)
+ self.assertRaises(GitRepositoryBlobNotFound, ref.getBlob, "dir/file")
+
+ @responses.activate
+ def test_remote_github_error(self):
+ ref = self.factory.makeGitRefRemote(
+ repository_url="https://github.com/owner/name";, path="HEAD")
+ responses.add(
+ "GET",
+ "https://raw.githubusercontent.com/owner/name/HEAD/dir/file";,
+ status=500)
+ self.assertRaises(GitRepositoryScanFault, ref.getBlob, "dir/file")
+
+ def test_remote_github_bad_repository_path(self):
+ ref = self.factory.makeGitRefRemote(
+ repository_url="https://github.com/owner/name/extra";)
+ self.assertRaises(GitRepositoryScanFault, ref.getBlob, "dir/file")
+
+ def test_remote_unknown_host(self):
+ ref = self.factory.makeGitRefRemote(
+ repository_url="https://example.com/foo";)
+ self.assertRaises(GitRepositoryScanFault, ref.getBlob, "dir/file")
+
+
class TestGitRefCreateMergeProposal(TestCaseWithFactory):
"""Exercise all the code paths for creating a merge proposal."""
=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py 2018-06-15 17:15:13 +0000
+++ lib/lp/snappy/model/snap.py 2018-06-15 17:15:14 +0000
@@ -1040,10 +1040,7 @@
)
for path in paths:
try:
- if IBranch.providedBy(context):
- blob = context.getBlob(path)
- else:
- blob = context.repository.getBlob(path, context.name)
+ blob = context.getBlob(path)
break
except (BranchFileNotFound, GitRepositoryBlobNotFound):
pass
=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py 2018-06-15 17:15:13 +0000
+++ lib/lp/snappy/tests/test_snap.py 2018-06-15 17:15:14 +0000
@@ -74,6 +74,7 @@
from lp.services.job.runner import JobRunner
from lp.services.log.logger import BufferLogger
from lp.services.propertycache import clear_property_cache
+from lp.services.timeout import default_timeout
from lp.services.webapp.interfaces import OAuthPermission
from lp.snappy.interfaces.snap import (
BadSnapSearchContext,
@@ -1196,6 +1197,21 @@
self.assertEqual(
{"name": "test-snap"}, getUtility(ISnapSet).getSnapcraftYaml(snap))
+ @responses.activate
+ def test_getSnapcraftYaml_snap_git_external_github(self):
+ responses.add(
+ "GET",
+ "https://raw.githubusercontent.com/owner/name/HEAD/";
+ "snap/snapcraft.yaml",
+ body=b"name: test-snap")
+ git_ref = self.factory.makeGitRefRemote(
+ repository_url="https://github.com/owner/name";, path="HEAD")
+ snap = self.factory.makeSnap(git_ref=git_ref)
+ with default_timeout(1.0):
+ self.assertEqual(
+ {"name": "test-snap"},
+ getUtility(ISnapSet).getSnapcraftYaml(snap))
+
def test_getSnapcraftYaml_invalid_data(self):
hosting_fixture = self.useFixture(GitHostingFixture())
for invalid_result in (None, 123, b"", b"[][]", b"#name:test", b"]"):
Follow ups
