launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snapcraft-yaml-paths into lp:launchpad.

Commit message:
Consider .snapcraft.yaml as well as snapcraft.yaml for new snaps.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snapcraft-yaml-paths/+merge/314903
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snapcraft-yaml-paths into lp:launchpad.

=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py	2016-10-15 02:01:23 +0000
+++ lib/lp/code/errors.py	2017-01-17 11:20:27 +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).
 
 """Errors used in the lp/code modules."""
@@ -33,6 +33,7 @@
     'ClaimReviewFailed',
     'DiffNotFound',
     'GitDefaultConflict',
+    'GitRepositoryBlobNotFound',
     'GitRepositoryCreationException',
     'GitRepositoryCreationFault',
     'GitRepositoryCreationForbidden',
@@ -406,6 +407,22 @@
     """Raised when there is a fault scanning a repository."""
 
 
+class GitRepositoryBlobNotFound(GitRepositoryScanFault):
+    """Raised when a blob does not exist in a repository."""
+
+    def __init__(self, path, filename, rev=None):
+        super(GitRepositoryBlobNotFound, self).__init__()
+        self.path = path
+        self.filename = filename
+        self.rev = rev
+
+    def __str__(self):
+        message = "Repository %s has no file %s" % (self.path, self.filename)
+        if self.rev is not None:
+            message += " at revision %s" % self.rev
+        return message
+
+
 class GitRepositoryDeletionFault(Exception):
     """Raised when there is a fault deleting a repository."""
 

=== modified file 'lib/lp/code/model/githosting.py'
--- lib/lp/code/model/githosting.py	2016-07-01 20:04:54 +0000
+++ lib/lp/code/model/githosting.py	2017-01-17 11:20:27 +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).
 
 """Communication with the Git hosting service."""
@@ -19,6 +19,7 @@
 from zope.interface import implementer
 
 from lp.code.errors import (
+    GitRepositoryBlobNotFound,
     GitRepositoryCreationFault,
     GitRepositoryDeletionFault,
     GitRepositoryScanFault,
@@ -55,6 +56,8 @@
             # Re-raise this directly so that it can be handled specially by
             # callers.
             raise
+        except requests.RequestException:
+            raise
         except Exception:
             _, val, tb = sys.exc_info()
             reraise(
@@ -62,7 +65,6 @@
                 tb)
         finally:
             action.finish()
-        response.raise_for_status()
         if response.content:
             return response.json()
         else:
@@ -209,8 +211,11 @@
             url = "/repo/%s/blob/%s" % (path, quote(filename))
             response = self._get(url, params={"rev": rev})
         except requests.RequestException as e:
-            raise GitRepositoryScanFault(
-                "Failed to get file from Git repository: %s" % unicode(e))
+            if e.response.status_code == requests.codes.NOT_FOUND:
+                raise GitRepositoryBlobNotFound(path, filename, rev=rev)
+            else:
+                raise GitRepositoryScanFault(
+                    "Failed to get file from Git repository: %s" % unicode(e))
         try:
             blob = response["data"].decode("base64")
             if len(blob) != response["size"]:

=== modified file 'lib/lp/code/model/tests/test_githosting.py'
--- lib/lp/code/model/tests/test_githosting.py	2016-07-01 20:04:54 +0000
+++ lib/lp/code/model/tests/test_githosting.py	2017-01-17 11:20:27 +0000
@@ -2,7 +2,7 @@
 # NOTE: The first line above must stay first; do not move the copyright
 # notice to the top.  See http://www.python.org/dev/peps/pep-0263/.
 #
-# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2016-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Unit tests for `GitHostingClient`.
@@ -29,6 +29,7 @@
 from zope.security.proxy import removeSecurityProxy
 
 from lp.code.errors import (
+    GitRepositoryBlobNotFound,
     GitRepositoryCreationFault,
     GitRepositoryDeletionFault,
     GitRepositoryScanFault,
@@ -306,6 +307,21 @@
         self.assertRequest(
             "repo/123/blob/dir/path/file/name?rev=dev", method="GET")
 
+    def test_getBlob_not_found(self):
+        with self.mockRequests(status_code=404, reason=b"Not found"):
+            self.assertRaisesWithContent(
+                GitRepositoryBlobNotFound,
+                "Repository 123 has no file dir/path/file/name",
+                self.client.getBlob, "123", "dir/path/file/name")
+
+    def test_getBlob_revision_not_found(self):
+        with self.mockRequests(status_code=404, reason=b"Not found"):
+            self.assertRaisesWithContent(
+                GitRepositoryBlobNotFound,
+                "Repository 123 has no file dir/path/file/name "
+                "at revision dev",
+                self.client.getBlob, "123", "dir/path/file/name", "dev")
+
     def test_getBlob_failure(self):
         with self.mockRequests(status_code=400, reason=b"Bad request"):
             self.assertRaisesWithContent(

=== modified file 'lib/lp/snappy/browser/snap.py'
--- lib/lp/snappy/browser/snap.py	2017-01-04 20:52:12 +0000
+++ lib/lp/snappy/browser/snap.py	2017-01-17 11:20:27 +0000
@@ -53,7 +53,10 @@
     )
 from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.code.browser.widgets.gitref import GitRefWidget
-from lp.code.errors import GitRepositoryScanFault
+from lp.code.errors import (
+    GitRepositoryBlobNotFound,
+    GitRepositoryScanFault,
+    )
 from lp.code.interfaces.gitref import IGitRef
 from lp.registry.enums import VCSType
 from lp.registry.interfaces.pocket import PackagePublishingPocket
@@ -433,8 +436,12 @@
         if self.has_snappy_distro_series and IGitRef.providedBy(self.context):
             # Try to extract Snap store name from snapcraft.yaml file.
             try:
-                blob = self.context.repository.getBlob(
-                    'snapcraft.yaml', self.context.name)
+                try:
+                    blob = self.context.repository.getBlob(
+                        'snapcraft.yaml', self.context.name)
+                except GitRepositoryBlobNotFound:
+                    blob = self.context.repository.getBlob(
+                        '.snapcraft.yaml', self.context.name)
                 # Beware of unsafe yaml.load()!
                 store_name = yaml.safe_load(blob).get('name')
             except GitRepositoryScanFault:

=== modified file 'lib/lp/snappy/browser/tests/test_snap.py'
--- lib/lp/snappy/browser/tests/test_snap.py	2017-01-04 20:52:12 +0000
+++ lib/lp/snappy/browser/tests/test_snap.py	2017-01-17 11:20:27 +0000
@@ -40,7 +40,10 @@
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.processor import IProcessorSet
-from lp.code.errors import GitRepositoryScanFault
+from lp.code.errors import (
+    GitRepositoryBlobNotFound,
+    GitRepositoryScanFault,
+    )
 from lp.code.tests.helpers import GitHostingFixture
 from lp.registry.enums import PersonVisibility
 from lp.registry.interfaces.pocket import PackagePublishingPocket
@@ -490,6 +493,20 @@
         self.assertIn('store_name', initial_values)
         self.assertEqual('test-snap', initial_values['store_name'])
 
+    def test_initial_name_extraction_git_dot_snapcraft_yaml(self):
+        def getBlob(filename, *args, **kwargs):
+            if filename == ".snapcraft.yaml":
+                return "name: test-snap"
+            else:
+                raise GitRepositoryBlobNotFound()
+
+        [git_ref] = self.factory.makeGitRefs()
+        git_ref.repository.getBlob = getBlob
+        view = create_initialized_view(git_ref, "+new-snap")
+        initial_values = view.initial_values
+        self.assertIn('store_name', initial_values)
+        self.assertIsNone(initial_values['store_name'])
+
     def test_initial_name_extraction_git_repo_error(self):
         [git_ref] = self.factory.makeGitRefs()
         git_ref.repository.getBlob = FakeMethod(failure=GitRepositoryScanFault)

=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py	2017-01-04 20:52:12 +0000
+++ lib/lp/snappy/interfaces/snap.py	2017-01-17 11:20:27 +0000
@@ -425,22 +425,22 @@
         title=_("Bazaar branch"), schema=IBranch, vocabulary="Branch",
         required=False, readonly=False,
         description=_(
-            "A Bazaar branch containing a snapcraft.yaml recipe at the top "
-            "level.")))
+            "A Bazaar branch containing a snapcraft.yaml or .snapcraft.yaml "
+            "recipe at the top level.")))
 
     git_repository = exported(ReferenceChoice(
         title=_("Git repository"),
         schema=IGitRepository, vocabulary="GitRepository",
         required=False, readonly=True,
         description=_(
-            "A Git repository with a branch containing a snapcraft.yaml "
-            "recipe at the top level.")))
+            "A Git repository with a branch containing a snapcraft.yaml or "
+            ".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."),
+            "snapcraft.yaml or .snapcraft.yaml recipe at the top level."),
         allowed_schemes=["git", "http", "https"],
         allow_userinfo=True,
         allow_port=True,
@@ -451,21 +451,22 @@
     git_path = exported(TextLine(
         title=_("Git branch path"), required=False, readonly=True,
         description=_(
-            "The path of the Git branch containing a snapcraft.yaml recipe at "
-            "the top level.")))
+            "The path of the Git branch containing a snapcraft.yaml or "
+            ".snapcraft.yaml recipe at the top level.")))
 
     git_ref = exported(Reference(
         IGitRef, title=_("Git branch"), required=False, readonly=False,
         description=_(
-            "The Git branch containing a snapcraft.yaml recipe at the top "
-            "level.")))
+            "The Git branch containing a snapcraft.yaml or .snapcraft.yaml "
+            "recipe at the top level.")))
 
     auto_build = exported(Bool(
         title=_("Automatically build when branch changes"),
         required=True, readonly=False,
         description=_(
             "Whether this snap package is built automatically when the branch "
-            "containing its snapcraft.yaml recipe changes.")))
+            "containing its snapcraft.yaml or .snapcraft.yaml recipe "
+            "changes.")))
 
     auto_build_archive = exported(Reference(
         IArchive, title=_("Source archive for automatic builds"),

=== modified file 'lib/lp/snappy/templates/snap-new.pt'
--- lib/lp/snappy/templates/snap-new.pt	2016-07-16 07:46:23 +0000
+++ lib/lp/snappy/templates/snap-new.pt	2017-01-17 11:20:27 +0000
@@ -15,7 +15,8 @@
       Core</a>.  Launchpad can build snap packages using <a
       href="https://developer.ubuntu.com/en/snappy/snapcraft/";>snapcraft</a>,
       from any Git or Bazaar branch on Launchpad that has a
-      <tt>snapcraft.yaml</tt> file at its top level.
+      <tt>snapcraft.yaml</tt> or <tt>.snapcraft.yaml</tt> file at its top
+      level.
     </p>
   </div>