git-rev-parse incorrectly uses --default parameter when invalid revision is supplied

Brandon Casey· Jan 7, 2008, 23:38 UTC · lore

When rev-parse is called with an invalid revision parameter and --default has been used to set a default revision, the output will be based on the parameter to --default.

For example:

  This likely fails:
    $ git rev-parse --verify foobar
    fatal: Needed a single revision

  This likely succeeds but probably should fail:
    $ git rev-parse --verify --default refs/heads/master foobar
    b2e62a7dc6ba20a354d7590bf6a1d9264de7efe3

The documentation for rev-parse says:
   --default <arg>::
         If there is no parameter given by the user, use `<arg>`
         instead.

git-stash uses this command, so a typo could cause the wrong stash to be applied.

   git stash apply stsh@{1}

If stash@{0} exists, it will be applied which is definitely not what the user would want.

It is not straight forward to me how to modify builtin-rev-parse.c to fix this.

-brandon

Junio C Hamano· Jan 8, 2008, 00:21 UTC · re: Brandon Casey · lore

Re: git-rev-parse incorrectly uses --default parameter when invalid revision is supplied

Brandon Casey <casey@nrlssc.navy.mil> writes:

Show 31 quoted lines

> When rev-parse is called with an invalid revision parameter and
> --default has been used to set a default revision, the output
> will be based on the parameter to --default.
>
> For example:
>
>  This likely fails:
>    $ git rev-parse --verify foobar
>    fatal: Needed a single revision
>
>  This likely succeeds but probably should fail:
>    $ git rev-parse --verify --default refs/heads/master foobar
>    b2e62a7dc6ba20a354d7590bf6a1d9264de7efe3
>
>
> The documentation for rev-parse says:
>   --default <arg>::
>         If there is no parameter given by the user, use `<arg>`
>         instead.
>
>
> git-stash uses this command, so a typo could cause the wrong stash
> to be applied.
>
>   git stash apply stsh@{1}
>
> If stash@{0} exists, it will be applied which is definitely not
> what the user would want.
>
> It is not straight forward to me how to modify builtin-rev-parse.c
> to fix this.

Two points.

 * The description for --default you quoted says "no parameter",
   but it should read "no non-flag rev parameter".

 * --verify needs to be whole lot more special cased in the
   codepath;

Currently, --verify just runs the command as if it was not specified at all, but dies (1) as soon as it sees a second non-flag rev arg (worse --- it dies *after* emitting it!), or (2) at the end if it notices it did not emit any. It does not even barf if you say "git rev-parse --verify ^HEAD", which I think is a misdesign.