Merge ~ilasc/launchpad:git-push-url-hint into launchpad:master

[Ioana Lasc]

Add git push URL hinting

Add git push URL hinting to LP UI on both repository and branch pages when the user doesn’t have permission to push to the repository.

[Thiago F. Pappacena (pappacena)]

Looks like a good job. I just have a question about the splitting of the URL itself.

review: Approve

[Colin Watson (cjwatson)]

review: Needs Fixing

17b83c2... by Ioana Lasc on 2020-07-06

Subclass GitIdentityMixin for contributor repository path

[Colin Watson (cjwatson)]

Thanks, this is an improvement. I'd like to see some more polishing of the contributor-identity code.

review: Needs Fixing

a83386a... by Ioana Lasc on 2020-07-08

Address code review comments

[Ioana Lasc (ilasc)]

Thank you Colin!
I've addressed everything and in relation to your comment on the 2 code paths in git_ssh_url_non_owner: I have 2 code paths because otherwise it breaks like this when self.context.target_default is true:

   Module lp.code.interfaces.gitrepository, line 1201, in getRepositoryIdentities
    for default in self.getRepositoryDefaults()]
    Module lp.code.interfaces.gitrepository, line 1192, in getRepositoryDefaults
    "Only projects, packages, or OCI projects can have "

The branch is now ready for another look.

[Colin Watson (cjwatson)]

git_ssh_url_non_owner works just fine with a single code path in my tests; you're passing target_default=False when constructing ContributorGitIdentity, so the breakage you mention doesn't happen. It does require some test changes to account for the resulting URLs being different, but IMO the changed URLs are an improvement. See this patch:

  https://paste.ubuntu.com/p/gS5YWSnDYB/

There's this new import policy warning from the test suite, which should be resolved one way or another:

There were 2 imports of names not appearing in the __all__.
You should not import ContributorGitIdentity from lp.code.interfaces.gitrepository:
    lp.code.browser.gitref
    lp.code.browser.gitrepository

There's also one further case that may have been overlooked here. Personal repositories (those handled by lp.code.model.gitnamespace.PersonalGitNamespace, of the form /~OWNER/+git/NAME) don't support merge proposals between different repositories, because there's no obvious link between them except possibly for the name. This isn't ideal, and it might be worth us revisiting it in the future, but for the meantime this is how it is. As such, it risks misleading users if, for example, I visit /~ilasc/+git/random-scripts and am told that I can push code to /~cjwatson/+git/random-scripts and propose changes from there. We should probably stick with the existing "You cannot push to this branch/repository" language in that case and not try to provide a push URL hint, even though it's unfortunate that that complicates this branch some more.

Despite my repeated "Needs fixing" votes, this is definitely improving, and I think is nearly there! Hopefully it should just be this one more round.

review: Needs Fixing

281c8a9... by Ioana Lasc on 2020-08-05

Address code review comments

[Ioana Lasc (ilasc)]

Colin thank you for the helpful comments.
MP is now ready for another review.

[Colin Watson (cjwatson)]

I'd like a few more bits of tidying up, explained in detail below, but this looks pretty close now. Thanks for your work.

review: Approve

a9e7a78... by Ioana Lasc on 2020-08-12

Merge branch 'master' into git-push-url-hint

5dd6be6... by Ioana Lasc on 2020-08-13

Address code review comments

[Colin Watson (cjwatson)]

review: Approve

6909a8b... by Ioana Lasc on 2020-08-14

Fix unit tests

[Ioana Lasc (ilasc)]

Thanks Colin! Agreed with all comments & suggestions.
Had to tweak a few tests I didn't write for the new code, possibly worth another look before I land this ?

[Colin Watson (cjwatson)]

review: Approve

