From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org,
Shubham Kanodia <shubham.kanodia10@gmail.com>,
Derrick Stolee <stolee@gmail.com>
Subject: Re: [PATCH v2] builtin/gc: fix crash when running `git maintenance start`
Date: Wed, 09 Oct 2024 10:10:25 -0700 [thread overview]
Message-ID: <xmqq8quxw6bi.fsf@gitster.g> (raw)
In-Reply-To: <5798c31e1ef9346e7faf73f8c80b32c436937a8a.1728455715.git.ps@pks.im> (Patrick Steinhardt's message of "Wed, 9 Oct 2024 09:08:33 +0200")
Patrick Steinhardt <ps@pks.im> writes:
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index a66d0e089d..e75b485319 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -646,6 +646,22 @@ test_expect_success !MINGW 'register and unregister with regex metacharacters' '
> maintenance.repo "$(pwd)/$META"
> '
>
> +test_expect_success 'start without GIT_TEST_MAINT_SCHEDULER' '
> + test_when_finished "rm -rf systemctl.log script repo" &&
> + mkdir script &&
> + write_script script/systemctl <<-EOF &&
> + echo "\$*" >>"$(pwd)"/systemctl.log
> + EOF
We deliberately include one SP in the name of the trash directory
when we create it under t/ so that we can catch a bug easily when we
forget quoting "$TRASH_DIRECTORY", but otherwise the name under t/
is safe.
But this use of "$(pwd)" is a bit iffy. It is expanded when the
script is written, which means the script is designed to write to
the systemctl.log file in a directory whose absolute path is known
and hardcoded in the script. We assume that the leading directories
down to the t/ directory from the root can also safely quoted by
simply enclosing inside "a pair of double quotes", which might be
iffy (obviously if $(pwd) had a double-quote in it, the quoting
would break).
I'll let it pass (and will forget blissfully if nobody screams).
Other than that, looking good. It certainly is good that there is
something more widely supported than "cron" so that we can write
this test as a generic one, even if some of us might not want to
touch "systemd" in the real life ;-)
Thanks, will queue.
next prev parent reply other threads:[~2024-10-09 17:10 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-08 10:50 Bug: `git maintenance start` is likely broken in 2.47 Shubham Kanodia
2024-10-08 11:45 ` Patrick Steinhardt
2024-10-08 12:15 ` [PATCH] builtin/gc: fix crash when running `git maintenance start` Patrick Steinhardt
2024-10-08 18:30 ` Derrick Stolee
2024-10-09 2:58 ` Derrick Stolee
2024-10-09 6:28 ` Patrick Steinhardt
2024-10-08 18:33 ` Junio C Hamano
2024-10-09 23:14 ` Junio C Hamano
2024-10-10 4:54 ` Patrick Steinhardt
2024-10-10 5:10 ` Patrick Steinhardt
2024-10-09 7:08 ` [PATCH v2] " Patrick Steinhardt
2024-10-09 13:13 ` Derrick Stolee
2024-10-09 17:28 ` Junio C Hamano
2024-10-09 17:10 ` Junio C Hamano [this message]
2024-10-10 5:33 ` [PATCH v3] " Patrick Steinhardt
2024-10-10 17:09 ` Junio C Hamano
2024-10-14 4:44 ` Shubham Kanodia
2024-10-14 8:38 ` Patrick Steinhardt
2024-10-15 0:36 ` Taylor Blau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq8quxw6bi.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=ps@pks.im \
--cc=shubham.kanodia10@gmail.com \
--cc=stolee@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).