Patchwork Regression when executing abbreviated shell aliases with arguments

login
register
mail settings
Submitter Thomas De Schampheleire
Date Sept. 3, 2014, 2:30 p.m.
Message ID <CAAXf6LX3iPtpVaC3VK8S44go_DBGu7PAsO+Wz7pBfbg-5HOfiQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/5691/
State Not Applicable
Headers show

Comments

Thomas De Schampheleire - Sept. 3, 2014, 2:30 p.m.
Hi,

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)


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'.

However, I'm not sure how this problem should be solved.
Any help appreciated.

Thanks,
Thomas
Matt Mackall - Sept. 3, 2014, 8:12 p.m.
On Wed, 2014-09-03 at 16:30 +0200, Thomas De Schampheleire wrote:
> Hi,
> 
> 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"

Excellent bug report, please put it on the BTS so we don't lose track of
it and you stay in the loop. Mark it "urgent" (because it's a
regression) and make sure foozy is cc:ed.
Thomas De Schampheleire - Sept. 4, 2014, 7:51 a.m.
On Wed, Sep 3, 2014 at 10:12 PM, Matt Mackall <mpm@selenic.com> wrote:
> On Wed, 2014-09-03 at 16:30 +0200, Thomas De Schampheleire wrote:
>> Hi,
>>
>> 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"
>
> Excellent bug report, please put it on the BTS so we don't lose track of
> it and you stay in the loop. Mark it "urgent" (because it's a
> regression) and make sure foozy is cc:ed.

Thanks for the feedback. Bug report at:
http://bz.selenic.com/show_bug.cgi?id=4355
Katsunori FUJIWARA - Sept. 5, 2014, 2:20 p.m.
At Wed, 3 Sep 2014 16:30:04 +0200,
Thomas De Schampheleire wrote:
> 
> Hi,
> 
> 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'.
> 
> However, I'm not sure how this problem should be solved.
> Any help appreciated.
> 
> Thanks,
> Thomas
> 

Oops, sorry for my regression and thank you for detail report.

I'll soon sent the patch to fix this.

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

--- /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]