* Re: [PATCH] gpg-interface: expand gpg.program as a path
2025-07-11 23:23 ` [PATCH] gpg-interface: expand gpg.program as a path Jonas Brandstötter
@ 2025-07-14 17:10 ` Junio C Hamano
2025-07-22 19:09 ` [PATCH v2 0/2] " Jonas Brandstötter
` (2 subsequent siblings)
3 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2025-07-14 17:10 UTC (permalink / raw)
To: Jonas Brandstötter; +Cc: git, Fabian Stelzer, Patrick Steinhardt
Jonas Brandstötter <jonas.brandstoetter@gmx.at> writes:
> This allows using a custom gpg program under the user's home directory
> by specifying a path starting with '~'
>
> [gpg]
> program = "~/.local/bin/mygpg"
>
> Signed-off-by: Jonas Brandstötter <jonas.brandstoetter@gmx.at>
> ---
> First time interacting with a project via a mailing list. Do let me know if
> I did something very dumb.
Thanks. The update to the codumentation to explicitly say that the
variable is about "pathname" is a very nice touch, and the code
change is trivially correct, I guess.
I wonder if we can have some test to protect this feature from
broken by mistake, though.
> Documentation/config/gpg.adoc | 2 +-
> gpg-interface.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/config/gpg.adoc b/Documentation/config/gpg.adoc
> index 5cf32b179d..240e46c050 100644
> --- a/Documentation/config/gpg.adoc
> +++ b/Documentation/config/gpg.adoc
> @@ -1,5 +1,5 @@
> gpg.program::
> - Use this custom program instead of "`gpg`" found on `$PATH` when
> + Pathname of the program to use instead of "`gpg`" when
> making or verifying a PGP signature. The program must support the
> same command-line interface as GPG, namely, to verify a detached
> signature, "`gpg --verify $signature - <$file`" is run, and the
> diff --git a/gpg-interface.c b/gpg-interface.c
> index 0896458de5..3dfbc45385 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -783,7 +783,7 @@ static int git_gpg_config(const char *var, const char *value,
>
> if (fmtname) {
> fmt = get_format_by_name(fmtname);
> - return git_config_string((char **) &fmt->program, var, value);
> + return git_config_pathname((char **) &fmt->program, var, value);
> }
>
> return 0;
>
> base-commit: a30f80fde927d70950b3b4d1820813480968fb0d
> --
> 2.50.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v2 0/2] gpg-interface: expand gpg.program as a path
2025-07-11 23:23 ` [PATCH] gpg-interface: expand gpg.program as a path Jonas Brandstötter
2025-07-14 17:10 ` Junio C Hamano
@ 2025-07-22 19:09 ` Jonas Brandstötter
2025-07-22 20:28 ` Junio C Hamano
2025-07-22 19:09 ` [PATCH v2 1/2] " Jonas Brandstötter
2025-07-22 19:09 ` [PATCH v2 2/2] t7510: add test cases for non-absolute gpg program Jonas Brandstötter
3 siblings, 1 reply; 15+ messages in thread
From: Jonas Brandstötter @ 2025-07-22 19:09 UTC (permalink / raw)
To: git
Cc: Jonas Brandstötter, Fabian Stelzer, Patrick Steinhardt,
Junio C Hamano
Allows users to specify a custom gpg program in their home directory.
V2 adds test cases for when the gpg program is not set as an absolute path.
The test for gpg in a home directory feels a bit wonky to me, because it
assumes that `~` is an alias for `$HOME` and just overriding that variable
with the test directory. But short of creating a user while running the
tests, this is the best solution I could come up with.
Jonas Brandstötter (2):
gpg-interface: expand gpg.program as a path
t7510: add test cases for non-absolute gpg program
Documentation/config/gpg.adoc | 2 +-
gpg-interface.c | 2 +-
t/t7510-signed-commit.sh | 12 +++++++++++-
3 files changed, 13 insertions(+), 3 deletions(-)
Range-diff against v1:
1: b551903c16 = 1: b551903c16 gpg-interface: expand gpg.program as a path
-: ---------- > 2: ca22bf2ee6 t7510: add test cases for non-absolute gpg program
--
2.50.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 0/2] gpg-interface: expand gpg.program as a path
2025-07-22 19:09 ` [PATCH v2 0/2] " Jonas Brandstötter
@ 2025-07-22 20:28 ` Junio C Hamano
0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2025-07-22 20:28 UTC (permalink / raw)
To: Jonas Brandstötter; +Cc: git, Fabian Stelzer, Patrick Steinhardt
Jonas Brandstötter <jonas.brandstoetter@gmx.at> writes:
> Allows users to specify a custom gpg program in their home directory.
>
> V2 adds test cases for when the gpg program is not set as an absolute path.
> The test for gpg in a home directory feels a bit wonky to me, because it
> assumes that `~` is an alias for `$HOME` and just overriding that variable
> with the test directory. But short of creating a user while running the
> tests, this is the best solution I could come up with.
>
> Jonas Brandstötter (2):
> gpg-interface: expand gpg.program as a path
> t7510: add test cases for non-absolute gpg program
>
> Documentation/config/gpg.adoc | 2 +-
> gpg-interface.c | 2 +-
> t/t7510-signed-commit.sh | 12 +++++++++++-
> 3 files changed, 13 insertions(+), 3 deletions(-)
>
> Range-diff against v1:
> 1: b551903c16 = 1: b551903c16 gpg-interface: expand gpg.program as a path
> -: ---------- > 2: ca22bf2ee6 t7510: add test cases for non-absolute gpg program
> --
> 2.50.1
The first iteration of the patch was merged in the 'next' branch
long time ago, and then to the 'master' branch already. It is way
too late to wholesale replace the patch from the previous iteration.
It appears the first patch hasn't changed at all, so I'll just take
the second patch as a separate, follow-up topic to add a missing
test for already-graduated topic.
Thanks!
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/2] gpg-interface: expand gpg.program as a path
2025-07-11 23:23 ` [PATCH] gpg-interface: expand gpg.program as a path Jonas Brandstötter
2025-07-14 17:10 ` Junio C Hamano
2025-07-22 19:09 ` [PATCH v2 0/2] " Jonas Brandstötter
@ 2025-07-22 19:09 ` Jonas Brandstötter
2025-07-22 19:09 ` [PATCH v2 2/2] t7510: add test cases for non-absolute gpg program Jonas Brandstötter
3 siblings, 0 replies; 15+ messages in thread
From: Jonas Brandstötter @ 2025-07-22 19:09 UTC (permalink / raw)
To: git; +Cc: Jonas Brandstötter
This allows using a custom gpg program under the user's home directory
by specifying a path starting with '~'
[gpg]
program = "~/.local/bin/mygpg"
Signed-off-by: Jonas Brandstötter <jonas.brandstoetter@gmx.at>
---
Documentation/config/gpg.adoc | 2 +-
gpg-interface.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/config/gpg.adoc b/Documentation/config/gpg.adoc
index 5cf32b179d..240e46c050 100644
--- a/Documentation/config/gpg.adoc
+++ b/Documentation/config/gpg.adoc
@@ -1,5 +1,5 @@
gpg.program::
- Use this custom program instead of "`gpg`" found on `$PATH` when
+ Pathname of the program to use instead of "`gpg`" when
making or verifying a PGP signature. The program must support the
same command-line interface as GPG, namely, to verify a detached
signature, "`gpg --verify $signature - <$file`" is run, and the
diff --git a/gpg-interface.c b/gpg-interface.c
index 0896458de5..3dfbc45385 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -783,7 +783,7 @@ static int git_gpg_config(const char *var, const char *value,
if (fmtname) {
fmt = get_format_by_name(fmtname);
- return git_config_string((char **) &fmt->program, var, value);
+ return git_config_pathname((char **) &fmt->program, var, value);
}
return 0;
--
2.50.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 2/2] t7510: add test cases for non-absolute gpg program
2025-07-11 23:23 ` [PATCH] gpg-interface: expand gpg.program as a path Jonas Brandstötter
` (2 preceding siblings ...)
2025-07-22 19:09 ` [PATCH v2 1/2] " Jonas Brandstötter
@ 2025-07-22 19:09 ` Jonas Brandstötter
2025-07-25 4:30 ` Jeff King
3 siblings, 1 reply; 15+ messages in thread
From: Jonas Brandstötter @ 2025-07-22 19:09 UTC (permalink / raw)
To: git; +Cc: Jonas Brandstötter
These cases cover scenarios where `gpg.program` is set as a program in
`$PATH` or as a path relative to the user's home directory.
Signed-off-by: Jonas Brandstötter <jonas.brandstoetter@gmx.at>
---
t/t7510-signed-commit.sh | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 39677e859a..95d2ebe277 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -449,7 +449,17 @@ test_expect_success 'custom `gpg.program`' '
test_must_fail env LET_GPG_PROGRAM_FAIL=1 \
git commit -S --allow-empty -m must-fail 2>err &&
- grep zOMG err
+ grep zOMG err &&
+
+ # `gpg.program` starts with `~`, the path should be interpreted to be relative to `$HOME`
+ test_config gpg.program "~/fake-gpg" &&
+ env HOME="$(pwd)" \
+ git commit -S --allow-empty -m signed-commit &&
+
+ # `gpg.program` does not specify an absolute path, it should find a program in `$PATH`
+ test_config gpg.program "fake-gpg" &&
+ env PATH="$(pwd):$PATH" \
+ git commit -S --allow-empty -m signed-commit
'
test_done
--
2.50.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 2/2] t7510: add test cases for non-absolute gpg program
2025-07-22 19:09 ` [PATCH v2 2/2] t7510: add test cases for non-absolute gpg program Jonas Brandstötter
@ 2025-07-25 4:30 ` Jeff King
2025-07-25 5:13 ` Jeff King
2025-07-29 0:05 ` Junio C Hamano
0 siblings, 2 replies; 15+ messages in thread
From: Jeff King @ 2025-07-25 4:30 UTC (permalink / raw)
To: Jonas Brandstötter; +Cc: Junio C Hamano, git
On Tue, Jul 22, 2025 at 09:09:22PM +0200, Jonas Brandstötter wrote:
> test_must_fail env LET_GPG_PROGRAM_FAIL=1 \
> git commit -S --allow-empty -m must-fail 2>err &&
> - grep zOMG err
> + grep zOMG err &&
> +
> + # `gpg.program` starts with `~`, the path should be interpreted to be relative to `$HOME`
> + test_config gpg.program "~/fake-gpg" &&
> + env HOME="$(pwd)" \
> + git commit -S --allow-empty -m signed-commit &&
> +
> + # `gpg.program` does not specify an absolute path, it should find a program in `$PATH`
> + test_config gpg.program "fake-gpg" &&
> + env PATH="$(pwd):$PATH" \
> + git commit -S --allow-empty -m signed-commit
This second test seems to fail on Windows. E.g., in this CI job:
https://github.com/git/git/actions/runs/16509422831/job/46688307091
Right before the failure, the trace shows that we are setting PATH like
this:
++env 'PATH=D:/a/git/git/t/trash directory.t7510-signed-commit:/d/a/git/git:/d/a/git/git/t/helper:/c/Users/runneradmin/path:/mingw64/bin:/usr/bin/:/usr/bin/core_perl:/c/WINDOWS/system32:/c/WINDOWS:/c/WINDOWS/System32/Wbem'
Should it be "/d/a/git/git/..." instead of "D:/a/git/git/..."? Which we
could get by using $PWD, I think.
The earlier one using $HOME uses D:/, but this one is different because
colons are meaningful separators in $PATH.
-Peff
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] t7510: add test cases for non-absolute gpg program
2025-07-25 4:30 ` Jeff King
@ 2025-07-25 5:13 ` Jeff King
2025-07-29 0:05 ` Junio C Hamano
1 sibling, 0 replies; 15+ messages in thread
From: Jeff King @ 2025-07-25 5:13 UTC (permalink / raw)
To: Jonas Brandstötter; +Cc: Junio C Hamano, git
On Fri, Jul 25, 2025 at 12:30:43AM -0400, Jeff King wrote:
> This second test seems to fail on Windows. E.g., in this CI job:
> [...]
> Should it be "/d/a/git/git/..." instead of "D:/a/git/git/..."? Which we
> could get by using $PWD, I think.
Yeah, that is it. I found a commit with some prior art, and here is the
passing CI result:
https://github.com/peff/git/actions/runs/16513873854
> The earlier one using $HOME uses D:/, but this one is different because
> colons are meaningful separators in $PATH.
I do wonder if that one needs to set $HOME at all. The test harness
already points $HOME to the trash directory. But maybe it has value to
document the expectation.
Anyway, here is the fix as a patch on top of jb/t7510-gpg-program-path.
I am also happy for it to be squashed in, since I think the topic is not
yet in 'next'.
-- >8 --
Subject: [PATCH] t7510: use $PWD instead of $(pwd) inside PATH
On Windows, $(pwd) will give us a Windows-style path like "D:/foo".
Putting that into $PATH confuses anybody parsing that variable, since
colon is a separator character in $PATH. Instead, we should use the
Unix-style value we get from $PWD ("/d/foo").
This is similar to the cases fixed by 71dd50472d (t0021, t5615: use $PWD
instead of $(pwd) in PATH-like shell variables, 2016-11-11).
Signed-off-by: Jeff King <peff@peff.net>
---
t/t7510-signed-commit.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 95d2ebe277..1201c85ba6 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -458,7 +458,7 @@ test_expect_success 'custom `gpg.program`' '
# `gpg.program` does not specify an absolute path, it should find a program in `$PATH`
test_config gpg.program "fake-gpg" &&
- env PATH="$(pwd):$PATH" \
+ env PATH="$PWD:$PATH" \
git commit -S --allow-empty -m signed-commit
'
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 2/2] t7510: add test cases for non-absolute gpg program
2025-07-25 4:30 ` Jeff King
2025-07-25 5:13 ` Jeff King
@ 2025-07-29 0:05 ` Junio C Hamano
2025-07-29 7:12 ` Jeff King
1 sibling, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2025-07-29 0:05 UTC (permalink / raw)
To: Jeff King; +Cc: Jonas Brandstötter, git
Jeff King <peff@peff.net> writes:
> This second test seems to fail on Windows. E.g., in this CI job:
>
> https://github.com/git/git/actions/runs/16509422831/job/46688307091
>
> Right before the failure, the trace shows that we are setting PATH like
> this:
>
> ++env 'PATH=D:/a/git/git/t/trash directory.t7510-signed-commit:/d/a/git/git:/d/a/git/git/t/helper:/c/Users/runneradmin/path:/mingw64/bin:/usr/bin/:/usr/bin/core_perl:/c/WINDOWS/system32:/c/WINDOWS:/c/WINDOWS/System32/Wbem'
>
> Should it be "/d/a/git/git/..." instead of "D:/a/git/git/..."? Which we
> could get by using $PWD, I think.
>
> The earlier one using $HOME uses D:/, but this one is different because
> colons are meaningful separators in $PATH.
Here is what I have on top of the posted patches. If today's
integration goes well, I plan to merge it to 'next'; the rest of the
series is already in 'next'.
---- >8 ----
Subject: [PATCH] t7510: Windows fix
$PATH and $(pwd) does not mix very well, because PATH is a colon
separated list of directories, but on Windows port of the shell
Git-for-Windows uses, $(pwd) looks like "D:/path/to/a/directory".
With $PWD, we would get /d/path/to/a/directory instead, which would
fit better on $PATH. This broke Windows CI job.
While at it, drop unnecessary use of "env VAR=VAL" before "git
commit"; one-shot export "VAR=VAL git commit" is sufficient.
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t7510-signed-commit.sh | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 95d2ebe277..c51e2e2589 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -453,13 +453,11 @@ test_expect_success 'custom `gpg.program`' '
# `gpg.program` starts with `~`, the path should be interpreted to be relative to `$HOME`
test_config gpg.program "~/fake-gpg" &&
- env HOME="$(pwd)" \
- git commit -S --allow-empty -m signed-commit &&
+ HOME="$(pwd)" git commit -S --allow-empty -m signed-commit &&
# `gpg.program` does not specify an absolute path, it should find a program in `$PATH`
test_config gpg.program "fake-gpg" &&
- env PATH="$(pwd):$PATH" \
- git commit -S --allow-empty -m signed-commit
+ PATH="$PWD:$PATH" git commit -S --allow-empty -m signed-commit
'
test_done
--
2.50.1-589-gf1cdebda82
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] t7510: add test cases for non-absolute gpg program
2025-07-29 0:05 ` Junio C Hamano
@ 2025-07-29 7:12 ` Jeff King
2025-07-29 15:06 ` Junio C Hamano
0 siblings, 1 reply; 15+ messages in thread
From: Jeff King @ 2025-07-29 7:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonas Brandstötter, git
On Mon, Jul 28, 2025 at 05:05:46PM -0700, Junio C Hamano wrote:
> > Should it be "/d/a/git/git/..." instead of "D:/a/git/git/..."? Which we
> > could get by using $PWD, I think.
> >
> > The earlier one using $HOME uses D:/, but this one is different because
> > colons are meaningful separators in $PATH.
>
> Here is what I have on top of the posted patches. If today's
> integration goes well, I plan to merge it to 'next'; the rest of the
> series is already in 'next'.
Looks good. Not sure if you saw the patch I posted in this thread. It's
roughly the same as yours, though I didn't drop the useless "env" (which
I agree is useless).
I did reference 71dd50472d (t0021, t5615: use $PWD instead of $(pwd) in
PATH-like shell variables, 2016-11-11) to try to give more explanation
of the two different sources. But re-reading it, it actually doesn't
really clarify much. ;) So maybe not worth worrying about.
-Peff
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] t7510: add test cases for non-absolute gpg program
2025-07-29 7:12 ` Jeff King
@ 2025-07-29 15:06 ` Junio C Hamano
0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2025-07-29 15:06 UTC (permalink / raw)
To: Jeff King; +Cc: Jonas Brandstötter, git
Jeff King <peff@peff.net> writes:
> On Mon, Jul 28, 2025 at 05:05:46PM -0700, Junio C Hamano wrote:
>
>> > Should it be "/d/a/git/git/..." instead of "D:/a/git/git/..."? Which we
>> > could get by using $PWD, I think.
>> >
>> > The earlier one using $HOME uses D:/, but this one is different because
>> > colons are meaningful separators in $PATH.
>>
>> Here is what I have on top of the posted patches. If today's
>> integration goes well, I plan to merge it to 'next'; the rest of the
>> series is already in 'next'.
>
> Looks good. Not sure if you saw the patch I posted in this thread. It's
> roughly the same as yours, though I didn't drop the useless "env" (which
> I agree is useless).
Ah, sorry, no I missed it. Will replace with your variant.
> I did reference 71dd50472d (t0021, t5615: use $PWD instead of $(pwd) in
> PATH-like shell variables, 2016-11-11) to try to give more explanation
> of the two different sources. But re-reading it, it actually doesn't
> really clarify much. ;) So maybe not worth worrying about.
>
> -Peff
Thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread