Merge lp:~mbp/bzr/506265-command-deprecation into lp:bzr

[Martin Pool]

Deprecate 'bzr clone' and 'bzr get' (bug 506265)

This makes cmd_branch give a warning if it's invoked as 'bzr get' or 'bzr clone'. See bug 506265.

This can't be suppressed by defining a user alias, but it also is not fatal.

[John A Meinel (jameinel)]

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

On 03/25/2011 10:45 AM, Martin Pool wrote:
> Martin Pool has proposed merging lp:~mbp/bzr/506265-command-deprecation into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
> For more details, see:
> https://code.launchpad.net/~mbp/bzr/506265-command-deprecation/+merge/54828
>
> This doesn't fix bug 506265, but it adds a new Command.invoked_as member, which gets the original name before alias expansion.

Does this distinguish between user aliases and built-in aliases?

I'm certainly fine with the addition. I think I see how you want to use
this, so I'm happy to bring it in, even though it isn't directly used now.

John
=:->
 merge: approve
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk2MZZ8ACgkQJdeBCYSNAANKEACfdTxJA44Pwnmn+mT8XFO0s8mt
xOUAoMlIE+GVuakK2qPUvht/ROUllozW
=wEc1
-----END PGP SIGNATURE-----

review: Approve

[Martin Pool (mbp)]

It doesn't distinguish between user and builtin aliases: it will give the actual name on the command line in either case.

[John A Meinel (jameinel)]

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

On 03/28/2011 03:38 AM, Martin Pool wrote:
> Martin Pool has proposed merging lp:~mbp/bzr/506265-command-deprecation into lp:bzr.
>
> Requested reviews:
> John A Meinel (jameinel)
>
> For more details, see:
> https://code.launchpad.net/~mbp/bzr/506265-command-deprecation/+merge/55030
>
> This doesn't fix bug 506265, but it adds a new Command.invoked_as member, which gets the original name before alias expansion.

I'm a little concerned about the "unable to be suppressed by user alias"
aspect.

I think I really prefer an implementation like:

class cmd_deprecated(Command):

 aliases = ['get', 'clone']

 def run(self, *args, **kwargs):
   if self.invoked_as in ('get', 'clone'):
     official_name = 'branch'
   trace.warning('Command %s deprecated in favor of its official name'
    ' %s' % (self.invoked_as, official_name))
   trace.warning('Please retry with that name')

We might also want to do a trick with self.run_argv_aliases, since we
won't be providing all the flags for every command we are deprecating on
this single command. I don't think it would be very hard to do.

Thoughts?

 review: needsfixing

John
=:->

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

iEYEARECAAYFAk2QckEACgkQJdeBCYSNAANa+QCgqG2DGer6DxXVxkU99icEgiuO
5aEAoIoqRLyOlEs2z95NAG206DOL7Z5s
=T4mo
-----END PGP SIGNATURE-----

review: Needs Fixing

[Martin Pool (mbp)]

Being able to easily turn it off would be good.

I wonder if the exact command you sketch would fail before it runs
because of option or alias mismatches. I can try it.

Alternatively we can add a user option to turn off particular
warnings; I intended to add one already.
Martin

[Andrew Bennetts (spiv)]

Chatting with poolie on IRC I think the inability to suppress the warnings is probably not enough of a problem to let this patch go stale. It'd be great to add the option, and I hope poolie does that soon, but for now I think we land this.

review: Approve

[Andrew Bennetts (spiv)]

sent to pqm by email

[Andrew Bennetts (spiv)]

PQM replied to me: Conflicts during merge: Text conflict in doc/en/release-notes/bzr-2.4.txt

[Martin Pool (mbp)]

sent to pqm by email

[Martin Pool (mbp)]

this apparently has some test failures in pqm

[Martin Pool (mbp)]

sent to pqm by email

[Martin Pool (mbp)]

sent to pqm by email

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2011-04-11 00:18:12 +0000
+++ bzrlib/builtins.py	2011-04-19 03:25:50 +0000
@@ -1205,6 +1205,8 @@
 
     To retrieve the branch as of a particular revision, supply the --revision
     parameter, as in "branch foo/bar -r 5".
