git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] builtin/gc: fix crash when running `git maintenance start`

From
Junio C Hamano <gitster@pobox.com>
Date
Oct 9, 2024, 17:10 UTC
Message-ID
<xmqq8quxw6bi.fsf@gitster.g>
In-Reply-To
<5798c31e1ef9346e7faf73f8c80b32c436937a8a.1728455715.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 14 quoted lines
> 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.
Previous: Junio C HamanoNext: Patrick Steinhardt
Message 14 of 19 in “Bug: `git maintenance start` is likely broken in 2.47”
  1. Shubham KanodiaOct 8, 2024
  2. Patrick SteinhardtOct 8, 2024
  3. builtin/gc: fix crash when running `git maintenance start`Patrick Steinhardt, Oct 8, 2024
  4. Derrick StoleeOct 8, 2024
  5. Derrick StoleeOct 9, 2024
  6. Patrick SteinhardtOct 9, 2024
  7. Junio C HamanoOct 8, 2024
  8. Junio C HamanoOct 9, 2024
  9. Patrick SteinhardtOct 10, 2024
  10. Patrick SteinhardtOct 10, 2024
  11. builtin/gc: fix crash when running `git maintenance start`Patrick Steinhardt, Oct 9, 2024
  12. Derrick StoleeOct 9, 2024
  13. Junio C HamanoOct 9, 2024
  14. Junio C HamanoOct 9, 2024
  15. builtin/gc: fix crash when running `git maintenance start`Patrick Steinhardt, Oct 10, 2024
  16. Junio C HamanoOct 10, 2024
  17. Shubham KanodiaOct 14, 2024
  18. Patrick SteinhardtOct 14, 2024
  19. Taylor BlauOct 15, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.