On Thu, Feb 25, 2016 at 5:54 PM SZEDER Gábor <szeder@xxxxxxxxxx> wrote:
>
> Some scripts can benefit from not having to deal with the possibility
> of relative paths to the repository, but the output of 'git rev-parse
> --git-dir' can be a relative path. Case in point: supporting 'git -C
> <path>' in our Bash completion script turned out to be considerably
> more difficult, error prone and required more subshells and git
> processes when we had to cope with a relative path to the .git
> directory.
>
> Help these use cases and teach 'git rev-parse' a new
> '--absolute-git-dir' option which always outputs a canonicalized
> absolute path to the .git directory, regardless of whether the path is
> discovered automatically or is specified via $GIT_DIR or 'git
> --git-dir=<path>'.
>
> Signed-off-by: SZEDER Gábor <szeder@xxxxxxxxxx>
> ---
> Documentation/git-rev-parse.txt | 4 ++++
> builtin/rev-parse.c | 29 +++++++++++++++++++++--------
> t/t1500-rev-parse.sh | 17 ++++++++++-------
> 3 files changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
> index b6c6326cdc7b..fb06e3118570 100644
> --- a/Documentation/git-rev-parse.txt
> +++ b/Documentation/git-rev-parse.txt
> @@ -216,6 +216,10 @@ If `$GIT_DIR` is not defined and the current directory
> is not detected to lie in a Git repository or work tree
> print a message to stderr and exit with nonzero status.
>
> +--absolute-git-dir::
> + Like `--git-dir`, but its output is always the canonicalized
> + absolute path.
> +
> --git-common-dir::
> Show `$GIT_COMMON_DIR` if defined, else `$GIT_DIR`.
>
After working a little bit with rev-parse [1], I feel that this might
be better served as a
stand-alone option. Consider that in addition to --git-dir, the
options --git-common-dir,
--git-path, and --git-shared-index produce relative paths.
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`?
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index cf8487b3b95f..90a4dd6032c0 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -744,17 +744,30 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
> putchar('\n');
> continue;
> }
> - if (!strcmp(arg, "--git-dir")) {
> + if (!strcmp(arg, "--git-dir") ||
> + !strcmp(arg, "--absolute-git-dir")) {
> const char *gitdir = getenv(GIT_DIR_ENVIRONMENT);
> char *cwd;
> int len;
> - if (gitdir) {
> - puts(gitdir);
> - continue;
> - }
> - if (!prefix) {
> - puts(".git");
> - continue;
> + if (arg[2] == 'g') { /* --git-dir */
> + if (gitdir) {
> + puts(gitdir);
> + continue;
> + }
> + if (!prefix) {
> + puts(".git");
> + continue;
> + }
> + } else { /* --absolute-git-dir */
> + if (!gitdir && !prefix)
> + gitdir = ".git";
> + if (gitdir) {
> + char absolute_path[PATH_MAX];
> + if (!realpath(gitdir, absolute_path))
> + die_errno(_("unable to get absolute path"));
> + puts(absolute_path);
> + continue;
> + }
> }
> cwd = xgetcwd();
> len = strlen(cwd);
> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
> index 48ee07779d64..617fcd821309 100755
> --- a/t/t1500-rev-parse.sh
> +++ b/t/t1500-rev-parse.sh
> @@ -31,23 +31,26 @@ test_rev_parse() {
> "test '$1' = \"\$(git rev-parse --git-dir)\""
> shift
> [ $# -eq 0 ] && return
> +
> + test_expect_success "$name: absolute-git-dir" \
> + "verbose test '$1' = \"\$(git rev-parse --absolute-git-dir)\""
> }
>
> -# label is-bare is-inside-git is-inside-work prefix git-dir
> +# label is-bare is-inside-git is-inside-work prefix git-dir absolute-git-dir
>
> ROOT=$(pwd)
>
> -test_rev_parse toplevel false false true '' .git
> +test_rev_parse toplevel false false true '' .git "$ROOT/.git"
>
> cd .git || exit 1
> -test_rev_parse .git/ false true false '' .
> +test_rev_parse .git/ false true false '' . "$ROOT/.git"
> cd objects || exit 1
> -test_rev_parse .git/objects/ false true false '' "$ROOT/.git"
> +test_rev_parse .git/objects/ false true false '' "$ROOT/.git" "$ROOT/.git"
> cd ../.. || exit 1
>
> mkdir -p sub/dir || exit 1
> cd sub/dir || exit 1
> -test_rev_parse subdirectory false false true sub/dir/ "$ROOT/.git"
> +test_rev_parse subdirectory false false true sub/dir/ "$ROOT/.git" "$ROOT/.git"
> cd ../.. || exit 1
>
> git config core.bare true
> @@ -63,7 +66,7 @@ GIT_CONFIG="$(pwd)"/../.git/config
> export GIT_DIR GIT_CONFIG
>
> git config core.bare false
> -test_rev_parse 'GIT_DIR=../.git, core.bare = false' false false true ''
> +test_rev_parse 'GIT_DIR=../.git, core.bare = false' false false true '' "../.git" "$ROOT/.git"
>
> git config core.bare true
> test_rev_parse 'GIT_DIR=../.git, core.bare = true' true false false ''
> @@ -76,7 +79,7 @@ GIT_DIR=../repo.git
> GIT_CONFIG="$(pwd)"/../repo.git/config
>
> git config core.bare false
> -test_rev_parse 'GIT_DIR=../repo.git, core.bare = false' false false true ''
> +test_rev_parse 'GIT_DIR=../repo.git, core.bare = false' false false true '' "../repo.git" "$ROOT/repo.git"
>
> git config core.bare true
> test_rev_parse 'GIT_DIR=../repo.git, core.bare = true' true false false ''
> --
> 2.7.2.410.g92cb358
>
> --
[1] http://thread.gmane.org/gmane.comp.version-control.git/292272
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html