Merge lp:~vila/bzr/206577-send-strict into lp:~bzr/bzr/trunk-old

[Vincent Ladeuil]

[Vincent Ladeuil (vila)]

This fixes bug #206577 by introducing a --strict boolean option for 'send' and
'bundle-revisions' and aborting when uncommitted changes are found in the working tree.

As for bug #284038 (see https://code.edge.launchpad.net/~vila/bzr/284038-push-strict/+merge/8005),
I plan to introduce a way to query config files for a bool option and a new method on working tree
for checking changes, in a later submission.

[Vincent Ladeuil (vila)]

Hmm, resubmit doesn't allow me to type a comment so here is the cover letter.

This new submission takes into account the remarks made during
the reviews of bug #284038:
- the help and error messages have been updated,
- a new check has been added to ensure that the tree is not out of
  sync with its branch

I still plan to introduce a way to query config files for a bool option
and a new method on working tree for checking changes, in a later submission.

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Vincent Ladeuil wrote:
> Vincent Ladeuil has proposed merging lp:~vila/bzr/206577-send-strict into lp:bzr.
>
> Hmm, resubmit doesn't allow me to type a comment so here is the cover letter.
>
> This new submission takes into account the remarks made during
> the reviews of bug #284038:
> - the help and error messages have been updated,
> - a new check has been added to ensure that the tree is not out of
> sync with its branch
>
> I still plan to introduce a way to query config files for a bool option
> and a new method on working tree for checking changes, in a later submission.
>
>
>

Unfortunately, there is still some lp:mad bits of showing your previous
(and now merged) patch. So I'm going to mark Resubmit and hope it gives
me the correct diff.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkpLXb4ACgkQJdeBCYSNAAODFwCgujso+F0Ns1DV4w2BKSUVKEl3
1dYAoJMVQiGL+DKCb3UrM5zyG68TfHKb
=0yxU
-----END PGP SIGNATURE-----

=== modified file 'NEWS'
--- NEWS	2009-06-30 17:00:26 +0000
+++ NEWS	2009-07-01 09:35:10 +0000
@@ -30,6 +30,12 @@
   show revision info for the working tree instead of the branch.
   (Matthew Fuller, John Arbash Meinel)
 
+* ``bzr send`` now aborts if uncommitted changes (including pending merges)
+  are present in the working tree and no revision is specified. The
+  configuration option ``send_strict`` can be used to set the default for this
+  behavior.
+  (Vincent Ladeuil, #206577)
+
 
 Bug Fixes
 *********

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2009-07-01 07:29:00 +0000
+++ bzrlib/builtins.py	2009-07-01 09:35:10 +0000
@@ -1080,7 +1080,7 @@
             type=unicode),
         Option('strict',
                help='Refuse to push if there are uncommitted changes in'
-               ' the working tree.'),
+               ' the working tree, --no-strict disables the check.'),
         ]
     takes_args = ['location?']
     encoding_type = 'replace'
@@ -4957,24 +4957,29 @@
                help='Write merge directive to this file; '
                     'use - for stdout.',
                type=unicode),
+        Option('strict',
+               help='Refuse to send if there are uncommitted changes in'
+               ' the working tree, --no-strict disables the check.'),
         Option('mail-to', help='Mail the request to this address.',
                type=unicode),
         'revision',
         'message',
         Option('body', help='Body for the email.', type=unicode),
         RegistryOption('format',
-                       help='Use the specified output format.', 
-                       lazy_registry=('bzrlib.send', 'format_registry'))
+                       help='Use the specified output format.',
+                       lazy_registry=('bzrlib.send', 'format_registry')),
         ]
 
     def run(self, submit_branch=None, public_branch=None, no_bundle=False,
             no_patch=False, revision=None, remember=False, output=None,
-            format=None, mail_to=None, message=None, body=None, **kwargs):
+            format=None, mail_to=None, message=None, body=None,
+            strict=None, **kwargs):
         from bzrlib.send import send
         return send(submit_branch, revision, public_branch, remember,
-                         format, no_bundle, no_patch, output,
-                         kwargs.get('from', '.'), mail_to, message, body,
-                         self.outf)
+                    format, no_bundle, no_patch, output,
+                    kwargs.get('from', '.'), mail_to, message, body,
+                    self.outf,
+                    strict=strict)
 
 
 class cmd_bundle_revisions(cmd_send):
@@ -5024,6 +5029,9 @@
                type=unicode),
         Option('output', short_name='o', help='Write directive to this file.',
                type=unicode),