diff --git a/lib/lp/code/browser/gitref.py b/lib/lp/code/browser/gitref.py
index a945fa0..f17207e 100644
--- a/lib/lp/code/browser/gitref.py
+++ b/lib/lp/code/browser/gitref.py
@@ -13,6 +13,7 @@ __all__ = [
 
 import json
 
+from breezy import urlutils
 from lazr.restful.interface import copy_field
 from six.moves.urllib_parse import (
     quote_plus,
@@ -46,8 +47,12 @@ from lp.code.errors import InvalidBranchMergeProposal
 from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
 from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference
 from lp.code.interfaces.gitref import IGitRef
-from lp.code.interfaces.gitrepository import IGitRepositorySet
+from lp.code.interfaces.gitrepository import (
+    ContributorGitIdentity,
+    IGitRepositorySet,
+    )
 from lp.registry.interfaces.person import IPerson
+from lp.services.config import config
 from lp.services.helpers import english_list
 from lp.services.propertycache import cachedproperty
 from lp.services.scripts import log
@@ -120,6 +125,22 @@ class GitRefView(LaunchpadView, HasSnapsViewMixin):
         return urlunsplit(url)
 
     @property
+    def git_ssh_url_non_owner(self):
+        """The git+ssh:// URL for this repository, adjusted for this user.
+
+        The user is not the owner of the repository.
+        """
+        contributor = ContributorGitIdentity(
+            owner=self.user,
+            target=self.context.repository.target,
+            repository=self.context.repository)
+        base_url = urlutils.join(
+            config.codehosting.git_ssh_root, contributor.shortened_path)
+        url = list(urlsplit(base_url))
+        url[1] = "{}@{}".format(self.user.name, url[1])
+        return urlunsplit(url)
+
+    @property
     def user_can_push(self):
         """Whether the user can push to this branch."""
         return (
diff --git a/lib/lp/code/browser/gitrepository.py b/lib/lp/code/browser/gitrepository.py
index 7dc1c32..76b9f7d 100644
--- a/lib/lp/code/browser/gitrepository.py
+++ b/lib/lp/code/browser/gitrepository.py
@@ -25,6 +25,7 @@ __all__ = [
 import base64
 from collections import defaultdict
 
+from breezy import urlutils
 from lazr.lifecycle.event import ObjectModifiedEvent
 from lazr.lifecycle.snapshot import Snapshot
 from lazr.restful.interface import (
@@ -99,7 +100,10 @@ from lp.code.errors import (
     )
 from lp.code.interfaces.gitnamespace import get_git_namespace
 from lp.code.interfaces.gitref import IGitRefBatchNavigator
-from lp.code.interfaces.gitrepository import IGitRepository
+from lp.code.interfaces.gitrepository import (
+    ContributorGitIdentity,
+    IGitRepository,
+    )
 from lp.code.vocabularies.gitrule import GitPermissionsVocabulary
 from lp.registry.interfaces.person import (
     IPerson,
@@ -385,6 +389,21 @@ class GitRepositoryView(InformationTypePortletMixin, LaunchpadView,
         return urlunsplit(url)
 
     @property
+    def git_ssh_url_non_owner(self):
+        """The git+ssh:// URL for this repository, adjusted for this user.
+        The user is not the owner of the repository."""
+
+        contributor = ContributorGitIdentity(
+            owner=self.user,
+            target=self.context.target,
+            repository=self.context)
+        base_url = urlutils.join(
+            config.codehosting.git_ssh_root, contributor.shortened_path)
+        url = list(urlsplit(base_url))
+        url[1] = "{}@{}".format(self.user.name, url[1])
+        return urlunsplit(url)
+
+    @property
     def user_can_push(self):
         """Whether the user can push to this branch."""
         return (
diff --git a/lib/lp/code/browser/tests/test_gitlisting.py b/lib/lp/code/browser/tests/test_gitlisting.py
index 72954ad..e7682fc 100644
--- a/lib/lp/code/browser/tests/test_gitlisting.py
+++ b/lib/lp/code/browser/tests/test_gitlisting.py
@@ -125,7 +125,7 @@ class TestTargetGitListingView:
                 repository=other_repo, user=other_repo.owner)
 
         self.assertThat(
-            self.target, BrowsesWithQueryLimit(36, self.owner, '+git'))
+            self.target, BrowsesWithQueryLimit(38, self.owner, '+git'))
 
     def test_copes_with_no_default(self):
         self.factory.makeGitRepository(
diff --git a/lib/lp/code/browser/tests/test_gitref.py b/lib/lp/code/browser/tests/test_gitref.py
index 0268ca4..0a36550 100644
--- a/lib/lp/code/browser/tests/test_gitref.py
+++ b/lib/lp/code/browser/tests/test_gitref.py
@@ -33,6 +33,8 @@ from lp.services.webapp.publisher import canonical_url
 from lp.testing import (
     admin_logged_in,
     BrowserTestCase,
+    login_person,
+    person_logged_in,
     StormStatementRecorder,
     TestCaseWithFactory,
     )
@@ -155,6 +157,119 @@ class TestGitRefView(BrowserTestCase):
             git clone -b branch git\+ssh://{username}@git.launchpad.test/.*
             """.format(username=username), text)
 
+    def test_push_directions_logged_in_cannot_push_individual(self):
+        repo = self.factory.makeGitRepository()
+        [ref] = self.factory.makeGitRefs(repository=repo,
+                                         paths=["refs/heads/branch"])
+        login_person(self.user)
+        view = create_initialized_view(ref, "+index", principal=self.user)
+        git_push_url_text_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url text', 'dt',
+                text='To fork this repository and propose '
+                     'fixes from there, push to this repository:'))
+
+        git_push_url_hint_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url hint', 'span',
+                text=('git+ssh://[e-mail]/'
+                      '~%s/%s') % (
+                    self.user.name, self.user.name, repo.target.name)))
+        with person_logged_in(self.user):
+            rendered_view = view.render()
+            self.assertThat(rendered_view, git_push_url_text_match)
+            self.assertThat(rendered_view, git_push_url_hint_match)
+
+    def test_push_directions_logged_in_cannot_push_individual_project(self):
+        # Repository is the default for a project
+        eric = self.factory.makePerson(name="eric")
+        fooix = self.factory.makeProduct(name="fooix", owner=eric)
+        repository = self.factory.makeGitRepository(
+            owner=eric, target=fooix, name="fooix-repo")
+        [ref] = self.factory.makeGitRefs(repository=repository,
+                                         paths=["refs/heads/branch"])
+        self.repository_set = getUtility(IGitRepositorySet)
+        with person_logged_in(fooix.owner) as user:
+            self.repository_set.setDefaultRepositoryForOwner(
+                repository.owner, fooix, repository, user)
+            self.repository_set.setDefaultRepository(fooix, repository)
+        login_person(self.user)
+        view = create_initialized_view(ref, "+index", principal=self.user)
+        git_push_url_text_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url text', 'dt',
+                text='To fork this repository and propose '
+                     'fixes from there, push to this repository:'))
+        git_push_url_hint_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url hint', 'span',
+                text='git+ssh://[e-mail]/~%s/%s' %
+                     (self.user.name,
+                      self.user.name,
+                      repository.target.name)))
+
+        with person_logged_in(self.user):
+            rendered_view = view.render()
+            self.assertThat(rendered_view, git_push_url_text_match)
+            self.assertThat(rendered_view, git_push_url_hint_match)
+
+    def test_push_directions_logged_in_cannot_push_individual_package(self):
+        # Repository is the default for a package
+        mint = self.factory.makeDistribution(name="mint")
+        eric = self.factory.makePerson(name="eric")
+        mint_choc = self.factory.makeDistributionSourcePackage(
+            distribution=mint, sourcepackagename="choc")
+        repository = self.factory.makeGitRepository(
+            owner=eric, target=mint_choc, name="choc-repo")
+        [ref] = self.factory.makeGitRefs(repository=repository,
+                                         paths=["refs/heads/branch"])
+        dsp = repository.target
+        self.repository_set = getUtility(IGitRepositorySet)
+        with admin_logged_in():
+            self.repository_set.setDefaultRepositoryForOwner(
+                repository.owner, dsp, repository, repository.owner)
+            self.repository_set.setDefaultRepository(dsp, repository)
+        login_person(self.user)
+        view = create_initialized_view(ref, "+index", principal=self.user)
+        git_push_url_text_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url text', 'dt',
+                text='To fork this repository and propose '
+                     'fixes from there, push to this repository:'))
+        git_push_url_hint_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url hint', 'span',
+                text='git+ssh://[e-mail]/~%s/%s/+source/%s' %
+                     (self.user.name,
+                      self.user.name,
+                      mint.name,
+                      mint_choc.name)))
+
+        with person_logged_in(self.user):
+            rendered_view = view.render()
+            self.assertThat(rendered_view, git_push_url_text_match)
+            self.assertThat(rendered_view, git_push_url_hint_match)
+
+    def test_push_directions_logged_in_cannot_push_personal_project(self):
+        repository = self.factory.makeGitRepository(
+            owner=self.user, target=self.user)
+        [ref] = self.factory.makeGitRefs(repository=repository,
+                                         paths=["refs/heads/branch"])
+        other_user = self.factory.makePerson()
+        login_person(other_user)
+        view = create_initialized_view(ref, "+index", principal=self.user)
+        git_push_url_text_match = soupmatchers.Tag(
+                'Push url text', 'a',
+                text=self.user.displayname)
+        with person_logged_in(other_user):
+            rendered_view = view.render()
+            div = soupmatchers.Tag("Push directions", "div",
+                                    attrs={"id": "push-directions"})
+            self.assertThat(rendered_view, soupmatchers.HTMLContains(
+                soupmatchers.Within(
+                    div,
+                    git_push_url_text_match)))
+
     def makeCommitLog(self):
         authors = [self.factory.makePerson() for _ in range(5)]
         with admin_logged_in():
diff --git a/lib/lp/code/browser/tests/test_gitrepository.py b/lib/lp/code/browser/tests/test_gitrepository.py
index d323976..d3211ba 100644
--- a/lib/lp/code/browser/tests/test_gitrepository.py
+++ b/lib/lp/code/browser/tests/test_gitrepository.py
@@ -50,6 +50,7 @@ from lp.code.enums import (
     GitRepositoryStatus,
     GitRepositoryType,
     )
+from lp.code.interfaces.gitrepository import IGitRepositorySet
 from lp.code.interfaces.revision import IRevisionSet
 from lp.code.model.gitjob import GitRefScanJob
 from lp.code.tests.helpers import GitHostingFixture
@@ -225,6 +226,7 @@ class TestGitRepositoryView(BrowserTestCase):
             Update this repository:
             git push
             git+ssh://{username}@git.launchpad.test/{repository.shortened_path}
+            BRANCHNAME
             """).format(username=username, repository=repository),
             flags=doctest.NORMALIZE_WHITESPACE))
 
@@ -246,30 +248,130 @@ class TestGitRepositoryView(BrowserTestCase):
         # If the user is logged in but cannot push to a repository owned by
         # a person, we explain who can push.
         repository = self.factory.makeGitRepository()
-        browser = self.getViewBrowser(repository)
-        directions = find_tag_by_id(browser.contents, "push-directions")
         login_person(self.user)
-        self.assertThat(directions.renderContents(), DocTestMatches(dedent("""
-            You cannot push to this repository. Only <a
-            href="http://launchpad.test/~{owner.name}">{owner.display_name}</a>
-            can push to this repository.
-            """).format(owner=repository.owner),
-            flags=doctest.NORMALIZE_WHITESPACE))
+        view = create_initialized_view(
+            repository, '+index', principal=self.user)
+        git_push_url_text_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url text', 'dt',
+                text='To fork this repository and propose '
+                     'fixes from there, push to this repository:'))
+        git_push_url_hint_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url hint', 'span',
+                text='git+ssh://[e-mail]/~%s/%s' %
+                     (self.user.name, self.user.name, repository.target.name)))
+        with person_logged_in(self.user):
+            rendered_view = view.render()
+            self.assertThat(rendered_view, git_push_url_text_match)
+            self.assertThat(rendered_view, git_push_url_hint_match)
+
+    def test_push_directions_logged_in_cannot_push_individual_project(self):
+        # Repository is the default for a project
+        eric = self.factory.makePerson(name="eric")
+        fooix = self.factory.makeProduct(name="fooix", owner=eric)
+        repository = self.factory.makeGitRepository(
+            owner=eric, target=fooix, name="fooix-repo")
+        self.repository_set = getUtility(IGitRepositorySet)
+        with person_logged_in(fooix.owner) as user:
+            self.repository_set.setDefaultRepositoryForOwner(
+                repository.owner, fooix, repository, user)
+            self.repository_set.setDefaultRepository(fooix, repository)
+        login_person(self.user)
+        view = create_initialized_view(
+            repository, '+index', principal=self.user)
+        git_push_url_text_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url text', 'dt',
+                text='To fork this repository and propose '
+                     'fixes from there, push to this repository:'))
+        git_push_url_hint_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url hint', 'span',
+                text='git+ssh://[e-mail]/~%s/%s' %
+                     (self.user.name, self.user.name,
+                      repository.target.name)))
+        with person_logged_in(self.user):
+            rendered_view = view.render()
+            self.assertThat(rendered_view, git_push_url_text_match)
+            self.assertThat(rendered_view, git_push_url_hint_match)
+
+    def test_push_directions_logged_in_cannot_push_individual_package(self):
+        # Repository is the default for a package
+        mint = self.factory.makeDistribution(name="mint")
+        eric = self.factory.makePerson(name="eric")
+        mint_choc = self.factory.makeDistributionSourcePackage(
+            distribution=mint, sourcepackagename="choc")
+        repository = self.factory.makeGitRepository(
+            owner=eric, target=mint_choc, name="choc-repo")
+        dsp = repository.target
+        self.repository_set = getUtility(IGitRepositorySet)
+        with admin_logged_in():
+            self.repository_set.setDefaultRepositoryForOwner(
+                repository.owner, dsp, repository, repository.owner)
+            self.repository_set.setDefaultRepository(dsp, repository)
+        login_person(self.user)
+        view = create_initialized_view(
+            repository, '+index', principal=self.user)
+        git_push_url_text_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url text', 'dt',
+                text='To fork this repository and propose '
+                     'fixes from there, push to this repository:'))
+        git_push_url_hint_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url hint', 'span',
+                text='git+ssh://[e-mail]/~%s/%s/+source/%s' %
+                     (self.user.name, self.user.name,
+                      mint.name, mint_choc.name)))
+        with person_logged_in(self.user):
+            rendered_view = view.render()
+            self.assertThat(rendered_view, git_push_url_text_match)
+            self.assertThat(rendered_view, git_push_url_hint_match)
+
+    def test_push_directions_logged_in_cannot_push_personal_project(self):
+        # If the user is logged in but cannot push to a repository owned by
+        # a person, we explain who can push.
+        repository = self.factory.makeGitRepository(
+            owner=self.user, target=self.user)
+        other_user = self.factory.makePerson()
+        login_person(other_user)
+        view = create_initialized_view(
+            repository, '+index', principal=other_user)
+        git_push_url_text_match = soupmatchers.Tag(
+                'Push url text', 'a',
+                text=self.user.displayname)
+        with person_logged_in(other_user):
+            rendered_view = view.render()
+            div = soupmatchers.Tag("Push directions", "div",
+                                    attrs={"id": "push-directions"})
+            self.assertThat(rendered_view, soupmatchers.HTMLContains(
+                soupmatchers.Within(
+                    div,
+                    git_push_url_text_match)))
 
     def test_push_directions_logged_in_cannot_push_team(self):
         # If the user is logged in but cannot push to a repository owned by
         # a team, we explain who can push.
         team = self.factory.makeTeam()
         repository = self.factory.makeGitRepository(owner=team)
