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( + "ad...@canonical.com") + + 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:passw...@bzr.example.com/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:passw...@svn.example.com/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:anonym...@cvs.example.com:/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 = ( ... ":anonym...@cvs.example.com:/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:anonym...@cvs.example.com:/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');
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp