* [PATCH v3 0/2] rev-parse options for absolute or relative paths
@ 2020-11-27 23:19 brian m. carlson
2020-11-27 23:19 ` [PATCH v3 1/2] abspath: add a function to resolve paths with missing components brian m. carlson
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: brian m. carlson @ 2020-11-27 23:19 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Emily Shaffer
There are a bunch of different situations in which one would like to
have an absolute and canonical or a relative path from Git. In many of
these cases, these values are already available from git rev-parse, but
some values only come in one form or another.
Many operating systems, such as macOS, lack a built-in realpath command
that can canonicalize paths properly, and additionally some programming
languages, like Go, currently do as well. It's therefore helpful for us
to provide a generic way to request that a path is fully canonicalized
before using it. Since users may wish for a relative path, we can
provide one of those as well.
The primary impetus for this was Git LFS, which needs paths to be
canonicalized in the same way as Git in order to function correctly.
See https://github.com/git-lfs/git-lfs/issues/4012 for more details.
Changes from v2:
* Incorporate multiple missing segment support into the strbuf_realpath
code.
* Switch some invocations to use DEFAULT_UNMODIFIED, which should not
result in a change in behavior.
* Rebase, resolving some conflicts.
Changes from v1:
* Add a function to handle missing trailing components when
canonicalizing paths and use it.
* Improve commit messages.
* Fix broken && chain.
* Fix situation where relative paths are not relative.
brian m. carlson (2):
abspath: add a function to resolve paths with missing components
rev-parse: add option for absolute or relative path formatting
Documentation/git-rev-parse.txt | 71 +++++++++++++---------
abspath.c | 33 +++++++++-
builtin/rev-parse.c | 104 ++++++++++++++++++++++++++++----
cache.h | 2 +
t/t1500-rev-parse.sh | 57 ++++++++++++++++-
5 files changed, 223 insertions(+), 44 deletions(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/2] abspath: add a function to resolve paths with missing components
2020-11-27 23:19 [PATCH v3 0/2] rev-parse options for absolute or relative paths brian m. carlson
@ 2020-11-27 23:19 ` brian m. carlson
2020-11-28 10:08 ` René Scharfe
2020-12-02 13:09 ` Johannes Schindelin
2020-11-27 23:19 ` [PATCH v3 2/2] rev-parse: add option for absolute or relative path formatting brian m. carlson
2020-12-02 13:12 ` [PATCH v3 0/2] rev-parse options for absolute or relative paths Johannes Schindelin
2 siblings, 2 replies; 8+ messages in thread
From: brian m. carlson @ 2020-11-27 23:19 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Emily Shaffer
We'd like to canonicalize paths such that we can preserve any number of
trailing components that may be missing. Let's add a function to do
that, taking the number of components to canonicalize, and make
strbuf_realpath a wrapper around it that allows just one component.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
abspath.c | 33 ++++++++++++++++++++++++++++++++-
cache.h | 2 ++
2 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/abspath.c b/abspath.c
index 6f15a418bb..1d8f3d007c 100644
--- a/abspath.c
+++ b/abspath.c
@@ -20,6 +20,7 @@ static void strip_last_component(struct strbuf *path)
/* Find start of the last component */
while (offset < len && !is_dir_sep(path->buf[len - 1]))
len--;
+
/* Skip sequences of multiple path-separators */
while (offset < len && is_dir_sep(path->buf[len - 1]))
len--;
@@ -66,6 +67,22 @@ static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)
#define MAXSYMLINKS 32
#endif
+/* Count non-contiguous directory separators, not including a trailing one. */
+static int count_dir_separators(const char *s)
+{
+ int count = 0;
+ int last_sep = 0;
+ const char *p = s;
+ while (*p) {
+ int is_sep = is_dir_sep(*p++);
+ if (is_sep && !last_sep)
+ count++;
+ last_sep = is_sep;
+ }
+ return count;
+}
+
+
/*
* Return the real path (i.e., absolute path, with symlinks resolved
* and extra slashes removed) equivalent to the specified path. (If
@@ -80,6 +97,16 @@ static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)
*/
char *strbuf_realpath(struct strbuf *resolved, const char *path,
int die_on_error)
+{
+ return strbuf_realpath_missing(resolved, path, 1, die_on_error);
+}
+
+/*
+ * Just like strbuf_realpath, but allows specifying how many missing components
+ * are permitted. -1 may be specified to allow an unlimited number.
+ */
+char *strbuf_realpath_missing(struct strbuf *resolved, const char *path,
+ int missing_components, int die_on_error)
{
struct strbuf remaining = STRBUF_INIT;
struct strbuf next = STRBUF_INIT;
@@ -128,8 +155,12 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,
strbuf_addbuf(resolved, &next);
if (lstat(resolved->buf, &st)) {
+ int trailing_components = count_dir_separators(remaining.buf) +
+ (remaining.len != 0);
/* error out unless this was the last component */
- if (errno != ENOENT || remaining.len) {
+ if (errno != ENOENT ||
+ !(missing_components == -1 ||
+ trailing_components < missing_components)) {
if (die_on_error)
die_errno("Invalid path '%s'",
resolved->buf);
diff --git a/cache.h b/cache.h
index c0072d43b1..ee4bc5ec04 100644
--- a/cache.h
+++ b/cache.h
@@ -1320,6 +1320,8 @@ static inline int is_absolute_path(const char *path)
int is_directory(const char *);
char *strbuf_realpath(struct strbuf *resolved, const char *path,
int die_on_error);
+char *strbuf_realpath_missing(struct strbuf *resolved, const char *path,
+ int missing_components, int die_on_error);
char *real_pathdup(const char *path, int die_on_error);
const char *absolute_path(const char *path);
char *absolute_pathdup(const char *path);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 2/2] rev-parse: add option for absolute or relative path formatting
2020-11-27 23:19 [PATCH v3 0/2] rev-parse options for absolute or relative paths brian m. carlson
2020-11-27 23:19 ` [PATCH v3 1/2] abspath: add a function to resolve paths with missing components brian m. carlson
@ 2020-11-27 23:19 ` brian m. carlson
2020-12-02 13:12 ` [PATCH v3 0/2] rev-parse options for absolute or relative paths Johannes Schindelin
2 siblings, 0 replies; 8+ messages in thread
From: brian m. carlson @ 2020-11-27 23:19 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Emily Shaffer
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.
Note that if the user needs both forms, it is possible to pass an
additional option in the middle of the command line which changes the
behavior of subsequent operations.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
Documentation/git-rev-parse.txt | 71 +++++++++++++---------
builtin/rev-parse.c | 104 ++++++++++++++++++++++++++++----
t/t1500-rev-parse.sh | 57 ++++++++++++++++-
3 files changed, 189 insertions(+), 43 deletions(-)
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 5013daa6ef..1708af8a66 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -212,6 +212,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
@@ -221,13 +230,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".
@@ -242,19 +280,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
@@ -265,20 +290,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 69ba7326cf..5386a6ae8b 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -583,6 +583,73 @@ 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)) {
+ /*
+ * In order for relative_path to work as expected, we need to
+ * make sure that both paths are absolute paths. If we don't,
+ * we can end up with an unexpected absolute path that the user
+ * didn't want.
+ */
+ struct strbuf buf = STRBUF_INIT, realbuf = STRBUF_INIT, prefixbuf = STRBUF_INIT;
+ if (!is_absolute_path(path)) {
+ strbuf_realpath_missing(&realbuf, path, -1, 1);
+ path = realbuf.buf;
+ }
+ if (!is_absolute_path(prefix)) {
+ strbuf_realpath_missing(&prefixbuf, prefix, -1, 1);
+ prefix = prefixbuf.buf;
+ }
+ puts(relative_path(path, prefix, &buf));
+ strbuf_release(&buf);
+ } else if (format == FORMAT_DEFAULT && def == DEFAULT_RELATIVE_IF_SHARED) {
+ struct strbuf buf = STRBUF_INIT;
+ puts(relative_path(path, prefix, &buf));
+ strbuf_release(&buf);
+ } else {
+ struct strbuf buf = STRBUF_INIT;
+ strbuf_realpath_missing(&buf, path, -1, 1);
+ puts(buf.buf);
+ strbuf_release(&buf);
+ }
+ 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;
@@ -596,6 +663,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
struct strbuf buf = STRBUF_INIT;
const int hexsz = the_hash_algo->hexsz;
int seen_end_of_options = 0;
+ enum format_type format = FORMAT_DEFAULT;
if (argc > 1 && !strcmp("--parseopt", argv[1]))
return cmd_parseopt(argc - 1, argv + 1, prefix);
@@ -668,8 +736,9 @@ 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;
}
@@ -687,6 +756,16 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
show(arg);
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)
@@ -807,7 +886,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_UNMODIFIED);
else
die("this operation must be run in a work tree");
continue;
@@ -815,7 +894,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_UNMODIFIED);
strbuf_release(&superproject);
continue;
}
@@ -850,16 +929,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) {
@@ -872,14 +953,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")) {
@@ -909,8 +990,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..51d7d40ec1 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,45 @@ 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 "sub/dir" "$ROOT/.git" --path-format=absolute --git-dir &&
+ test_one "sub/dir" "$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_one "." "$ROOT/.git/objects/foo/bar/baz" --path-format=absolute --git-path objects/foo/bar/baz
+'
+
+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 "sub/dir" "../../.git" --path-format=relative --git-dir &&
+ test_one "sub/dir" "../../.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_one "." ".git/objects/foo/bar/baz" --path-format=relative --git-path objects/foo/bar/baz
+'
+
+test_expect_success '--path-format=relative does not affect --absolute-git-dir' '
+ git rev-parse --path-format=relative --absolute-git-dir >actual &&
+ echo "$ROOT/.git" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '--path-format can change in the middle of the command line' '
+ git rev-parse --path-format=absolute --git-dir --path-format=relative --git-path objects/foo/bar >actual &&
+ cat >expect <<-EOF &&
+ $ROOT/.git
+ .git/objects/foo/bar
+ EOF
+ test_cmp expect actual
+'
+
test_expect_success 'git-common-dir from worktree root' '
echo .git >expect &&
git rev-parse --git-common-dir >actual &&
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] abspath: add a function to resolve paths with missing components
2020-11-27 23:19 ` [PATCH v3 1/2] abspath: add a function to resolve paths with missing components brian m. carlson
@ 2020-11-28 10:08 ` René Scharfe
2020-11-28 18:41 ` brian m. carlson
2020-12-02 13:09 ` Johannes Schindelin
1 sibling, 1 reply; 8+ messages in thread
From: René Scharfe @ 2020-11-28 10:08 UTC (permalink / raw)
To: brian m. carlson, git; +Cc: Johannes Schindelin, Emily Shaffer
Am 28.11.20 um 00:19 schrieb brian m. carlson:
> We'd like to canonicalize paths such that we can preserve any number of
> trailing components that may be missing. Let's add a function to do
> that, taking the number of components to canonicalize, and make
> strbuf_realpath a wrapper around it that allows just one component.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> abspath.c | 33 ++++++++++++++++++++++++++++++++-
> cache.h | 2 ++
> 2 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/abspath.c b/abspath.c
> index 6f15a418bb..1d8f3d007c 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -20,6 +20,7 @@ static void strip_last_component(struct strbuf *path)
> /* Find start of the last component */
> while (offset < len && !is_dir_sep(path->buf[len - 1]))
> len--;
> +
> /* Skip sequences of multiple path-separators */
> while (offset < len && is_dir_sep(path->buf[len - 1]))
> len--;
Stray change?
> @@ -66,6 +67,22 @@ static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)
> #define MAXSYMLINKS 32
> #endif
>
> +/* Count non-contiguous directory separators, not including a trailing one. */
> +static int count_dir_separators(const char *s)
> +{
> + int count = 0;
> + int last_sep = 0;
> + const char *p = s;
> + while (*p) {
> + int is_sep = is_dir_sep(*p++);
> + if (is_sep && !last_sep)
> + count++;
> + last_sep = is_sep;
> + }
> + return count;
> +}
count_dir_separators("a/") returns 1; is this intended? It seems to
contradict the "not including a trailing one" comment.
> +
> +
Nitpicking: Do we need both empty lines?
> /*
> * Return the real path (i.e., absolute path, with symlinks resolved
> * and extra slashes removed) equivalent to the specified path. (If
> @@ -80,6 +97,16 @@ static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)
> */
> char *strbuf_realpath(struct strbuf *resolved, const char *path,
> int die_on_error)
> +{
> + return strbuf_realpath_missing(resolved, path, 1, die_on_error);
> +}
> +
> +/*
> + * Just like strbuf_realpath, but allows specifying how many missing components
> + * are permitted. -1 may be specified to allow an unlimited number.
> + */
> +char *strbuf_realpath_missing(struct strbuf *resolved, const char *path,
> + int missing_components, int die_on_error)
> {
> struct strbuf remaining = STRBUF_INIT;
> struct strbuf next = STRBUF_INIT;
> @@ -128,8 +155,12 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,
> strbuf_addbuf(resolved, &next);
>
> if (lstat(resolved->buf, &st)) {
> + int trailing_components = count_dir_separators(remaining.buf) +
> + (remaining.len != 0);
Hmm, so you actually want to count path components, not separators.
Perhaps like this?
static size_t count_components(const char *p)
{
size_t n = 0;
while (*p) {
while (*p && !is_dir_sep(*p))
p++;
while (is_dir_sep(*p))
p++;
n++;
}
return n;
}
> /* error out unless this was the last component */
> - if (errno != ENOENT || remaining.len) {
> + if (errno != ENOENT ||
> + !(missing_components == -1 ||
> + trailing_components < missing_components)) {
> if (die_on_error)
> die_errno("Invalid path '%s'",
> resolved->buf);
> diff --git a/cache.h b/cache.h
> index c0072d43b1..ee4bc5ec04 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1320,6 +1320,8 @@ static inline int is_absolute_path(const char *path)
> int is_directory(const char *);
> char *strbuf_realpath(struct strbuf *resolved, const char *path,
> int die_on_error);
> +char *strbuf_realpath_missing(struct strbuf *resolved, const char *path,
> + int missing_components, int die_on_error);
> char *real_pathdup(const char *path, int die_on_error);
> const char *absolute_path(const char *path);
> char *absolute_pathdup(const char *path);
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] abspath: add a function to resolve paths with missing components
2020-11-28 10:08 ` René Scharfe
@ 2020-11-28 18:41 ` brian m. carlson
0 siblings, 0 replies; 8+ messages in thread
From: brian m. carlson @ 2020-11-28 18:41 UTC (permalink / raw)
To: René Scharfe; +Cc: git, Johannes Schindelin, Emily Shaffer
[-- Attachment #1: Type: text/plain, Size: 2877 bytes --]
On 2020-11-28 at 10:08:09, René Scharfe wrote:
> Am 28.11.20 um 00:19 schrieb brian m. carlson:
> > We'd like to canonicalize paths such that we can preserve any number of
> > trailing components that may be missing. Let's add a function to do
> > that, taking the number of components to canonicalize, and make
> > strbuf_realpath a wrapper around it that allows just one component.
> >
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> > abspath.c | 33 ++++++++++++++++++++++++++++++++-
> > cache.h | 2 ++
> > 2 files changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/abspath.c b/abspath.c
> > index 6f15a418bb..1d8f3d007c 100644
> > --- a/abspath.c
> > +++ b/abspath.c
> > @@ -20,6 +20,7 @@ static void strip_last_component(struct strbuf *path)
> > /* Find start of the last component */
> > while (offset < len && !is_dir_sep(path->buf[len - 1]))
> > len--;
> > +
> > /* Skip sequences of multiple path-separators */
> > while (offset < len && is_dir_sep(path->buf[len - 1]))
> > len--;
>
> Stray change?
Ah, yes. I pulled out the old code from v2, but left the whitespace.
Will fix.
> Nitpicking: Do we need both empty lines?
No, we don't.
> > /*
> > * Return the real path (i.e., absolute path, with symlinks resolved
> > * and extra slashes removed) equivalent to the specified path. (If
> > @@ -80,6 +97,16 @@ static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)
> > */
> > char *strbuf_realpath(struct strbuf *resolved, const char *path,
> > int die_on_error)
> > +{
> > + return strbuf_realpath_missing(resolved, path, 1, die_on_error);
> > +}
> > +
> > +/*
> > + * Just like strbuf_realpath, but allows specifying how many missing components
> > + * are permitted. -1 may be specified to allow an unlimited number.
> > + */
> > +char *strbuf_realpath_missing(struct strbuf *resolved, const char *path,
> > + int missing_components, int die_on_error)
> > {
> > struct strbuf remaining = STRBUF_INIT;
> > struct strbuf next = STRBUF_INIT;
> > @@ -128,8 +155,12 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,
> > strbuf_addbuf(resolved, &next);
> >
> > if (lstat(resolved->buf, &st)) {
> > + int trailing_components = count_dir_separators(remaining.buf) +
> > + (remaining.len != 0);
>
> Hmm, so you actually want to count path components, not separators.
> Perhaps like this?
>
> static size_t count_components(const char *p)
> {
> size_t n = 0;
> while (*p) {
> while (*p && !is_dir_sep(*p))
> p++;
> while (is_dir_sep(*p))
> p++;
> n++;
> }
> return n;
> }
Yeah, I think that's nicer, and simpler, too. Will reroll with that
fix.
--
brian m. carlson (he/him or they/them)
Houston, Texas, US
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] abspath: add a function to resolve paths with missing components
2020-11-27 23:19 ` [PATCH v3 1/2] abspath: add a function to resolve paths with missing components brian m. carlson
2020-11-28 10:08 ` René Scharfe
@ 2020-12-02 13:09 ` Johannes Schindelin
2020-12-02 23:54 ` brian m. carlson
1 sibling, 1 reply; 8+ messages in thread
From: Johannes Schindelin @ 2020-12-02 13:09 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Emily Shaffer
Hi brian,
On Fri, 27 Nov 2020, brian m. carlson wrote:
> @@ -80,6 +97,16 @@ static void get_root_part(struct strbuf *resolved, struct strbuf *remaining)
> */
> char *strbuf_realpath(struct strbuf *resolved, const char *path,
> int die_on_error)
> +{
> + return strbuf_realpath_missing(resolved, path, 1, die_on_error);
> +}
> +
> +/*
> + * Just like strbuf_realpath, but allows specifying how many missing components
> + * are permitted. -1 may be specified to allow an unlimited number.
> + */
> +char *strbuf_realpath_missing(struct strbuf *resolved, const char *path,
> + int missing_components, int die_on_error)
I am having a hard time with the name `missing_components`. Taking a step
back, I think that we are interested in essentially two modes: allow one
missing component, or an arbitrary amount of missing components.
If this assumption is correct, we may not even need to count components at
all. We only need a flag indicating whether we allow only the last
component to be missing, or any number of components. Maybe something like
`error_on_missing_parent_directory` or some such?
Ciao,
Dscho
> {
> struct strbuf remaining = STRBUF_INIT;
> struct strbuf next = STRBUF_INIT;
> @@ -128,8 +155,12 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,
> strbuf_addbuf(resolved, &next);
>
> if (lstat(resolved->buf, &st)) {
> + int trailing_components = count_dir_separators(remaining.buf) +
> + (remaining.len != 0);
> /* error out unless this was the last component */
> - if (errno != ENOENT || remaining.len) {
> + if (errno != ENOENT ||
> + !(missing_components == -1 ||
> + trailing_components < missing_components)) {
> if (die_on_error)
> die_errno("Invalid path '%s'",
> resolved->buf);
> diff --git a/cache.h b/cache.h
> index c0072d43b1..ee4bc5ec04 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1320,6 +1320,8 @@ static inline int is_absolute_path(const char *path)
> int is_directory(const char *);
> char *strbuf_realpath(struct strbuf *resolved, const char *path,
> int die_on_error);
> +char *strbuf_realpath_missing(struct strbuf *resolved, const char *path,
> + int missing_components, int die_on_error);
> char *real_pathdup(const char *path, int die_on_error);
> const char *absolute_path(const char *path);
> char *absolute_pathdup(const char *path);
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/2] rev-parse options for absolute or relative paths
2020-11-27 23:19 [PATCH v3 0/2] rev-parse options for absolute or relative paths brian m. carlson
2020-11-27 23:19 ` [PATCH v3 1/2] abspath: add a function to resolve paths with missing components brian m. carlson
2020-11-27 23:19 ` [PATCH v3 2/2] rev-parse: add option for absolute or relative path formatting brian m. carlson
@ 2020-12-02 13:12 ` Johannes Schindelin
2 siblings, 0 replies; 8+ messages in thread
From: Johannes Schindelin @ 2020-12-02 13:12 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Emily Shaffer
Hi brian,
On Fri, 27 Nov 2020, brian m. carlson wrote:
> There are a bunch of different situations in which one would like to
> have an absolute and canonical or a relative path from Git. In many of
> these cases, these values are already available from git rev-parse, but
> some values only come in one form or another.
>
> Many operating systems, such as macOS, lack a built-in realpath command
> that can canonicalize paths properly, and additionally some programming
> languages, like Go, currently do as well. It's therefore helpful for us
> to provide a generic way to request that a path is fully canonicalized
> before using it. Since users may wish for a relative path, we can
> provide one of those as well.
>
> The primary impetus for this was Git LFS, which needs paths to be
> canonicalized in the same way as Git in order to function correctly.
> See https://github.com/git-lfs/git-lfs/issues/4012 for more details.
Sounds good!
I offered a step back that would allow us to do without
`count_dir_separators()` (or `count_components()`) altogether.
Otherwise, this patch series still looks good to me.
Thanks,
Dscho
>
> Changes from v2:
>
> * Incorporate multiple missing segment support into the strbuf_realpath
> code.
> * Switch some invocations to use DEFAULT_UNMODIFIED, which should not
> result in a change in behavior.
> * Rebase, resolving some conflicts.
>
> Changes from v1:
>
> * Add a function to handle missing trailing components when
> canonicalizing paths and use it.
> * Improve commit messages.
> * Fix broken && chain.
> * Fix situation where relative paths are not relative.
>
> brian m. carlson (2):
> abspath: add a function to resolve paths with missing components
> rev-parse: add option for absolute or relative path formatting
>
> Documentation/git-rev-parse.txt | 71 +++++++++++++---------
> abspath.c | 33 +++++++++-
> builtin/rev-parse.c | 104 ++++++++++++++++++++++++++++----
> cache.h | 2 +
> t/t1500-rev-parse.sh | 57 ++++++++++++++++-
> 5 files changed, 223 insertions(+), 44 deletions(-)
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] abspath: add a function to resolve paths with missing components
2020-12-02 13:09 ` Johannes Schindelin
@ 2020-12-02 23:54 ` brian m. carlson
0 siblings, 0 replies; 8+ messages in thread
From: brian m. carlson @ 2020-12-02 23:54 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Emily Shaffer
[-- Attachment #1: Type: text/plain, Size: 726 bytes --]
On 2020-12-02 at 13:09:49, Johannes Schindelin wrote:
> I am having a hard time with the name `missing_components`. Taking a step
> back, I think that we are interested in essentially two modes: allow one
> missing component, or an arbitrary amount of missing components.
>
> If this assumption is correct, we may not even need to count components at
> all. We only need a flag indicating whether we allow only the last
> component to be missing, or any number of components. Maybe something like
> `error_on_missing_parent_directory` or some such?
That's certainly easier. I hope to get a reroll out this week, and I'll
simplify when I do that.
--
brian m. carlson (he/him or they/them)
Houston, Texas, US
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-12-02 23:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-27 23:19 [PATCH v3 0/2] rev-parse options for absolute or relative paths brian m. carlson
2020-11-27 23:19 ` [PATCH v3 1/2] abspath: add a function to resolve paths with missing components brian m. carlson
2020-11-28 10:08 ` René Scharfe
2020-11-28 18:41 ` brian m. carlson
2020-12-02 13:09 ` Johannes Schindelin
2020-12-02 23:54 ` brian m. carlson
2020-11-27 23:19 ` [PATCH v3 2/2] rev-parse: add option for absolute or relative path formatting brian m. carlson
2020-12-02 13:12 ` [PATCH v3 0/2] rev-parse options for absolute or relative paths Johannes Schindelin
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).