{"thread":{"id":"54777","subject":"[PATCH v4 1/2] abspath: add a function to resolve paths with missing components","startedAt":"2020-12-06T22:56:30Z","lastAt":"2020-12-08T02:52:37Z","messageCount":9,"participants":["brian m. carlson","Eric Sunshine","René Scharfe"],"isPatch":true,"patchVersion":4,"patchTotal":2},"messages":[{"id":"411567","messageId":"20201206225349.3392790-2-sandals@crustytoothpaste.net","threadId":"54777","inReplyTo":"20201206225349.3392790-1-sandals@crustytoothpaste.net","subject":"[PATCH v4 1/2] abspath: add a function to resolve paths with missing components","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-12-06T22:53:48Z","receivedAt":"2020-12-06T22:56:30Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"We'd like to canonicalize paths such that we can preserve any number of\ntrailing components that may be missing.  Let's add a function to do\nthat, allowing either one or an unlimited number of components to\ncanonicalize, and make strbuf_realpath a wrapper around it that allows\njust one component.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n abspath.c | 13 ++++++++++++-\n cache.h   |  2 ++\n 2 files changed, 14 insertions(+), 1 deletion(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex 6f15a418bb..e6630b3618 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -80,6 +80,17 @@ static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)\n  */\n char *strbuf_realpath(struct strbuf *resolved, const char *path,\n \t\t      int die_on_error)\n+{\n+\treturn strbuf_realpath_missing(resolved, path, 0, die_on_error);\n+}\n+\n+/*\n+ * Just like strbuf_realpath, but allows specifying how many missing components\n+ * are permitted.  If many_missing is true, an arbitrary number of path\n+ * components may be missing; otherwise, only the last component may be missing.\n+ */\n+char *strbuf_realpath_missing(struct strbuf *resolved, const char *path,\n+\t\t\t      int many_missing, int die_on_error)\n {\n \tstruct strbuf remaining = STRBUF_INIT;\n \tstruct strbuf next = STRBUF_INIT;\n@@ -129,7 +140,7 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,\n \n \t\tif (lstat(resolved->buf, &st)) {\n \t\t\t/* error out unless this was the last component */\n-\t\t\tif (errno != ENOENT || remaining.len) {\n+\t\t\tif (errno != ENOENT || (!many_missing && remaining.len)) {\n \t\t\t\tif (die_on_error)\n \t\t\t\t\tdie_errno(\"Invalid path '%s'\",\n \t\t\t\t\t\t  resolved->buf);\ndiff --git a/cache.h b/cache.h\nindex e986cf4ea9..a1386235fc 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1320,6 +1320,8 @@ static inline int is_absolute_path(const char *path)\n int is_directory(const char *);\n char *strbuf_realpath(struct strbuf *resolved, const char *path,\n \t\t      int die_on_error);\n+char *strbuf_realpath_missing(struct strbuf *resolved, const char *path,\n+\t\t\t      int missing_components, int die_on_error);\n char *real_pathdup(const char *path, int die_on_error);\n const char *absolute_path(const char *path);\n char *absolute_pathdup(const char *path);\n"},{"id":"411568","messageId":"20201206225349.3392790-1-sandals@crustytoothpaste.net","threadId":"54777","inReplyTo":null,"subject":"[PATCH v4 0/2] rev-parse options for absolute or relative paths","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-12-06T22:53:47Z","receivedAt":"2020-12-06T22:56:31Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"There are a bunch of different situations in which one would like to\nhave an absolute and canonical or a relative path from Git.  In many of\nthese cases, these values are already available from git rev-parse, but\nsome values only come in one form or another.\n\nChanges from v3:\n\n* Switch to a binary option instead of a number of missing path\n  components to skip.\n\nChanges from v2:\n\n* Incorporate multiple missing segment support into the strbuf_realpath\n  code.\n* Switch some invocations to use DEFAULT_UNMODIFIED, which should not\n  result in a change in behavior.\n* Rebase, resolving some conflicts.\n\nChanges from v1:\n\n* Add a function to handle missing trailing components when\n  canonicalizing paths and use it.\n* Improve commit messages.\n* Fix broken && chain.\n* Fix situation where relative paths are not relative.\n\nbrian m. carlson (2):\n  abspath: add a function to resolve paths with missing components\n  rev-parse: add option for absolute or relative path formatting\n\n Documentation/git-rev-parse.txt |  71 +++++++++++++---------\n abspath.c                       |  13 +++-\n builtin/rev-parse.c             | 104 ++++++++++++++++++++++++++++----\n cache.h                         |   2 +\n t/t1500-rev-parse.sh            |  57 ++++++++++++++++-\n 5 files changed, 203 insertions(+), 44 deletions(-)\n\n"},{"id":"411569","messageId":"20201206225349.3392790-3-sandals@crustytoothpaste.net","threadId":"54777","inReplyTo":"20201206225349.3392790-1-sandals@crustytoothpaste.net","subject":"[PATCH v4 2/2] rev-parse: add option for absolute or relative path formatting","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-12-06T22:53:49Z","receivedAt":"2020-12-06T22:57:35Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"git rev-parse has several options which print various paths.  Some of\nthese paths are printed relative to the current working directory, and\nsome are absolute.\n\nNormally, this is not a problem, but there are times when one wants\npaths entirely in one format or another.  This can be done trivially if\nthe paths are canonical, but canonicalizing paths is not possible on\nsome shell scripting environments which lack realpath(1) and also in Go,\nwhich lacks functions that properly canonicalize paths on Windows.\n\nTo help out the scripter, let's provide an option which turns most of\nthe paths printed by git rev-parse to be either relative to the current\nworking directory or absolute and canonical.  Document which options are\naffected and which are not so that users are not confused.\n\nThis approach is cleaner and tidier than providing duplicates of\nexisting options which are either relative or absolute.\n\nNote that if the user needs both forms, it is possible to pass an\nadditional option in the middle of the command line which changes the\nbehavior of subsequent operations.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n Documentation/git-rev-parse.txt |  71 +++++++++++++---------\n builtin/rev-parse.c             | 104 ++++++++++++++++++++++++++++----\n t/t1500-rev-parse.sh            |  57 ++++++++++++++++-\n 3 files changed, 189 insertions(+), 43 deletions(-)\n\ndiff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt\nindex 5013daa6ef..1708af8a66 100644\n--- a/Documentation/git-rev-parse.txt\n+++ b/Documentation/git-rev-parse.txt\n@@ -212,6 +212,15 @@ Options for Files\n \tOnly the names of the variables are listed, not their value,\n \teven if they are set.\n \n+--path-format=(absolute|relative)::\n+\tControls the behavior of certain other options following it on the command\n+\tline. If specified as absolute, the paths printed by those options will be\n+\tabsolute and canonical. If specified as relative, the paths will be relative\n+\tto the current working directory if that is possible.  The default is option\n+\tspecific.\n+\n+The following options are modified by `--path-format`:\n+\n --git-dir::\n \tShow `$GIT_DIR` if defined. Otherwise show the path to\n \tthe .git directory. The path shown, when relative, is\n@@ -221,13 +230,42 @@ If `$GIT_DIR` is not defined and the current directory\n is not detected to lie in a Git repository or work tree\n print a message to stderr and exit with nonzero status.\n \n+--git-common-dir::\n+\tShow `$GIT_COMMON_DIR` if defined, else `$GIT_DIR`.\n+\n+--resolve-git-dir <path>::\n+\tCheck if <path> is a valid repository or a gitfile that\n+\tpoints at a valid repository, and print the location of the\n+\trepository.  If <path> is a gitfile then the resolved path\n+\tto the real repository is printed.\n+\n+--git-path <path>::\n+\tResolve \"$GIT_DIR/<path>\" and takes other path relocation\n+\tvariables such as $GIT_OBJECT_DIRECTORY,\n+\t$GIT_INDEX_FILE... into account. For example, if\n+\t$GIT_OBJECT_DIRECTORY is set to /foo/bar then \"git rev-parse\n+\t--git-path objects/abc\" returns /foo/bar/abc.\n+\n+--show-toplevel::\n+\tShow the (by default, absolute) path of the top-level directory\n+\tof the working tree. If there is no working tree, report an error.\n+\n+--show-superproject-working-tree::\n+\tShow the absolute path of the root of the superproject's\n+\tworking tree (if exists) that uses the current repository as\n+\tits submodule.  Outputs nothing if the current repository is\n+\tnot used as a submodule by any project.\n+\n+--shared-index-path::\n+\tShow the path to the shared index file in split index mode, or\n+\tempty if not in split-index mode.\n+\n+The following options are unaffected by `--path-format`:\n+\n --absolute-git-dir::\n \tLike `--git-dir`, but its output is always the canonicalized\n \tabsolute path.\n \n---git-common-dir::\n-\tShow `$GIT_COMMON_DIR` if defined, else `$GIT_DIR`.\n-\n --is-inside-git-dir::\n \tWhen the current working directory is below the repository\n \tdirectory print \"true\", otherwise \"false\".\n@@ -242,19 +280,6 @@ print a message to stderr and exit with nonzero status.\n --is-shallow-repository::\n \tWhen the repository is shallow print \"true\", otherwise \"false\".\n \n---resolve-git-dir <path>::\n-\tCheck if <path> is a valid repository or a gitfile that\n-\tpoints at a valid repository, and print the location of the\n-\trepository.  If <path> is a gitfile then the resolved path\n-\tto the real repository is printed.\n-\n---git-path <path>::\n-\tResolve \"$GIT_DIR/<path>\" and takes other path relocation\n-\tvariables such as $GIT_OBJECT_DIRECTORY,\n-\t$GIT_INDEX_FILE... into account. For example, if\n-\t$GIT_OBJECT_DIRECTORY is set to /foo/bar then \"git rev-parse\n-\t--git-path objects/abc\" returns /foo/bar/abc.\n-\n --show-cdup::\n \tWhen the command is invoked from a subdirectory, show the\n \tpath of the top-level directory relative to the current\n@@ -265,20 +290,6 @@ print a message to stderr and exit with nonzero status.\n \tpath of the current directory relative to the top-level\n \tdirectory.\n \n---show-toplevel::\n-\tShow the absolute path of the top-level directory of the working\n-\ttree. If there is no working tree, report an error.\n-\n---show-superproject-working-tree::\n-\tShow the absolute path of the root of the superproject's\n-\tworking tree (if exists) that uses the current repository as\n-\tits submodule.  Outputs nothing if the current repository is\n-\tnot used as a submodule by any project.\n-\n---shared-index-path::\n-\tShow the path to the shared index file in split index mode, or\n-\tempty if not in split-index mode.\n-\n --show-object-format[=(storage|input|output)]::\n \tShow the object format (hash algorithm) used for the repository\n \tfor storage inside the `.git` directory, input, or output. For\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 69ba7326cf..8b487cd7a9 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -583,6 +583,73 @@ static void handle_ref_opt(const char *pattern, const char *prefix)\n \tclear_ref_exclusion(&ref_excludes);\n }\n \n+enum format_type {\n+\t/* We would like a relative path. */\n+\tFORMAT_RELATIVE,\n+\t/* We would like a canonical absolute path. */\n+\tFORMAT_CANONICAL,\n+\t/* We would like the default behavior. */\n+\tFORMAT_DEFAULT,\n+};\n+\n+enum default_type {\n+\t/* Our default is a relative path. */\n+\tDEFAULT_RELATIVE,\n+\t/* Our default is a relative path if there's a shared root. */\n+\tDEFAULT_RELATIVE_IF_SHARED,\n+\t/* Our default is a canonical absolute path. */\n+\tDEFAULT_CANONICAL,\n+\t/* Our default is not to modify the item. */\n+\tDEFAULT_UNMODIFIED,\n+};\n+\n+static void print_path(const char *path, const char *prefix, enum format_type format, enum default_type def)\n+{\n+\tchar *cwd = NULL;\n+\t/*\n+\t * We don't ever produce a relative path if prefix is NULL, so set the\n+\t * prefix to the current directory so that we can produce a relative\n+\t * path whenever possible.  If we're using RELATIVE_IF_SHARED mode, then\n+\t * we want an absolute path unless the two share a common prefix, so don't\n+\t * set it in that case, since doing so causes a relative path to always\n+\t * be produced if possible.\n+\t */\n+\tif (!prefix && (format != FORMAT_DEFAULT || def != DEFAULT_RELATIVE_IF_SHARED))\n+\t\tprefix = cwd = xgetcwd();\n+\tif (format == FORMAT_DEFAULT && def == DEFAULT_UNMODIFIED) {\n+\t\tputs(path);\n+\t} else if (format == FORMAT_RELATIVE ||\n+\t\t  (format == FORMAT_DEFAULT && def == DEFAULT_RELATIVE)) {\n+\t\t/*\n+\t\t * In order for relative_path to work as expected, we need to\n+\t\t * make sure that both paths are absolute paths.  If we don't,\n+\t\t * we can end up with an unexpected absolute path that the user\n+\t\t * didn't want.\n+\t\t */\n+\t\tstruct strbuf buf = STRBUF_INIT, realbuf = STRBUF_INIT, prefixbuf = STRBUF_INIT;\n+\t\tif (!is_absolute_path(path)) {\n+\t\t\tstrbuf_realpath_missing(&realbuf, path, 1, 1);\n+\t\t\tpath = realbuf.buf;\n+\t\t}\n+\t\tif (!is_absolute_path(prefix)) {\n+\t\t\tstrbuf_realpath_missing(&prefixbuf, prefix, 1, 1);\n+\t\t\tprefix = prefixbuf.buf;\n+\t\t}\n+\t\tputs(relative_path(path, prefix, &buf));\n+\t\tstrbuf_release(&buf);\n+\t} else if (format == FORMAT_DEFAULT && def == DEFAULT_RELATIVE_IF_SHARED) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tputs(relative_path(path, prefix, &buf));\n+\t\tstrbuf_release(&buf);\n+\t} else {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tstrbuf_realpath_missing(&buf, path, 1, 1);\n+\t\tputs(buf.buf);\n+\t\tstrbuf_release(&buf);\n+\t}\n+\tfree(cwd);\n+}\n+\n int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n {\n \tint i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;\n@@ -596,6 +663,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \tstruct strbuf buf = STRBUF_INIT;\n \tconst int hexsz = the_hash_algo->hexsz;\n \tint seen_end_of_options = 0;\n+\tenum format_type format = FORMAT_DEFAULT;\n \n \tif (argc > 1 && !strcmp(\"--parseopt\", argv[1]))\n \t\treturn cmd_parseopt(argc - 1, argv + 1, prefix);\n@@ -668,8 +736,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tif (!argv[i + 1])\n \t\t\t\t\tdie(\"--git-path requires an argument\");\n \t\t\t\tstrbuf_reset(&buf);\n-\t\t\t\tputs(relative_path(git_path(\"%s\", argv[i + 1]),\n-\t\t\t\t\t\t   prefix, &buf));\n+\t\t\t\tprint_path(git_path(\"%s\", argv[i + 1]), prefix,\n+\t\t\t\t\t\tformat,\n+\t\t\t\t\t\tDEFAULT_RELATIVE_IF_SHARED);\n \t\t\t\ti++;\n \t\t\t\tcontinue;\n \t\t\t}\n@@ -687,6 +756,16 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tshow(arg);\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (opt_with_value(arg, \"--path-format\", &arg)) {\n+\t\t\t\tif (!strcmp(arg, \"absolute\")) {\n+\t\t\t\t\tformat = FORMAT_CANONICAL;\n+\t\t\t\t} else if (!strcmp(arg, \"relative\")) {\n+\t\t\t\t\tformat = FORMAT_RELATIVE;\n+\t\t\t\t} else {\n+\t\t\t\t\tdie(\"unknown argument to --path-format: %s\", arg);\n+\t\t\t\t}\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--default\")) {\n \t\t\t\tdef = argv[++i];\n \t\t\t\tif (!def)\n@@ -807,7 +886,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\tif (!strcmp(arg, \"--show-toplevel\")) {\n \t\t\t\tconst char *work_tree = get_git_work_tree();\n \t\t\t\tif (work_tree)\n-\t\t\t\t\tputs(work_tree);\n+\t\t\t\t\tprint_path(work_tree, prefix, format, DEFAULT_UNMODIFIED);\n \t\t\t\telse\n \t\t\t\t\tdie(\"this operation must be run in a work tree\");\n \t\t\t\tcontinue;\n@@ -815,7 +894,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\tif (!strcmp(arg, \"--show-superproject-working-tree\")) {\n \t\t\t\tstruct strbuf superproject = STRBUF_INIT;\n \t\t\t\tif (get_superproject_working_tree(&superproject))\n-\t\t\t\t\tputs(superproject.buf);\n+\t\t\t\t\tprint_path(superproject.buf, prefix, format, DEFAULT_UNMODIFIED);\n \t\t\t\tstrbuf_release(&superproject);\n \t\t\t\tcontinue;\n \t\t\t}\n@@ -850,16 +929,18 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tconst char *gitdir = getenv(GIT_DIR_ENVIRONMENT);\n \t\t\t\tchar *cwd;\n \t\t\t\tint len;\n+\t\t\t\tenum format_type wanted = format;\n \t\t\t\tif (arg[2] == 'g') {\t/* --git-dir */\n \t\t\t\t\tif (gitdir) {\n-\t\t\t\t\t\tputs(gitdir);\n+\t\t\t\t\t\tprint_path(gitdir, prefix, format, DEFAULT_UNMODIFIED);\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\t}\n \t\t\t\t\tif (!prefix) {\n-\t\t\t\t\t\tputs(\".git\");\n+\t\t\t\t\t\tprint_path(\".git\", prefix, format, DEFAULT_UNMODIFIED);\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\t}\n \t\t\t\t} else {\t\t/* --absolute-git-dir */\n+\t\t\t\t\twanted = FORMAT_CANONICAL;\n \t\t\t\t\tif (!gitdir && !prefix)\n \t\t\t\t\t\tgitdir = \".git\";\n \t\t\t\t\tif (gitdir) {\n@@ -872,14 +953,14 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\t}\n \t\t\t\tcwd = xgetcwd();\n \t\t\t\tlen = strlen(cwd);\n-\t\t\t\tprintf(\"%s%s.git\\n\", cwd, len && cwd[len-1] != '/' ? \"/\" : \"\");\n+\t\t\t\tstrbuf_reset(&buf);\n+\t\t\t\tstrbuf_addf(&buf, \"%s%s.git\", cwd, len && cwd[len-1] != '/' ? \"/\" : \"\");\n \t\t\t\tfree(cwd);\n+\t\t\t\tprint_path(buf.buf, prefix, wanted, DEFAULT_CANONICAL);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--git-common-dir\")) {\n-\t\t\t\tstrbuf_reset(&buf);\n-\t\t\t\tputs(relative_path(get_git_common_dir(),\n-\t\t\t\t\t\t   prefix, &buf));\n+\t\t\t\tprint_path(get_git_common_dir(), prefix, format, DEFAULT_RELATIVE_IF_SHARED);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--is-inside-git-dir\")) {\n@@ -909,8 +990,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tif (the_index.split_index) {\n \t\t\t\t\tconst struct object_id *oid = &the_index.split_index->base_oid;\n \t\t\t\t\tconst char *path = git_path(\"sharedindex.%s\", oid_to_hex(oid));\n-\t\t\t\t\tstrbuf_reset(&buf);\n-\t\t\t\t\tputs(relative_path(path, prefix, &buf));\n+\t\t\t\t\tprint_path(path, prefix, format, DEFAULT_RELATIVE);\n \t\t\t\t}\n \t\t\t\tcontinue;\n \t\t\t}\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex 408b97d5af..51d7d40ec1 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -3,6 +3,16 @@\n test_description='test git rev-parse'\n . ./test-lib.sh\n \n+test_one () {\n+\tdir=\"$1\" &&\n+\texpect=\"$2\" &&\n+\tshift &&\n+\tshift &&\n+\techo \"$expect\" >expect &&\n+\tgit -C \"$dir\" rev-parse \"$@\" >actual &&\n+\ttest_cmp expect actual\n+}\n+\n # usage: [options] label is-bare is-inside-git is-inside-work prefix git-dir absolute-git-dir\n test_rev_parse () {\n \td=\n@@ -60,7 +70,13 @@ ROOT=$(pwd)\n \n test_expect_success 'setup' '\n \tmkdir -p sub/dir work &&\n-\tcp -R .git repo.git\n+\tcp -R .git repo.git &&\n+\tgit checkout -B main &&\n+\ttest_commit abc &&\n+\tgit checkout -b side &&\n+\ttest_commit def &&\n+\tgit checkout main &&\n+\tgit worktree add worktree side\n '\n \n test_rev_parse toplevel false false true '' .git \"$ROOT/.git\"\n@@ -88,6 +104,45 @@ test_rev_parse -C work -g ../repo.git -b t 'GIT_DIR=../repo.git, core.bare = tru\n \n test_rev_parse -C work -g ../repo.git -b u 'GIT_DIR=../repo.git, core.bare undefined' false false true ''\n \n+test_expect_success 'rev-parse --path-format=absolute' '\n+\ttest_one \".\" \"$ROOT/.git\" --path-format=absolute --git-dir &&\n+\ttest_one \".\" \"$ROOT/.git\" --path-format=absolute --git-common-dir &&\n+\ttest_one \"sub/dir\" \"$ROOT/.git\" --path-format=absolute --git-dir &&\n+\ttest_one \"sub/dir\" \"$ROOT/.git\" --path-format=absolute --git-common-dir &&\n+\ttest_one \"worktree\" \"$ROOT/.git/worktrees/worktree\" --path-format=absolute --git-dir &&\n+\ttest_one \"worktree\" \"$ROOT/.git\" --path-format=absolute --git-common-dir &&\n+\ttest_one \".\" \"$ROOT\" --path-format=absolute --show-toplevel &&\n+\ttest_one \".\" \"$ROOT/.git/objects\" --path-format=absolute --git-path objects &&\n+\ttest_one \".\" \"$ROOT/.git/objects/foo/bar/baz\" --path-format=absolute --git-path objects/foo/bar/baz\n+'\n+\n+test_expect_success 'rev-parse --path-format=relative' '\n+\ttest_one \".\" \".git\" --path-format=relative --git-dir &&\n+\ttest_one \".\" \".git\" --path-format=relative --git-common-dir &&\n+\ttest_one \"sub/dir\" \"../../.git\" --path-format=relative --git-dir &&\n+\ttest_one \"sub/dir\" \"../../.git\" --path-format=relative --git-common-dir &&\n+\ttest_one \"worktree\" \"../.git/worktrees/worktree\" --path-format=relative --git-dir &&\n+\ttest_one \"worktree\" \"../.git\" --path-format=relative --git-common-dir &&\n+\ttest_one \".\" \"./\" --path-format=relative --show-toplevel &&\n+\ttest_one \".\" \".git/objects\" --path-format=relative --git-path objects &&\n+\ttest_one \".\" \".git/objects/foo/bar/baz\" --path-format=relative --git-path objects/foo/bar/baz\n+'\n+\n+test_expect_success '--path-format=relative does not affect --absolute-git-dir' '\n+\tgit rev-parse --path-format=relative --absolute-git-dir >actual &&\n+\techo \"$ROOT/.git\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--path-format can change in the middle of the command line' '\n+\tgit rev-parse --path-format=absolute --git-dir --path-format=relative --git-path objects/foo/bar >actual &&\n+\tcat >expect <<-EOF &&\n+\t$ROOT/.git\n+\t.git/objects/foo/bar\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'git-common-dir from worktree root' '\n \techo .git >expect &&\n \tgit rev-parse --git-common-dir >actual &&\n"},{"id":"411570","messageId":"CAPig+cTbtpzwcQPHUgyf=0Oe5h2_=zory2oj9oFEUrdtaRR6ng@mail.gmail.com","threadId":"54777","inReplyTo":"20201206225349.3392790-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH v4 1/2] abspath: add a function to resolve paths with missing components","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-12-07T00:02:16Z","receivedAt":"2020-12-07T00:03:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Dec 6, 2020 at 5:56 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> We'd like to canonicalize paths such that we can preserve any number of\n> trailing components that may be missing.  Let's add a function to do\n> that, allowing either one or an unlimited number of components to\n> canonicalize, and make strbuf_realpath a wrapper around it that allows\n> just one component.\n\nThis commit message is too barebones. As a justification, \"We'd\nlike...\" is insufficient; it doesn't help the reader understand why\nthis change is desirable.\n\nFurther, the lack of explanation about the seemingly arbitrary \"one or\ninfinite\" condition confuses the issue. The first question which\npopped into this reader's head was \"why those two specific choices?\".\nWhat makes one missing path component special as opposed to any number\nof missing components? (These questions are mostly rhetorical; I can\nfigure out reasonable answers, but the commit message ought to do a\nbetter job of explaining.)\n\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n> +/*\n> + * Just like strbuf_realpath, but allows specifying how many missing components\n> + * are permitted.  If many_missing is true, an arbitrary number of path\n> + * components may be missing; otherwise, only the last component may be missing.\n> + */\n> +char *strbuf_realpath_missing(struct strbuf *resolved, const char *path,\n> +                             int many_missing, int die_on_error)\n\nThis interface feels odd. Why would a caller ever want to call this\nfunction with many_missing=0 when it would be easier, shorter, more\ndirect to simply call strbuf_realpath()? Is the plan to retire\nstrbuf_realpath() down the road?\n\nA more orthogonal-feeling interface would be to make this function\n_always_ allow any number of missing trailing path components (that\nis, drop `many_missing` altogether). Doing so would simplify the\ndocumentation and the signature. If the caller needs the original\nbehavior of only allowing the final path component to be missing, then\nstrbuf_realpath() can be used, as usual.\n\nThe name of the function is somewhat confusing, especially if you take\nthe suggestion of dropping the `many_missing` argument. Perhaps a name\nsuch as strbuf_realpath_forgiving() would be more understandable.\n\nNote that the above comments are only about the API. It's perfectly\nfine if the two functions share an underlying private implementation\n(making them each one-liners calling the actual worker function).\n"},{"id":"411571","messageId":"CAPig+cS-2Vw84rejMFAiDeF8dd5gtBOmQZUMpOy2ufA8nU7W7g@mail.gmail.com","threadId":"54777","inReplyTo":"20201206225349.3392790-3-sandals@crustytoothpaste.net","subject":"Re: [PATCH v4 2/2] rev-parse: add option for absolute or relative path formatting","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-12-07T00:30:13Z","receivedAt":"2020-12-07T00:31:28Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Dec 6, 2020 at 5:58 PM brian m. carlson\n<sandals@crustytoothpaste.net> wrote:\n> [...]\n> To help out the scripter, let's provide an option which turns most of\n> the paths printed by git rev-parse to be either relative to the current\n> working directory or absolute and canonical.  Document which options are\n> affected and which are not so that users are not confused.\n> [...]\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt\n> @@ -212,6 +212,15 @@ Options for Files\n> +--path-format=(absolute|relative)::\n> +       Controls the behavior of certain other options following it on the command\n> +       line. If specified as absolute, the paths printed by those options will be\n> +       absolute and canonical. If specified as relative, the paths will be relative\n> +       to the current working directory if that is possible.  The default is option\n> +       specific.\n\nI read the commit message and looked at the implementation so I know\nthat this option can be specified multiple times, but this\ndocumentation only vaguely hints at it by saying \"options following\nit\". We could do a better job of imparting that knowledge to the\nreader by saying so explicitly, perhaps like this:\n\n    Controls the behavior of some path-printing options. If\n    'absolute', [...]. If 'relative', [...]. May be specified multiple\n    times, each time affecting the path-printing options which\n    follow it. The default path format is option-specific.\n\nSince this option can be specified multiple times, should it also\nrecognize `default` to request the default behavior in addition to\n`absolute` and `relative`? (Genuine question. Maybe real-world\nuse-cases wouldn't need it, but it would be easy to support and make\nit functionally complete.)\n\n> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\n> @@ -583,6 +583,73 @@ static void handle_ref_opt(const char *pattern, const char *prefix)\n> +static void print_path(const char *path, const char *prefix, enum format_type format, enum default_type def)\n> +{\n> +               struct strbuf buf = STRBUF_INIT, realbuf = STRBUF_INIT, prefixbuf = STRBUF_INIT;\n> +               if (!is_absolute_path(path)) {\n> +                       strbuf_realpath_missing(&realbuf, path, 1, 1);\n> +                       path = realbuf.buf;\n> +               }\n> +               if (!is_absolute_path(prefix)) {\n> +                       strbuf_realpath_missing(&prefixbuf, prefix, 1, 1);\n> +                       prefix = prefixbuf.buf;\n> +               }\n> +               puts(relative_path(path, prefix, &buf));\n> +               strbuf_release(&buf);\n\nLeaking `realbuf` and `prefixbuf` here.\n"},{"id":"411579","messageId":"X82PS2RDlpXY0srv@camp.crustytoothpaste.net","threadId":"54777","inReplyTo":"CAPig+cTbtpzwcQPHUgyf=0Oe5h2_=zory2oj9oFEUrdtaRR6ng@mail.gmail.com","subject":"Re: [PATCH v4 1/2] abspath: add a function to resolve paths with missing components","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-12-07T02:11:23Z","receivedAt":"2020-12-07T02:12:44Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-12-07 at 00:02:16, Eric Sunshine wrote:\n> This commit message is too barebones. As a justification, \"We'd\n> like...\" is insufficient; it doesn't help the reader understand why\n> this change is desirable.\n> \n> Further, the lack of explanation about the seemingly arbitrary \"one or\n> infinite\" condition confuses the issue. The first question which\n> popped into this reader's head was \"why those two specific choices?\".\n> What makes one missing path component special as opposed to any number\n> of missing components? (These questions are mostly rhetorical; I can\n> figure out reasonable answers, but the commit message ought to do a\n> better job of explaining.)\n\nSure, I can expand the commit message to be a little more descriptive.\n\n> The name of the function is somewhat confusing, especially if you take\n> the suggestion of dropping the `many_missing` argument. Perhaps a name\n> such as strbuf_realpath_forgiving() would be more understandable.\n\nSure, I agree that's a better name.  It shouldn't be surprising to\nanyone on the list that I am absolutely terrible at naming things, so I\nappreciate the suggestion.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"411580","messageId":"X82Vl7YmZ1/Sr4xS@camp.crustytoothpaste.net","threadId":"54777","inReplyTo":"CAPig+cS-2Vw84rejMFAiDeF8dd5gtBOmQZUMpOy2ufA8nU7W7g@mail.gmail.com","subject":"Re: [PATCH v4 2/2] rev-parse: add option for absolute or relative path formatting","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-12-07T02:38:15Z","receivedAt":"2020-12-07T02:39:32Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-12-07 at 00:30:13, Eric Sunshine wrote:\n> I read the commit message and looked at the implementation so I know\n> that this option can be specified multiple times, but this\n> documentation only vaguely hints at it by saying \"options following\n> it\". We could do a better job of imparting that knowledge to the\n> reader by saying so explicitly, perhaps like this:\n> \n>     Controls the behavior of some path-printing options. If\n>     'absolute', [...]. If 'relative', [...]. May be specified multiple\n>     times, each time affecting the path-printing options which\n>     follow it. The default path format is option-specific.\n\nI'll improve the documentation in v5.\n\n> Since this option can be specified multiple times, should it also\n> recognize `default` to request the default behavior in addition to\n> `absolute` and `relative`? (Genuine question. Maybe real-world\n> use-cases wouldn't need it, but it would be easy to support and make\n> it functionally complete.)\n\nI don't think adding the default is helpful.  I can think of situations\nwhere people want absolute path names or relative path names, but in\nwhat case would someone want, \"meh, whatever Git decides to give me\"?\n\nThe problem we're solving here is that Git isn't consistent and can't be\nused to provide valuable information that the user wants.  If a user\ndoesn't care what format the path is in, then they can always specify\nabsolute and it will almost certainly be correct and work for them.\n\n> Leaking `realbuf` and `prefixbuf` here.\n\nWill fix.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"411608","messageId":"c1d1fe44-6a7d-3578-cd89-9aea59c4637a@web.de","threadId":"54777","inReplyTo":"20201206225349.3392790-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH v4 1/2] abspath: add a function to resolve paths with missing components","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2020-12-07T17:19:32Z","receivedAt":"2020-12-07T17:22:10Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 06.12.20 um 23:53 schrieb brian m. carlson:\n> We'd like to canonicalize paths such that we can preserve any number of\n> trailing components that may be missing.  Let's add a function to do\n> that, allowing either one or an unlimited number of components to\n> canonicalize, and make strbuf_realpath a wrapper around it that allows\n> just one component.\n>\n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n>  abspath.c | 13 ++++++++++++-\n>  cache.h   |  2 ++\n>  2 files changed, 14 insertions(+), 1 deletion(-)\n>\n> diff --git a/abspath.c b/abspath.c\n> index 6f15a418bb..e6630b3618 100644\n> --- a/abspath.c\n> +++ b/abspath.c\n> @@ -80,6 +80,17 @@ static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)\n>   */\n>  char *strbuf_realpath(struct strbuf *resolved, const char *path,\n>  \t\t      int die_on_error)\n> +{\n> +\treturn strbuf_realpath_missing(resolved, path, 0, die_on_error);\n> +}\n> +\n> +/*\n> + * Just like strbuf_realpath, but allows specifying how many missing components\n> + * are permitted.  If many_missing is true, an arbitrary number of path\n> + * components may be missing; otherwise, only the last component may be missing.\n> + */\n> +char *strbuf_realpath_missing(struct strbuf *resolved, const char *path,\n> +\t\t\t      int many_missing, int die_on_error)\n\nSo this uses two 32-bit values to pass two bits.  Hmm.\n\nI find the concept of a \"real\" path with imaginary components strangely\namusing.  But perhaps a name like strbuf_resolve_path() would fit better?\n\n>  {\n>  \tstruct strbuf remaining = STRBUF_INIT;\n>  \tstruct strbuf next = STRBUF_INIT;\n> @@ -129,7 +140,7 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,\n>\n>  \t\tif (lstat(resolved->buf, &st)) {\n>  \t\t\t/* error out unless this was the last component */\n> -\t\t\tif (errno != ENOENT || remaining.len) {\n> +\t\t\tif (errno != ENOENT || (!many_missing && remaining.len)) {\n>  \t\t\t\tif (die_on_error)\n>  \t\t\t\t\tdie_errno(\"Invalid path '%s'\",\n>  \t\t\t\t\t\t  resolved->buf);\n\nSo the original code errors out if there is a real error\n(errno != ENOENT).  It also errors out if any component except the last\none is missing (errno == ENOENT && remaining.len); that's what the\ncomment is about.  This patch adds the ability to ignore ENOENT for all\ncomponents.\n\nPerhaps convert many_missing and die_on_error into a single flags\nparameter and implement the flags DIE_ON_ERR and REQUIRE_BASENAME or\nsimilar?  Callers would be easier to read because such an interface is\nself-documenting -- provided we find good flag names.\n\n> diff --git a/cache.h b/cache.h\n> index e986cf4ea9..a1386235fc 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -1320,6 +1320,8 @@ static inline int is_absolute_path(const char *path)\n>  int is_directory(const char *);\n>  char *strbuf_realpath(struct strbuf *resolved, const char *path,\n>  \t\t      int die_on_error);\n> +char *strbuf_realpath_missing(struct strbuf *resolved, const char *path,\n> +\t\t\t      int missing_components, int die_on_error);\n>  char *real_pathdup(const char *path, int die_on_error);\n>  const char *absolute_path(const char *path);\n>  char *absolute_pathdup(const char *path);\n>\n\n"},{"id":"411711","messageId":"X87qNTiQDlnzm6e2@camp.crustytoothpaste.net","threadId":"54777","inReplyTo":"c1d1fe44-6a7d-3578-cd89-9aea59c4637a@web.de","subject":"Re: [PATCH v4 1/2] abspath: add a function to resolve paths with missing components","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-12-08T02:51:33Z","receivedAt":"2020-12-08T02:52:37Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-12-07 at 17:19:32, René Scharfe wrote:\n> I find the concept of a \"real\" path with imaginary components strangely\n> amusing.  But perhaps a name like strbuf_resolve_path() would fit better?\n\nI think I'm going to take the strbuf_realpath_forgiving solution from\nEric Sunshine because I think having similar names for similar functions\nhelps discoverability.\n\n> So the original code errors out if there is a real error\n> (errno != ENOENT).  It also errors out if any component except the last\n> one is missing (errno == ENOENT && remaining.len); that's what the\n> comment is about.  This patch adds the ability to ignore ENOENT for all\n> components.\n> \n> Perhaps convert many_missing and die_on_error into a single flags\n> parameter and implement the flags DIE_ON_ERR and REQUIRE_BASENAME or\n> similar?  Callers would be easier to read because such an interface is\n> self-documenting -- provided we find good flag names.\n\nAs discussed elsewhere in the thread, this will be moving to an internal\nfunction, but I can make that function take two flag parameters.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"}]}