← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] ~cjwatson/launchpad:oci-show-default-git-repository into launchpad:master

 

Colin Watson has proposed merging ~cjwatson/launchpad:oci-show-default-git-repository into launchpad:master.

Commit message:
Encourage the use of target-default repositories for OCI projects

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/401743

Unrestrict the lp:PILLAR/+oci/NAME form a little further, allowing setting the default repository by pushing to the corresponding URL, and do more work to encourage people towards them in the web UI.  (Depending on the pillar's default traversal policies, lp:PILLAR/NAME may also work.)
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:oci-show-default-git-repository into launchpad:master.
diff --git a/lib/lp/code/model/gitnamespace.py b/lib/lp/code/model/gitnamespace.py
index deaa9d9..86169a2 100644
--- a/lib/lp/code/model/gitnamespace.py
+++ b/lib/lp/code/model/gitnamespace.py
@@ -552,7 +552,7 @@ class OCIProjectGitNamespace(_BaseGitNamespace):
     """
 
     has_defaults = True
-    allow_push_to_set_default = False
+    allow_push_to_set_default = True
     supports_merge_proposals = True
     supports_code_imports = True
     allow_recipe_name_from_target = True
diff --git a/lib/lp/code/xmlrpc/tests/test_git.py b/lib/lp/code/xmlrpc/tests/test_git.py
index ea38315..b87b2a3 100644
--- a/lib/lp/code/xmlrpc/tests/test_git.py
+++ b/lib/lp/code/xmlrpc/tests/test_git.py
@@ -1492,9 +1492,9 @@ class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
         self.assertPermissionDenied(
             requester, path, message=message, permission="write")
 
-    def test_translatePath_create_oci_project_default_denied(self):
-        # A repository cannot (yet) be created and immediately set as the
-        # default for an OCI project.
+    def test_translatePath_create_oci_project_default(self):
+        # A repository can be created and immediately set as the default for
+        # an OCI project.
         requester = self.factory.makePerson()
         oci_project = self.factory.makeOCIProject()
         path = u"/%s/+oci/%s" % (oci_project.pillar.name, oci_project.name)
diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py
index f55bf1d..eb62596 100644
--- a/lib/lp/oci/browser/ocirecipe.py
+++ b/lib/lp/oci/browser/ocirecipe.py
@@ -64,6 +64,7 @@ from lp.app.widgets.itemswidgets import (
     )
 from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.code.browser.widgets.gitref import GitRefWidget
+from lp.code.interfaces.gitrepository import IGitRepositorySet
 from lp.oci.interfaces.ocipushrule import (
     IOCIPushRuleSet,
     OCIPushRuleAlreadyExists,
@@ -957,7 +958,7 @@ class OCIRecipeAddView(LaunchpadFormView, EnableProcessorsMixin,
         """Setup GitRef widget indicating the user to use the default
         oci project's git repository, if possible.
         """
-        path = self.context.getDefaultGitRepositoryPath(self.user)
+        path = canonical_url(self.context, force_local_path=True)[1:]
         widget = self.widgets["git_ref"]
         widget.setUpSubWidgets()
         widget.setBranchFormatValidator(self._branch_format_validator)
@@ -965,12 +966,14 @@ class OCIRecipeAddView(LaunchpadFormView, EnableProcessorsMixin,
         if widget.error():
             # Do not override more important git_ref errors.
             return
-        default_repo = self.context.getDefaultGitRepository(self.user)
+        default_repo = getUtility(IGitRepositorySet).getDefaultRepository(
+            self.context)
         if default_repo is None:
             msg = (
-                "Your git repository for this OCI project was not created yet."
-                "<br/>Check the <a href='%s'>OCI project page"
-                "</a> for instructions on how to create one.")
+                'The default git repository for this OCI project was not '
+                'created yet.<br/>'
+                'Check the <a href="%s">OCI project page</a> for instructions '
+                'on how to create one.')
             msg = structured(msg, canonical_url(self.context))
             self.widget_errors["git_ref"] = msg.escapedtext
 
@@ -1122,14 +1125,15 @@ class OCIRecipeEditView(BaseOCIRecipeEditView, EnableProcessorsMixin,
         if self.context.git_ref.namespace.target != self.context.oci_project:
             msg = ("This recipe's git repository is not in the "
                    "correct namespace.<br/>")
-            default_repo = oci_proj.getDefaultGitRepository(self.context.owner)
+            default_repo = getUtility(IGitRepositorySet).getDefaultRepository(
+                oci_proj)
             if default_repo:
                 link = GitRepositoryFormatterAPI(default_repo).link('')
                 msg += "Consider using %s instead." % link
             else:
                 msg += (
-                    "Check the <a href='%(oci_proj_url)s'>OCI project page</a>"
-                    " for instructions on how to create it correctly.")
+                    'Check the <a href="%(oci_proj_url)s">OCI project page</a>'
+                    ' for instructions on how to create it correctly.')
         if msg:
             msg = structured(msg, oci_proj_url=oci_proj_url)
             self.widget_errors["git_ref"] = msg.escapedtext
diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py
index 54d4b10..b51097e 100644
--- a/lib/lp/oci/browser/tests/test_ocirecipe.py
+++ b/lib/lp/oci/browser/tests/test_ocirecipe.py
@@ -38,6 +38,7 @@ from lp.app.enums import InformationType
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.processor import IProcessorSet
+from lp.code.interfaces.gitrepository import IGitRepositorySet
 from lp.oci.browser.ocirecipe import (
     OCIRecipeAdminView,
     OCIRecipeEditView,
@@ -382,24 +383,28 @@ class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
         browser = self.getViewBrowser(
             oci_project, view_name="+new-recipe", user=self.person)
         error_message = (
-            'Your git repository for this OCI project was not created yet.'
-            "<br/>Check the <a href='{url}'>OCI project page</a> for "
-            'instructions on how to create one.').format(url=oci_project_url)
+            'The default git repository for this OCI project was not created '
+            'yet.<br/>'
+            'Check the <a href="{url}">OCI project page</a> for instructions '
+            'on how to create one.').format(url=oci_project_url)
         self.assertIn(error_message, browser.contents)
 
     def test_create_new_recipe_with_default_repo_already_created(self):
         self.setUpDistroSeries()
         oci_project = self.factory.makeOCIProject(pillar=self.distribution)
-        self.factory.makeGitRepository(
+        repository = self.factory.makeGitRepository(
             name=oci_project.name,
             target=oci_project, owner=self.person, registrant=self.person)
-        with admin_logged_in():
-            default_repo_path = oci_project.getDefaultGitRepositoryPath(
-                self.person)
+        with person_logged_in(self.distribution.owner):
+            getUtility(IGitRepositorySet).setDefaultRepository(
+                oci_project, repository)
+            default_repo_path = "%s/+oci/%s" % (
+                self.distribution.name, oci_project.name)
         browser = self.getViewBrowser(
             oci_project, view_name="+new-recipe", user=self.person)
         error_message = (
-            'Your git repository for this OCI project was not created yet.')
+            'The default git repository for this OCI project was not created '
+            'yet.')
         self.assertNotIn(error_message, browser.contents)
         self.assertThat(browser.contents, soupmatchers.HTMLContains(
             soupmatchers.Tag(
@@ -971,7 +976,7 @@ class TestOCIRecipeEditView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
                 recipe, view_name="+edit", user=self.person)
         error_message = (
             "This recipe's git repository is not in the correct "
-            "namespace.<br/>Check the <a href='{url}'>OCI project page</a> "
+            'namespace.<br/>Check the <a href="{url}">OCI project page</a> '
             "for instructions on how to create it correctly.")
         self.assertIn(
             error_message.format(url=oci_project_url), browser.contents)
@@ -987,12 +992,14 @@ class TestOCIRecipeEditView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
 
         # Make the default git repository that should have been used by the
         # recipe.
-        self.factory.makeGitRepository(
+        default_repo = self.factory.makeGitRepository(
             name=oci_project.name,
             target=oci_project, owner=self.person, registrant=self.person)
+        with person_logged_in(self.distribution.owner):
+            getUtility(IGitRepositorySet).setDefaultRepository(
+                oci_project, default_repo)
 
         with person_logged_in(self.person):
-            default_repo = oci_project.getDefaultGitRepository(recipe.owner)
             repo_link = GitRepositoryFormatterAPI(default_repo).link('')
             browser = self.getViewBrowser(
                 recipe, view_name="+edit", user=self.person)
diff --git a/lib/lp/registry/browser/ociproject.py b/lib/lp/registry/browser/ociproject.py
index 3f938d8..d49bfb3 100644
--- a/lib/lp/registry/browser/ociproject.py
+++ b/lib/lp/registry/browser/ociproject.py
@@ -15,7 +15,11 @@ __all__ = [
     'OCIProjectURL',
     ]
 
-from six.moves.urllib.parse import urlsplit
+from breezy import urlutils
+from six.moves.urllib.parse import (
+    urlsplit,
+    urlunsplit,
+    )
 from zope.component import getUtility
 from zope.interface import implementer
 
@@ -27,6 +31,7 @@ from lp.app.browser.launchpadform import (
 from lp.app.browser.tales import CustomizableFormatter
 from lp.app.errors import NotFoundError
 from lp.code.browser.vcslisting import TargetDefaultVCSNavigationMixin
+from lp.code.interfaces.gitrepository import IGitRepositorySet
 from lp.oci.interfaces.ocirecipe import IOCIRecipeSet
 from lp.registry.enums import DistributionDefaultTraversalPolicy
 from lp.registry.interfaces.distribution import IDistribution
@@ -55,6 +60,7 @@ from lp.services.webapp import (
     StandardLaunchpadFacets,
     stepthrough,
     )
+from lp.services.webapp.authorization import check_permission
 from lp.services.webapp.batching import BatchNavigator
 from lp.services.webapp.breadcrumb import Breadcrumb
 from lp.services.webapp.interfaces import (
@@ -225,15 +231,21 @@ class OCIProjectContextMenu(ContextMenu):
 class OCIProjectIndexView(LaunchpadView):
     @property
     def git_repository(self):
-        return self.context.getDefaultGitRepository(self.user)
+        return getUtility(IGitRepositorySet).getDefaultRepository(self.context)
 
     @property
-    def git_repository_path(self):
-        return self.context.getDefaultGitRepositoryPath(self.user)
+    def git_ssh_url(self):
+        base_url = urlsplit(
+            urlutils.join(
+                config.codehosting.git_ssh_root,
+                canonical_url(self.context, force_local_path=True)[1:]))
+        url = list(base_url)
+        url[1] = "{}@{}".format(self.user.name, base_url.hostname)
+        return urlunsplit(url)
 
     @property
-    def git_ssh_hostname(self):
-        return urlsplit(config.codehosting.git_ssh_root).hostname
+    def user_can_push_default(self):
+        return check_permission("launchpad.Edit", self.context)
 
     @property
     def official_recipes(self):
diff --git a/lib/lp/registry/browser/tests/test_ociproject.py b/lib/lp/registry/browser/tests/test_ociproject.py
index 54a9baf..ee97cc0 100644
--- a/lib/lp/registry/browser/tests/test_ociproject.py
+++ b/lib/lp/registry/browser/tests/test_ociproject.py
@@ -10,11 +10,14 @@ __metaclass__ = type
 __all__ = []
 
 from datetime import datetime
+import re
 
 import pytz
+from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.enums import InformationType
+from lp.code.interfaces.gitrepository import IGitRepositorySet
 from lp.oci.tests.helpers import OCIConfigHelperMixin
 from lp.registry.interfaces.ociproject import (
     OCI_PROJECT_ALLOW_CREATE,
@@ -157,47 +160,60 @@ class TestOCIProjectView(OCIConfigHelperMixin, BrowserTestCase):
         expected_links = ["Create OCI recipe", "View all recipes"]
         self.assertEqual("\n".join(expected_links), actions)
 
-    def test_git_repo_hint(self):
-        owner = self.factory.makePerson(name="a-usr")
+    def test_git_repo_hint_cannot_push(self):
+        owner = self.factory.makePerson()
+        pillar = self.factory.makeProduct(name="a-pillar")
+        oci_project = self.factory.makeOCIProject(
+            pillar=pillar, ociprojectname="oci-name")
+        self.assertNotIn(
+            "You can create a git repository",
+            self.getMainText(oci_project, user=owner))
+
+    def test_git_repo_hint_can_push(self):
         pillar = self.factory.makeProduct(name="a-pillar")
         oci_project = self.factory.makeOCIProject(
             pillar=pillar, ociprojectname="oci-name")
-        git_url = (
-            "git\+ssh://a-usr@xxxxxxxxxxxxxxxxxx/"
-            "~a-usr/a-pillar/\+oci/oci-name"
-        )
+        self.factory.makeGitRepository(
+            name=oci_project.name,
+            target=oci_project, owner=pillar.owner, registrant=pillar.owner)
+        git_url = "git+ssh://%s@xxxxxxxxxxxxxxxxxx/a-pillar/+oci/oci-name" % (
+            pillar.owner.name)
         self.assertTextMatchesExpressionIgnoreWhitespace("""\
             OCI project oci-name for A-pillar
             .*
             You can create a git repository for this OCI project in order
             to build your OCI recipes by using the following commands:
-            git remote add origin
-            %s
+            git remote add origin %s
             git push --set-upstream origin master
 
             OCI project information
             Project: A-pillar
+            Edit OCI project
             Name: oci-name
-            """ % git_url, self.getMainText(oci_project, user=owner))
+            Edit OCI project
+            """ % re.escape(git_url),
+            self.getMainText(oci_project, user=pillar.owner))
 