+
+    The synonyms 'clone' and 'get' for this command are deprecated.
     """
 
     _see_also = ['checkout']
@@ -1240,6 +1242,12 @@
             files_from=None):
         from bzrlib import switch as _mod_switch
         from bzrlib.tag import _merge_tags_if_possible
+        if self.invoked_as in ['get', 'clone']:
+            ui.ui_factory.show_user_warning(
+                'deprecated_command',
+                deprecated_name=self.invoked_as,
+                recommended_name='branch',
+                deprecated_in_version='2.4')
         accelerator_tree, br_from = bzrdir.BzrDir.open_tree_or_branch(
             from_location)
         if not (hardlink or files_from):

=== modified file 'bzrlib/commands.py'
--- bzrlib/commands.py	2011-04-15 18:11:31 +0000
+++ bzrlib/commands.py	2011-04-19 03:25:50 +0000
@@ -273,6 +273,8 @@
     # Allow plugins to extend commands
     for hook in Command.hooks['extend_command']:
         hook(cmd)
+    if getattr(cmd, 'invoked_as', None) is None:
+        cmd.invoked_as = cmd_name
     return cmd
 
 
@@ -394,7 +396,13 @@
             sys.stdout is forced to be a binary stream, and line-endings
             will not mangled.
 
+    :ivar invoked_as:
+        A string indicating the real name under which this command was
+        invoked, before expansion of aliases. 
+        (This may be None if the command was constructed and run in-process.)
+
     :cvar hooks: An instance of CommandHooks.
+
     :ivar __doc__: The help shown by 'bzr help command' for this command.
         This is set by assigning explicitly to __doc__ so that -OO can
         be used::
@@ -406,6 +414,7 @@
     takes_args = []
     takes_options = []
     encoding_type = 'strict'
+    invoked_as = None
 
     hidden = False
 
@@ -749,6 +758,10 @@
         return getdoc(self)
 
     def name(self):
+        """Return the canonical name for this command.
+
+        The name under which it was actually invoked is available in invoked_as.
+        """
         return _unsquish_command_name(self.__class__.__name__)
 
     def plugin_name(self):

=== modified file 'bzrlib/tests/blackbox/test_branch.py'
--- bzrlib/tests/blackbox/test_branch.py	2011-04-15 07:01:22 +0000
+++ bzrlib/tests/blackbox/test_branch.py	2011-04-19 03:25:50 +0000
@@ -33,6 +33,7 @@
     test_server,
     )
 from bzrlib.tests.test_sftp_transport import TestCaseWithSFTPServer
+from bzrlib.tests.script import run_script
 from bzrlib.urlutils import local_path_to_url, strip_trailing_slash
 from bzrlib.workingtree import WorkingTree
 
@@ -501,3 +502,17 @@
         # Ensure that no working tree what created remotely
         self.assertFalse(t.has('remote/file'))
 
+
+class TestDeprecatedAliases(TestCaseWithTransport):
+
+    def test_deprecated_aliases(self):
+        """bzr branch can be called clone or get, but those names are deprecated.
+
+        See bug 506265.
+        """
+        for command in ['clone', 'get']:
+            run_script(self, """
+            $ bzr %(command)s A B
+            2>The command 'bzr %(command)s' has been deprecated in bzr 2.4. Please use 'bzr branch' instead.
+            2>bzr: ERROR: Not a branch...
+            """ % locals())

=== modified file 'bzrlib/tests/test_commands.py'
--- bzrlib/tests/test_commands.py	2011-01-12 01:01:53 +0000
+++ bzrlib/tests/test_commands.py	2011-04-19 03:25:50 +0000
@@ -91,6 +91,18 @@
         self.assertContainsRe(c.get_help_text(), '--foo')
 
 
+class TestInvokedAs(tests.TestCase):
+
+    def test_invoked_as(self):
+        """The command object knows the actual name used to invoke it."""
+        commands.install_bzr_command_hooks()
+        commands._register_builtin_commands()
+        # get one from the real get_cmd_object.
+        c = commands.get_cmd_object('ci')
+        self.assertIsInstance(c, builtins.cmd_commit)
+        self.assertEquals(c.invoked_as, 'ci')
+
+
 class TestGetAlias(tests.TestCase):
 
     def _get_config(self, config_text):

=== modified file 'bzrlib/ui/__init__.py'
--- bzrlib/ui/__init__.py	2011-04-07 10:36:24 +0000
+++ bzrlib/ui/__init__.py	2011-04-19 03:25:50 +0000
@@ -145,6 +145,10 @@
             "This may take some time. Upgrade the repositories to the "
             "same format for better performance."
             ),
+        deprecated_command=(
+            "The command 'bzr %(deprecated_name)s' "
+            "has been deprecated in bzr %(deprecated_in_version)s. "
+            "Please use 'bzr %(recommended_name)s' instead."),
         recommend_upgrade=("%(current_format_name)s is deprecated "
             "and a better format is available.\n"
             "It is recommended that you upgrade by "

=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- doc/en/release-notes/bzr-2.4.txt	2011-04-18 03:15:03 +0000
+++ doc/en/release-notes/bzr-2.4.txt	2011-04-19 03:25:50 +0000
@@ -15,6 +15,10 @@
 
 .. These may require users to change the way they use Bazaar.
 
+* Two command synonyms for ``bzr branch`` have been deprecated, to avoid
+  confusion and to allow the names to later be reused.  The removed names
+  are: ``get`` and ``clone``.   (Martin Pool, #506265)
+
 New Features
 ************
 
@@ -107,6 +111,10 @@
 .. Changes that may require updates in plugins or other code that uses
    bzrlib.
 
+* Commands now have an `invoked_as` attribute, showing the name under
+  which they were called before alias expansion.
+  (Martin Pool)
+
 * ``Hooks.create_hook`` is now deprecated in favour of ``Hooks.add_hook``.
   (Jelmer Vernooij)
 

=== modified file 'doc/en/whats-new/whats-new-in-2.4.txt'
--- doc/en/whats-new/whats-new-in-2.4.txt	2011-03-15 08:07:04 +0000
+++ doc/en/whats-new/whats-new-in-2.4.txt	2011-04-19 03:25:50 +0000
@@ -16,14 +16,14 @@
 2.1, 2.2 and 2.3, and can read and write repositories generated by all
 previous versions.
 
-External Merge Tools
+External merge tools
 ********************
 
 External merge tool configuration has been added to ``bzr`` core. The name
 and commandline of one or more external merge tools can be defined in
 bazaar.conf. See the help topic ``configuration`` for more details.
 
-Tagged Revisions are Copied
+Tagged revisions are copied
 ***************************
 
 When tags are copied from a branch, the associated revisions are now copied
@@ -32,6 +32,12 @@
 revisions from tags will always be present, so that operations like ``bzr
 log -r tag:foo`` will always work.
 
+Deprecated command synonyms
+***************************
+
+Two built-in synonyms for ``bzr branch`` have been deprecated: ``clone`` and
+``get``.
+
 Configuration files
 *******************