git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: "brian m. carlson" <sandals@crustytoothpaste.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] rev-parse: add option for absolute or relative path formatting
Date: Wed, 9 Sep 2020 05:23:32 +0200 (CEST)	[thread overview]
Message-ID: <nycvar.QRO.7.76.6.2009090513300.54@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <20200908185017.2005159-1-sandals@crustytoothpaste.net>

Hi brian,

On Tue, 8 Sep 2020, brian m. carlson wrote:

> git rev-parse has several options which print various paths.  Some of
> these paths are printed relative to the current working directory, and
> some are absolute.
>
> Normally, this is not a problem, but there are times when one wants
> paths entirely in one format or another.  This can be done trivially if
> the paths are canonical, but canonicalizing paths is not possible on
> some shell scripting environments which lack realpath(1) and also in Go,
> which lacks functions that properly canonicalize paths on Windows.
>
> To help out the scripter, let's provide an option which turns most of
> the paths printed by git rev-parse to be either relative to the current
> working directory or absolute and canonical.  Document which options are
> affected and which are not so that users are not confused.
>
> This approach is cleaner and tidier than providing duplicates of
> existing options which are either relative or absolute.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---

Nicely explained and implemented.

> This impetus for this patch is Git LFS, which is written in Go.  Go
> lacks a cross-platform way to canonicalize paths in the same way that
> Git does, so when Git produces relative paths, such as in some cases
> with --git-common-dir, we end up with problems when users are doing
> things with unusual paths on Windows, such as when using SUBST or
> OneDrive or various other edge cases.  Go upstream does not appear eager
> to address this problem.
>
> The obvious solution to this is to have Git canonicalize all paths, and
> rather than adding yet another one-off, I decided to implement a more
> generic solution.  This can also be valuable for shell scripting
> environments which lack realpath(1) (e.g. macOS, IIRC).
>
> Dscho CC'd for visibility into how this will work on Windows.

On Windows, at least in the version from git-for-windows/git,
`strbuf_realpath()` uses the Win32 API function
`GetFinalPathNameByHandleW()` to canonicalize paths (whenever possible),
meaning that the `subst` issue you mentioned above will be addressed
adequately.

Note: this platform-specific real path handling has not yet made it
upstream, it is still "battle-tested" in Git for Windows.

Thanks,
Dscho