-        browser = self.getViewBrowser(repository)
-        directions = find_tag_by_id(browser.contents, "push-directions")
         login_person(self.user)
-        self.assertThat(directions.renderContents(), DocTestMatches(dedent("""
-            You cannot push to this repository. Members of <a
-            href="http://launchpad.test/~{owner.name}">{owner.display_name}</a>
-            can push to this repository.
-            """).format(owner=repository.owner),
-            flags=doctest.NORMALIZE_WHITESPACE))
+        view = create_initialized_view(
+            repository, '+index', principal=self.user)
+        git_push_url_text_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url text', 'dt',
+                text='To fork this repository and propose '
+                     'fixes from there, push to this repository:'))
+        git_push_url_hint_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url hint', 'span',
+                text='git+ssh://[e-mail]/~%s/%s' %
+                     (self.user.name, self.user.name, repository.target.name)))
+        with person_logged_in(self.user):
+            rendered_view = view.render()
+            self.assertThat(rendered_view, git_push_url_text_match)
+            self.assertThat(rendered_view, git_push_url_hint_match)
 
     def test_no_push_directions_for_imported_repository(self):
         # Imported repositories never show push directions.
diff --git a/lib/lp/code/interfaces/gitnamespace.py b/lib/lp/code/interfaces/gitnamespace.py
index fd7e3e7..9c6886b 100644
--- a/lib/lp/code/interfaces/gitnamespace.py
+++ b/lib/lp/code/interfaces/gitnamespace.py
@@ -99,6 +99,9 @@ class IGitNamespacePolicy(Interface):
         "True iff this namespace permits automatically setting a default "
         "repository on push.")
 
