Re: Bug? Bad permissions in $PATH breaks Git aliases

From: Jeff King <peff@peff.net>
Date: Mar 27, 2012, 17:59 UTC
Message-ID: <20120327175933.GA1716@sigill.intra.peff.net>
In-Reply-To: <7vbonikrj4.fsf@alter.siamese.dyndns.org>

On Tue, Mar 27, 2012 at 08:11:27AM -0700, Junio C Hamano wrote:

Show 7 quoted lines

> > I think the general feeling last time this came up was "why not just
> > remove the cruft from your PATH?" But I would personally be OK with
> > option (3) above, and it is probably not that hard to implement.
> 
> http://thread.gmane.org/gmane.comp.version-control.git/171755/focus=171838
> shows that it was almost exactly a year ago; we tried (2) and nobody liked
> it.

I was actually thinking of:

  http://thread.gmane.org/gmane.comp.version-control.git/189077

> I got an impression from the discussion in it that #3 may give confusing
> messages to the end users, but I didn't think the issues through.

The implementation for #3 is straight-forward; I'll post the patches in a moment. However, it still ends up being confusing, because git ends up talking about permissions instead of offering its usual help.

Here are a few cases with stock git and no broken entries in PATH:

  (1)
  $ git does-not-exist
  git: 'does-not-exist' is not a git command. See 'git --help'.

  (2)
  $ git cerry-pick
  git: 'cerry-pick' is not a git command. See 'git --help'.

  Did you mean this?
          cherry-pick

  (3)
  $ git config alias.broken does-not-exist
  $ git broken
  Expansion of alias 'broken' failed; 'does-not-exist' is not a git command

  (4)
  $ git config alias.ok '!echo ok'
  $ git ok
  ok

Here are the same cases with a broken entry in PATH:

  $ mkdir foo; chmod 0 foo; PATH=$PWD/foo:$PATH

  (1)
  $ git does-not-exist
  fatal: cannot exec 'git-does-not-exist': Permission denied

  (2)
  $ git cerry-pick
  fatal: cannot exec 'git-cerry-pick': Permission denied

  (3)
  $ git broken
  fatal: cannot exec 'git-broken': Permission denied

  (4)
  $ git ok
  fatal: cannot exec 'git-ok': Permission denied

Case (1) is OK; we report the differing error. But case (2) is worse, as we don't offer suggestions any more. Cases (3) and (4) are both worse, because we don't even try to expand the alias (whether it would work or not).

Here are the same cases with my patches:

  (1)
  $ git does-not-exist
  Failed to run command 'does-not-exist': Permission denied

  (2)
  $ git cerry-pick
  Failed to run command 'cerry-pick': Permission denied

  (3)
  $ git broken
  Expansion of alias 'broken' failed; 'does-not-exist': Permission
  denied

  (4)
  $ git ok
  ok

This is somewhat improved. Case (4) now runs the alias. Case (3) has a better error message, which is that it tells you it was not "broken" which was a problem, but its subcommand. But the "permission denied" error still ends up being somewhat confusing. And in case (2), you don't get a list of suggestions (nor should you, because we still don't know whether "cerry-pick" exists and cannot be executed, or if there is a broken directory in the PATH).

So we've made the situation better, but it's still way less nice than having a fixed PATH. Which makes me wonder if this half-way effort is worth it.

-Peff