launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21113
[Merge] lp:~cjwatson/launchpad/codeimport-git-new-view into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/codeimport-git-new-view into lp:launchpad with lp:~cjwatson/launchpad/codeimport-git-worker-fixes as a prerequisite.
Commit message:
Extend CodeImportNewView to be able to create Git-to-Git code imports.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1469459 in Launchpad itself: "import external code into a LP git repo (natively)"
https://bugs.launchpad.net/launchpad/+bug/1469459
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/codeimport-git-new-view/+merge/308545
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/codeimport-git-new-view into lp:launchpad.
=== modified file 'lib/lp/code/browser/codeimport.py'
--- lib/lp/code/browser/codeimport.py 2016-10-14 17:29:44 +0000
+++ lib/lp/code/browser/codeimport.py 2016-10-14 17:29:44 +0000
@@ -57,7 +57,10 @@
RevisionControlSystems,
TargetRevisionControlSystems,
)
-from lp.code.errors import BranchExists
+from lp.code.errors import (
+ BranchExists,
+ GitRepositoryExists,
+ )
from lp.code.interfaces.branch import (
IBranch,
user_has_special_branch_access,
@@ -71,6 +74,10 @@
ICodeImportSet,
)
from lp.code.interfaces.codeimportmachine import ICodeImportMachineSet
+from lp.code.interfaces.gitnamespace import (
+ get_git_namespace,
+ IGitNamespacePolicy,
+ )
from lp.registry.interfaces.product import IProduct
from lp.registry.interfaces.role import IPersonRoles
from lp.services.fields import URIField
@@ -243,9 +250,10 @@
git_repo_url = URIField(
title=_("Repo URL"), required=False,
description=_(
- "The URL of the git repository. The HEAD branch will be "
- "imported. You can import different branches by appending "
- "',branch=$name' to the URL."),
+ "The URL of the git repository. For imports to Bazaar, the "
+ "HEAD branch will be imported by default, but you can import "
+ "different branches by appending ',branch=$name' to the URL. "
+ "For imports to Git, the entire repository will be imported."),
allowed_schemes=["git", "http", "https"],
allow_userinfo=True,
allow_port=True,
@@ -253,6 +261,13 @@
allow_fragment=False,
trailing_slash=False)
+ git_target_rcs_type = Choice(
+ title=_("Target version control system"),
+ description=_(
+ "The version control system that the source code should be "
+ "imported into on the Launchpad side."),
+ required=False, vocabulary=TargetRevisionControlSystems)
+
bzr_branch_url = URIField(
title=_("Branch URL"), required=False,
description=_("The URL of the Bazaar branch."),
@@ -266,10 +281,10 @@
branch_name = copy_field(
IBranch['name'],
__name__='branch_name',
- title=_('Branch Name'),
+ title=_('Name'),
description=_(
- "This will be used in the branch URL to identify the "
- "imported branch. Examples: main, trunk."),
+ "This will be used in the branch or repository URL to identify "
+ "the import. Examples: main, trunk."),
)
product = Choice(
@@ -286,6 +301,7 @@
for_input = True
custom_widget('rcs_type', LaunchpadRadioWidget)
+ custom_widget('git_target_rcs_type', LaunchpadRadioWidget)
@property
def initial_values(self):
@@ -293,6 +309,7 @@
'owner': self.user,
'rcs_type': RevisionControlSystems.BZR,
'branch_name': 'trunk',
+ 'git_target_rcs_type': TargetRevisionControlSystems.BZR,
}
@property
@@ -354,6 +371,9 @@
self.rcs_type_git = str(git_button)
self.rcs_type_bzr = str(bzr_button)
self.rcs_type_emptymarker = str(empty_marker)
+ # This widget is only conditionally required in the rcs_type == GIT
+ # case, but we still don't want a "(nothing selected)" item.
+ self.widgets['git_target_rcs_type']._displayItemForMissingValue = False
def _getImportLocation(self, data):
"""Return the import location based on type."""
@@ -374,13 +394,18 @@
"""Create the code import."""
product = self.getProduct(data)
cvs_root, cvs_module, url = self._getImportLocation(data)
+ if data['rcs_type'] == RevisionControlSystems.GIT:
+ target_rcs_type = data.get(
+ 'git_target_rcs_type', TargetRevisionControlSystems.BZR)
+ else:
+ target_rcs_type = TargetRevisionControlSystems.BZR
return getUtility(ICodeImportSet).new(
registrant=self.user,
owner=data['owner'],
context=product,
branch_name=data['branch_name'],
rcs_type=data['rcs_type'],
- target_rcs_type=TargetRevisionControlSystems.BZR,
+ target_rcs_type=target_rcs_type,
url=url,
cvs_root=cvs_root,
cvs_module=cvs_module,
@@ -408,16 +433,19 @@
except BranchExists as e:
self._setBranchExists(e.existing_branch)
return
+ except GitRepositoryExists as e:
+ self._setBranchExists(e.existing_repository)
+ return
# Subscribe the user.
- code_import.branch.subscribe(
+ code_import.target.subscribe(
self.user,
BranchSubscriptionNotificationLevel.FULL,
BranchSubscriptionDiffSize.NODIFF,
CodeReviewNotificationLevel.NOEMAIL,
self.user)
- self.next_url = canonical_url(code_import.branch)
+ self.next_url = canonical_url(code_import.target)
self.request.response.addNotification("""
New code import created. The code import will start shortly.""")
@@ -429,24 +457,41 @@
else:
return data.get('product')
+ def validate_widgets(self, data, names=None):
+ """See `LaunchpadFormView`."""
+ self.widgets['git_target_rcs_type'].context.required = (
+ data.get('rcs_type') == RevisionControlSystems.GIT)
+ super(CodeImportNewView, self).validate_widgets(data, names=names)
+
def validate(self, data):
"""See `LaunchpadFormView`."""
- # Make sure that the user is able to create branches for the specified
- # namespace.
+ rcs_type = data['rcs_type']
+ if rcs_type == RevisionControlSystems.GIT:
+ target_rcs_type = data.get(
+ 'git_target_rcs_type', TargetRevisionControlSystems.BZR)
+ else:
+ target_rcs_type = TargetRevisionControlSystems.BZR
+
+ # Make sure that the user is able to create branches/repositories
+ # for the specified namespace.
product = self.getProduct(data)
# 'owner' in data may be None if it failed validation.
owner = data.get('owner')
if product is not None and owner is not None:
- namespace = get_branch_namespace(owner, product)
- policy = IBranchNamespacePolicy(namespace)
- if not policy.canCreateBranches(self.user):
+ if target_rcs_type == TargetRevisionControlSystems.BZR:
+ namespace = get_branch_namespace(owner, product)
+ policy = IBranchNamespacePolicy(namespace)
+ can_create = policy.canCreateBranches(self.user)
+ else:
+ namespace = get_git_namespace(product, owner)
+ policy = IGitNamespacePolicy(namespace)
+ can_create = policy.canCreateRepositories(self.user)
+ if not can_create:
self.setFieldError(
'product',
"You are not allowed to register imports for %s."
% product.displayname)
- rcs_type = data['rcs_type']
- target_rcs_type = TargetRevisionControlSystems.BZR
# Make sure fields for unselected revision control systems
# are blanked out:
if rcs_type == RevisionControlSystems.CVS:
=== modified file 'lib/lp/code/interfaces/gitnamespace.py'
--- lib/lp/code/interfaces/gitnamespace.py 2016-10-12 23:24:24 +0000
+++ lib/lp/code/interfaces/gitnamespace.py 2016-10-14 17:29:44 +0000
@@ -104,6 +104,13 @@
"Can recipe names reasonably be generated from the target name "
"rather than the branch name?")
+ def canCreateRepositories(user):
+ """Is the user allowed to create repositories for this namespace?
+
+ :param user: An `IPerson`.
+ :return: A Boolean value.
+ """
+
def getAllowedInformationTypes(who):
"""Get the information types that a repository in this namespace can
have.
=== modified file 'lib/lp/code/model/gitnamespace.py'
--- lib/lp/code/model/gitnamespace.py 2016-10-12 23:24:24 +0000
+++ lib/lp/code/model/gitnamespace.py 2016-10-14 17:29:44 +0000
@@ -211,6 +211,19 @@
match = default
return match
+ def canCreateRepositories(self, user):
+ """See `IGitNamespacePolicy`."""
+ try:
+ self.validateRegistrant(user)
+ except GitRepositoryCreatorNotMemberOfOwnerTeam:
+ return False
+ except GitRepositoryCreatorNotOwner:
+ return False
+ except GitRepositoryCreationForbidden:
+ return False
+ else:
+ return True
+
def getAllowedInformationTypes(self, who=None):
"""See `IGitNamespace`."""
raise NotImplementedError
=== modified file 'lib/lp/code/model/tests/test_gitnamespace.py'
--- lib/lp/code/model/tests/test_gitnamespace.py 2016-10-05 10:18:56 +0000
+++ lib/lp/code/model/tests/test_gitnamespace.py 2016-10-14 17:29:44 +0000
@@ -520,6 +520,105 @@
self.assertEqual(dsp, namespace.target)
+class BaseCanCreateRepositoriesMixin:
+ """Common tests for all namespaces."""
+
+ layer = DatabaseFunctionalLayer
+
+ def _getNamespace(self, owner):
+ # Return a namespace appropriate for the owner specified.
+ raise NotImplementedError(self._getNamespace)
+
+ def test_individual(self):
+ # For a GitNamespace for an individual, only the individual can own
+ # repositories there.
+ person = self.factory.makePerson()
+ namespace = self._getNamespace(person)
+ self.assertTrue(namespace.canCreateRepositories(person))
+
+ def test_other_user(self):
+ # Any other individual cannot own repositories targeted to the
+ # person.
+ person = self.factory.makePerson()
+ namespace = self._getNamespace(person)
+ self.assertFalse(
+ namespace.canCreateRepositories(self.factory.makePerson()))
+
+ def test_team_member(self):
+ # A member of a team is able to create a repository in this
+ # namespace.
+ person = self.factory.makePerson()
+ self.factory.makeTeam(owner=person)
+ namespace = self._getNamespace(person)
+ self.assertTrue(namespace.canCreateRepositories(person))
+
+ def test_team_non_member(self):
+ # A person who is not part of the team cannot create repositories
+ # for the personal team target.
+ person = self.factory.makePerson()
+ self.factory.makeTeam(owner=person)
+ namespace = self._getNamespace(person)
+ self.assertFalse(
+ namespace.canCreateRepositories(self.factory.makePerson()))
+
+
+class TestPersonalGitNamespaceCanCreateRepositories(
+ TestCaseWithFactory, BaseCanCreateRepositoriesMixin):
+
+ def _getNamespace(self, owner):
+ return PersonalGitNamespace(owner)
+
+
+class TestPackageGitNamespaceCanCreateBranches(
+ TestCaseWithFactory, BaseCanCreateRepositoriesMixin):
+
+ def _getNamespace(self, owner):
+ source_package = self.factory.makeSourcePackage()
+ return PackageGitNamespace(owner, source_package)
+
+
+class TestProjectGitNamespaceCanCreateBranches(
+ TestCaseWithFactory, BaseCanCreateRepositoriesMixin):
+
+ def _getNamespace(self, owner,
+ branch_sharing_policy=BranchSharingPolicy.PUBLIC):
+ project = self.factory.makeProduct(
+ branch_sharing_policy=branch_sharing_policy)
+ return ProjectGitNamespace(owner, project)
+
+ def setUp(self):
+ # Setting visibility policies is an admin-only task.
+ super(TestProjectGitNamespaceCanCreateBranches, self).setUp(
+ "admin@xxxxxxxxxxxxx")
+
+ def test_any_person(self):
+ # If there is no privacy set up, any person can create a personal
+ # branch on the product.
+ person = self.factory.makePerson()
+ namespace = self._getNamespace(person, BranchSharingPolicy.PUBLIC)
+ self.assertTrue(namespace.canCreateRepositories(person))
+
+ def test_any_person_with_proprietary_repositories(self):
+ # If the sharing policy defaults to PROPRIETARY, then non-privileged
+ # users cannot create a repository.
+ person = self.factory.makePerson()
+ namespace = self._getNamespace(person, BranchSharingPolicy.PROPRIETARY)
+ self.assertFalse(namespace.canCreateRepositories(person))
+
+ def test_grantee_with_proprietary_repositories(self):
+ # If the sharing policy defaults to PROPRIETARY, then non-privileged
+ # users cannot create a repository.
+ person = self.factory.makePerson()
+ other_person = self.factory.makePerson()
+ team = self.factory.makeTeam(members=[person])
+ namespace = self._getNamespace(team, BranchSharingPolicy.PROPRIETARY)
+ getUtility(IService, "sharing").sharePillarInformation(
+ namespace.target, team, namespace.target.owner,
+ {InformationType.PROPRIETARY: SharingPermission.ALL})
+ self.assertTrue(namespace.canCreateRepositories(person))
+ self.assertFalse(namespace.canCreateRepositories(other_person))
+
+
class TestNamespaceSet(TestCaseWithFactory):
"""Tests for `get_namespace`."""
=== modified file 'lib/lp/code/stories/codeimport/xx-create-codeimport.txt'
--- lib/lp/code/stories/codeimport/xx-create-codeimport.txt 2016-10-14 17:29:44 +0000
+++ lib/lp/code/stories/codeimport/xx-create-codeimport.txt 2016-10-14 17:29:44 +0000
@@ -66,7 +66,7 @@
The user is required to enter a project that the import is for, a name
for the import branch, and a Bazaar branch location.
- >>> browser.getControl('Branch Name').value = "mirrored"
+ >>> browser.getControl('Name').value = "mirrored"
>>> browser.getControl('Branch URL', index=0).value = (
... "http://bzr.example.com/firefox/trunk";)
>>> browser.getControl('Request Import').click()
@@ -87,7 +87,7 @@
URL.
>>> browser.open("http://code.launchpad.dev/+code-imports/+new";)
- >>> browser.getControl('Branch Name').value = "with-password"
+ >>> browser.getControl('Name').value = "with-password"
>>> browser.getControl('Branch URL', index=0).value = (
... "http://user:password@xxxxxxxxxxxxxxx/firefox/trunk";)
>>> browser.getControl('Project').value = "firefox"
@@ -102,7 +102,7 @@
Specifying a Launchpad URL results in an error.
>>> browser.open("http://code.launchpad.dev/+code-imports/+new";)
- >>> browser.getControl('Branch Name').value = "invalid"
+ >>> browser.getControl('Name').value = "invalid"
>>> browser.getControl('Branch URL', index=0).value = (
... "http://bazaar.launchpad.net/firefox/trunk";)
>>> browser.getControl('Project').value = "firefox"
@@ -116,7 +116,7 @@
>>> browser.open("http://code.launchpad.dev/+code-imports/+new";)
>>> browser.getControl('Project').value = "firefox"
- >>> browser.getControl('Branch Name').value = "lp-git-import"
+ >>> browser.getControl('Name').value = "lp-git-import"
>>> browser.getControl('Git').click()
>>> browser.getControl('Repo URL', index=0).value = (
... "git://git.launchpad.net/firefox.git")
@@ -136,7 +136,7 @@
>>> browser.open("http://code.launchpad.dev/+code-imports/+new";)
>>> browser.getControl('Subversion').click()
>>> browser.getControl('Project').value = "firefox"
- >>> browser.getControl('Branch Name').value = "imported"
+ >>> browser.getControl('Name').value = "imported"
>>> browser.getControl('Branch URL', index=1).value = (
... "http://svn.example.com/firefox/trunk";)
>>> browser.getControl('Request Import').click()
@@ -167,7 +167,7 @@
>>> browser.open("http://code.launchpad.dev/+code-imports/+new";)
>>> browser.getControl('Subversion').click()
- >>> browser.getControl('Branch Name').value = "svn-with-password"
+ >>> browser.getControl('Name').value = "svn-with-password"
>>> browser.getControl('Branch URL', index=1).value = (
... "http://user:password@xxxxxxxxxxxxxxx/firefox/trunk";)
>>> browser.getControl('Project').value = "firefox"
@@ -180,21 +180,21 @@
as soon as possible.
-Requesting a Git import
-=======================
+Requesting a Git-to-Bazaar import
+=================================
The user is required to enter a project that the import is for,
-a name for the import branch, and a subversion branch location.
+a name for the import branch, and a Git repository location.
>>> browser.open("http://code.launchpad.dev/+code-imports/+new";)
>>> browser.getControl('Project').value = "firefox"
- >>> browser.getControl('Branch Name').value = "git-import"
+ >>> browser.getControl('Name').value = "git-import"
>>> browser.getControl('Git').click()
>>> browser.getControl('Repo URL', index=0).value = (
... "git://example.com/firefox.git")
>>> browser.getControl('Request Import').click()
-When the user clicks continue, the approved import branch is created
+When the user clicks continue, the approved import branch is created.
>>> print extract_text(find_tag_by_id(browser.contents, "import-details"))
Import Status: Reviewed
@@ -203,6 +203,38 @@
The next import is scheduled to run as soon as possible.
+Requesting a Git-to-Git import
+==============================
+
+The user is required to enter a project that the import is for,
+a name for the import repository, and a Git repository location.
+
+ >>> from lp.code.interfaces.codeimport import (
+ ... CODE_IMPORT_GIT_TARGET_FEATURE_FLAG,
+ ... )
+ >>> from lp.code.tests.helpers import GitHostingFixture
+ >>> from lp.services.features.testing import FeatureFixture
+
+ >>> with FeatureFixture({CODE_IMPORT_GIT_TARGET_FEATURE_FLAG: u'on'}):
+ ... browser.open("http://code.launchpad.dev/+code-imports/+new";)
+ ... browser.getControl('Project').value = "firefox"
+ ... browser.getControl('Name').value = "upstream"
+ ... browser.getControl('Git', index=0).click()
+ ... browser.getControl('Git', index=1).click()
+ ... browser.getControl('Repo URL', index=0).value = (
+ ... "git://example.com/firefox2.git")
+ ... with GitHostingFixture():
+ ... browser.getControl('Request Import').click()
+
+When the user clicks continue, the approved import repository is created.
+
+ >>> print extract_text(find_tag_by_id(browser.contents, "import-details"))
+ Import Status: Reviewed
+ This repository is an import of the Git repository at
+ git://example.com/firefox2.git.
+ The next import is scheduled to run as soon as possible.
+
+
Requesting a CVS import
=======================
@@ -211,7 +243,7 @@
>>> browser.open("http://code.launchpad.dev/+code-imports/+new";)
>>> browser.getControl('Project').value = "firefox"
- >>> browser.getControl('Branch Name').value = "import2"
+ >>> browser.getControl('Name').value = "import2"
>>> browser.getControl('CVS').click()
>>> browser.getControl('Repository').value = (
... ":pserver:anonymous@xxxxxxxxxxxxxxx:/mozilla/cvs")
@@ -233,7 +265,7 @@
>>> browser.open("http://code.launchpad.dev/+code-imports/+new";)
>>> browser.getControl('Project').value = "firefox"
- >>> browser.getControl('Branch Name').value = "import2"
+ >>> browser.getControl('Name').value = "import2"
>>> browser.getControl('CVS').click()
>>> browser.getControl('Repository').value = (
... ":anonymous@xxxxxxxxxxxxxxx:/mozilla/cvs")
@@ -256,7 +288,7 @@
The error is shown even if the project is different.
>>> browser.getControl('Project').value = "thunderbird"
- >>> browser.getControl('Branch Name').value = "imported"
+ >>> browser.getControl('Name').value = "imported"
>>> browser.getControl('CVS').click()
>>> browser.getControl('Repository').value = (
... ":pserver:anonymous@xxxxxxxxxxxxxxx:/mozilla/cvs")
@@ -290,7 +322,7 @@
>>> browser.open("http://code.launchpad.dev/+code-imports/+new";)
>>> browser.getControl('Subversion').click()
>>> browser.getControl('Project').value = "firefox"
- >>> browser.getControl('Branch Name').value = "imported"
+ >>> browser.getControl('Name').value = "imported"
>>> browser.getControl('Branch URL', index=1).value = (
... "http://svn.example.com/firefox/other";)
>>> browser.getControl('Request Import').click()
@@ -307,7 +339,7 @@
>>> browser.open("http://code.launchpad.dev/+code-imports/+new";)
>>> browser.getControl('Project').value = "launchpad"
- >>> browser.getControl('Branch Name').value = "imported"
+ >>> browser.getControl('Name').value = "imported"
>>> browser.getControl('Branch URL', index=0).value = (
... "http://svn.example.com/launchpage/fake";)
>>> browser.getControl('Request Import').click()
@@ -324,7 +356,7 @@
>>> browser.open("http://code.launchpad.dev/+code-imports/+new";)
>>> browser.getControl('Project').value = "no-such-product"
- >>> browser.getControl('Branch Name').value = "imported"
+ >>> browser.getControl('Name').value = "imported"
>>> browser.getControl('Branch URL', index=0).value = (
... "http://svn.example.com/launchpage/fake";)
>>> browser.getControl('Request Import').click()
=== modified file 'lib/lp/code/templates/codeimport-new.pt'
--- lib/lp/code/templates/codeimport-new.pt 2012-10-09 01:07:52 +0000
+++ lib/lp/code/templates/codeimport-new.pt 2016-10-14 17:29:44 +0000
@@ -45,7 +45,7 @@
<tr>
<td colspan="2">
<div class="formHelp">
- Enter details for the selected repository type.
+ Enter details for the selected version control system.
</div>
</td>
</tr>
@@ -74,6 +74,10 @@
<tal:widget define="widget nocall:view/widgets/git_repo_url">
<metal:block use-macro="context/@@launchpad_form/widget_row" />
</tal:widget>
+ <tal:widget define="widget nocall:view/widgets/git_target_rcs_type"
+ condition="request/features/code.import.git_target">
+ <metal:block use-macro="context/@@launchpad_form/widget_row" />
+ </tal:widget>
</table>
</td>
</tr>
@@ -131,6 +135,12 @@
}
}
updateField(form['field.git_repo_url'], rcs_type === 'GIT');
+ for (i = 0; i < form.elements.length; i++) {
+ if (form.elements[i].id.startsWith(
+ 'field.git_target_rcs_type.')) {
+ updateField(form.elements[i], rcs_type === 'GIT');
+ }
+ }
updateField(form['field.cvs_root'], rcs_type === 'CVS');
updateField(form['field.cvs_module'], rcs_type === 'CVS');
updateField(form['field.svn_branch_url'], rcs_type === 'BZR_SVN');
Follow ups
