Bug 4355 - Regression when executing abbreviated shell aliases with arguments
Summary: Regression when executing abbreviated shell aliases with arguments
Status: RESOLVED FIXED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: Mercurial (show other bugs)
Version: 3.1
Hardware: PC Linux
: urgent bug
Assignee: Bugzilla
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-04 03:10 UTC by Thomas De Schampheleire
Modified: 2014-09-10 04:54 UTC (History)
5 users (show)

See Also:
Python Version: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas De Schampheleire 2014-09-04 03:10 UTC
[originally reported at http://www.selenic.com/pipermail/mercurial-devel/2014-September/061345.html, turned into bug report on advise of Matt Mackall]

Commit 03d345da0579 introduced a regression when executing shell
aliases with arguments through an abbreviated form of the alias.

changeset:   20328:03d345da0579
branch:      stable
user:        FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
date:        Wed Jan 29 23:47:54 2014 +0900
summary:     dispatch: make "_checkshellalias()" invoke "findcmd()"
with "strict=True"


Following test shows the problem:

$ cat tests/test-blaat.t
  $ cat >> $HGRCPATH <<EOF
  > [alias]
  > args_foo = !echo foo \$HG_ARGS
  > EOF

abbreviated alias with arguments
  $ hg args
  foo args_foo
  $ hg args --bar
  foo args_foo --bar

Output at commit 03d345da0579:

$ (cd tests; python run-tests.py test-blaat.t)

--- /home/tdescham/repo/contrib/hg-dev/tests/test-blaat.t
+++ /home/tdescham/repo/contrib/hg-dev/tests/test-blaat.t.err
@@ -7,4 +7,10 @@
   $ hg args
   foo args_foo
   $ hg args --bar
-  foo args_foo --bar
+  hg args_foo: option --bar not recognized
+  hg args_foo
+
+  shell alias for:
+
+  use "hg help args_foo" to show the full help text
+  [255]

ERROR: /home/tdescham/repo/contrib/hg-dev/tests/test-blaat.t output changed
!
Failed test-blaat.t: output changed
# Ran 1 tests, 0 skipped, 1 failed.
python hash seed: 4280300372


while output at commit 03d345da0579^:

$ (cd tests; python run-tests.py test-blaat.t)
.
# Ran 1 tests, 0 skipped, 0 failed.


I already did some analysis and found that due to the mentioned
commit, abbreviated invocations of shell aliases are not captured by
the __checkshellalias function, but instead by the later invoked
_parse method of mercurial/dispatch.py.

In that method there are these lines:

    # combine global options into local
    for o in commands.globalopts:
        c.append((o[0], o[1], options[o[1]], o[3]))

    try:
        args = fancyopts.fancyopts(args, c, cmdoptions, True)
    except fancyopts.getopt.GetoptError, inst:
        raise error.CommandError(cmd, inst)

and the 'args' argument to fancyopts is '--bar' at this moment. The
knowledge about the alias command 'args_foo' is gone here.
fancyopts then throws the error 'unrecognized option'.
Comment 1 HG Bot 2014-09-09 18:45 UTC
Fixed by http://selenic.com/repo/hg/rev/d821fff9b0b9
FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
dispatch: make "_checkshellalias" reusable regardless of adding aliases

To reduce changes in the subsequent patch fixing issue4355, this patch
makes "_checkshellalias" reusable regardless of adding aliases.

In this patch, alias definitions are added and restored, only when
"precheck=True".

(please test the fix)
Comment 2 HG Bot 2014-09-09 18:45 UTC
Fixed by http://selenic.com/repo/hg/rev/f98abe3146b2
FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
dispatch: check shell alias again after loading extensions (issue4355)

Before this patch, the shell alias causes failure when it takes its
specific (= unknown for "hg") options in the command line, because
"_parse()" can't accept them.

This is the regression introduced by 03d345da0579.

It fixed the issue that ambiguity between shell aliases and commands
defined by extensions was ignored. But it also caused that ambiguous
shell alias is handled in "_parse()" even if it takes specific options
in the command line.

To avoid such failure, this patch checks shell alias again after
loading extensions.

All aliases and commands (including ones defined by extensions) are
completely defined before the 2nd (= newly added in this patch)
"_checkshellalias()" invocation, and "cmdutil.findcmd(strict=False)"
can detect ambiguity between them correctly.

For efficiency, this patch does:

  - omit the 2nd "_checkshellalias()" invocation if "[ui] strict= True"

    it causes "cmdutil.findcmd(strict=True)", of which result should
    be equal to one of the 1st invocation before adding aliases

  - avoid removing the 1st "_checkshellalias()" invocation

    it causes "cmdutil.findcmd(strict=True)" invocation preventing
    shell alias execution from loading extensions uselessly

(please test the fix)
Comment 3 Thomas De Schampheleire 2014-09-10 04:54 UTC
Problem is indeed fixed with the patches in comment #1 and comment #2. 
Thanks!