+        Option('strict',
+               help='Refuse to bundle revisions if there are uncommitted'
+               ' changes in the working tree, --no-strict disables the check.'),
         'revision',
         RegistryOption('format',
                        help='Use the specified output format.',
@@ -5037,14 +5045,14 @@
 
     def run(self, submit_branch=None, public_branch=None, no_bundle=False,
             no_patch=False, revision=None, remember=False, output=None,
-            format=None, **kwargs):
+            format=None, strict=None, **kwargs):
         if output is None:
             output = '-'
         from bzrlib.send import send
         return send(submit_branch, revision, public_branch, remember,
                          format, no_bundle, no_patch, output,
                          kwargs.get('from', '.'), None, None, None,
-                         self.outf)
+                         self.outf, strict=strict)
 
 
 class cmd_tag(Command):

=== modified file 'bzrlib/help_topics/en/configuration.txt'
--- bzrlib/help_topics/en/configuration.txt	2009-06-10 13:18:48 +0000
+++ bzrlib/help_topics/en/configuration.txt	2009-07-01 09:35:10 +0000
@@ -421,3 +421,10 @@
 
 If set to "True", the branch should act as a checkout, and push each commit to
 the bound_location.  This option is normally set by ``bind``/``unbind``.
+
+send_strict
+~~~~~~~~~~~
+
+If present, defines the ``--strict`` option default value for checking
+uncommitted changes before sending a merge directive.
+

=== modified file 'bzrlib/send.py'
--- bzrlib/send.py	2009-05-28 16:14:16 +0000
+++ bzrlib/send.py	2009-07-01 09:35:10 +0000
@@ -37,8 +37,8 @@
 
 
 def send(submit_branch, revision, public_branch, remember, format,
-         no_bundle, no_patch, output, from_, mail_to, message, body, 
-         to_file):
+         no_bundle, no_patch, output, from_, mail_to, message, body,
+         to_file, strict=None):
     tree, branch = bzrdir.BzrDir.open_containing_tree_or_branch(from_)[:2]
     # we may need to write data into branch's repository to calculate
     # the data to send.
@@ -107,13 +107,35 @@
             if len(revision) == 2:
                 base_revision_id = revision[0].as_revision_id(branch)
         if revision_id is None:
+            if strict is None:
+                strict = branch.get_config().get_user_option('send_strict')
+                if strict is not None:
+                    # FIXME: This should be better supported by config
+                    # -- vila 20090626
+                    bools = dict(yes=True, no=False, on=True, off=False,
+                                 true=True, false=False)
+                    try:
+                        strict = bools[strict.lower()]
+                    except KeyError:
+                        strict = None
+            if tree is not None and (strict is None or strict):
+                changes = tree.changes_from(tree.basis_tree())
+                if changes.has_changed() or len(tree.get_parent_ids()) > 1:
+                    raise errors.UncommittedChanges(
+                        tree, more='Use --no-strict to force the push.')
+                if tree.last_revision() != tree.branch.last_revision():
+                    # The tree has lost sync with its branch, there is little
+                    # chance that the user is aware of it but he can still force
+                    # the push with --no-strict
+                    raise errors.OutOfDateTree(
+                        tree, more='Use --no-strict to force the push.')
             revision_id = branch.last_revision()
         if revision_id == NULL_REVISION:
             raise errors.BzrCommandError('No revisions to submit.')
         if format is None:
             # TODO: Query submit branch for its preferred format
             format = format_registry.get()
