On 2025-07-05 at 22:50:43, redoste wrote:
> On Sat Jul 5, 2025 at 22:07 CEST, brian m. carlson wrote:
> > On 2025-07-05 at 19:21:13, Jeff King wrote:
> > I don't have a strong view either way, but I do wonder if it's a good
> > idea to have the testsuite poking around in `/tmp`, although maybe if we
> > honour `TMPDIR` then it would be possible to do in a tidy way.
> I looked into adding a test, but I didn't find any other tests checking
> for temporary files and I agree that messing in /tmp doesn't feel really
> appropriate for the testsuite.
>
> Maybe something like this?
>
> diff --git a/t/t7528-signed-commit-ssh.sh b/t/t7528-signed-commit-ssh.sh
> index 065f780636..359dc8eba8 100755
> --- a/t/t7528-signed-commit-ssh.sh
> +++ b/t/t7528-signed-commit-ssh.sh
> @@ -85,6 +85,7 @@ test_expect_success GPGSSH 'sign commits using literal public keys with ssh-agen
> eval $(ssh-agent) &&
> test_when_finished "kill ${SSH_AGENT_PID}" &&
> ssh-add "${GPGSSH_KEY_PRIMARY}" &&
> + export TMPDIR=$(pwd) &&
> echo 1 >file && git add file &&
> git commit -a -m rsa-inline -S"$(cat "${GPGSSH_KEY_PRIMARY}.pub")" &&
> echo 2 >file &&
> @@ -95,7 +96,8 @@ test_expect_success GPGSSH 'sign commits using literal public keys with ssh-agen
> git commit -a -m ecdsa-inline -S"key::$(cat "${GPGSSH_KEY_ECDSA}.pub")" &&
> echo 4 >file &&
> test_config user.signingkey "key::$(cat "${GPGSSH_KEY_ECDSA}.pub")" &&
> - git commit -a -m ecdsa-config -S
> + git commit -a -m ecdsa-config -S &&
> + ! ls .git_signing_key_tmp*
> '
What I would recommend is adding another test (that is, another
`test_expect_success` assertion) that when signing we don't leave any
temporary files behind. You can then squash that into your patch and
send a v2.
As for `ls`, I wouldn't use that in scripting. I'd use something like
this:
----
mkdir tmpdir &&
TMPDIR="$(pwd)/tmpdir" &&
export TMPDIR &&
# Other code
find tmpdir -type f >files &&
test_line_count = 0 files
----
I think `find` is typically better in scripting and also in this case
with a separate directory as the temporary directory we'll find any left
over temporary files, not just the pattern we've fixed here.
Our coding guidelines point out that some shells don't like `export`
with assignments, so we do those as two lines. Normally I would write
what you have in my day-to-day life, but we run on a variety of systems
that most people will never use.
> test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'create signed commits with keys having defined lifetimes' '
>
> I can add it in a v2 if you think it's a good way to test it.
I think the basic premise is sound, but a separate assertion would make
it easier to readers why we're setting the temporary directory.
> > I also have some friends who are trans and have transitioned or are in
> > the process of transitioning but have simply not gotten around to
> > getting legal paperwork done[1].
> This is the exact reason why I'm not very comfortable with using my
> legal or real name, (well, it's mostly because I still can't find a name
> I like).
> And since it's a simple patch that's probably not even copyrightable, I
> figured out that using a pseudonym was fine.
>
> Since I knew that the Linux kernel changed their documentation to remove
> the use of "real name", I thought it was more common and didn't relly
> think about it a lot.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d4563201f33a022fc0353033d9dfeb1606a88330
>
> I'm sorry, I should have read the git documentation more thoroughly.
>
> If it's really an issue I don't mind signing off with a different and
> more distinctive name.
No, I think this is fine. You weren't obligated to explain that and our
policies should gracefully handle this situation regardless, but given
what I said before and this context, I think the name you have is fine.
Git unfortunately has poor support for replacing names and I would't
want you to have to put your deadname into our history for all of
eternity.
I'll send a patch to fix the policy.
--
brian m. carlson (they/them)
Toronto, Ontario, CA
Attachment:
signature.asc
Description: PGP signature