-    def test_shows_existing_git_repo(self):
-        owner = self.factory.makePerson(name="a-usr")
+    def test_shows_existing_default_git_repo(self):
         pillar = self.factory.makeProduct(name="a-pillar")
         oci_project = self.factory.makeOCIProject(
             pillar=pillar, ociprojectname="oci-name")
-        self.factory.makeGitRepository(
-            name=oci_project.name,
-            target=oci_project, owner=owner, registrant=owner)
+        repository = self.factory.makeGitRepository(
+            name=oci_project.name, target=oci_project)
+        with person_logged_in(pillar.owner):
+            getUtility(IGitRepositorySet).setDefaultRepository(
+                oci_project, repository)
+        git_url = "lp:a-pillar/+oci/oci-name"
         self.assertTextMatchesExpressionIgnoreWhitespace("""\
             OCI project oci-name for A-pillar
             .*
-            Your default git repository for this project is
-            lp:~a-usr/a-pillar/\+oci/oci-name/\+git/oci-name.
+            The default git repository for this project is %s.
 
             OCI project information
             Project: A-pillar
             Name: oci-name
-            """, self.getMainText(oci_project, user=owner))
+            """ % re.escape(git_url),
+            self.getMainText(oci_project))
 
     def test_shows_official_recipes(self):
         distribution = self.factory.makeDistribution(displayname="My Distro")
diff --git a/lib/lp/registry/interfaces/ociproject.py b/lib/lp/registry/interfaces/ociproject.py
index bb0a785..b8fca14 100644
--- a/lib/lp/registry/interfaces/ociproject.py
+++ b/lib/lp/registry/interfaces/ociproject.py
@@ -108,15 +108,6 @@ class IOCIProjectView(IHasGitRepositories, Interface):
         If the user is a Launchpad admin, any type is acceptable.
         """
 