-        directive = format(branch, revision_id, submit_branch, 
+        directive = format(branch, revision_id, submit_branch,
             public_branch, no_patch, no_bundle, message, base_revision_id)
         if output is None:
             directive.compose_merge_request(mail_client, mail_to, body,

=== modified file 'bzrlib/tests/blackbox/test_send.py'
--- bzrlib/tests/blackbox/test_send.py	2009-06-26 08:20:13 +0000
+++ bzrlib/tests/blackbox/test_send.py	2009-07-01 09:35:10 +0000
@@ -28,12 +28,54 @@
 from bzrlib.bundle import serializer
 
 
-def read_bundle(fileobj):
-    md = merge_directive.MergeDirective.from_lines(fileobj.readlines())
-    return serializer.read_bundle(StringIO(md.get_raw_bundle()))
-
-
-class TestSend(tests.TestCaseWithTransport):
+def load_tests(standard_tests, module, loader):
+    """Multiply tests for the send command."""
+    result = loader.suiteClass()
+
+    # one for each king of change
+    changes_tests, remaining_tests = tests.split_suite_by_condition(
+        standard_tests, tests.condition_isinstance((
+                TestSendStrictWithChanges,
+                )))
+    changes_scenarios = [
+        ('uncommitted',
+         dict(_changes_type= '_uncommitted_changes')),
+        ('pending_merges',
+         dict(_changes_type= '_pending_merges')),
+        ('out-of-sync-trees',
+         dict(_changes_type= '_out_of_sync_trees')),
+        ]
+    tests.multiply_tests(changes_tests, changes_scenarios, result)
+    # No parametrization for the remaining tests
+    result.addTests(remaining_tests)
+
+    return result
+
+
+class TestSendMixin(object):
+
+    _default_command = ['send', '-o-']
+    _default_wd = 'branch'
+
+    def run_send(self, args, cmd=None, rc=0, wd=None, err_re=None):
+        if cmd is None: cmd = self._default_command
+        if wd is None: wd = self._default_wd
+        if err_re is None: err_re = []
+        return self.run_bzr(cmd + args, retcode=rc,
+                            working_dir=wd,
+                            error_regexes=err_re)
+
+    def get_MD(self, args, cmd=None, wd='branch'):
+        out = StringIO(self.run_send(args, cmd=cmd, wd=wd)[0])
+        return merge_directive.MergeDirective.from_lines(out.readlines())
+
+    def assertBundleContains(self, revs, args, cmd=None, wd='branch'):
+        md = self.get_MD(args, cmd=cmd, wd=wd)
+        br = serializer.read_bundle(StringIO(md.get_raw_bundle()))
+        self.assertEqual(set(revs), set(r.revision_id for r in br.revisions))
+
+
+class TestSend(tests.TestCaseWithTransport, TestSendMixin):
 
     def setUp(self):
         super(TestSend, self).setUp()
@@ -51,24 +93,6 @@
         self.build_tree_contents([('branch/file1', 'branch')])
         branch_tree.commit('last commit', rev_id='rev3')
 
-    def run_send(self, args, cmd=None, rc=0, wd='branch'):
-        if cmd is None:
-            cmd = ['send', '-o-']
-        return self.run_bzr(cmd + args, retcode=rc, working_dir=wd)
-
-    def get_MD(self, args, cmd=None, wd='branch'):
-        out = StringIO(self.run_send(args, cmd=cmd, wd=wd)[0])
-        return merge_directive.MergeDirective.from_lines(out.readlines())
-
-    def get_bundle(self, args, cmd=None, wd='branch'):
-        md = self.get_MD(args, cmd=cmd, wd=wd)
-        return serializer.read_bundle(StringIO(md.get_raw_bundle()))
-
-    def assertBundleContains(self, revs, args, cmd=None, wd='branch'):
-        out = self.run_send(args, cmd=cmd, wd=wd)[0]
-        br = read_bundle(StringIO(out))
-        self.assertEqual(set(revs), set(r.revision_id for r in br.revisions))
-
     def assertFormatIs(self, fmt_string, md):
         self.assertEqual(fmt_string, md.get_raw_bundle().splitlines()[0])
 
@@ -263,3 +287,147 @@
         out, err = self.run_bzr(["send", "--from", location], retcode=3)
         self.assertEqual(out, '')
         self.assertEqual(err, 'bzr: ERROR: Not a branch: "%s".\n' % location)
+
+
+class TestSendStrictMixin(TestSendMixin):
+
+    def make_parent_and_local_branches(self):
+        # Create a 'parent' branch as the base
+        self.parent_tree = bzrdir.BzrDir.create_standalone_workingtree('parent')
+        self.build_tree_contents([('parent/file', 'parent')])
+        self.parent_tree.add('file')
+        self.parent_tree.commit('first commit', rev_id='parent')
+        # Branch 'local' from parent and do a change
+        local_bzrdir = self.parent_tree.bzrdir.sprout('local')
+        self.local_tree = local_bzrdir.open_workingtree()
+        self.build_tree_contents([('local/file', 'local')])
+        self.local_tree.commit('second commit', rev_id='local')
+
+    _default_command = ['send', '-o-', '../parent']
+    _default_wd = 'local'
+    _default_sent_revs = ['local']
+    _default_errors = ['Working tree ".*/local/" has uncommitted '
+                       'changes \(See bzr status\)\.',]
+
+    def set_config_send_strict(self, value):
+        # set config var (any of bazaar.conf, locations.conf, branch.conf
+        # should do)
+        conf = self.local_tree.branch.get_config()
+        conf.set_user_option('send_strict', value)
+
+    def assertSendFails(self, args):
+        self.run_send(args, rc=3, err_re=self._default_errors)
+
+    def assertSendSucceeds(self, args, revs=None):
+        if revs is None:
+            revs = self._default_sent_revs
+        out, err = self.run_send(args)
+        self.assertEquals(
+            'Bundling %d revision(s).\n' % len(revs), err)
+        md = merge_directive.MergeDirective.from_lines(
+                StringIO(out).readlines())
+        self.assertEqual('parent', md.base_revision_id)
+        br = serializer.read_bundle(StringIO(md.get_raw_bundle()))
+        self.assertEqual(set(revs), set(r.revision_id for r in br.revisions))
+
+
+class TestSendStrictWithoutChanges(tests.TestCaseWithTransport,
+                                   TestSendStrictMixin):
+
+    def setUp(self):
+        super(TestSendStrictWithoutChanges, self).setUp()
+        self.make_parent_and_local_branches()
+
+    def test_send_default(self):
+        self.assertSendSucceeds([])
+
+    def test_send_strict(self):
+        self.assertSendSucceeds(['--strict'])
+
+    def test_send_no_strict(self):
+        self.assertSendSucceeds(['--no-strict'])
+
+    def test_send_config_var_strict(self):
+        self.set_config_send_strict('true')
+        self.assertSendSucceeds([])
+
+    def test_send_config_var_no_strict(self):
+        self.set_config_send_strict('false')
+        self.assertSendSucceeds([])
+
+
+class TestSendStrictWithChanges(tests.TestCaseWithTransport,
+                                   TestSendStrictMixin):
+
+    _changes_type = None # Set by load_tests
+
+    def setUp(self):
+        super(TestSendStrictWithChanges, self).setUp()
+        getattr(self, self._changes_type)()
+
+    def _uncommitted_changes(self):
+        self.make_parent_and_local_branches()
+        # Make a change without committing it
+        self.build_tree_contents([('local/file', 'modified')])
+
+    def _pending_merges(self):
+        self.make_parent_and_local_branches()
+        # Create 'other' branch containing a new file
+        other_bzrdir = self.parent_tree.bzrdir.sprout('other')
+        other_tree = other_bzrdir.open_workingtree()
+        self.build_tree_contents([('other/other-file', 'other')])
+        other_tree.add('other-file')
+        other_tree.commit('other commit', rev_id='other')
+        # Merge and revert, leaving a pending merge
+        self.local_tree.merge_from_branch(other_tree.branch)
+        self.local_tree.revert(filenames=['other-file'], backups=False)
+
+    def _out_of_sync_trees(self):
+        self.make_parent_and_local_branches()
+        self.run_bzr(['checkout', '--lightweight', 'local', 'checkout'])
+        # Make a change and commit it
+        self.build_tree_contents([('local/file', 'modified in local')])
+        self.local_tree.commit('modify file', rev_id='modified-in-local')
+        # Exercise commands from the checkout directory
+        self._default_wd = 'checkout'
+        self._default_errors = ["Working tree is out of date, please run"
+                                " 'bzr update'\.",]
+        self._default_sent_revs = ['modified-in-local', 'local']
+
+    def test_send_default(self):
+        self.assertSendFails([])
+
+    def test_send_with_revision(self):
+        self.assertSendSucceeds(['-r', 'revid:local'], revs=['local'])
+
+    def test_send_no_strict(self):
+        self.assertSendSucceeds(['--no-strict'])
+
+    def test_send_strict_with_changes(self):
+        self.assertSendFails(['--strict'])
+
+    def test_send_respect_config_var_strict(self):
+        self.set_config_send_strict('true')
+        self.assertSendFails([])
+        self.assertSendSucceeds(['--no-strict'])
+
+
+    def test_send_bogus_config_var_ignored(self):
+        self.set_config_send_strict("I'm unsure")
+        self.assertSendFails([])
+
+
+    def test_send_no_strict_command_line_override_config(self):
+        self.set_config_send_strict('true')
+        self.assertSendFails([])
+        self.assertSendSucceeds(['--no-strict'])
+
+    def test_push_strict_command_line_override_config(self):
+        self.set_config_send_strict('false')
+        self.assertSendSucceeds([])
+        self.assertSendFails(['--strict'])
+
+
+class TestBundleStrictWithoutChanges(TestSendStrictWithoutChanges):
+
+    _default_command = ['bundle-revisions', '../parent']