>
>  Documentation/git-rev-parse.txt | 71 ++++++++++++++++------------
>  builtin/rev-parse.c             | 84 ++++++++++++++++++++++++++++-----
>  t/t1500-rev-parse.sh            | 36 +++++++++++++-
>  3 files changed, 148 insertions(+), 43 deletions(-)
>
> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
> index 19b12b6d43..6b95292559 100644
> --- a/Documentation/git-rev-parse.txt
> +++ b/Documentation/git-rev-parse.txt
> @@ -208,6 +208,15 @@ Options for Files
>  	Only the names of the variables are listed, not their value,
>  	even if they are set.
>
> +--path-format=(absolute|relative)::
> +	Controls the behavior of certain other options following it on the command
> +	line. If specified as absolute, the paths printed by those options will be
> +	absolute and canonical. If specified as relative, the paths will be relative
> +	to the current working directory if that is possible.  The default is option
> +	specific.
> +
> +The following options are modified by `--path-format`:
> +
>  --git-dir::
>  	Show `$GIT_DIR` if defined. Otherwise show the path to
>  	the .git directory. The path shown, when relative, is
> @@ -217,13 +226,42 @@ 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.
>
> +--git-common-dir::
> +	Show `$GIT_COMMON_DIR` if defined, else `$GIT_DIR`.
> +
> +--resolve-git-dir <path>::
> +	Check if <path> is a valid repository or a gitfile that
> +	points at a valid repository, and print the location of the
> +	repository.  If <path> is a gitfile then the resolved path
> +	to the real repository is printed.
> +
> +--git-path <path>::
> +	Resolve "$GIT_DIR/<path>" and takes other path relocation
> +	variables such as $GIT_OBJECT_DIRECTORY,
> +	$GIT_INDEX_FILE... into account. For example, if
> +	$GIT_OBJECT_DIRECTORY is set to /foo/bar then "git rev-parse
> +	--git-path objects/abc" returns /foo/bar/abc.
> +
> +--show-toplevel::
> +	Show the (by default, absolute) path of the top-level directory
> +	of the working tree. If there is no working tree, report an error.
> +
> +--show-superproject-working-tree::
> +	Show the absolute path of the root of the superproject's
> +	working tree (if exists) that uses the current repository as
> +	its submodule.  Outputs nothing if the current repository is
> +	not used as a submodule by any project.
> +
> +--shared-index-path::
> +	Show the path to the shared index file in split index mode, or
> +	empty if not in split-index mode.
> +
> +The following options are unaffected by `--path-format`:
> +
>  --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`.
> -
>  --is-inside-git-dir::
>  	When the current working directory is below the repository
>  	directory print "true", otherwise "false".
> @@ -238,19 +276,6 @@ print a message to stderr and exit with nonzero status.
>  --is-shallow-repository::
>  	When the repository is shallow print "true", otherwise "false".
>
> ---resolve-git-dir <path>::
> -	Check if <path> is a valid repository or a gitfile that
> -	points at a valid repository, and print the location of the
> -	repository.  If <path> is a gitfile then the resolved path
> -	to the real repository is printed.
> -
> ---git-path <path>::
> -	Resolve "$GIT_DIR/<path>" and takes other path relocation
> -	variables such as $GIT_OBJECT_DIRECTORY,
> -	$GIT_INDEX_FILE... into account. For example, if
> -	$GIT_OBJECT_DIRECTORY is set to /foo/bar then "git rev-parse
> -	--git-path objects/abc" returns /foo/bar/abc.
> -
>  --show-cdup::
>  	When the command is invoked from a subdirectory, show the
>  	path of the top-level directory relative to the current
> @@ -261,20 +286,6 @@ print a message to stderr and exit with nonzero status.
>  	path of the current directory relative to the top-level
>  	directory.
>
> ---show-toplevel::
> -	Show the absolute path of the top-level directory of the working
> -	tree. If there is no working tree, report an error.
> -
> ---show-superproject-working-tree::
> -	Show the absolute path of the root of the superproject's
> -	working tree (if exists) that uses the current repository as
> -	its submodule.  Outputs nothing if the current repository is
> -	not used as a submodule by any project.
> -
> ---shared-index-path::
> -	Show the path to the shared index file in split index mode, or
> -	empty if not in split-index mode.
> -
>  --show-object-format[=(storage|input|output)]::
>  	Show the object format (hash algorithm) used for the repository
>  	for storage inside the `.git` directory, input, or output. For
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index 669dd2fd6f..311a56ba01 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -583,6 +583,55 @@ static void handle_ref_opt(const char *pattern, const char *prefix)
>  	clear_ref_exclusion(&ref_excludes);
>  }
>
> +enum format_type {
> +	/* We would like a relative path. */
> +	FORMAT_RELATIVE,
> +	/* We would like a canonical absolute path. */
> +	FORMAT_CANONICAL,
> +	/* We would like the default behavior. */
> +	FORMAT_DEFAULT,
> +};
> +
> +enum default_type {
> +	/* Our default is a relative path. */
> +	DEFAULT_RELATIVE,
> +	/* Our default is a relative path if there's a shared root. */
> +	DEFAULT_RELATIVE_IF_SHARED,
> +	/* Our default is a canonical absolute path. */
> +	DEFAULT_CANONICAL,
> +	/* Our default is not to modify the item. */
> +	DEFAULT_UNMODIFIED,
> +};
> +
> +static void print_path(const char *path, const char *prefix, enum format_type format, enum default_type def)
> +{
> +	char *cwd = NULL;
> +	/*
> +	 * We don't ever produce a relative path if prefix is NULL, so set the
> +	 * prefix to the current directory so that we can produce a relative
> +	 * path whenever possible.  If we're using RELATIVE_IF_SHARED mode, then
> +	 * we want an absolute path unless the two share a common prefix, so don't
> +	 * set it in that case, since doing so causes a relative path to always
> +	 * be produced if possible.
> +	 */
> +	if (!prefix && (format != FORMAT_DEFAULT || def != DEFAULT_RELATIVE_IF_SHARED))
> +		prefix = cwd = xgetcwd();
> +	if (format == FORMAT_DEFAULT && def == DEFAULT_UNMODIFIED) {
> +		puts(path);
> +	} else if (format == FORMAT_RELATIVE ||
> +		  (format == FORMAT_DEFAULT && def == DEFAULT_RELATIVE) ||
> +		  (format == FORMAT_DEFAULT && def == DEFAULT_RELATIVE_IF_SHARED)) {
> +		struct strbuf buf = STRBUF_INIT;
> +		puts(relative_path(path, prefix, &buf));
> +		strbuf_reset(&buf);
> +	} else {
> +		char *real = real_pathdup(path, 1);
> +		puts(real);
> +		free(real);
> +	}
> +	free(cwd);
> +}
> +
>  int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>  {
>  	int i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;
> @@ -595,6 +644,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>  	struct object_context unused;
>  	struct strbuf buf = STRBUF_INIT;
>  	const int hexsz = the_hash_algo->hexsz;
> +	enum format_type format = FORMAT_DEFAULT;
>
>  	if (argc > 1 && !strcmp("--parseopt", argv[1]))
>  		return cmd_parseopt(argc - 1, argv + 1, prefix);
> @@ -650,8 +700,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>  			if (!argv[i + 1])
>  				die("--git-path requires an argument");
>  			strbuf_reset(&buf);
> -			puts(relative_path(git_path("%s", argv[i + 1]),
> -					   prefix, &buf));
> +			print_path(git_path("%s", argv[i + 1]), prefix, format, DEFAULT_RELATIVE_IF_SHARED);
>  			i++;
>  			continue;
>  		}
> @@ -683,6 +732,16 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>  					show_file(arg, 0);
>  				continue;
>  			}
> +			if (opt_with_value(arg, "--path-format", &arg)) {
> +				if (!strcmp(arg, "absolute")) {
> +					format = FORMAT_CANONICAL;
> +				} else if (!strcmp(arg, "relative")) {
> +					format = FORMAT_RELATIVE;
> +				} else {
> +					die("unknown argument to --path-format: %s", arg);
> +				}
> +				continue;
> +			}
>  			if (!strcmp(arg, "--default")) {
>  				def = argv[++i];
>  				if (!def)
> @@ -803,7 +862,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>  			if (!strcmp(arg, "--show-toplevel")) {
>  				const char *work_tree = get_git_work_tree();
>  				if (work_tree)
> -					puts(work_tree);
> +					print_path(work_tree, prefix, format, DEFAULT_CANONICAL);
>  				else
>  					die("this operation must be run in a work tree");
>  				continue;
> @@ -811,7 +870,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>  			if (!strcmp(arg, "--show-superproject-working-tree")) {
>  				struct strbuf superproject = STRBUF_INIT;
>  				if (get_superproject_working_tree(&superproject))
> -					puts(superproject.buf);
> +					print_path(superproject.buf, prefix, format, DEFAULT_CANONICAL);
>  				strbuf_release(&superproject);
>  				continue;
>  			}
> @@ -846,16 +905,18 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>  				const char *gitdir = getenv(GIT_DIR_ENVIRONMENT);
>  				char *cwd;
>  				int len;
> +				enum format_type wanted = format;
>  				if (arg[2] == 'g') {	/* --git-dir */
>  					if (gitdir) {
> -						puts(gitdir);
> +						print_path(gitdir, prefix, format, DEFAULT_UNMODIFIED);
>  						continue;
>  					}
>  					if (!prefix) {
> -						puts(".git");
> +						print_path(".git", prefix, format, DEFAULT_UNMODIFIED);
>  						continue;
>  					}
>  				} else {		/* --absolute-git-dir */
> +					wanted = FORMAT_CANONICAL;
>  					if (!gitdir && !prefix)
>  						gitdir = ".git";
>  					if (gitdir) {
> @@ -868,14 +929,14 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>  				}
>  				cwd = xgetcwd();
>  				len = strlen(cwd);
> -				printf("%s%s.git\n", cwd, len && cwd[len-1] != '/' ? "/" : "");
> +				strbuf_reset(&buf);
> +				strbuf_addf(&buf, "%s%s.git", cwd, len && cwd[len-1] != '/' ? "/" : "");
>  				free(cwd);
> +				print_path(buf.buf, prefix, wanted, DEFAULT_CANONICAL);
>  				continue;
>  			}
>  			if (!strcmp(arg, "--git-common-dir")) {
> -				strbuf_reset(&buf);
> -				puts(relative_path(get_git_common_dir(),
> -						   prefix, &buf));
> +				print_path(get_git_common_dir(), prefix, format, DEFAULT_RELATIVE_IF_SHARED);
>  				continue;
>  			}
>  			if (!strcmp(arg, "--is-inside-git-dir")) {
> @@ -905,8 +966,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>  				if (the_index.split_index) {
>  					const struct object_id *oid = &the_index.split_index->base_oid;
>  					const char *path = git_path("sharedindex.%s", oid_to_hex(oid));
> -					strbuf_reset(&buf);
> -					puts(relative_path(path, prefix, &buf));
> +					print_path(path, prefix, format, DEFAULT_RELATIVE);
>  				}
>  				continue;
>  			}
> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
> index 408b97d5af..6f3811d189 100755
> --- a/t/t1500-rev-parse.sh
> +++ b/t/t1500-rev-parse.sh
> @@ -3,6 +3,16 @@
>  test_description='test git rev-parse'
>  . ./test-lib.sh
>
> +test_one () {
> +	dir="$1" &&
> +	expect="$2" &&
> +	shift &&
> +	shift &&
> +	echo "$expect" >expect &&
> +	git -C "$dir" rev-parse "$@" >actual
> +	test_cmp expect actual
> +}
> +
>  # usage: [options] label is-bare is-inside-git is-inside-work prefix git-dir absolute-git-dir
>  test_rev_parse () {
>  	d=
> @@ -60,7 +70,13 @@ ROOT=$(pwd)
>
>  test_expect_success 'setup' '
>  	mkdir -p sub/dir work &&
> -	cp -R .git repo.git
> +	cp -R .git repo.git &&
> +	git checkout -b main &&
> +	test_commit abc &&
> +	git checkout -b side &&
> +	test_commit def &&
> +	git checkout main &&
> +	git worktree add worktree side
>  '
>
>  test_rev_parse toplevel false false true '' .git "$ROOT/.git"
> @@ -88,6 +104,24 @@ test_rev_parse -C work -g ../repo.git -b t 'GIT_DIR=../repo.git, core.bare = tru
>
>  test_rev_parse -C work -g ../repo.git -b u 'GIT_DIR=../repo.git, core.bare undefined' false false true ''
>
> +test_expect_success 'rev-parse --path-format=absolute' '
> +	test_one "." "$ROOT/.git" --path-format=absolute --git-dir &&
> +	test_one "." "$ROOT/.git" --path-format=absolute --git-common-dir &&
> +	test_one "worktree" "$ROOT/.git/worktrees/worktree" --path-format=absolute --git-dir &&
> +	test_one "worktree" "$ROOT/.git" --path-format=absolute --git-common-dir &&
> +	test_one "." "$ROOT" --path-format=absolute --show-toplevel &&
> +	test_one "." "$ROOT/.git/objects" --path-format=absolute --git-path objects
> +'
> +
> +test_expect_success 'rev-parse --path-format=relative' '
> +	test_one "." ".git" --path-format=relative --git-dir &&
> +	test_one "." ".git" --path-format=relative --git-common-dir &&
> +	test_one "worktree" "../.git/worktrees/worktree" --path-format=relative --git-dir &&
> +	test_one "worktree" "../.git" --path-format=relative --git-common-dir &&
> +	test_one "." "./" --path-format=relative --show-toplevel &&
> +	test_one "." ".git/objects" --path-format=relative --git-path objects
> +'
> +
>  test_expect_success 'git-common-dir from worktree root' '
>  	echo .git >expect &&
>  	git rev-parse --git-common-dir >actual &&
>

  reply	other threads:[~2020-09-09  8:05 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-08 18:50 [PATCH] rev-parse: add option for absolute or relative path formatting brian m. carlson
2020-09-09  3:23 ` Johannes Schindelin [this message]
2020-09-09 22:31   ` brian m. carlson
2020-09-09 14:51 ` SZEDER Gábor
2020-09-09 22:23   ` brian m. carlson
2020-09-10 15:19     ` SZEDER Gábor
2020-09-11  0:03       ` brian m. carlson
2020-11-04 22:16 ` Jonathan Nieder
2020-11-05  3:11   ` brian m. carlson
2020-11-06  0:51     ` Jonathan Nieder
2020-11-06  1:57       ` brian m. carlson

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=nycvar.QRO.7.76.6.2009090513300.54@tvgsbejvaqbjf.bet \
    --to=johannes.schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=sandals@crustytoothpaste.net \
    /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).