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.

=== modified file 'NEWS'
--- NEWS	2009-06-30 17:00:26 +0000
+++ NEWS	2009-06-30 20:35:32 +0000
@@ -21,6 +21,7 @@
 New Features
 ************
 
+<<<<<<< TREE
 * ``bzr push`` now aborts if uncommitted changes (including pending merges)
   are present in the working tree (if one is present) and no revision is
   speified. The configuration option ``push_strict`` can be used to set the
@@ -29,6 +30,18 @@
 * ``bzr revno`` and ``bzr revision-info`` now have a ``--tree`` option to
   show revision info for the working tree instead of the branch.
   (Matthew Fuller, John Arbash Meinel)
+=======
+* ``bzr push`` now checks if uncommitted changes are present in the working
+  tree if the ``--strict`` option is used. The ``push_strict`` option can
+  be declared in a configuration file.
+  (Vincent Ladeuil, #284038)
+>>>>>>> MERGE-SOURCE
+
+* ``bzr send`` now aborts if uncommitted changes (including pending merges)
+  are present in the working tree and no revision is speficied. The
+  ``--no-strict`` option can be used to force the sending. The
+  ``send_strict`` option can be declared in a configuration file.
+  (Vincent Ladeuil, #206577)
 
 
 Bug Fixes

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2009-06-30 17:00:26 +0000
+++ bzrlib/builtins.py	2009-06-30 20:35:32 +0000
@@ -4950,24 +4950,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.'),
         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):
@@ -5017,6 +5022,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.'),
         'revision',
         RegistryOption('format',
                        help='Use the specified output format.',
@@ -5030,14 +5038,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-06-30 20:35:32 +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-06-30 20:35:32 +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,6 +107,21 @@
             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 strict is None or strict: # Default to True
+                changes = tree.changes_from(tree.basis_tree())
+                if changes.has_changed() or len(tree.get_parent_ids()) > 1:
+                    raise errors.UncommittedChanges(tree)
             revision_id = branch.last_revision()
         if revision_id == NULL_REVISION:
             raise errors.BzrCommandError('No revisions to submit.')

=== 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-06-30 20:35:32 +0000
@@ -28,12 +28,52 @@
 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')),
+        ]
+    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 +91,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 +285,131 @@
         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):
+
+    _default_command = ['send', '-o-', '../parent']
+    _default_wd = 'local'
+
+    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')
+
+    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=['Working tree ".*/local/"'
+                              ' has uncommitted changes.$',])
+
+    def assertSendSucceeds(self, revs, args):
+        out, err = self.run_send(args)
+        self.assertEquals('Bundling 1 revision(s).\n', 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(['local'], [])
+
+    def test_send_strict(self):
+        self.assertSendSucceeds(['local'], ['--strict'])
+
+    def test_send_no_strict(self):
+        self.assertSendSucceeds(['local'], ['--no-strict'])
+
+    def test_send_config_var_strict(self):
+        self.set_config_send_strict('true')
+        self.assertSendSucceeds(['local'], [])
+
+    def test_send_config_var_no_strict(self):
+        self.set_config_send_strict('false')
+        self.assertSendSucceeds(['local'], [])
+
+
+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 test_send_default(self):
+        self.assertSendFails([])
+
+    def test_send_with_revision(self):
+        self.assertSendSucceeds(['local'], ['-r', 'revid:local'])
+
+    def test_send_no_strict(self):
+        self.assertSendSucceeds(['local'], ['--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(['local'], ['--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(['local'], ['--no-strict'])
+
+    def test_push_strict_command_line_override_config(self):
+        self.set_config_send_strict('false')
+        self.assertSendSucceeds(['local'], [])
+        self.assertSendFails(['--strict'])
+
+
+class TestBundleStrictWithoutChanges(TestSendStrictWithoutChanges):
+
+    _default_command = ['bundle-revisions', '../parent']