launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #32985
[Merge] ~finnrg/launchpad:refactor/remove-bzr-codeimports into launchpad:master
Finn Gärtner has proposed merging ~finnrg/launchpad:refactor/remove-bzr-codeimports into launchpad:master.
Commit message:
refactor: Disable creating code imports to and from Bazaar
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~finnrg/launchpad/+git/launchpad/+merge/492719
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~finnrg/launchpad:refactor/remove-bzr-codeimports into launchpad:master.
diff --git a/lib/lp/code/browser/codeimport.py b/lib/lp/code/browser/codeimport.py
index 9ea3dcf..60175d1 100644
--- a/lib/lp/code/browser/codeimport.py
+++ b/lib/lp/code/browser/codeimport.py
@@ -57,10 +57,6 @@ from lp.code.errors import (
GitRepositoryExists,
)
from lp.code.interfaces.branch import IBranch, user_has_special_branch_access
-from lp.code.interfaces.branchnamespace import (
- IBranchNamespacePolicy,
- get_branch_namespace,
-)
from lp.code.interfaces.codeimport import ICodeImport, ICodeImportSet
from lp.code.interfaces.codeimportmachine import ICodeImportMachineSet
from lp.code.interfaces.gitnamespace import (
@@ -267,23 +263,7 @@ class NewCodeImportForm(Interface):
"""The fields presented on the form for editing a code import."""
use_template(IBranch, ["owner"])
- use_template(ICodeImport, ["rcs_type", "cvs_root", "cvs_module"])
-
- svn_branch_url = URIField(
- title=_("Branch URL"),
- required=False,
- description=_(
- "The URL of a Subversion branch, starting with svn:// or "
- "http(s)://. You can include a username and password as part "
- "of the url, but this will be displayed on the branch page."
- ),
- allowed_schemes=["http", "https", "svn"],
- allow_userinfo=True,
- allow_port=True,
- allow_query=False,
- allow_fragment=False,
- trailing_slash=False,
- )
+ use_template(ICodeImport, ["rcs_type"])
git_repo_url = URIField(
title=_("Repo URL"),
@@ -302,28 +282,6 @@ class NewCodeImportForm(Interface):
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."),
- allowed_schemes=["http", "https", "bzr", "ftp"],
- allow_userinfo=True,
- allow_port=True,
- allow_query=False, # Query makes no sense in Bazaar
- allow_fragment=False, # Fragment makes no sense in Bazaar
- trailing_slash=False,
- )
-
branch_name = copy_field(
IBranch["name"],
__name__="branch_name",
@@ -349,15 +307,13 @@ class CodeImportNewView(CodeImportBaseView, CodeImportNameValidationMixin):
for_input = True
custom_widget_rcs_type = LaunchpadRadioWidget
- custom_widget_git_target_rcs_type = LaunchpadRadioWidget
@property
def initial_values(self):
return {
"owner": self.user,
- "rcs_type": RevisionControlSystems.BZR,
+ "rcs_type": RevisionControlSystems.GIT,
"branch_name": "trunk",
- "git_target_rcs_type": TargetRevisionControlSystems.BZR,
}
@property
@@ -409,36 +365,19 @@ class CodeImportNewView(CodeImportBaseView, CodeImportNameValidationMixin):
# display them separately in the form.
soup = BeautifulSoup(self.widgets["rcs_type"]())
fields = soup.find_all("input")
- [cvs_button, svn_button, git_button, bzr_button, empty_marker] = [
- field
- for field in fields
- if field.get("value") in ["CVS", "BZR_SVN", "GIT", "BZR", "1"]
+ [git_button, empty_marker] = [
+ field for field in fields if field.get("value") in ["GIT", "1"]
]
- bzr_button["onclick"] = "updateWidgets()"
- cvs_button["onclick"] = "updateWidgets()"
- svn_button["onclick"] = "updateWidgets()"
git_button["onclick"] = "updateWidgets()"
# The following attributes are used only in the page template.
- self.rcs_type_cvs = str(cvs_button)
- self.rcs_type_svn = str(svn_button)
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."""
rcs_type = data["rcs_type"]
- if rcs_type == RevisionControlSystems.CVS:
- return data.get("cvs_root"), data.get("cvs_module"), None
- elif rcs_type == RevisionControlSystems.BZR_SVN:
- return None, None, data.get("svn_branch_url")
- elif rcs_type == RevisionControlSystems.GIT:
- return None, None, data.get("git_repo_url")
- elif rcs_type == RevisionControlSystems.BZR:
- return None, None, data.get("bzr_branch_url")
+ if rcs_type == RevisionControlSystems.GIT:
+ return data.get("git_repo_url")
else:
raise AssertionError(
"Unexpected revision control type %r." % rcs_type
@@ -447,23 +386,19 @@ class CodeImportNewView(CodeImportBaseView, CodeImportNameValidationMixin):
def _create_import(self, data, status):
"""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
+ url = self._getImportLocation(data)
+ assert data["rcs_type"] == RevisionControlSystems.GIT
+
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=target_rcs_type,
+ target_rcs_type=TargetRevisionControlSystems.GIT,
url=url,
- cvs_root=cvs_root,
- cvs_module=cvs_module,
+ cvs_root=None,
+ cvs_module=None,
review_status=status,
)
@@ -504,20 +439,12 @@ class CodeImportNewView(CodeImportBaseView, CodeImportNameValidationMixin):
def validate_widgets(self, data, names=None):
"""See `LaunchpadFormView`."""
- self.widgets["git_target_rcs_type"].context.required = (
- data.get("rcs_type") == RevisionControlSystems.GIT
- )
super().validate_widgets(data, names=names)
def validate(self, data):
"""See `LaunchpadFormView`."""
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
+ assert rcs_type == RevisionControlSystems.GIT
# Make sure that the user is able to create branches/repositories
# for the specified namespace.
@@ -525,14 +452,9 @@ class CodeImportNewView(CodeImportBaseView, CodeImportNameValidationMixin):
# 'owner' in data may be None if it failed validation.
owner = data.get("owner")
if product is not None and owner is not None:
- 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)
+ namespace = get_git_namespace(product, owner)
+ policy = IGitNamespacePolicy(namespace)
+ can_create = policy.canCreateRepositories(self.user)
if not can_create:
self.setFieldError(
"product",
@@ -542,29 +464,13 @@ class CodeImportNewView(CodeImportBaseView, CodeImportNameValidationMixin):
# Make sure fields for unselected revision control systems
# are blanked out:
- if rcs_type == RevisionControlSystems.CVS:
- self._validateCVS(data.get("cvs_root"), data.get("cvs_module"))
- elif rcs_type == RevisionControlSystems.BZR_SVN:
- self._validateURL(
- data.get("svn_branch_url"),
- rcs_type,
- target_rcs_type,
- field_name="svn_branch_url",
- )
- elif rcs_type == RevisionControlSystems.GIT:
+ if rcs_type == RevisionControlSystems.GIT:
self._validateURL(
data.get("git_repo_url"),
rcs_type,
- target_rcs_type,
+ target_rcs_type=TargetRevisionControlSystems.GIT,
field_name="git_repo_url",
)
- elif rcs_type == RevisionControlSystems.BZR:
- self._validateURL(
- data.get("bzr_branch_url"),
- rcs_type,
- target_rcs_type,
- field_name="bzr_branch_url",
- )
else:
raise AssertionError(
"Unexpected revision control type %r." % rcs_type
diff --git a/lib/lp/code/browser/tests/test_codeimport.py b/lib/lp/code/browser/tests/test_codeimport.py
index 25bc91c..0e7fe85 100644
--- a/lib/lp/code/browser/tests/test_codeimport.py
+++ b/lib/lp/code/browser/tests/test_codeimport.py
@@ -46,19 +46,6 @@ class TestImportDetails(TestCaseWithFactory):
text = re.sub(r"\s+", " ", extract_text(details))
self.assertThat(text, StartsWith(prefix_text))
- def test_bzr_svn_import(self):
- # The branch page for a bzr-svn-imported branch contains a summary
- # of the import details.
- code_import = self.factory.makeCodeImport(
- rcs_type=RevisionControlSystems.BZR_SVN
- )
- self.assertImportDetailsDisplayed(
- code_import.target,
- "svn-import-details",
- "This branch is an import of the Subversion branch",
- span_title=RevisionControlSystems.BZR_SVN.title,
- )
-
def test_git_to_git_import(self):
# The repository page for a git-to-git-imported repository contains
# a summary of the import details.
diff --git a/lib/lp/code/mail/tests/test_codeimport.py b/lib/lp/code/mail/tests/test_codeimport.py
index d3d4481..5e9e55e 100644
--- a/lib/lp/code/mail/tests/test_codeimport.py
+++ b/lib/lp/code/mail/tests/test_codeimport.py
@@ -8,11 +8,7 @@ import textwrap
import transaction
-from lp.code.enums import (
- CodeImportReviewStatus,
- RevisionControlSystems,
- TargetRevisionControlSystems,
-)
+from lp.code.enums import CodeImportReviewStatus, TargetRevisionControlSystems
from lp.code.tests.helpers import GitHostingFixture
from lp.services.mail import stub
from lp.testing import TestCaseWithFactory, login_celebrity, login_person
@@ -24,90 +20,6 @@ class TestNewCodeImports(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
- def test_cvs_to_bzr_import(self):
- # Test the email for a new CVS-to-Bazaar import.
- eric = self.factory.makePerson(name="eric")
- fooix = self.factory.makeProduct(name="fooix")
- # Eric needs to be logged in for the mail to be sent.
- login_person(eric)
- self.factory.makeProductCodeImport(
- cvs_root=":pserver:anon@xxxxxxxxxxxxxxx:/cvsroot",
- cvs_module="a_module",
- branch_name="import",
- product=fooix,
- registrant=eric,
- )
- transaction.commit()
- msg = email.message_from_bytes(stub.test_emails[0][2])
- self.assertEqual("code-import", msg["X-Launchpad-Notification-Type"])
- self.assertEqual("~eric/fooix/import", msg["X-Launchpad-Branch"])
- self.assertEqual(
- "A new CVS code import has been requested by Eric:\n"
- " http://code.launchpad.test/~eric/fooix/import\n";
- "from\n"
- " :pserver:anon@xxxxxxxxxxxxxxx:/cvsroot, a_module\n"
- "\n"
- "-- \nYou are getting this email because you are a member of the "
- "vcs-imports team.\n",
- msg.get_payload(decode=True).decode(),
- )
-
- def test_svn_to_bzr_import(self):
- # Test the email for a new Subversion-to-Bazaar import.
- eric = self.factory.makePerson(name="eric")
- fooix = self.factory.makeProduct(name="fooix")
- # Eric needs to be logged in for the mail to be sent.
- login_person(eric)
- self.factory.makeProductCodeImport(
- svn_branch_url="svn://svn.example.com/fooix/trunk",
- branch_name="trunk",
- product=fooix,
- registrant=eric,
- rcs_type=RevisionControlSystems.BZR_SVN,
- )
- transaction.commit()
- msg = email.message_from_bytes(stub.test_emails[0][2])
- self.assertEqual("code-import", msg["X-Launchpad-Notification-Type"])
- self.assertEqual("~eric/fooix/trunk", msg["X-Launchpad-Branch"])
- self.assertEqual(
- "A new subversion code import has been requested by Eric:\n"
- " http://code.launchpad.test/~eric/fooix/trunk\n";
- "from\n"
- " svn://svn.example.com/fooix/trunk\n"
- "\n"
- "-- \nYou are getting this email because you are a member of the "
- "vcs-imports team.\n",
- msg.get_payload(decode=True).decode(),
- )
-
- def test_git_to_bzr_import(self):
- # Test the email for a new git-to-Bazaar import.
- eric = self.factory.makePerson(name="eric")
- fooix = self.factory.makeProduct(name="fooix")
- # Eric needs to be logged in for the mail to be sent.
- login_person(eric)
- self.factory.makeProductCodeImport(
- git_repo_url="git://git.example.com/fooix.git",
- branch_name="master",
- product=fooix,
- registrant=eric,
- )
- transaction.commit()
- msg = email.message_from_bytes(stub.test_emails[0][2])
- self.assertEqual("code-import", msg["X-Launchpad-Notification-Type"])
- self.assertEqual("~eric/fooix/master", msg["X-Launchpad-Branch"])
- self.assertEqual(
- "A new git code import has been requested "
- "by Eric:\n"
- " http://code.launchpad.test/~eric/fooix/master\n";
- "from\n"
- " git://git.example.com/fooix.git\n"
- "\n"
- "-- \nYou are getting this email because you are a member of the "
- "vcs-imports team.\n",
- msg.get_payload(decode=True).decode(),
- )
-
def test_git_to_git_import(self):
# Test the email for a new git-to-git import.
self.useFixture(GitHostingFixture())
@@ -138,43 +50,6 @@ class TestNewCodeImports(TestCaseWithFactory):
msg.get_payload(decode=True).decode(),
)
- def test_new_source_package_import(self):
- # Test the email for a new sourcepackage import.
- eric = self.factory.makePerson(name="eric")
- distro = self.factory.makeDistribution(name="foobuntu")
- series = self.factory.makeDistroSeries(
- name="manic", distribution=distro
- )
- fooix = self.factory.makeSourcePackage(
- sourcepackagename="fooix", distroseries=series
- )
- # Eric needs to be logged in for the mail to be sent.
- login_person(eric)
- self.factory.makePackageCodeImport(
- git_repo_url="git://git.example.com/fooix.git",
- branch_name="master",
- sourcepackage=fooix,
- registrant=eric,
- )
- transaction.commit()
- msg = email.message_from_bytes(stub.test_emails[0][2])
- self.assertEqual("code-import", msg["X-Launchpad-Notification-Type"])
- self.assertEqual(
- "~eric/foobuntu/manic/fooix/master", msg["X-Launchpad-Branch"]
- )
- self.assertEqual(
- "A new git code import has been requested "
- "by Eric:\n"
- " http://code.launchpad.test/";
- "~eric/foobuntu/manic/fooix/master\n"
- "from\n"
- " git://git.example.com/fooix.git\n"
- "\n"
- "-- \nYou are getting this email because you are a member of the "
- "vcs-imports team.\n",
- msg.get_payload(decode=True).decode(),
- )
-
class TestUpdatedCodeImports(TestCaseWithFactory):
"""Test the emails sent out for updated code imports."""
@@ -231,97 +106,6 @@ class TestUpdatedCodeImports(TestCaseWithFactory):
msg.get_payload(decode=True).decode(),
)
- def test_cvs_to_bzr_import_same_details(self):
- code_import = self.factory.makeProductCodeImport(
- cvs_root=":pserver:anon@xxxxxxxxxxxxxxx:/cvsroot",
- cvs_module="a_module",
- )
- unique_name = code_import.target.unique_name
- user = login_celebrity("vcs_imports")
- code_import.updateFromData(
- {"review_status": CodeImportReviewStatus.FAILING}, user
- )
- transaction.commit()
- self.assertSameDetailsEmail(
- "a_module from :pserver:anon@xxxxxxxxxxxxxxx:/cvsroot",
- unique_name,
- )
-
- def test_cvs_to_bzr_import_different_details(self):
- code_import = self.factory.makeProductCodeImport(
- cvs_root=":pserver:anon@xxxxxxxxxxxxxxx:/cvsroot",
- cvs_module="a_module",
- )
- unique_name = code_import.target.unique_name
- user = login_celebrity("vcs_imports")
- code_import.updateFromData({"cvs_module": "another_module"}, user)
- transaction.commit()
- self.assertDifferentDetailsEmail(
- "a_module from :pserver:anon@xxxxxxxxxxxxxxx:/cvsroot",
- "another_module from :pserver:anon@xxxxxxxxxxxxxxx:/cvsroot",
- unique_name,
- )
-
- def test_svn_to_bzr_import_same_details(self):
- code_import = self.factory.makeProductCodeImport(
- svn_branch_url="svn://svn.example.com/fooix/trunk"
- )
- unique_name = code_import.target.unique_name
- user = login_celebrity("vcs_imports")
- code_import.updateFromData(
- {"review_status": CodeImportReviewStatus.FAILING}, user
- )
- transaction.commit()
- self.assertSameDetailsEmail(
- "svn://svn.example.com/fooix/trunk", unique_name
- )
-
- def test_svn_to_bzr_import_different_details(self):
- code_import = self.factory.makeProductCodeImport(
- svn_branch_url="svn://svn.example.com/fooix/trunk"
- )
- unique_name = code_import.target.unique_name
- user = login_celebrity("vcs_imports")
- code_import.updateFromData(
- {"url": "https://svn.example.com/fooix/trunk"}, user
- )
- transaction.commit()
- self.assertDifferentDetailsEmail(
- "svn://svn.example.com/fooix/trunk",
- "https://svn.example.com/fooix/trunk";,
- unique_name,
- )
-
- def test_git_to_bzr_import_same_details(self):
- code_import = self.factory.makeProductCodeImport(
- git_repo_url="git://git.example.com/fooix.git"
- )
- unique_name = code_import.target.unique_name
- user = login_celebrity("vcs_imports")
- code_import.updateFromData(
- {"review_status": CodeImportReviewStatus.FAILING}, user
- )
- transaction.commit()
- self.assertSameDetailsEmail(
- "git://git.example.com/fooix.git", unique_name
- )
-
- def test_git_to_bzr_import_different_details(self):
- code_import = self.factory.makeProductCodeImport(
- git_repo_url="git://git.example.com/fooix.git"
- )
- unique_name = code_import.target.unique_name
- user = login_celebrity("vcs_imports")
- code_import.updateFromData(
- {"url": "https://git.example.com/fooix.git"}, user
- )
- transaction.commit()
- self.assertDifferentDetailsEmail(
- "git://git.example.com/fooix.git",
- "https://git.example.com/fooix.git";,
- unique_name,
- )
-
def test_git_to_git_import_same_details(self):
self.useFixture(GitHostingFixture())
code_import = self.factory.makeProductCodeImport(
diff --git a/lib/lp/code/model/codeimport.py b/lib/lp/code/model/codeimport.py
index 5ba1ed7..9dea434 100644
--- a/lib/lp/code/model/codeimport.py
+++ b/lib/lp/code/model/codeimport.py
@@ -28,8 +28,6 @@ from zope.security.proxy import removeSecurityProxy
from lp.app.errors import NotFoundError
from lp.code.enums import (
- NON_CVS_RCS_TYPES,
- BranchType,
CodeImportJobState,
CodeImportResultStatus,
CodeImportReviewStatus,
@@ -44,7 +42,6 @@ from lp.code.errors import (
CodeImportNotInReviewedState,
)
from lp.code.interfaces.branch import IBranch
-from lp.code.interfaces.branchtarget import IBranchTarget
from lp.code.interfaces.codeimport import ICodeImport, ICodeImportSet
from lp.code.interfaces.codeimportevent import ICodeImportEventSet
from lp.code.interfaces.codeimportjob import ICodeImportJobWorkflow
@@ -338,61 +335,35 @@ class CodeImportSet:
owner=None,
):
"""See `ICodeImportSet`."""
- if rcs_type == RevisionControlSystems.CVS:
- assert cvs_root is not None and cvs_module is not None
- assert url is None
- elif rcs_type in NON_CVS_RCS_TYPES:
- assert cvs_root is None and cvs_module is None
- assert url is not None
- else:
- raise AssertionError(
- "Don't know how to sanity check source details for unknown "
- "rcs_type %s" % rcs_type
- )
+ if (
+ rcs_type != RevisionControlSystems.GIT
+ and target_rcs_type != TargetRevisionControlSystems.GIT
+ ):
+ raise AssertionError("Unsupported rcs system")
+ assert cvs_root is None and cvs_module is None
+ assert url is not None
if owner is None:
owner = registrant
- if target_rcs_type == TargetRevisionControlSystems.BZR:
- # XXX cjwatson 2016-10-15: Testing
- # IHasBranches.providedBy(context) would seem more in line with
- # the Git case, but for some reason ProductSeries doesn't
- # provide that. We should sync this up somehow.
- try:
- target = IBranchTarget(context)
- except TypeError:
- raise CodeImportInvalidTargetType(context, target_rcs_type)
- namespace = target.getNamespace(owner)
- elif target_rcs_type == TargetRevisionControlSystems.GIT:
- if not IHasGitRepositories.providedBy(context):
- raise CodeImportInvalidTargetType(context, target_rcs_type)
- if rcs_type != RevisionControlSystems.GIT:
- raise AssertionError(
- "Can't import rcs_type %s into a Git repository" % rcs_type
- )
- target = namespace = get_git_namespace(context, owner)
- else:
+ if not IHasGitRepositories.providedBy(context):
+ raise CodeImportInvalidTargetType(context, target_rcs_type)
+ if rcs_type != RevisionControlSystems.GIT:
raise AssertionError(
- "Can't import to target_rcs_type %s" % target_rcs_type
+ "Can't import rcs_type %s into a Git repository" % rcs_type
)
+ target = namespace = get_git_namespace(context, owner)
if review_status is None:
# Auto approve imports.
review_status = CodeImportReviewStatus.REVIEWED
if not target.supports_code_imports:
raise AssertionError("%r doesn't support code imports" % target)
# Create the branch for the CodeImport.
- if target_rcs_type == TargetRevisionControlSystems.BZR:
- import_target = namespace.createBranch(
- branch_type=BranchType.IMPORTED,
- name=branch_name,
- registrant=registrant,
- )
- else:
- import_target = namespace.createRepository(
- repository_type=GitRepositoryType.IMPORTED,
- name=branch_name,
- registrant=registrant,
- )
- hosting_path = import_target.getInternalPath()
- getUtility(IGitHostingClient).create(hosting_path)
+ import_target = namespace.createRepository(
+ repository_type=GitRepositoryType.IMPORTED,
+ name=branch_name,
+ registrant=registrant,
+ )
+ hosting_path = import_target.getInternalPath()
+ getUtility(IGitHostingClient).create(hosting_path)
code_import = CodeImport(
registrant=registrant,
diff --git a/lib/lp/code/model/tests/test_codeimportjob.py b/lib/lp/code/model/tests/test_codeimportjob.py
index b88a17e..7fff06e 100644
--- a/lib/lp/code/model/tests/test_codeimportjob.py
+++ b/lib/lp/code/model/tests/test_codeimportjob.py
@@ -17,7 +17,6 @@ from zope.component import getUtility
from zope.publisher.xmlrpc import TestRequest
from zope.security.proxy import removeSecurityProxy
-from lp.app.enums import InformationType
from lp.code.enums import (
CodeImportEventType,
CodeImportJobState,
@@ -26,7 +25,6 @@ from lp.code.enums import (
GitRepositoryType,
TargetRevisionControlSystems,
)
-from lp.code.interfaces.codehosting import branch_id_alias, compose_public_url
from lp.code.interfaces.codeimport import ICodeImportSet
from lp.code.interfaces.codeimportevent import ICodeImportEventSet
from lp.code.interfaces.codeimportjob import (
@@ -48,10 +46,7 @@ from lp.services.database.interfaces import IStore
from lp.services.database.sqlbase import get_transaction_timestamp
from lp.services.librarian.interfaces import ILibraryFileAliasSet
from lp.services.librarian.interfaces.client import ILibrarianClient
-from lp.services.macaroons.interfaces import (
- BadMacaroonContext,
- IMacaroonIssuer,
-)
+from lp.services.macaroons.interfaces import IMacaroonIssuer
from lp.services.macaroons.testing import MacaroonTestMixin, MacaroonVerifies
from lp.services.webapp import canonical_url
from lp.testing import (
@@ -90,41 +85,8 @@ class TestCodeImportJob(TestCaseWithFactory):
if start_job:
machine = self.factory.makeCodeImportMachine(set_online=True)
getUtility(ICodeImportJobWorkflow).startJob(job, machine)
- self.assertThat(job.makeWorkerArguments(), matcher)
-
- def test_bzr_arguments(self):
- code_import = self.factory.makeCodeImport(
- bzr_branch_url="http://example.com/foo";
- )
- self.assertArgumentsMatch(
- code_import,
- Equals(
- [
- str(code_import.branch.id),
- "bzr",
- "bzr",
- "http://example.com/foo";,
- "--exclude-host",
- "launchpad.test",
- ]
- ),
- )
-
- def test_git_arguments(self):
- code_import = self.factory.makeCodeImport(
- git_repo_url="git://git.example.com/project.git"
- )
- self.assertArgumentsMatch(
- code_import,
- Equals(
- [
- str(code_import.branch.id),
- "git",
- "bzr",
- "git://git.example.com/project.git",
- ]
- ),
- )
+ args = job.makeWorkerArguments()
+ self.assertThat(args, matcher)
def test_git_to_git_arguments(self):
self.pushConfig(
@@ -155,91 +117,13 @@ class TestCodeImportJob(TestCaseWithFactory):
start_job=True,
)
- def test_cvs_arguments(self):
- code_import = self.factory.makeCodeImport(
- cvs_root=":pserver:foo@xxxxxxxxxxx/bar", cvs_module="bar"
- )
- self.assertArgumentsMatch(
- code_import,
- Equals(
- [
- str(code_import.branch.id),
- "cvs",
- "bzr",
- ":pserver:foo@xxxxxxxxxxx/bar",
- "--cvs-module",
- "bar",
- ]
- ),
- )
-
- def test_bzr_svn_arguments(self):
- code_import = self.factory.makeCodeImport(
- svn_branch_url="svn://svn.example.com/trunk"
- )
- self.assertArgumentsMatch(
- code_import,
- Equals(
- [
- str(code_import.branch.id),
- "bzr-svn",
- "bzr",
- "svn://svn.example.com/trunk",
- ]
- ),
- )
-
- def test_bzr_stacked(self):
- devfocus = self.factory.makeAnyBranch()
- code_import = self.factory.makeCodeImport(
- bzr_branch_url="bzr://bzr.example.com/foo",
- context=devfocus.target.context,
- )
- code_import.branch.stacked_on = devfocus
- self.assertArgumentsMatch(
- code_import,
- Equals(
- [
- str(code_import.branch.id),
- "bzr",
- "bzr",
- "bzr://bzr.example.com/foo",
- "--stacked-on",
- compose_public_url("http", branch_id_alias(devfocus)),
- "--exclude-host",
- "launchpad.test",
- ]
- ),
- )
-
- def test_bzr_stacked_private(self):
- # Code imports can't be stacked on private branches.
- devfocus = self.factory.makeAnyBranch(
- information_type=InformationType.USERDATA
- )
- code_import = self.factory.makeCodeImport(
- context=removeSecurityProxy(devfocus).target.context,
- bzr_branch_url="bzr://bzr.example.com/foo",
- )
- branch = removeSecurityProxy(code_import.branch)
- branch.stacked_on = devfocus
- self.assertArgumentsMatch(
- code_import,
- Equals(
- [
- str(branch.id),
- "bzr",
- "bzr",
- "bzr://bzr.example.com/foo",
- "--exclude-host",
- "launchpad.test",
- ]
- ),
- )
-
def test_blacklisted_hostnames(self):
# Additional blacklisted hostnames are passed as --exclude-host
# options.
+ self.useFixture(GitHostingFixture())
+ self.pushConfig(
+ "launchpad", internal_macaroon_secret_key="some-secret"
+ )
self.pushConfig(
"codehosting", blacklisted_hostnames="localhost,127.0.0.1"
)
@@ -248,18 +132,26 @@ class TestCodeImportJob(TestCaseWithFactory):
)
self.assertArgumentsMatch(
code_import,
- Equals(
+ MatchesListwise(
[
- str(code_import.branch.id),
- "git",
- "bzr",
- "git://git.example.com/project.git",
- "--exclude-host",
- "localhost",
- "--exclude-host",
- "127.0.0.1",
+ Equals(str(code_import.git_repository.unique_name)),
+ Equals("git"),
+ Equals("git"),
+ Equals("git://git.example.com/project.git"),
+ Equals("--macaroon"),
+ MacaroonVerifies(
+ "code-import-job", code_import.import_job
+ ),
+ Equals("--exclude-host"),
+ Equals("launchpad.test"),
+ Equals("--exclude-host"),
+ Equals("localhost"),
+ Equals("--exclude-host"),
+ Equals("127.0.0.1"),
]
),
+ # Start the job so that the macaroon can be verified.
+ start_job=True,
)
@@ -271,6 +163,7 @@ class TestCodeImportJobSet(TestCaseWithFactory):
def setUp(self):
super().setUp()
login_for_code_imports()
+ self.useFixture(GitHostingFixture())
def test_getByIdExisting(self):
# CodeImportJobSet.getById retrieves a CodeImportJob by database id.
@@ -322,6 +215,7 @@ class TestCodeImportJobSetGetJobForMachine(TestCaseWithFactory):
for job in IStore(CodeImportJob).find(CodeImportJob):
job.destroySelf()
self.machine = self.factory.makeCodeImportMachine(set_online=True)
+ self.useFixture(GitHostingFixture())
def makeJob(self, state, date_due_delta, requesting_user=None):
"""Create a CodeImportJob object from a spec."""
@@ -435,6 +329,7 @@ class ReclaimableJobTests(TestCaseWithFactory):
login_for_code_imports()
for job in IStore(CodeImportJob).find(CodeImportJob):
job.destroySelf()
+ self.useFixture(GitHostingFixture())
def makeJobWithHeartbeatInPast(self, seconds_in_past):
code_import = make_running_import(factory=self.factory)
@@ -592,6 +487,7 @@ class TestCodeImportJobWorkflowNewJob(TestCaseWithFactory, AssertFailureMixin):
def setUp(self):
super().setUp()
login_for_code_imports()
+ self.useFixture(GitHostingFixture())
def test_wrongReviewStatus(self):
# CodeImportJobWorkflow.newJob fails if the CodeImport review_status
@@ -599,7 +495,7 @@ class TestCodeImportJobWorkflowNewJob(TestCaseWithFactory, AssertFailureMixin):
new_import = self.factory.makeCodeImport(
review_status=CodeImportReviewStatus.SUSPENDED
)
- branch_name = new_import.branch.unique_name
+ branch_name = new_import.git_repository.unique_name
# Testing newJob failure.
self.assertFailure(
"Review status of %s is not REVIEWED: SUSPENDED" % (branch_name,),
@@ -612,7 +508,7 @@ class TestCodeImportJobWorkflowNewJob(TestCaseWithFactory, AssertFailureMixin):
# associated to a CodeImportJob.
job = self.factory.makeCodeImportJob()
reviewed_import = job.code_import
- branch_name = reviewed_import.branch.unique_name
+ branch_name = reviewed_import.git_repository.unique_name
self.assertFailure(
"Already associated to a CodeImportJob: %s" % (branch_name,),
getUtility(ICodeImportJobWorkflow).newJob,
@@ -720,6 +616,7 @@ class TestCodeImportJobWorkflowDeletePendingJob(
def setUp(self):
super().setUp()
self.import_admin = login_for_code_imports()
+ self.useFixture(GitHostingFixture())
def test_wrongReviewStatus(self):
# CodeImportJobWorkflow.deletePendingJob fails if the
@@ -729,7 +626,7 @@ class TestCodeImportJobWorkflowDeletePendingJob(
{"review_status": CodeImportReviewStatus.REVIEWED},
self.import_admin,
)
- branch_name = reviewed_import.branch.unique_name
+ branch_name = reviewed_import.git_repository.unique_name
# Testing deletePendingJob failure.
self.assertFailure(
"The review status of %s is REVIEWED." % (branch_name,),
@@ -743,7 +640,7 @@ class TestCodeImportJobWorkflowDeletePendingJob(
new_import = self.factory.makeCodeImport(
review_status=CodeImportReviewStatus.NEW
)
- branch_name = new_import.branch.unique_name
+ branch_name = new_import.git_repository.unique_name
# Testing deletePendingJob failure.
self.assertFailure(
"Not associated to a CodeImportJob: %s" % (branch_name,),
@@ -756,7 +653,7 @@ class TestCodeImportJobWorkflowDeletePendingJob(
# the CodeImportJob is different from PENDING.
job = self.factory.makeCodeImportJob()
code_import = job.code_import
- branch_name = job.code_import.branch.unique_name
+ branch_name = job.code_import.git_repository.unique_name
# ICodeImport does not allow setting 'review_status', so we must use
# removeSecurityProxy.
INVALID = CodeImportReviewStatus.INVALID
@@ -783,6 +680,7 @@ class TestCodeImportJobWorkflowRequestJob(
def setUp(self):
super().setUp()
login_for_code_imports()
+ self.useFixture(GitHostingFixture())
def test_wrongJobState(self):
# CodeImportJobWorkflow.requestJob fails if the state of the
@@ -800,7 +698,7 @@ class TestCodeImportJobWorkflowRequestJob(
removeSecurityProxy(import_job).state = CodeImportJobState.RUNNING
self.assertFailure(
"The CodeImportJob associated with %s is "
- "RUNNING." % code_import.branch.unique_name,
+ "RUNNING." % code_import.git_repository.unique_name,
getUtility(ICodeImportJobWorkflow).requestJob,
import_job,
person,
@@ -818,7 +716,7 @@ class TestCodeImportJobWorkflowRequestJob(
removeSecurityProxy(import_job).requesting_user = person
self.assertFailure(
"The CodeImportJob associated with %s was already requested by "
- "%s." % (code_import.branch.unique_name, person.name),
+ "%s." % (code_import.git_repository.unique_name, person.name),
getUtility(ICodeImportJobWorkflow).requestJob,
import_job,
other_person,
@@ -884,6 +782,7 @@ class TestCodeImportJobWorkflowStartJob(
def setUp(self):
super().setUp()
login_for_code_imports()
+ self.useFixture(GitHostingFixture())
def test_wrongJobState(self):
# Calling startJob with a job whose state is not PENDING is an error.
@@ -897,7 +796,7 @@ class TestCodeImportJobWorkflowStartJob(
# Testing startJob failure.
self.assertFailure(
"The CodeImportJob associated with %s is "
- "RUNNING." % code_import.branch.unique_name,
+ "RUNNING." % code_import.git_repository.unique_name,
getUtility(ICodeImportJobWorkflow).requestJob,
job,
machine,
@@ -942,6 +841,7 @@ class TestCodeImportJobWorkflowUpdateHeartbeat(
def setUp(self):
super().setUp()
login_for_code_imports()
+ self.useFixture(GitHostingFixture())
def test_wrongJobState(self):
# Calling updateHeartbeat with a job whose state is not RUNNING is an
@@ -950,7 +850,7 @@ class TestCodeImportJobWorkflowUpdateHeartbeat(
job = self.factory.makeCodeImportJob(code_import)
self.assertFailure(
"The CodeImportJob associated with %s is "
- "PENDING." % code_import.branch.unique_name,
+ "PENDING." % code_import.git_repository.unique_name,
getUtility(ICodeImportJobWorkflow).updateHeartbeat,
job,
"",
@@ -981,6 +881,7 @@ class TestCodeImportJobWorkflowFinishJob(
super().setUp()
self.vcs_imports = login_for_code_imports()
self.machine = self.factory.makeCodeImportMachine(set_online=True)
+ self.useFixture(GitHostingFixture())
def makeRunningJob(self, code_import=None):
"""Make and return a CodeImportJob object with state==RUNNING.
@@ -1003,7 +904,7 @@ class TestCodeImportJobWorkflowFinishJob(
job = self.factory.makeCodeImportJob(code_import)
self.assertFailure(
"The CodeImportJob associated with %s is "
- "PENDING." % code_import.branch.unique_name,
+ "PENDING." % code_import.git_repository.unique_name,
getUtility(ICodeImportJobWorkflow).finishJob,
job,
CodeImportResultStatus.SUCCESS,
@@ -1252,23 +1153,6 @@ class TestCodeImportJobWorkflowFinishJob(
else:
self.assertTrue(code_import.date_last_successful is None)
- def test_successfulResultCallsRequestMirror(self):
- # finishJob() calls requestMirror() on the import branch if and only
- # if the status was success.
- status_jobs = []
- for status in CodeImportResultStatus.items:
- status_jobs.append((status, self.makeRunningJob()))
- for status, job in status_jobs:
- code_import = job.code_import
- self.assertTrue(code_import.date_last_successful is None)
- getUtility(ICodeImportJobWorkflow).finishJob(job, status, None)
- if status == CodeImportResultStatus.SUCCESS:
- self.assertTrue(
- code_import.branch.next_mirror_time is not None
- )
- else:
- self.assertTrue(code_import.branch.next_mirror_time is None)
-
def test_enoughFailuresMarksAsFailing(self):
# If a code import fails config.codeimport.consecutive_failure_limit
# times in a row, the import is marked as FAILING.
@@ -1314,6 +1198,7 @@ class TestCodeImportJobWorkflowReclaimJob(
super().setUp()
login_for_code_imports()
self.machine = self.factory.makeCodeImportMachine(set_online=True)
+ self.useFixture(GitHostingFixture())
def makeRunningJob(self, code_import=None):
"""Make and return a CodeImportJob object with state==RUNNING.
@@ -1374,13 +1259,18 @@ class TestRequestJobUIRaces(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
+ def setUp(self):
+ super().setUp()
+ self.useFixture(GitHostingFixture())
+
@logged_in_for_code_imports
def getNewCodeImportIDAndBranchURL(self):
- """Create a code import and return its ID and the URL of its branch."""
+ """Create a code import and return its ID
+ and the URL of the git repository."""
code_import = make_finished_import(factory=self.factory)
- branch_url = canonical_url(code_import.branch)
+ repository_url = canonical_url(code_import.git_repository)
code_import_id = code_import.id
- return code_import_id, branch_url
+ return code_import_id, repository_url
def requestJobByUserWithDisplayName(self, code_import_id, displayname):
"""Record a request for the job by a user with the given name."""
@@ -1458,19 +1348,10 @@ class TestCodeImportJobMacaroonIssuer(MacaroonTestMixin, TestCaseWithFactory):
)
self.useFixture(GitHostingFixture())
- def makeJob(self, target_rcs_type=TargetRevisionControlSystems.GIT):
- code_import = self.factory.makeCodeImport(
- target_rcs_type=target_rcs_type
- )
+ def makeJob(self):
+ code_import = self.factory.makeCodeImport()
return self.factory.makeCodeImportJob(code_import=code_import)
- def test_issueMacaroon_refuses_branch(self):
- job = self.makeJob(target_rcs_type=TargetRevisionControlSystems.BZR)
- issuer = getUtility(IMacaroonIssuer, "code-import-job")
- self.assertRaises(
- BadMacaroonContext, removeSecurityProxy(issuer).issueMacaroon, job
- )
-
def test_issueMacaroon_good(self):
job = self.makeJob()
issuer = getUtility(IMacaroonIssuer, "code-import-job")
diff --git a/lib/lp/code/templates/codeimport-new.pt b/lib/lp/code/templates/codeimport-new.pt
index ef544dd..4db0594 100644
--- a/lib/lp/code/templates/codeimport-new.pt
+++ b/lib/lp/code/templates/codeimport-new.pt
@@ -55,20 +55,6 @@
<tr>
<td>
<label>
- <input tal:replace="structure view/rcs_type_bzr" />
- Bazaar
- </label>
- <table class="importdetails">
- <tal:widget define="widget nocall:view/widgets/bzr_branch_url">
- <metal:block use-macro="context/@@launchpad_form/widget_row" />
- </tal:widget>
- </table>
- </td>
- </tr>
-
- <tr>
- <td>
- <label>
<input tal:replace="structure view/rcs_type_git" />
Git
</label>
@@ -76,40 +62,6 @@
<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">
- <metal:block use-macro="context/@@launchpad_form/widget_row" />
- </tal:widget>
- </table>
- </td>
- </tr>
-
- <tr>
- <td>
- <label>
- <input tal:replace="structure view/rcs_type_svn" />
- Subversion
- </label>
- <table class="importdetails">
- <tal:widget define="widget nocall:view/widgets/svn_branch_url">
- <metal:block use-macro="context/@@launchpad_form/widget_row" />
- </tal:widget>
- </table>
- </td>
- </tr>
-
- <tr>
- <td>
- <label>
- <input tal:replace="structure view/rcs_type_cvs" />
- CVS
- </label>
- <table class="importdetails">
- <tal:widget define="widget nocall:view/widgets/cvs_root">
- <metal:block use-macro="context/@@launchpad_form/widget_row" />
- </tal:widget>
- <tal:widget define="widget nocall:view/widgets/cvs_module">
- <metal:block use-macro="context/@@launchpad_form/widget_row" />
- </tal:widget>
</table>
</td>
</tr>
@@ -136,12 +88,6 @@
}
}
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');
diff --git a/lib/lp/registry/scripts/tests/test_closeaccount.py b/lib/lp/registry/scripts/tests/test_closeaccount.py
index 13a750c..8d9b37d 100644
--- a/lib/lp/registry/scripts/tests/test_closeaccount.py
+++ b/lib/lp/registry/scripts/tests/test_closeaccount.py
@@ -826,10 +826,7 @@ class TestCloseAccount(TestCaseWithFactory):
self.factory.makeCodeImport(
registrant=person, target_rcs_type=target_rcs_type, owner=team
)
- for target_rcs_type in (
- TargetRevisionControlSystems.BZR,
- TargetRevisionControlSystems.GIT,
- )
+ for target_rcs_type in (TargetRevisionControlSystems.GIT,)
]
person_id = person.id
account_id = person.account.id
@@ -838,41 +835,35 @@ class TestCloseAccount(TestCaseWithFactory):
self.runScript(script)
self.assertRemoved(account_id, person_id)
self.assertEqual(person, code_imports[0].registrant)
- self.assertEqual(person, code_imports[1].registrant)
def test_skips_import_job_requester(self):
self.useFixture(GitHostingFixture())
person = self.factory.makePerson()
team = self.factory.makeTeam(members=[person])
- code_imports = [
- self.factory.makeCodeImport(
- registrant=person, target_rcs_type=target_rcs_type, owner=team
- )
- for target_rcs_type in (
- TargetRevisionControlSystems.BZR,
- TargetRevisionControlSystems.GIT,
- )
- ]
+ code_import = self.factory.makeCodeImport(
+ registrant=person,
+ target_rcs_type=TargetRevisionControlSystems.GIT,
+ owner=team,
+ )
- for code_import in code_imports:
- getUtility(ICodeImportJobWorkflow).requestJob(
- code_import.import_job, person
- )
- self.assertEqual(person, code_import.import_job.requesting_user)
- result = self.factory.makeCodeImportResult(
- code_import=code_import,
- requesting_user=person,
- result_status=CodeImportResultStatus.SUCCESS,
- )
- person_id = person.id
- account_id = person.account.id
- script = self.makeScript([person.name])
- with dbuser("launchpad"):
- self.runScript(script)
- self.assertRemoved(account_id, person_id)
- self.assertEqual(person, code_import.registrant)
- self.assertEqual(person, result.requesting_user)
- self.assertEqual(person, code_import.import_job.requesting_user)
+ getUtility(ICodeImportJobWorkflow).requestJob(
+ code_import.import_job, person
+ )
+ self.assertEqual(person, code_import.import_job.requesting_user)
+ result = self.factory.makeCodeImportResult(
+ code_import=code_import,
+ requesting_user=person,
+ result_status=CodeImportResultStatus.SUCCESS,
+ )
+ person_id = person.id
+ account_id = person.account.id
+ script = self.makeScript([person.name])
+ with dbuser("launchpad"):
+ self.runScript(script)
+ self.assertRemoved(account_id, person_id)
+ self.assertEqual(person, code_import.registrant)
+ self.assertEqual(person, result.requesting_user)
+ self.assertEqual(person, code_import.import_job.requesting_user)
def test_skip_requester_package_diff_job(self):
person = self.factory.makePerson()
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 5c5807f..d9bc633 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -2986,7 +2986,7 @@ class LaunchpadObjectFactory(ObjectFactory):
the source type instead defaults to Git.)
"""
if target_rcs_type is None:
- target_rcs_type = TargetRevisionControlSystems.BZR
+ target_rcs_type = TargetRevisionControlSystems.GIT
if (
svn_branch_url
is cvs_root
Follow ups
