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

From: Derrick Stolee <stolee@gmail.com>
Date: Oct 9, 2024, 02:58 UTC
Message-ID: <b4d75b35-bfe2-4950-b2de-b56d95918b3c@gmail.com>
In-Reply-To: <c1271bf6-3648-4eca-916f-67a8518fe4e1@gmail.com>

On 10/8/24 2:30 PM, Derrick Stolee wrote:

Show 25 quoted lines

> On 10/8/24 8:15 AM, Patrick Steinhardt wrote:
>> It was reported on the mailing list that running `git maintenance start`
>> immediately segfaults starting with b6c3f8e12c (builtin/maintenance: fix
>> leak in `get_schedule_cmd()`, 2024-09-26). And indeed, this segfault is
>> trivial to reproduce up to a point where one is scratching their head
>> why we didn't catch this regression in our test suite.
> 
>> +test_expect_success 'start without GIT_TEST_MAINT_SCHEDULER' '
>> +    test_when_finished "rm -rf crontab.log script repo" &&
>> +    mkdir script &&
>> +    write_script script/crontab <<-EOF &&
>> +    echo "\$*" >>"$(pwd)"/crontab.log
>> +    EOF
>> +    git init repo &&
>> +    (
>> +        cd repo &&
>> +        sane_unset GIT_TEST_MAINT_SCHEDULER &&
>> +        PATH="$(pwd)/../script:$PATH" git maintenance start --scheduler=crontab
>> +    ) &&
>> +    test_grep -- -l crontab.log &&
>> +    test_grep -- git_cron_edit_tmp crontab.log
>> +'
>> +
> I see why we didn't catch this immediately. This is a good way to work
> around this issue of "mocking" the scheduler.

Unfortunately, this test is broken on macOS and Windows. Those platforms will fail when asked for 'crontab' without the test variable.

Here is a potential fixup that will make your test succeed:

--- >8 ---

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 4008e4e45e..86bc77e73f 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -646,7 +646,7 @@ test_expect_success !MINGW 'register and unregister with 
regex metacharacters' '
  		maintenance.repo "$(pwd)/$META"
  '

-test_expect_success 'start without GIT_TEST_MAINT_SCHEDULER' '
+test_expect_success !MINGW,!DARWIN 'start without GIT_TEST_MAINT_SCHEDULER' '
  	test_when_finished "rm -rf crontab.log script repo" &&
  	mkdir script &&
  	write_script script/crontab <<-EOF &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index b1a8ee5c00..f12f3a7609 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1715,6 +1715,12 @@ case $uname_s in
  	test_set_prereq GREP_STRIPS_CR
  	test_set_prereq WINDOWS
  	;;
+*Darwin*)
+	test_set_prereq POSIXPERM
+	test_set_prereq BSLASHPSPEC
+	test_set_prereq EXECKEEPSPID
+	test_set_prereq DARWIN
+	;;
  *)
  	test_set_prereq POSIXPERM
  	test_set_prereq BSLASHPSPEC