+    show_push_url_hints = Attribute(
+        "True if this namespace permits display of the push URL hint.")
+
     supports_merge_proposals = Attribute(
         "Does this namespace support merge proposals at all?")
 
diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py
index 189b426..344bc43 100644
--- a/lib/lp/code/interfaces/gitrepository.py
+++ b/lib/lp/code/interfaces/gitrepository.py
@@ -6,6 +6,7 @@
 __metaclass__ = type
 
 __all__ = [
+    'ContributorGitIdentity',
     'GitIdentityMixin',
     'GIT_REPOSITORY_NAME_VALIDATION_ERROR_MESSAGE',
     'git_repository_name_validator',
@@ -1033,7 +1034,8 @@ class IGitRepositorySet(Interface):
                     "checked."),
             schema=IPerson),
         repository_names=List(value_type=Text(),
-            title=_('List of repository unique names'), required=True),
+                              title=_('List of repository unique names'),
+                              required=True),
     )
     @export_read_operation()
     @operation_for_version("devel")
@@ -1228,3 +1230,20 @@ def user_has_special_git_repository_access(user, repository=None):
     if code_import is None:
         return False
     return roles.in_vcs_imports
+
+
+class ContributorGitIdentity(GitIdentityMixin):
+
+    def __init__(self, owner,
+                 target, repository):
+        self.target_default = False
+        self.owner_default = True
+        self.owner = owner
+        self.target = target
+        self.repository = repository
+
+    def getRepositoryIdentities(self):
+        identities = [
+            (default.path, default.context)
+            for default in self.getRepositoryDefaults()]
+        return identities
diff --git a/lib/lp/code/model/gitnamespace.py b/lib/lp/code/model/gitnamespace.py
index 8a08a97..f9ac2f5 100644
--- a/lib/lp/code/model/gitnamespace.py
+++ b/lib/lp/code/model/gitnamespace.py
@@ -273,6 +273,7 @@ class PersonalGitNamespace(_BaseGitNamespace):
     supports_merge_proposals = True
     supports_code_imports = False
     allow_recipe_name_from_target = False
+    show_push_url_hints = False
 
     def __init__(self, person):
         self.owner = person
@@ -356,6 +357,7 @@ class ProjectGitNamespace(_BaseGitNamespace):
     supports_merge_proposals = True
     supports_code_imports = True
     allow_recipe_name_from_target = True
+    show_push_url_hints = True
 
     def __init__(self, person, project):
         self.owner = person
@@ -447,6 +449,7 @@ class PackageGitNamespace(_BaseGitNamespace):
     supports_merge_proposals = True
     supports_code_imports = True
     allow_recipe_name_from_target = True
+    show_push_url_hints = True
 
     def __init__(self, person, distro_source_package):
         self.owner = person
@@ -538,6 +541,7 @@ class OCIProjectGitNamespace(_BaseGitNamespace):
     supports_merge_proposals = True
     supports_code_imports = True
     allow_recipe_name_from_target = True
+    show_push_url_hints = False
 
     def __init__(self, person, oci_project):
         self.owner = person
diff --git a/lib/lp/code/templates/git-macros.pt b/lib/lp/code/templates/git-macros.pt
index 9407df1..928bf83 100644
--- a/lib/lp/code/templates/git-macros.pt
+++ b/lib/lp/code/templates/git-macros.pt
@@ -64,31 +64,41 @@
             <tt class="command">
             git push
             <span class="ssh-url" tal:content="view/git_ssh_url" />
-            <tal:branch condition="branch_name|nothing" replace="branch_name" />
+            <em>BRANCHNAME</em>
             </tt>
           </dd>
         </dl>
-        <p tal:condition="not:view/user/sshkeys" id="ssh-key-directions">
-          To authenticate with the Launchpad Git hosting service, you need to
-          <a tal:attributes="href string:${view/user/fmt:url}/+editsshkeys">
-            register an SSH key</a>.
-        </p>
       </tal:can-push>
 
       <tal:cannot-push condition="not:view/user_can_push">
         <tal:individual condition="not:context/owner/is_team">
-          You cannot push to this <tal:kind replace="kind" />. Only
+          You cannot push directly to this <tal:kind replace="kind" />. Only
           <a tal:attributes="href context/owner/fmt:url"
              tal:content="context/owner/displayname">Person</a>
           can push to this <tal:kind replace="kind" />.
         </tal:individual>
         <tal:team condition="context/owner/is_team">
-          You cannot push to this <tal:kind replace="kind" />. Members of
+          You cannot push directly to this <tal:kind replace="kind" />. Members of
           <a tal:attributes="href context/owner/fmt:url"
              tal:content="context/owner/displayname">Team</a>
           can push to this <tal:kind replace="kind" />.
         </tal:team>
+        <dl tal:condition="context/namespace/show_push_url_hints" id="push-url">
+          <dt>To fork this repository and propose fixes from there, push to this repository:</dt>
+          <dd>
+            <tt class="command">
+            git push
+            <span class="ssh-url" tal:content="view/git_ssh_url_non_owner" />
+            <em>BRANCHNAME</em>
+            </tt>
+          </dd>
+        </dl>
       </tal:cannot-push>
+      <p tal:condition="not:view/user/sshkeys" id="ssh-key-directions">
+        To authenticate with the Launchpad Git hosting service, you need to
+        <a tal:attributes="href string:${view/user/fmt:url}/+editsshkeys">
+          register an SSH key</a>.
+      </p>
     </tal:logged-in>
 
   </div>