From: Mike Rappazzo <rappazzo@gmail.com>
To: "SZEDER Gábor" <szeder@ira.uka.de>, "Junio C Hamano" <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>,
Sebastian Schuberth <sschuberth@gmail.com>,
Git List <git@vger.kernel.org>
Subject: Re: [PATCH 13/21] rev-parse: add '--absolute-git-dir' option
Date: Mon, 25 Apr 2016 22:33:13 -0400 [thread overview]
Message-ID: <CANoM8SXO_Rz_CVOz9ptsaVCzcQ2D1FQrSuFFW4vZ4SdRYMzD=w@mail.gmail.com> (raw)
In-Reply-To: <1456440650-32623-14-git-send-email-szeder@ira.uka.de>
On Thu, Feb 25, 2016 at 5:54 PM SZEDER Gábor <szeder@ira.uka.de> 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@ira.uka.de>
> ---
> 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
next prev parent reply other threads:[~2016-04-26 2:33 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-25 22:50 [PATCH 00/21] completion: __gitdir()-related improvements SZEDER Gábor
2016-02-25 22:50 ` [PATCH 01/21] completion: improve __git_refs()'s in-code documentation SZEDER Gábor
2016-02-25 22:50 ` [PATCH 02/21] completion tests: don't add test cruft to the test repository SZEDER Gábor
2016-02-25 22:50 ` [PATCH 03/21] completion tests: make the $cur variable local to the test helper functions SZEDER Gábor
2016-02-25 22:50 ` [PATCH 04/21] completion tests: consolidate getting path of current working directory SZEDER Gábor
2016-02-25 22:50 ` [PATCH 05/21] completion tests: check __gitdir()'s output in the error cases SZEDER Gábor
2016-02-25 22:50 ` [PATCH 06/21] completion tests: add tests for the __git_refs() helper function SZEDER Gábor
2016-02-25 22:50 ` [PATCH 07/21] completion: ensure that the repository path given on the command line exists SZEDER Gábor
2016-02-25 22:50 ` [PATCH 08/21] completion: fix most spots not respecting 'git --git-dir=<path>' SZEDER Gábor
2016-02-25 22:50 ` [PATCH 09/21] completion: respect 'git --git-dir=<path>' when listing remote refs SZEDER Gábor
2016-02-25 22:50 ` [PATCH 10/21] completion: list refs from remote when remote's name matches a directory SZEDER Gábor
2016-02-25 22:50 ` [PATCH 11/21] completion: don't list 'HEAD' when trying refs completion outside of a repo SZEDER Gábor
2016-02-25 22:50 ` [PATCH 12/21] completion: list short refs from a remote given as a URL SZEDER Gábor
2016-02-25 22:50 ` [PATCH 13/21] rev-parse: add '--absolute-git-dir' option SZEDER Gábor
2016-04-26 2:33 ` Mike Rappazzo [this message]
2016-04-26 5:35 ` Jeff King
2016-05-18 16:58 ` SZEDER Gábor
2016-02-25 22:50 ` [PATCH 14/21] completion: don't offer commands when 'git --opt' needs an argument SZEDER Gábor
2016-02-25 22:50 ` [PATCH 15/21] completion: fix completion after 'git -C <path>' SZEDER Gábor
2016-02-25 22:50 ` [PATCH 16/21] completion: respect " SZEDER Gábor
2016-02-25 22:50 ` [PATCH 17/21] completion: don't use __gitdir() for git commands SZEDER Gábor
2016-02-25 22:50 ` [PATCH 18/21] completion: consolidate silencing errors from " SZEDER Gábor
2016-02-25 22:50 ` [PATCH 19/21] completion: don't guard git executions with __gitdir() SZEDER Gábor
2016-02-25 22:50 ` [PATCH 20/21] completion: extract repository discovery from __gitdir() SZEDER Gábor
2016-02-25 22:50 ` [PATCH 21/21] completion: cache the path to the repository SZEDER Gábor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CANoM8SXO_Rz_CVOz9ptsaVCzcQ2D1FQrSuFFW4vZ4SdRYMzD=w@mail.gmail.com' \
--to=rappazzo@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
--cc=sschuberth@gmail.com \
--cc=szeder@ira.uka.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).