launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #19313
[Merge] lp:~cjwatson/launchpad/snap-canonicalise-git-path into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-canonicalise-git-path into lp:launchpad.
Commit message:
Introduce Snap.git_ref as a combined field for modification purposes.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1476405 in Launchpad itself: "Add support for building snaps"
https://bugs.launchpad.net/launchpad/+bug/1476405
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-canonicalise-git-path/+merge/270543
Introduce Snap.git_ref as a combined field for modification purposes.
This has the effect of validating and canonicalising the git_path database column; of course it's still possible for a branch to be removed after a Snap was created using it, but at least it means that typos in web forms and such are caught. Adding a new GitRefWidget makes the Snap browser code simpler, and opens the way for improved branch picker code later. I had to fix the build behaviour to pass ref.name rather than ref.path, as "git clone -b" doesn't accept fully-qualified ref paths (thanks, git).
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-canonicalise-git-path into lp:launchpad.
=== modified file 'lib/lp/app/templates/launchpad-widget-macros.pt'
--- lib/lp/app/templates/launchpad-widget-macros.pt 2013-04-22 06:20:18 +0000
+++ lib/lp/app/templates/launchpad-widget-macros.pt 2015-09-09 14:30:28 +0000
@@ -29,7 +29,7 @@
tal:define="error widget/error">
<label tal:attributes="for widget/name"
- tal:content="widget/label">Label</label>
+ tal:content="string:${widget/label}:">Label</label>
<span tal:condition="widget/required"
class="fieldRequired"
=== added file 'lib/lp/code/browser/widgets/gitref.py'
--- lib/lp/code/browser/widgets/gitref.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/browser/widgets/gitref.py 2015-09-09 14:30:28 +0000
@@ -0,0 +1,129 @@
+# Copyright 2015 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+__all__ = [
+ 'GitRefWidget',
+ ]
+
+from z3c.ptcompat import ViewPageTemplateFile
+from zope.formlib.interfaces import (
+ ConversionError,
+ IInputWidget,
+ MissingInputError,
+ WidgetInputError,
+ )
+from zope.formlib.utility import setUpWidget
+from zope.formlib.widget import (
+ BrowserWidget,
+ InputErrors,
+ InputWidget,
+ )
+from zope.interface import implementer
+from zope.schema import (
+ Choice,
+ TextLine,
+ )
+
+from lp.app.errors import UnexpectedFormData
+from lp.app.validators import LaunchpadValidationError
+from lp.services.webapp.interfaces import (
+ IAlwaysSubmittedWidget,
+ IMultiLineWidgetLayout,
+ )
+
+
+@implementer(IMultiLineWidgetLayout, IAlwaysSubmittedWidget, IInputWidget)
+class GitRefWidget(BrowserWidget, InputWidget):
+
+ template = ViewPageTemplateFile("templates/gitref.pt")
+ display_label = False
+ _widgets_set_up = False
+
+ def setUpSubWidgets(self):
+ if self._widgets_set_up:
+ return
+ fields = [
+ Choice(
+ __name__="repository", title=u"Git repository",
+ required=False, vocabulary="GitRepository"),
+ TextLine(
+ __name__="path", title=u"Git branch path", required=False),
+ ]
+ for field in fields:
+ setUpWidget(
+ self, field.__name__, field, IInputWidget, prefix=self.name)
+ self._widgets_set_up = True
+
+ def setRenderedValue(self, value):
+ """See `IWidget`."""
+ self.setUpSubWidgets()
+ if value is not None:
+ self.repository_widget.setRenderedValue(value.repository)
+ self.path_widget.setRenderedValue(value.path)
+ else:
+ self.repository_widget.setRenderedValue(None)
+ self.path_widget.setRenderedValue(None)
+
+ def hasInput(self):
+ """See `IInputWidget`."""
+ return (
+ ("%s.repository" % self.name) in self.request.form or
+ ("%s.path" % self.name) in self.request.form)
+
+ def hasValidInput(self):
+ """See `IInputWidget`."""
+ try:
+ self.getInputValue()
+ return True
+ except (InputErrors, UnexpectedFormData):
+ return False
+
+ def getInputValue(self):
+ """See `IInputWidget`."""
+ self.setUpSubWidgets()
+ try:
+ repository = self.repository_widget.getInputValue()
+ except MissingInputError:
+ raise WidgetInputError(
+ self.name, self.label,
+ LaunchpadValidationError("Please choose a Git repository."))
+ except ConversionError:
+ entered_name = self.request.form_ng.getOne(
+ "%s.repository" % self.name)
+ raise WidgetInputError(
+ self.name, self.label,
+ LaunchpadValidationError(
+ "There is no Git repository named '%s' registered in "
+ "Launchpad." % entered_name))
+ if self.path_widget.hasInput():
+ path = self.path_widget.getInputValue()
+ else:
+ path = None
+ if not path:
+ raise WidgetInputError(
+ self.name, self.label,
+ LaunchpadValidationError("Please enter a Git branch path."))
+ ref = repository.getRefByPath(path)
+ if ref is None:
+ raise WidgetInputError(
+ self.name, self.label,
+ LaunchpadValidationError(
+ "The repository at %s does not contain a branch named "
+ "'%s'." % (repository.display_name, path)))
+ return ref
+
+ def error(self):
+ """See `IBrowserWidget`."""
+ try:
+ if self.hasInput():
+ self.getInputValue()
+ except InputErrors as error:
+ self._error = error
+ return super(GitRefWidget, self).error()
+
+ def __call__(self):
+ """See `IBrowserWidget`."""
+ self.setUpSubWidgets()
+ return self.template()
=== added file 'lib/lp/code/browser/widgets/templates/gitref.pt'
--- lib/lp/code/browser/widgets/templates/gitref.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/code/browser/widgets/templates/gitref.pt 2015-09-09 14:30:28 +0000
@@ -0,0 +1,18 @@
+<table>
+ <tr>
+ <td>
+ <tal:widget define="widget nocall:view/repository_widget">
+ <metal:block
+ use-macro="context/@@launchpad_widget_macros/launchpad_widget_row" />
+ </tal:widget>
+ </td>
+ </tr>
+ <tr>
+ <td>
+ <tal:widget define="widget nocall:view/path_widget">
+ <metal:block
+ use-macro="context/@@launchpad_widget_macros/launchpad_widget_row" />
+ </tal:widget>
+ </td>
+ </tr>
+</table>
=== added file 'lib/lp/code/browser/widgets/tests/test_gitrefwidget.py'
--- lib/lp/code/browser/widgets/tests/test_gitrefwidget.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/browser/widgets/tests/test_gitrefwidget.py 2015-09-09 14:30:28 +0000
@@ -0,0 +1,199 @@
+# Copyright 2015 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from BeautifulSoup import BeautifulSoup
+from lazr.restful.fields import Reference
+from zope.formlib.interfaces import (
+ IBrowserWidget,
+ IInputWidget,
+ WidgetInputError,
+ )
+from zope.interface import (
+ implementer,
+ Interface,
+ )
+
+from lp.app.validators import LaunchpadValidationError
+from lp.code.browser.widgets.gitref import GitRefWidget
+from lp.code.vocabularies.gitrepository import GitRepositoryVocabulary
+from lp.services.webapp.escaping import html_escape
+from lp.services.webapp.servers import LaunchpadTestRequest
+from lp.testing import (
+ TestCaseWithFactory,
+ verifyObject,
+ )
+from lp.testing.layers import DatabaseFunctionalLayer
+
+
+class IThing(Interface):
+ pass
+
+
+@implementer(IThing)
+class Thing:
+ pass
+
+
+class TestGitRefWidget(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestGitRefWidget, self).setUp()
+ field = Reference(
+ __name__="git_ref", schema=Interface, title=u"Git reference")
+ self.context = Thing()
+ field = field.bind(self.context)
+ request = LaunchpadTestRequest()
+ self.widget = GitRefWidget(field, request)
+
+ def test_implements(self):
+ self.assertTrue(verifyObject(IBrowserWidget, self.widget))
+ self.assertTrue(verifyObject(IInputWidget, self.widget))
+
+ def test_template(self):
+ self.assertTrue(
+ self.widget.template.filename.endswith("gitref.pt"),
+ "Template was not set up.")
+
+ def test_setUpSubWidgets_first_call(self):
+ # The subwidgets are set up and a flag is set.
+ self.widget.setUpSubWidgets()
+ self.assertTrue(self.widget._widgets_set_up)
+ self.assertIsInstance(
+ self.widget.repository_widget.context.vocabulary,
+ GitRepositoryVocabulary)
+ self.assertIsNotNone(self.widget.path_widget)
+
+ def test_setUpSubWidgets_second_call(self):
+ # The setUpSubWidgets method exits early if a flag is set to
+ # indicate that the widgets were set up.
+ self.widget._widgets_set_up = True
+ self.widget.setUpSubWidgets()
+ self.assertIsNone(getattr(self.widget, "repository_widget", None))
+ self.assertIsNone(getattr(self.widget, "path_widget", None))
+
+ def test_setRenderedValue(self):
+ # The widget's render state is set from the provided reference's
+ # repository and path.
+ self.widget.setUpSubWidgets()
+ [ref] = self.factory.makeGitRefs()
+ self.widget.setRenderedValue(ref)
+ self.assertEqual(
+ ref.repository, self.widget.repository_widget._getCurrentValue())
+ self.assertEqual(ref.path, self.widget.path_widget._getCurrentValue())
+
+ def test_hasInput_false(self):
+ # hasInput is false when the widget's name is not in the form data.
+ self.widget.request = LaunchpadTestRequest(form={})
+ self.assertFalse(self.widget.hasInput())
+
+ def test_hasInput_true(self):
+ # hasInput is true when the subwidgets are in the form data.
+ form = {
+ "field.git_ref.repository": "",
+ "field.git_ref.path": "",
+ }
+ self.widget.request = LaunchpadTestRequest(form=form)
+ self.assertEqual("field.git_ref", self.widget.name)
+ self.assertTrue(self.widget.hasInput())
+
+ def test_hasValidInput_false(self):
+ # The field input is invalid if any of the submitted parts are
+ # invalid.
+ form = {
+ "field.git_ref.repository": "non-existent",
+ "field.git_ref.path": "non-existent",
+ }
+ self.widget.request = LaunchpadTestRequest(form=form)
+ self.assertFalse(self.widget.hasValidInput())
+
+ def test_hasValidInput_true(self):
+ # The field input is valid when all submitted parts are valid.
+ [ref] = self.factory.makeGitRefs()
+ form = {
+ "field.git_ref.repository": ref.repository.unique_name,
+ "field.git_ref.path": ref.path,
+ }
+ self.widget.request = LaunchpadTestRequest(form=form)
+ self.assertTrue(self.widget.hasValidInput())
+
+ def assertGetInputValueError(self, form, message):
+ self.widget.request = LaunchpadTestRequest(form=form)
+ e = self.assertRaises(WidgetInputError, self.widget.getInputValue)
+ self.assertEqual(LaunchpadValidationError(message), e.errors)
+ self.assertEqual(html_escape(message), self.widget.error())
+
+ def test_getInputValue_repository_missing(self):
+ # An error is raised when the repository field is missing.
+ form = {
+ "field.git_ref.path": "master",
+ }
+ self.assertGetInputValueError(form, "Please choose a Git repository.")
+
+ def test_getInputValue_repository_invalid(self):
+ # An error is raised when the repository does not exist.
+ form = {
+ "field.git_ref.repository": "non-existent",
+ "field.git_ref.path": "master",
+ }
+ self.assertGetInputValueError(
+ form,
+ "There is no Git repository named 'non-existent' registered in "
+ "Launchpad.")
+
+ def test_getInputValue_path_empty(self):
+ # An error is raised when the path field is empty.
+ repository = self.factory.makeGitRepository()
+ form = {
+ "field.git_ref.repository": repository.unique_name,
+ "field.git_ref.path": "",
+ }
+ self.assertGetInputValueError(form, "Please enter a Git branch path.")
+
+ def test_getInputValue_path_invalid(self):
+ # An error is raised when the branch path does not identify a
+ # reference in the repository.
+ [ref] = self.factory.makeGitRefs()
+ form = {
+ "field.git_ref.repository": ref.repository.unique_name,
+ "field.git_ref.path": "non-existent",
+ }
+ self.assertGetInputValueError(
+ form,
+ "The repository at %s does not contain a branch named "
+ "'non-existent'." % ref.repository.display_name)
+
+ def test_getInputValue_valid(self):
+ # When both the repository and the path are valid, the field value
+ # is the reference they identify.
+ [ref] = self.factory.makeGitRefs()
+ form = {
+ "field.git_ref.repository": ref.repository.unique_name,
+ "field.git_ref.path": ref.path,
+ }
+ self.widget.request = LaunchpadTestRequest(form=form)
+ self.assertEqual(ref, self.widget.getInputValue())
+
+ def test_getInputValue_canonicalises_path(self):
+ # A shortened version of the branch path may be used.
+ [ref] = self.factory.makeGitRefs()
+ form = {
+ "field.git_ref.repository": ref.repository.unique_name,
+ "field.git_ref.path": ref.name,
+ }
+ self.widget.request = LaunchpadTestRequest(form=form)
+ self.assertEqual(ref, self.widget.getInputValue())
+
+ def test_call(self):
+ # The __call__ method sets up the widgets.
+ markup = self.widget()
+ self.assertIsNotNone(self.widget.repository_widget)
+ self.assertIsNotNone(self.widget.path_widget)
+ soup = BeautifulSoup(markup)
+ fields = soup.findAll("input", id=True)
+ ids = [field["id"] for field in fields]
+ self.assertContentEqual(
+ ["field.git_ref.repository", "field.git_ref.path"], ids)
=== modified file 'lib/lp/snappy/browser/snap.py'
--- lib/lp/snappy/browser/snap.py 2015-09-07 16:05:51 +0000
+++ lib/lp/snappy/browser/snap.py 2015-09-09 14:30:28 +0000
@@ -29,6 +29,7 @@
from lp.app.browser.lazrjs import InlinePersonEditPickerWidget
from lp.app.browser.tales import format_link
from lp.app.widgets.itemswidgets import LaunchpadRadioWidget
+from lp.code.browser.widgets.gitref import GitRefWidget
from lp.registry.enums import VCSType
from lp.services.webapp import (
canonical_url,
@@ -116,9 +117,8 @@
def source(self):
if self.context.branch is not None:
return self.context.branch
- elif self.context.git_repository is not None:
- return self.context.git_repository.getRefByPath(
- self.context.git_path)
+ elif self.context.git_ref is not None:
+ return self.context.git_ref
else:
return None
@@ -162,8 +162,7 @@
# Each of these is only required if vcs has an appropriate value. Later
# validation takes care of adjusting the required attribute.
branch = copy_field(ISnap['branch'], required=True)
- git_repository = copy_field(ISnap['git_repository'], required=True)
- git_path = copy_field(ISnap['git_path'], required=True)
+ git_ref = copy_field(ISnap['git_ref'], required=True)
class BaseSnapAddEditView(LaunchpadEditFormView):
@@ -193,12 +192,10 @@
vcs = data.get('vcs')
if vcs == VCSType.BZR:
self.widgets['branch'].context.required = True
- self.widgets['git_repository'].context.required = False
- self.widgets['git_path'].context.required = False
+ self.widgets['git_ref'].context.required = False
elif vcs == VCSType.GIT:
self.widgets['branch'].context.required = False
- self.widgets['git_repository'].context.required = True
- self.widgets['git_path'].context.required = True
+ self.widgets['git_ref'].context.required = True
else:
raise AssertionError("Unknown branch type %s" % vcs)
super(BaseSnapAddEditView, self).validate_widgets(data, names=names)
@@ -210,8 +207,7 @@
def request_action(self, action, data):
vcs = data.pop('vcs', None)
if vcs == VCSType.BZR:
- data['git_repository'] = None
- data['git_path'] = None
+ data['git_ref'] = None
elif vcs == VCSType.GIT:
data['branch'] = None
self.updateContextFromData(data)
@@ -245,14 +241,14 @@
page_title = 'Edit'
field_names = [
- 'owner', 'name', 'distro_series', 'vcs', 'branch', 'git_repository',
- 'git_path']
+ 'owner', 'name', 'distro_series', 'vcs', 'branch', 'git_ref']
custom_widget('distro_series', LaunchpadRadioWidget)
custom_widget('vcs', LaunchpadRadioWidget)
+ custom_widget('git_ref', GitRefWidget)
@property
def initial_values(self):
- if self.context.git_repository is not None:
+ if self.context.git_ref is not None:
vcs = VCSType.GIT
else:
vcs = VCSType.BZR
=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py 2015-08-05 00:27:16 +0000
+++ lib/lp/snappy/interfaces/snap.py 2015-09-09 14:30:28 +0000
@@ -65,6 +65,7 @@
from lp.app.validators.name import name_validator
from lp.buildmaster.interfaces.processor import IProcessor
from lp.code.interfaces.branch import IBranch
+from lp.code.interfaces.gitref import IGitRef
from lp.code.interfaces.gitrepository import IGitRepository
from lp.registry.interfaces.distroseries import IDistroSeries
from lp.registry.interfaces.person import IPerson
@@ -149,12 +150,12 @@
@error_status(httplib.BAD_REQUEST)
class NoSourceForSnap(Exception):
- """Snap packages must have a source (Bazaar branch or Git repository)."""
+ """Snap packages must have a source (Bazaar or Git branch)."""
def __init__(self):
super(NoSourceForSnap, self).__init__(
"New snap packages must have either a Bazaar branch or a Git "
- "repository.")
+ "branch.")
@error_status(httplib.BAD_REQUEST)
@@ -264,17 +265,23 @@
git_repository = exported(ReferenceChoice(
title=_("Git repository"),
schema=IGitRepository, vocabulary="GitRepository",
- required=False, readonly=False,
+ required=False, readonly=True,
description=_(
"A Git repository with a branch containing a snapcraft.yaml "
"recipe at the top level.")))
git_path = exported(TextLine(
- title=_("Git branch path"), required=False, readonly=False,
+ title=_("Git branch path"), required=False, readonly=True,
description=_(
"The path of the Git branch containing a snapcraft.yaml recipe at "
"the top level.")))
+ git_ref = exported(Reference(
+ IGitRef, title=_("Git branch"), required=False, readonly=False,
+ description=_(
+ "The Git branch containing a snapcraft.yaml recipe at the top "
+ "level.")))
+
class ISnapAdminAttributes(Interface):
"""`ISnap` attributes that can be edited by admins.
@@ -325,11 +332,11 @@
@export_factory_operation(
ISnap, [
"owner", "distro_series", "name", "description", "branch",
- "git_repository", "git_path"])
+ "git_ref"])
@operation_for_version("devel")
def new(registrant, owner, distro_series, name, description=None,
- branch=None, git_repository=None, git_path=None,
- require_virtualized=True, processors=None, date_created=None):
+ branch=None, git_ref=None, require_virtualized=True,
+ processors=None, date_created=None):
"""Create an `ISnap`."""
def exists(owner, name):
=== modified file 'lib/lp/snappy/javascript/snap.edit.js'
--- lib/lp/snappy/javascript/snap.edit.js 2015-09-04 16:20:26 +0000
+++ lib/lp/snappy/javascript/snap.edit.js 2015-09-09 14:30:28 +0000
@@ -23,8 +23,8 @@
}
});
module.set_enabled('field.branch', selected_vcs === 'BZR');
- module.set_enabled('field.git_repository', selected_vcs === 'GIT');
- module.set_enabled('field.git_path', selected_vcs === 'GIT');
+ module.set_enabled('field.git_ref.repository', selected_vcs === 'GIT');
+ module.set_enabled('field.git_ref.path', selected_vcs === 'GIT');
};
module.setup = function() {
=== modified file 'lib/lp/snappy/javascript/tests/test_snap.edit.html'
--- lib/lp/snappy/javascript/tests/test_snap.edit.html 2015-09-04 16:20:26 +0000
+++ lib/lp/snappy/javascript/tests/test_snap.edit.html 2015-09-09 14:30:28 +0000
@@ -99,12 +99,12 @@
<tr>
<td colspan="2">
<div>
- <label for="field.git_repository">Git repository:</label>
+ <label for="field.git_ref.repository">Git repository:</label>
<div>
<input type="text"
value=""
- id="field.git_repository"
- name="field.git_repository"
+ id="field.git_ref.repository"
+ name="field.git_ref.repository"
size="20"
class="" />
</div>
@@ -114,11 +114,11 @@
<tr>
<td colspan="2">
<div>
- <label for="field.git_path">Git path:</label>
+ <label for="field.git_ref.path">Git path:</label>
<div>
<input class="textType"
- id="field.git_path"
- name="field.git_path"
+ id="field.git_ref.path"
+ name="field.git_ref.path"
size="20"
type="text"
value="" />
=== modified file 'lib/lp/snappy/javascript/tests/test_snap.edit.js'
--- lib/lp/snappy/javascript/tests/test_snap.edit.js 2015-09-04 16:20:26 +0000
+++ lib/lp/snappy/javascript/tests/test_snap.edit.js 2015-09-09 14:30:28 +0000
@@ -20,8 +20,8 @@
// Get the input widgets.
this.input_branch = Y.DOM.byId('field.branch');
- this.input_git_repository = Y.DOM.byId('field.git_repository');
- this.input_git_path = Y.DOM.byId('field.git_path');
+ this.input_git_repository = Y.DOM.byId('field.git_ref.repository');
+ this.input_git_path = Y.DOM.byId('field.git_ref.path');
},
tearDown: function() {
@@ -53,11 +53,12 @@
// The branch input field is enabled.
Y.Assert.isFalse(this.input_branch.disabled,
'branch field disabled');
- // The git_repository and git_path input fields are disabled.
+ // The git_ref.repository and git_ref.path input fields are
+ // disabled.
Y.Assert.isTrue(this.input_git_repository.disabled,
- 'git_repository field not disabled');
+ 'git_ref.repository field not disabled');
Y.Assert.isTrue(this.input_git_path.disabled,
- 'git_path field not disabled');
+ 'git_ref.path field not disabled');
},
test_select_vcs_git: function() {
@@ -66,11 +67,12 @@
// The branch input field is disabled.
Y.Assert.isTrue(this.input_branch.disabled,
'branch field not disabled');
- // The git_repository and git_path input fields are enabled.
+ // The git_ref.repository and git_ref.path input fields are
+ // enabled.
Y.Assert.isFalse(this.input_git_repository.disabled,
- 'git_repository field disabled');
+ 'git_ref.repository field disabled');
Y.Assert.isFalse(this.input_git_path.disabled,
- 'git_path field disabled');
+ 'git_ref.path field disabled');
}
}));
}, '0.1', {
=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py 2015-08-07 10:09:26 +0000
+++ lib/lp/snappy/model/snap.py 2015-09-09 14:30:28 +0000
@@ -110,12 +110,12 @@
require_virtualized = Bool(name='require_virtualized')
def __init__(self, registrant, owner, distro_series, name,
- description=None, branch=None, git_repository=None,
- git_path=None, require_virtualized=True,
- date_created=DEFAULT):
+ description=None, branch=None, git_ref=None,
+ require_virtualized=True, date_created=DEFAULT):
"""Construct a `Snap`."""
if not getFeatureFlag(SNAP_FEATURE_FLAG):
raise SnapFeatureDisabled
+
super(Snap, self).__init__()
self.registrant = registrant
self.owner = owner
@@ -123,12 +123,30 @@
self.name = name
self.description = description
self.branch = branch
- self.git_repository = git_repository
- self.git_path = git_path
+ if git_ref is not None:
+ self.git_repository = git_ref.repository
+ self.git_path = git_ref.path
+ else:
+ self.git_repository = None
+ self.git_path = None
self.require_virtualized = require_virtualized
self.date_created = date_created
self.date_last_modified = date_created
+ @property
+ def git_ref(self):
+ """See `ISnap`."""
+ if self.git_repository is not None:
+ return self.git_repository.getRefByPath(self.git_path)
+ else:
+ return None
+
+ @git_ref.setter
+ def git_ref(self, value):
+ """See `ISnap`."""
+ self.git_repository = value.repository
+ self.git_path = value.path
+
def _getProcessors(self):
return list(Store.of(self).find(
Processor,
@@ -280,8 +298,8 @@
"""See `ISnapSet`."""
def new(self, registrant, owner, distro_series, name, description=None,
- branch=None, git_repository=None, git_path=None,
- require_virtualized=True, processors=None, date_created=DEFAULT):
+ branch=None, git_ref=None, require_virtualized=True,
+ processors=None, date_created=DEFAULT):
"""See `ISnapSet`."""
if not registrant.inTeam(owner):
if owner.is_team:
@@ -293,13 +311,13 @@
"%s cannot create snap packages owned by %s." %
(registrant.displayname, owner.displayname))
- if branch is None and git_repository is None:
+ if branch is None and git_ref is None:
raise NoSourceForSnap
store = IMasterStore(Snap)
snap = Snap(
registrant, owner, distro_series, name, description=description,
- branch=branch, git_repository=git_repository, git_path=git_path,
+ branch=branch, git_ref=git_ref,
require_virtualized=require_virtualized, date_created=date_created)
store.add(snap)
=== modified file 'lib/lp/snappy/model/snapbuildbehaviour.py'
--- lib/lp/snappy/model/snapbuildbehaviour.py 2015-08-05 12:25:30 +0000
+++ lib/lp/snappy/model/snapbuildbehaviour.py 2015-09-09 14:30:28 +0000
@@ -86,9 +86,9 @@
args["archive_private"] = build.archive.private
if build.snap.branch is not None:
args["branch"] = build.snap.branch.bzr_identity
- elif build.snap.git_repository is not None:
+ elif build.snap.git_ref is not None:
args["git_repository"] = build.snap.git_repository.git_https_url
- args["git_path"] = build.snap.git_path
+ args["git_path"] = build.snap.git_ref.name
else:
raise CannotBuild(
"Source branch/repository for ~%s/%s has been deleted." %
=== modified file 'lib/lp/snappy/templates/snap-edit.pt'
--- lib/lp/snappy/templates/snap-edit.pt 2015-09-07 16:05:51 +0000
+++ lib/lp/snappy/templates/snap-edit.pt 2015-09-09 14:30:28 +0000
@@ -49,10 +49,7 @@
<td>
<label tal:replace="structure view/vcs_git_radio" />
<table class="subordinate">
- <tal:widget define="widget nocall:view/widgets/git_repository">
- <metal:block use-macro="context/@@launchpad_form/widget_row" />
- </tal:widget>
- <tal:widget define="widget nocall:view/widgets/git_path">
+ <tal:widget define="widget nocall:view/widgets/git_ref">
<metal:block use-macro="context/@@launchpad_form/widget_row" />
</tal:widget>
</table>
=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py 2015-08-05 10:52:13 +0000
+++ lib/lp/snappy/tests/test_snap.py 2015-09-09 14:30:28 +0000
@@ -382,8 +382,7 @@
if branch is not None:
components["branch"] = branch
else:
- components["git_repository"] = git_ref.repository
- components["git_path"] = git_ref.path
+ components["git_ref"] = git_ref
return components
def test_creation_bzr(self):
@@ -392,7 +391,6 @@
branch = self.factory.makeAnyBranch()
components = self.makeSnapComponents(branch=branch)
snap = getUtility(ISnapSet).new(**components)
- transaction.commit()
self.assertEqual(components["registrant"], snap.registrant)
self.assertEqual(components["owner"], snap.owner)
self.assertEqual(components["distro_series"], snap.distro_series)
@@ -400,6 +398,7 @@
self.assertEqual(branch, snap.branch)
self.assertIsNone(snap.git_repository)
self.assertIsNone(snap.git_path)
+ self.assertIsNone(snap.git_ref)
self.assertTrue(snap.require_virtualized)
def test_creation_git(self):
@@ -408,7 +407,6 @@
[ref] = self.factory.makeGitRefs()
components = self.makeSnapComponents(git_ref=ref)
snap = getUtility(ISnapSet).new(**components)
- transaction.commit()
self.assertEqual(components["registrant"], snap.registrant)
self.assertEqual(components["owner"], snap.owner)
self.assertEqual(components["distro_series"], snap.distro_series)
@@ -416,6 +414,7 @@
self.assertIsNone(snap.branch)
self.assertEqual(ref.repository, snap.git_repository)
self.assertEqual(ref.path, snap.git_path)
+ self.assertEqual(ref, snap.git_ref)
self.assertTrue(snap.require_virtualized)
def test_creation_no_source(self):
@@ -499,10 +498,12 @@
repositories = [self.factory.makeGitRepository() for i in range(2)]
snaps = []
paths = []
+ refs = []
for repository in repositories:
for i in range(2):
[ref] = self.factory.makeGitRefs(repository=repository)
paths.append(ref.path)
+ refs.append(ref)
snaps.append(self.factory.makeSnap(
git_ref=ref, date_created=ONE_DAY_AGO))
getUtility(ISnapSet).detachFromGitRepository(repositories[0])
@@ -512,6 +513,8 @@
self.assertEqual(
[None, None, paths[2], paths[3]],
[snap.git_path for snap in snaps])
+ self.assertEqual(
+ [None, None, refs[2], refs[3]], [snap.git_ref for snap in snaps])
for snap in snaps[:2]:
self.assertSqlAttributeEqualsDate(
snap, "date_last_modified", UTC_NOW)
@@ -601,8 +604,7 @@
if branch is not None:
kwargs["branch"] = api_url(branch)
if git_ref is not None:
- kwargs["git_repository"] = api_url(git_ref.repository)
- kwargs["git_path"] = git_ref.path
+ kwargs["git_ref"] = api_url(git_ref)
if processors is not None:
kwargs["processors"] = [
api_url(processor) for processor in processors]
@@ -635,6 +637,7 @@
self.assertEqual(self.getURL(branch), snap["branch_link"])
self.assertIsNone(snap["git_repository_link"])
self.assertIsNone(snap["git_path"])
+ self.assertIsNone(snap["git_ref_link"])
self.assertTrue(snap["require_virtualized"])
def test_new_git(self):
@@ -654,6 +657,7 @@
self.assertEqual(
self.getURL(ref.repository), snap["git_repository_link"])
self.assertEqual(ref.path, snap["git_path"])
+ self.assertEqual(self.getURL(ref), snap["git_ref_link"])
self.assertTrue(snap["require_virtualized"])
def test_duplicate(self):
=== modified file 'lib/lp/snappy/tests/test_snapbuildbehaviour.py'
--- lib/lp/snappy/tests/test_snapbuildbehaviour.py 2015-08-05 12:25:30 +0000
+++ lib/lp/snappy/tests/test_snapbuildbehaviour.py 2015-09-09 14:30:28 +0000
@@ -180,7 +180,7 @@
"archives": expected_archives,
"arch_tag": "i386",
"git_repository": ref.repository.git_https_url,
- "git_path": ref.path,
+ "git_path": ref.name,
"name": u"test-snap",
}, job._extraBuildArgs())
@@ -208,6 +208,21 @@
"deleted.",
job.composeBuildRequest, None)
+ def test_composeBuildRequest_git_ref_deleted(self):
+ # If the source Git reference has been deleted, composeBuildRequest
+ # raises CannotBuild.
+ repository = self.factory.makeGitRepository()
+ [ref] = self.factory.makeGitRefs(repository=repository)
+ owner = self.factory.makePerson(name="snap-owner")
+ job = self.makeJob(registrant=owner, owner=owner, git_ref=ref)
+ repository.removeRefs([ref.path])
+ self.assertIsNone(job.build.snap.git_ref)
+ self.assertRaisesWithContent(
+ CannotBuild,
+ "Source branch/repository for ~snap-owner/test-snap has been "
+ "deleted.",
+ job.composeBuildRequest, None)
+
class MakeSnapBuildMixin:
"""Provide the common makeBuild method returning a queued build."""
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2015-08-10 07:36:52 +0000
+++ lib/lp/testing/factory.py 2015-09-09 14:30:28 +0000
@@ -4548,18 +4548,12 @@
distroseries = self.makeDistroSeries()
if name is None:
name = self.getUniqueString(u"snap-name")
- kwargs = {}
if branch is None and git_ref is None:
branch = self.makeAnyBranch()
- if branch is not None:
- kwargs["branch"] = branch
- elif git_ref is not None:
- kwargs["git_repository"] = git_ref.repository
- kwargs["git_path"] = git_ref.path
snap = getUtility(ISnapSet).new(
registrant, owner, distroseries, name,
require_virtualized=require_virtualized, processors=processors,
- date_created=date_created, **kwargs)
+ date_created=date_created, branch=branch, git_ref=git_ref)
IStore(snap).flush()
return snap
Follow ups