-    def getDefaultGitRepository(person):
-        """Returns the default git repository for the given user under the
-        namespace of this OCI project"""
-
-    def getDefaultGitRepositoryPath(person):
-        """Returns the default git repository path for this OCI Project,
-        regardless if the repository exists or not.
-        """
-
 
 class IOCIProjectEditableAttributes(IBugTarget):
     """IOCIProject attributes that can be edited.
diff --git a/lib/lp/registry/model/ociproject.py b/lib/lp/registry/model/ociproject.py
index e98b3ea..18e8670 100644
--- a/lib/lp/registry/model/ociproject.py
+++ b/lib/lp/registry/model/ociproject.py
@@ -38,7 +38,6 @@ from lp.app.enums import (
     )
 from lp.app.interfaces.services import IService
 from lp.bugs.model.bugtarget import BugTargetBase
-from lp.code.interfaces.gitnamespace import IGitNamespaceSet
 from lp.code.model.branchnamespace import (
     BRANCH_POLICY_ALLOWED_TYPES,
     BRANCH_POLICY_REQUIRED_GRANTS,
@@ -289,14 +288,6 @@ class OCIProject(BugTargetBase, StormBase):
             return []
         return BRANCH_POLICY_ALLOWED_TYPES[self.pillar.branch_sharing_policy]
 
-    def getDefaultGitRepository(self, person):
-        namespace = getUtility(IGitNamespaceSet).get(person, oci_project=self)
-        return namespace.getByName(self.name)
-
-    def getDefaultGitRepositoryPath(self, person):
-        namespace = getUtility(IGitNamespaceSet).get(person, oci_project=self)
-        return namespace.name
-
 
 @implementer(IOCIProjectSet)
 class OCIProjectSet:
diff --git a/lib/lp/registry/templates/ociproject-index.pt b/lib/lp/registry/templates/ociproject-index.pt
index 75023b0..dde9aea 100644
--- a/lib/lp/registry/templates/ociproject-index.pt
+++ b/lib/lp/registry/templates/ociproject-index.pt
@@ -26,20 +26,21 @@
   </metal:heading>
 
   <div metal:fill-slot="main">
-    <div tal:condition="python: view.git_repository is None">
+    <div tal:condition="python: view.git_repository is None and
+                                view.user_can_push_default">
       <p>
         You can create a git repository for this OCI project in order to
         build your OCI recipes by using the following commands:
         <br />
         <pre class="command">
-          git remote add origin git+ssh://<tal:name replace="view/user/name"/>@<tal:host replace="view/git_ssh_hostname" />/<tal:name replace="view/git_repository_path"/>
+          git remote add origin <span class="ssh-url" tal:content="view/git_ssh_url" />
           git push --set-upstream origin master
         </pre>
       </p>
     </div>
     <div tal:define="repo view/git_repository"
          tal:condition="repo">
-      Your default git repository for this project is <a tal:replace="structure repo/fmt:link"/>.
+      The default git repository for this project is <a tal:replace="structure repo/fmt:link"/>.
     </div>
 
     <h2>OCI project information</h2>