Re: [PATCH 13/21] rev-parse: add '--absolute-git-dir' option

[Jeff King <peff@peff.net>]

On Mon, Apr 25, 2016 at 10:33:13PM -0400, Mike Rappazzo wrote:

> I propose that it might make more sense to use something like
> `--abs-path` to indicate
> that the result should include an absolute path (or we could also just spell out
> `--absolute-path`).  That way we don't have to add additional options
> for any other type
> that might want an absolute path.
> 
>     git rev-parse --git-dir --abs-path
>     git rev-parse --git-common-dir --absolute-path
> 
> I do understand that this might be more work than is necessary for the
> completion series
> here.  Would it be unreasonable to suggest a partial implementation
> that, for now, only
> works with `--git-dir`?

I do like the concept of keeping "--absolute-path" orthogonal. The only
trick is that we need to either support it for all appropriate options,
or document which options it _does_ work with. Otherwise, we're going to
get bug reports when somebody tries "--absolute-path --git-common-dir".

It would be cleaner to provide a separate option to let people compose
the options, like:

  git rev-parse --git-dir | git rev-parse --realpath

but that's a lot less efficient.

> > +                                       if (gitdir) {
> > +                                               char absolute_path[PATH_MAX];
> > +                                               if (!realpath(gitdir, absolute_path))
> > +                                                       die_errno(_("unable to get absolute path"));
> > +                                               puts(absolute_path);
> > +                                               continue;
> > +                                       }

I don't recall if this came up in earlier review, but I happened to
notice the use of realpath() here. We should be using our custom
real_path() instead. There are some platforms without realpath(), I
think, and our real_path() is not limited to the static PATH_MAX (which
is too small on some platforms).

-Peff