* [PATCH] apply: strip ./ prefix from --directory argument
@ 2026-02-13 17:08 Joaquim Rocha via GitGitGadget
2026-02-17 8:06 ` Patrick Steinhardt
2026-02-18 0:15 ` [PATCH v2] apply: normalize path in " Joaquim Rocha via GitGitGadget
0 siblings, 2 replies; 7+ messages in thread
From: Joaquim Rocha via GitGitGadget @ 2026-02-13 17:08 UTC (permalink / raw)
To: git; +Cc: Joaquim Rocha, Joaquim Rocha
From: Joaquim Rocha <joaquim@amutable.com>
When passing a relative path like --directory=./some/sub, the leading
"./" caused apply to prepend it literally to patch filenames, resulting
in an error (invalid path).
Since using "./" is almost memory muscle for many, strip the "./"
prefix so it behaves the same as --directory=some/sub.
Signed-off-by: Joaquim Rocha <joaquim@amutable.com>
---
apply: strip ./ prefix from --directory argument
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2198%2Fjoaquimrocha%2Fapply-directory-dot-slash-prefix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2198/joaquimrocha/apply-directory-dot-slash-prefix-v1
Pull-Request: https://github.com/git/git/pull/2198
apply.c | 4 ++++
t/t4128-apply-root.sh | 9 +++++++++
2 files changed, 13 insertions(+)
diff --git a/apply.c b/apply.c
index 3de4aa4d2e..a44c54077c 100644
--- a/apply.c
+++ b/apply.c
@@ -5001,6 +5001,10 @@ static int apply_option_parse_directory(const struct option *opt,
BUG_ON_OPT_NEG(unset);
strbuf_reset(&state->root);
+
+ if (starts_with(arg, "./"))
+ arg += 2;
+
strbuf_addstr(&state->root, arg);
strbuf_complete(&state->root, '/');
return 0;
diff --git a/t/t4128-apply-root.sh b/t/t4128-apply-root.sh
index f6db5a79dd..2f446a4d69 100755
--- a/t/t4128-apply-root.sh
+++ b/t/t4128-apply-root.sh
@@ -43,6 +43,15 @@ test_expect_success 'apply --directory -p (2) ' '
'
+test_expect_success 'apply --directory (./ prefix)' '
+ git reset --hard initial &&
+ git apply --directory=./some/sub -p3 --index patch &&
+ echo Bello >expect &&
+ git show :some/sub/dir/file >actual &&
+ test_cmp expect actual &&
+ test_cmp expect some/sub/dir/file
+'
+
cat > patch << EOF
diff --git a/newfile b/newfile
new file mode 100644
base-commit: 6fcee4785280a08e7f271bd015a4dc33753e2886
--
gitgitgadget
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] apply: strip ./ prefix from --directory argument
2026-02-13 17:08 [PATCH] apply: strip ./ prefix from --directory argument Joaquim Rocha via GitGitGadget
@ 2026-02-17 8:06 ` Patrick Steinhardt
2026-02-17 20:27 ` Junio C Hamano
2026-02-18 0:15 ` [PATCH v2] apply: normalize path in " Joaquim Rocha via GitGitGadget
1 sibling, 1 reply; 7+ messages in thread
From: Patrick Steinhardt @ 2026-02-17 8:06 UTC (permalink / raw)
To: Joaquim Rocha via GitGitGadget; +Cc: git, Joaquim Rocha, Joaquim Rocha
On Fri, Feb 13, 2026 at 05:08:30PM +0000, Joaquim Rocha via GitGitGadget wrote:
> From: Joaquim Rocha <joaquim@amutable.com>
>
> When passing a relative path like --directory=./some/sub, the leading
> "./" caused apply to prepend it literally to patch filenames, resulting
> in an error (invalid path).
>
> Since using "./" is almost memory muscle for many, strip the "./"
> prefix so it behaves the same as --directory=some/sub.
Isn't the problem wider than that though? For example, if you had
"././some/sub" it would break again. Or if you had "some/./sub", or
"some/sub/../sub", or "some//sub".
> diff --git a/apply.c b/apply.c
> index 3de4aa4d2e..a44c54077c 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -5001,6 +5001,10 @@ static int apply_option_parse_directory(const struct option *opt,
> BUG_ON_OPT_NEG(unset);
>
> strbuf_reset(&state->root);
> +
> + if (starts_with(arg, "./"))
> + arg += 2;
> +
> strbuf_addstr(&state->root, arg);
> strbuf_complete(&state->root, '/');
> return 0;
While this change here fixes your observed issues, the next person might
run into a totally different one. So more generally, I think what we'd
rather want to do is to fully normalize the path. How about this
instead:
diff --git a/apply.c b/apply.c
index 9de2eb953e..8946b133a3 100644
--- a/apply.c
+++ b/apply.c
@@ -5002,6 +5002,7 @@ static int apply_option_parse_directory(const struct option *opt,
strbuf_reset(&state->root);
strbuf_addstr(&state->root, arg);
+ strbuf_normalize_path(&state->root);
strbuf_complete(&state->root, '/');
return 0;
}
`strbuf_normalize_path()` drops "." components, removes ".." and it
squashes multiple directory separators. So it handles your specific use
case, but also others.
Thanks!
Patrick
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] apply: strip ./ prefix from --directory argument
2026-02-17 8:06 ` Patrick Steinhardt
@ 2026-02-17 20:27 ` Junio C Hamano
2026-02-18 14:14 ` Patrick Steinhardt
0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2026-02-17 20:27 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Joaquim Rocha via GitGitGadget, git, Joaquim Rocha, Joaquim Rocha
Patrick Steinhardt <ps@pks.im> writes:
> While this change here fixes your observed issues, the next person might
> run into a totally different one. So more generally, I think what we'd
> rather want to do is to fully normalize the path. How about this
> instead:
Sorry, but I am confused. Why isn't "don't do it then" a good
answer for a case like this?
>
> diff --git a/apply.c b/apply.c
> index 9de2eb953e..8946b133a3 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -5002,6 +5002,7 @@ static int apply_option_parse_directory(const struct option *opt,
>
> strbuf_reset(&state->root);
> strbuf_addstr(&state->root, arg);
> + strbuf_normalize_path(&state->root);
> strbuf_complete(&state->root, '/');
> return 0;
> }
>
> `strbuf_normalize_path()` drops "." components, removes ".." and it
> squashes multiple directory separators. So it handles your specific use
> case, but also others.
>
> Thanks!
>
> Patrick
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] apply: strip ./ prefix from --directory argument
2026-02-17 20:27 ` Junio C Hamano
@ 2026-02-18 14:14 ` Patrick Steinhardt
2026-02-18 14:40 ` Joaquim Rocha
0 siblings, 1 reply; 7+ messages in thread
From: Patrick Steinhardt @ 2026-02-18 14:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: Joaquim Rocha via GitGitGadget, git, Joaquim Rocha, Joaquim Rocha
On Tue, Feb 17, 2026 at 12:27:39PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > While this change here fixes your observed issues, the next person might
> > run into a totally different one. So more generally, I think what we'd
> > rather want to do is to fully normalize the path. How about this
> > instead:
>
> Sorry, but I am confused. Why isn't "don't do it then" a good
> answer for a case like this?
I guess that's fair, but especially with command line completion it
might be easy to arrive at such paths. That may or may not be a good
argument, I'm not sure myself.
Patrick
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] apply: strip ./ prefix from --directory argument
2026-02-18 14:14 ` Patrick Steinhardt
@ 2026-02-18 14:40 ` Joaquim Rocha
0 siblings, 0 replies; 7+ messages in thread
From: Joaquim Rocha @ 2026-02-18 14:40 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Junio C Hamano, Joaquim Rocha via GitGitGadget, git,
Joaquim Rocha
On Wed, Feb 18, 2026 at 2:14 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> I guess that's fair, but especially with command line completion it
> might be easy to arrive at such paths. That may or may not be a good
> argument, I'm not sure myself.
>
> Patrick
I understand that. TBH, while not at your (plural) level of expertise
in git, I am certainly an experienced user and still the error from
the ./ use got me confused. So my proposal goes towards developer
experience, like the similar command suggestions it also has. If
there's no down side to it, I think it makes sense to have it.
An alternative would be to have a better error message if we detect
there are redundant path artifacts, but in that case why wouldn't we
just normalize the path as it's the user's clear intention?
Cheers,
Joaquim Rocha
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] apply: normalize path in --directory argument
2026-02-13 17:08 [PATCH] apply: strip ./ prefix from --directory argument Joaquim Rocha via GitGitGadget
2026-02-17 8:06 ` Patrick Steinhardt
@ 2026-02-18 0:15 ` Joaquim Rocha via GitGitGadget
2026-02-20 20:26 ` Junio C Hamano
1 sibling, 1 reply; 7+ messages in thread
From: Joaquim Rocha via GitGitGadget @ 2026-02-18 0:15 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Joaquim Rocha, Joaquim Rocha
From: Joaquim Rocha <joaquim@amutable.com>
When passing a relative path like --directory=./some/sub, the leading
"./" caused apply to prepend it literally to patch filenames, resulting
in an error (invalid path).
There may be more cases like this where users pass some/./path to the
directory which can easily be normalized to an acceptable path, so
these changes try to normalize the path before using it.
Signed-off-by: Joaquim Rocha <joaquim@amutable.com>
---
apply: strip ./ prefix from --directory argument
Changes since v1:
* Normalized the path as Patrick recommended
* Return error is we have ../ at the beginning of the --directory
argument
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2198%2Fjoaquimrocha%2Fapply-directory-dot-slash-prefix-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2198/joaquimrocha/apply-directory-dot-slash-prefix-v2
Pull-Request: https://github.com/git/git/pull/2198
Range-diff vs v1:
1: 82e24c3471 ! 1: 8ac66a876d apply: strip ./ prefix from --directory argument
@@ Metadata
Author: Joaquim Rocha <joaquim@amutable.com>
## Commit message ##
- apply: strip ./ prefix from --directory argument
+ apply: normalize path in --directory argument
When passing a relative path like --directory=./some/sub, the leading
"./" caused apply to prepend it literally to patch filenames, resulting
in an error (invalid path).
-
- Since using "./" is almost memory muscle for many, strip the "./"
- prefix so it behaves the same as --directory=some/sub.
+ There may be more cases like this where users pass some/./path to the
+ directory which can easily be normalized to an acceptable path, so
+ these changes try to normalize the path before using it.
Signed-off-by: Joaquim Rocha <joaquim@amutable.com>
## apply.c ##
@@ apply.c: static int apply_option_parse_directory(const struct option *opt,
- BUG_ON_OPT_NEG(unset);
strbuf_reset(&state->root);
+ strbuf_addstr(&state->root, arg);
+
-+ if (starts_with(arg, "./"))
-+ arg += 2;
++ if (strbuf_normalize_path(&state->root) < 0)
++ return error(_("unable to normalize directory: '%s'"), arg);
+
- strbuf_addstr(&state->root, arg);
strbuf_complete(&state->root, '/');
return 0;
+ }
## t/t4128-apply-root.sh ##
@@ t/t4128-apply-root.sh: test_expect_success 'apply --directory -p (2) ' '
@@ t/t4128-apply-root.sh: test_expect_success 'apply --directory -p (2) ' '
+ test_cmp expect actual &&
+ test_cmp expect some/sub/dir/file
+'
++
++test_expect_success 'apply --directory (double slash)' '
++ git reset --hard initial &&
++ git apply --directory=some//sub -p3 --index patch &&
++ echo Bello >expect &&
++ git show :some/sub/dir/file >actual &&
++ test_cmp expect actual &&
++ test_cmp expect some/sub/dir/file
++'
++
++test_expect_success 'apply --directory (./ in the middle)' '
++ git reset --hard initial &&
++ git apply --directory=some/./sub -p3 --index patch &&
++ echo Bello >expect &&
++ git show :some/sub/dir/file >actual &&
++ test_cmp expect actual &&
++ test_cmp expect some/sub/dir/file
++'
++
++test_expect_success 'apply --directory (../ in the middle)' '
++ git reset --hard initial &&
++ git apply --directory=some/../some/sub -p3 --index patch &&
++ echo Bello >expect &&
++ git show :some/sub/dir/file >actual &&
++ test_cmp expect actual &&
++ test_cmp expect some/sub/dir/file
++'
++
++test_expect_success 'apply --directory rejects leading ../' '
++ test_must_fail git apply --directory=../foo -p3 patch 2>err &&
++ test_grep "unable to normalize directory" err
++'
+
cat > patch << EOF
diff --git a/newfile b/newfile
apply.c | 4 ++++
t/t4128-apply-root.sh | 41 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 45 insertions(+)
diff --git a/apply.c b/apply.c
index 3de4aa4d2e..7f0ce5918b 100644
--- a/apply.c
+++ b/apply.c
@@ -5002,6 +5002,10 @@ static int apply_option_parse_directory(const struct option *opt,
strbuf_reset(&state->root);
strbuf_addstr(&state->root, arg);
+
+ if (strbuf_normalize_path(&state->root) < 0)
+ return error(_("unable to normalize directory: '%s'"), arg);
+
strbuf_complete(&state->root, '/');
return 0;
}
diff --git a/t/t4128-apply-root.sh b/t/t4128-apply-root.sh
index f6db5a79dd..5eba15fa66 100755
--- a/t/t4128-apply-root.sh
+++ b/t/t4128-apply-root.sh
@@ -43,6 +43,47 @@ test_expect_success 'apply --directory -p (2) ' '
'
+test_expect_success 'apply --directory (./ prefix)' '
+ git reset --hard initial &&
+ git apply --directory=./some/sub -p3 --index patch &&
+ echo Bello >expect &&
+ git show :some/sub/dir/file >actual &&
+ test_cmp expect actual &&
+ test_cmp expect some/sub/dir/file
+'
+
+test_expect_success 'apply --directory (double slash)' '
+ git reset --hard initial &&
+ git apply --directory=some//sub -p3 --index patch &&
+ echo Bello >expect &&
+ git show :some/sub/dir/file >actual &&
+ test_cmp expect actual &&
+ test_cmp expect some/sub/dir/file
+'
+
+test_expect_success 'apply --directory (./ in the middle)' '
+ git reset --hard initial &&
+ git apply --directory=some/./sub -p3 --index patch &&
+ echo Bello >expect &&
+ git show :some/sub/dir/file >actual &&
+ test_cmp expect actual &&
+ test_cmp expect some/sub/dir/file
+'
+
+test_expect_success 'apply --directory (../ in the middle)' '
+ git reset --hard initial &&
+ git apply --directory=some/../some/sub -p3 --index patch &&
+ echo Bello >expect &&
+ git show :some/sub/dir/file >actual &&
+ test_cmp expect actual &&
+ test_cmp expect some/sub/dir/file
+'
+
+test_expect_success 'apply --directory rejects leading ../' '
+ test_must_fail git apply --directory=../foo -p3 patch 2>err &&
+ test_grep "unable to normalize directory" err
+'
+
cat > patch << EOF
diff --git a/newfile b/newfile
new file mode 100644
base-commit: 6fcee4785280a08e7f271bd015a4dc33753e2886
--
gitgitgadget
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] apply: normalize path in --directory argument
2026-02-18 0:15 ` [PATCH v2] apply: normalize path in " Joaquim Rocha via GitGitGadget
@ 2026-02-20 20:26 ` Junio C Hamano
0 siblings, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2026-02-20 20:26 UTC (permalink / raw)
To: Joaquim Rocha via GitGitGadget
Cc: git, Patrick Steinhardt, Joaquim Rocha, Joaquim Rocha
"Joaquim Rocha via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Joaquim Rocha <joaquim@amutable.com>
>
> When passing a relative path like --directory=./some/sub, the leading
> "./" caused apply to prepend it literally to patch filenames, resulting
> in an error (invalid path).
> There may be more cases like this where users pass some/./path to the
> directory which can easily be normalized to an acceptable path, so
> these changes try to normalize the path before using it.
>
> Signed-off-by: Joaquim Rocha <joaquim@amutable.com>
> ---
> apply: strip ./ prefix from --directory argument
>
> Changes since v1:
>
> * Normalized the path as Patrick recommended
Sounds like a sensible direction to go.
Will queue. Thanks. (of course, further reviews welcome).
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-02-20 20:27 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-13 17:08 [PATCH] apply: strip ./ prefix from --directory argument Joaquim Rocha via GitGitGadget
2026-02-17 8:06 ` Patrick Steinhardt
2026-02-17 20:27 ` Junio C Hamano
2026-02-18 14:14 ` Patrick Steinhardt
2026-02-18 14:40 ` Joaquim Rocha
2026-02-18 0:15 ` [PATCH v2] apply: normalize path in " Joaquim Rocha via GitGitGadget
2026-02-20 20:26 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox