{"thread":{"id":"66137","subject":"[PATCH 0/2] t7900: fix flaky \"maintenance.strategy\" test","startedAt":"2026-08-07T10:59:08Z","lastAt":"2026-08-13T12:08:13Z","messageCount":13,"participants":["Patrick Steinhardt","Karthik Nayak"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"549977","messageId":"20260807-pks-t7900-fix-flaky-test-v1-0-08d0ea0fbbc5@pks.im","threadId":"66137","inReplyTo":null,"subject":"[PATCH 0/2] t7900: fix flaky \"maintenance.strategy\" test","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-07T10:59:00Z","receivedAt":"2026-08-07T10:59:08Z","isPatch":true,"body":"Hi,\n\nI've recently noticed that t7900 is flaky, see for example [1].\nThe root cause of the flake is the auto-detaching logic of\ngit-maintenance(1), which sometimes causes us to skip maintenance\naltogether when the foreground process is racing with background\nmaintenance.\n\nThanks!\n\nPatrick\n\n[1]: https://gitlab.com/gitlab-org/git/-/jobs/15762975482\n\n---\nPatrick Steinhardt (2):\n      t7900: adapt some tests to use a throwaway repository\n      t7900: fix flaky \"maintenance.strategy\" test\n\n t/t7900-maintenance.sh | 76 ++++++++++++++++++++++++++++++--------------------\n 1 file changed, 46 insertions(+), 30 deletions(-)\n\n\n---\nbase-commit: 2c78326f810173a4f3aefd8021f1e07575412481\nchange-id: 20260807-pks-t7900-fix-flaky-test-160abfcef65a\n\n"},{"id":"549978","messageId":"20260807-pks-t7900-fix-flaky-test-v1-1-08d0ea0fbbc5@pks.im","threadId":"66137","inReplyTo":"20260807-pks-t7900-fix-flaky-test-v1-0-08d0ea0fbbc5@pks.im","subject":"[PATCH 1/2] t7900: adapt some tests to use a throwaway repository","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-07T10:59:01Z","receivedAt":"2026-08-07T10:59:09Z","isPatch":true,"body":"Many of the tests in t7900 operate inside the main trash repository\nthat's set up by default by our test suite. This is overall quite\nfragile as we're exercising repository maintenance in those tests, and\nmaintenance is of course intricately tied towards the on-disk state of a\nrepository. Consequently, the tests can easily impact one another.\n\nFurthermore, in the next commit we'll have to modify the environment in\na handful of those tests. As tests don't run in a subshell, doing so\nwould impact all subsequent tests by default, as well.\n\nAdapt exactly those tests to use a throwaway repository. This makes the\ntests more neatly self-contained and allows us to trivially modify the\nenvironment in the next commit.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/t7900-maintenance.sh | 70 +++++++++++++++++++++++++++++++-------------------\n 1 file changed, 43 insertions(+), 27 deletions(-)\n\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 4238569b68..6735a9e082 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -67,41 +67,57 @@ test_expect_success 'run [--auto|--quiet] with gc strategy' '\n '\n \n test_expect_success 'maintenance.auto config option' '\n-\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n-\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n-\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n-\t\tgit -c maintenance.auto=true \\\n-\t\tcommit --quiet --allow-empty -m 2 &&\n-\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n-\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n-\t\tgit -c maintenance.auto=false \\\n-\t\tcommit --quiet --allow-empty -m 3 &&\n-\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n+\t\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n+\t\t\tgit -c maintenance.auto=true \\\n+\t\t\tcommit --quiet --allow-empty -m 2 &&\n+\t\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n+\t\t\tgit -c maintenance.auto=false \\\n+\t\t\tcommit --quiet --allow-empty -m 3 &&\n+\t\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n+\t)\n '\n \n test_expect_success 'gc.auto config option' '\n-\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n-\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n-\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n-\t\tgit -c gc.auto=1 commit --quiet --allow-empty -m 2 &&\n-\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n-\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n-\t\tgit -c gc.auto=0 commit --quiet --allow-empty -m 3 &&\n-\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n+\t\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n+\t\t\tgit -c gc.auto=1 commit --quiet --allow-empty -m 2 &&\n+\t\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n+\t\t\tgit -c gc.auto=0 commit --quiet --allow-empty -m 3 &&\n+\t\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n+\t)\n '\n \n test_expect_success 'maintenance.auto overrides gc.auto' '\n-\ttest_when_finished \"rm -f trace\" &&\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n \n-\ttest_config maintenance.auto false &&\n-\ttest_config gc.auto 1 &&\n-\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n-\ttest_subcommand ! git maintenance run --auto --quiet --detach <trace &&\n+\t\tgit config set maintenance.auto false &&\n+\t\tgit config set gc.auto 1 &&\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n+\t\ttest_subcommand ! git maintenance run --auto --quiet --detach <trace &&\n \n-\ttest_config maintenance.auto true &&\n-\ttest_config gc.auto 0 &&\n-\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n-\ttest_subcommand git maintenance run --auto --quiet --detach <trace\n+\t\tgit config set maintenance.auto true &&\n+\t\tgit config set gc.auto 0 &&\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n+\t\ttest_subcommand git maintenance run --auto --quiet --detach <trace\n+\t)\n '\n \n for cfg in maintenance.autoDetach gc.autoDetach\n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"549979","messageId":"20260807-pks-t7900-fix-flaky-test-v1-2-08d0ea0fbbc5@pks.im","threadId":"66137","inReplyTo":"20260807-pks-t7900-fix-flaky-test-v1-0-08d0ea0fbbc5@pks.im","subject":"[PATCH 2/2] t7900: fix flaky \"maintenance.strategy\" test","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-07T10:59:02Z","receivedAt":"2026-08-07T10:59:12Z","isPatch":true,"body":"One of our tests for whether \"maintenance.strategy\" is being respected\nin t7900 is flaky in our CI systems:\n\n    + GIT_TRACE2_EVENT=/tmp/test-output/trash directory.t7900-maintenance/repo/trace2.txt git -c maintenance.strategy=incremental maintenance run --quiet\n    + test_maintenance_tasks trace2.txt\n    + cat\n    + sed -ne s/.*\"region_enter\".*\"category\":\"maintenance\\([^\"]*\\)\".*\"label\":\"\\([^\"][^\"]*\\)\".*/\\2\\1/p trace2.txt\n    + test_cmp expect actual\n    + test 2 -ne 2\n    + eval /usr/bin/diff -u \"$@\"\n    + /usr/bin/diff -u expect actual\n    --- expect\t2026-08-07 06:20:51.388322602 +0000\n    +++ actual\t2026-08-07 06:20:51.388322602 +0000\n    @@ -1,2 +0,0 @@\n    -gc foreground\n    -gc\n\nWhen running with the \"incremental\" strategy, we expect two git-gc(1)\ntasks to have been executed, but sometimes the test simply doesn't\nexecute any of those tasks.\n\nA first hunch may be that maybe the disk-state is sometimes different\nand thus we decide not to run maintenance. But git-maintenance(1)\ndoesn't run with the \"--auto\" switch, so we should execute those tasks\nregardless of the on-disk state.\n\nBut there's a second condition that may cause us to not execute tasks,\nnamely when the \"maintenance.lock\" file exists due to a concurrently\nrunning tasks. We usually disable auto-maintenance from detaching in our\ntest suite to avoid exactly these kinds of race conditions, but in t7900\nwe unset \"GIT_TEST_MAINT_AUTO_DETACH\" and thus enable the auto-detach\nlogic. The intent of this is to exercise git-maintenance(1) closer to\nhow it would run in a real-world scenario, but it does cause us to race\nwhen the detached maintenance job that was triggered by `test_commit()`\nlives long enough.\n\nWe could trivially fix this race by disabling auto-maintenance for this\nspecific test. But that doesn't fix this class of races in this test\nsuite: while I haven't seen any of the other tests fail in the same way,\na bunch of them have this race, as well.\n\nInstead, let's retain \"GIT_TEST_MAINT_AUTO_DETACH\" and only unset it as\nrequired.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/t7900-maintenance.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 6735a9e082..5fbb16f0f0 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -7,9 +7,6 @@ test_description='git maintenance builtin'\n GIT_TEST_COMMIT_GRAPH=0\n GIT_TEST_MULTI_PACK_INDEX=0\n \n-# Ensure that auto-maintenance detaches as usual.\n-sane_unset GIT_TEST_MAINT_AUTO_DETACH\n-\n test_lazy_prereq XMLLINT '\n \txmllint --version\n '\n@@ -71,6 +68,7 @@ test_expect_success 'maintenance.auto config option' '\n \tgit init repo &&\n \t(\n \t\tcd repo &&\n+\t\tsane_unset GIT_TEST_MAINT_AUTO_DETACH &&\n \n \t\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n \t\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n@@ -90,6 +88,7 @@ test_expect_success 'gc.auto config option' '\n \tgit init repo &&\n \t(\n \t\tcd repo &&\n+\t\tsane_unset GIT_TEST_MAINT_AUTO_DETACH &&\n \n \t\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n \t\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n@@ -107,6 +106,7 @@ test_expect_success 'maintenance.auto overrides gc.auto' '\n \tgit init repo &&\n \t(\n \t\tcd repo &&\n+\t\tsane_unset GIT_TEST_MAINT_AUTO_DETACH &&\n \n \t\tgit config set maintenance.auto false &&\n \t\tgit config set gc.auto 1 &&\n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"550379","messageId":"CAOLa=ZTAV=JqOvE0xkE4zmHMm=xx40_3g42ob9RDBRXmw3u6_g@mail.gmail.com","threadId":"66137","inReplyTo":"20260807-pks-t7900-fix-flaky-test-v1-1-08d0ea0fbbc5@pks.im","subject":"Re: [PATCH 1/2] t7900: adapt some tests to use a throwaway repository","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-12T08:19:13Z","receivedAt":"2026-08-12T08:19:17Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Many of the tests in t7900 operate inside the main trash repository\n> that's set up by default by our test suite. This is overall quite\n> fragile as we're exercising repository maintenance in those tests, and\n> maintenance is of course intricately tied towards the on-disk state of a\n> repository. Consequently, the tests can easily impact one another.\n>\n> Furthermore, in the next commit we'll have to modify the environment in\n> a handful of those tests. As tests don't run in a subshell, doing so\n> would impact all subsequent tests by default, as well.\n>\n> Adapt exactly those tests to use a throwaway repository. This makes the\n> tests more neatly self-contained and allows us to trivially modify the\n> environment in the next commit.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  t/t7900-maintenance.sh | 70 +++++++++++++++++++++++++++++++-------------------\n>  1 file changed, 43 insertions(+), 27 deletions(-)\n>\n> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\n> index 4238569b68..6735a9e082 100755\n> --- a/t/t7900-maintenance.sh\n> +++ b/t/t7900-maintenance.sh\n> @@ -67,41 +67,57 @@ test_expect_success 'run [--auto|--quiet] with gc strategy' '\n>  '\n>\n>  test_expect_success 'maintenance.auto config option' '\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n> -\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n> -\t\tgit -c maintenance.auto=true \\\n> -\t\tcommit --quiet --allow-empty -m 2 &&\n> -\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n> -\t\tgit -c maintenance.auto=false \\\n> -\t\tcommit --quiet --allow-empty -m 3 &&\n> -\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n> +\ttest_when_finished \"rm -rf repo\" &&\n> +\tgit init repo &&\n> +\t(\n> +\t\tcd repo &&\n> +\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n> +\t\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n> +\t\t\tgit -c maintenance.auto=true \\\n> +\t\t\tcommit --quiet --allow-empty -m 2 &&\n> +\t\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n> +\t\t\tgit -c maintenance.auto=false \\\n> +\t\t\tcommit --quiet --allow-empty -m 3 &&\n> +\t\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n> +\t)\n>  '\n>\n>  test_expect_success 'gc.auto config option' '\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n> -\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n> -\t\tgit -c gc.auto=1 commit --quiet --allow-empty -m 2 &&\n> -\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n> -\t\tgit -c gc.auto=0 commit --quiet --allow-empty -m 3 &&\n> -\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n> +\ttest_when_finished \"rm -rf repo\" &&\n> +\tgit init repo &&\n> +\t(\n> +\t\tcd repo &&\n> +\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n> +\t\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n> +\t\t\tgit -c gc.auto=1 commit --quiet --allow-empty -m 2 &&\n> +\t\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n> +\t\t\tgit -c gc.auto=0 commit --quiet --allow-empty -m 3 &&\n> +\t\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n> +\t)\n>  '\n>\n>  test_expect_success 'maintenance.auto overrides gc.auto' '\n> -\ttest_when_finished \"rm -f trace\" &&\n> +\ttest_when_finished \"rm -rf repo\" &&\n> +\tgit init repo &&\n> +\t(\n> +\t\tcd repo &&\n>\n> -\ttest_config maintenance.auto false &&\n> -\ttest_config gc.auto 1 &&\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n> -\ttest_subcommand ! git maintenance run --auto --quiet --detach <trace &&\n> +\t\tgit config set maintenance.auto false &&\n> +\t\tgit config set gc.auto 1 &&\n\nSo we change from using `test_config` to `git config`, I assume this is\nbecause earlier since we used a shared folder, we had to undo any config\nchanges made. Now that's no longer needed. Nit: This is okay, but\nwould've been nicer to call out.\n\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n> +\t\ttest_subcommand ! git maintenance run --auto --quiet --detach <trace &&\n>\n> -\ttest_config maintenance.auto true &&\n> -\ttest_config gc.auto 0 &&\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n> -\ttest_subcommand git maintenance run --auto --quiet --detach <trace\n> +\t\tgit config set maintenance.auto true &&\n> +\t\tgit config set gc.auto 0 &&\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n> +\t\ttest_subcommand git maintenance run --auto --quiet --detach <trace\n> +\t)\n>  '\n>\n>  for cfg in maintenance.autoDetach gc.autoDetach\n>\n> --\n> 2.55.0.679.g6767b8d81c.dirty\n\nThe rest looks as expected.\n"},{"id":"550380","messageId":"CAOLa=ZTVZh0_S+J57GVx-KHUr4hMyNFHQMrtjyNF5Q+Og7BiZA@mail.gmail.com","threadId":"66137","inReplyTo":"20260807-pks-t7900-fix-flaky-test-v1-1-08d0ea0fbbc5@pks.im","subject":"Re: [PATCH 1/2] t7900: adapt some tests to use a throwaway repository","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-12T08:19:18Z","receivedAt":"2026-08-12T08:19:20Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Many of the tests in t7900 operate inside the main trash repository\n> that's set up by default by our test suite. This is overall quite\n> fragile as we're exercising repository maintenance in those tests, and\n> maintenance is of course intricately tied towards the on-disk state of a\n> repository. Consequently, the tests can easily impact one another.\n>\n> Furthermore, in the next commit we'll have to modify the environment in\n> a handful of those tests. As tests don't run in a subshell, doing so\n> would impact all subsequent tests by default, as well.\n>\n> Adapt exactly those tests to use a throwaway repository. This makes the\n> tests more neatly self-contained and allows us to trivially modify the\n> environment in the next commit.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  t/t7900-maintenance.sh | 70 +++++++++++++++++++++++++++++++-------------------\n>  1 file changed, 43 insertions(+), 27 deletions(-)\n>\n> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\n> index 4238569b68..6735a9e082 100755\n> --- a/t/t7900-maintenance.sh\n> +++ b/t/t7900-maintenance.sh\n> @@ -67,41 +67,57 @@ test_expect_success 'run [--auto|--quiet] with gc strategy' '\n>  '\n>\n>  test_expect_success 'maintenance.auto config option' '\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n> -\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n> -\t\tgit -c maintenance.auto=true \\\n> -\t\tcommit --quiet --allow-empty -m 2 &&\n> -\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n> -\t\tgit -c maintenance.auto=false \\\n> -\t\tcommit --quiet --allow-empty -m 3 &&\n> -\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n> +\ttest_when_finished \"rm -rf repo\" &&\n> +\tgit init repo &&\n> +\t(\n> +\t\tcd repo &&\n> +\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n> +\t\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n> +\t\t\tgit -c maintenance.auto=true \\\n> +\t\t\tcommit --quiet --allow-empty -m 2 &&\n> +\t\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n> +\t\t\tgit -c maintenance.auto=false \\\n> +\t\t\tcommit --quiet --allow-empty -m 3 &&\n> +\t\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n> +\t)\n>  '\n>\n>  test_expect_success 'gc.auto config option' '\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n> -\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n> -\t\tgit -c gc.auto=1 commit --quiet --allow-empty -m 2 &&\n> -\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n> -\t\tgit -c gc.auto=0 commit --quiet --allow-empty -m 3 &&\n> -\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n> +\ttest_when_finished \"rm -rf repo\" &&\n> +\tgit init repo &&\n> +\t(\n> +\t\tcd repo &&\n> +\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n> +\t\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n> +\t\t\tgit -c gc.auto=1 commit --quiet --allow-empty -m 2 &&\n> +\t\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n> +\t\t\tgit -c gc.auto=0 commit --quiet --allow-empty -m 3 &&\n> +\t\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n> +\t)\n>  '\n>\n>  test_expect_success 'maintenance.auto overrides gc.auto' '\n> -\ttest_when_finished \"rm -f trace\" &&\n> +\ttest_when_finished \"rm -rf repo\" &&\n> +\tgit init repo &&\n> +\t(\n> +\t\tcd repo &&\n>\n> -\ttest_config maintenance.auto false &&\n> -\ttest_config gc.auto 1 &&\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n> -\ttest_subcommand ! git maintenance run --auto --quiet --detach <trace &&\n> +\t\tgit config set maintenance.auto false &&\n> +\t\tgit config set gc.auto 1 &&\n\nSo we change from using `test_config` to `git config`, I assume this is\nbecause earlier since we used a shared folder, we had to undo any config\nchanges made. Now that's no longer needed. Nit: This is okay, but\nwould've been nicer to call out.\n\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n> +\t\ttest_subcommand ! git maintenance run --auto --quiet --detach <trace &&\n>\n> -\ttest_config maintenance.auto true &&\n> -\ttest_config gc.auto 0 &&\n> -\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n> -\ttest_subcommand git maintenance run --auto --quiet --detach <trace\n> +\t\tgit config set maintenance.auto true &&\n> +\t\tgit config set gc.auto 0 &&\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n> +\t\ttest_subcommand git maintenance run --auto --quiet --detach <trace\n> +\t)\n>  '\n>\n>  for cfg in maintenance.autoDetach gc.autoDetach\n>\n> --\n> 2.55.0.679.g6767b8d81c.dirty\n\nThe rest looks as expected.\n"},{"id":"550382","messageId":"CAOLa=ZSW+Ta5ktauamTUvp+fmjC4HHDpKOQ0sri+pBfLGq6mOg@mail.gmail.com","threadId":"66137","inReplyTo":"20260807-pks-t7900-fix-flaky-test-v1-2-08d0ea0fbbc5@pks.im","subject":"Re: [PATCH 2/2] t7900: fix flaky \"maintenance.strategy\" test","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-12T08:46:14Z","receivedAt":"2026-08-12T08:46:16Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> One of our tests for whether \"maintenance.strategy\" is being respected\n> in t7900 is flaky in our CI systems:\n>\n>     + GIT_TRACE2_EVENT=/tmp/test-output/trash directory.t7900-maintenance/repo/trace2.txt git -c maintenance.strategy=incremental maintenance run --quiet\n>     + test_maintenance_tasks trace2.txt\n>     + cat\n>     + sed -ne s/.*\"region_enter\".*\"category\":\"maintenance\\([^\"]*\\)\".*\"label\":\"\\([^\"][^\"]*\\)\".*/\\2\\1/p trace2.txt\n>     + test_cmp expect actual\n>     + test 2 -ne 2\n>     + eval /usr/bin/diff -u \"$@\"\n>     + /usr/bin/diff -u expect actual\n>     --- expect\t2026-08-07 06:20:51.388322602 +0000\n>     +++ actual\t2026-08-07 06:20:51.388322602 +0000\n>     @@ -1,2 +0,0 @@\n>     -gc foreground\n>     -gc\n>\n> When running with the \"incremental\" strategy, we expect two git-gc(1)\n> tasks to have been executed, but sometimes the test simply doesn't\n> execute any of those tasks.\n>\n> A first hunch may be that maybe the disk-state is sometimes different\n> and thus we decide not to run maintenance. But git-maintenance(1)\n> doesn't run with the \"--auto\" switch, so we should execute those tasks\n> regardless of the on-disk state.\n>\n> But there's a second condition that may cause us to not execute tasks,\n> namely when the \"maintenance.lock\" file exists due to a concurrently\n\nNit: s/a//\n\n> running tasks. We usually disable auto-maintenance from detaching in our\n> test suite to avoid exactly these kinds of race conditions, but in t7900\n> we unset \"GIT_TEST_MAINT_AUTO_DETACH\" and thus enable the auto-detach\n> logic. The intent of this is to exercise git-maintenance(1) closer to\n> how it would run in a real-world scenario, but it does cause us to race\n> when the detached maintenance job that was triggered by `test_commit()`\n> lives long enough.\n\nGIT_TEST_MAINT_AUTO_DETACH when set to true enables auto-detach, but\nalso the default value when unset is true. That's why unsetting it\nenables auto-detach. That's a bit confusing.\n\n>\n> We could trivially fix this race by disabling auto-maintenance for this\n> specific test. But that doesn't fix this class of races in this test\n> suite: while I haven't seen any of the other tests fail in the same way,\n> a bunch of them have this race, as well.\n>\n> Instead, let's retain \"GIT_TEST_MAINT_AUTO_DETACH\" and only unset it as\n> required.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  t/t7900-maintenance.sh | 6 +++---\n>  1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\n> index 6735a9e082..5fbb16f0f0 100755\n> --- a/t/t7900-maintenance.sh\n> +++ b/t/t7900-maintenance.sh\n> @@ -7,9 +7,6 @@ test_description='git maintenance builtin'\n>  GIT_TEST_COMMIT_GRAPH=0\n>  GIT_TEST_MULTI_PACK_INDEX=0\n>\n> -# Ensure that auto-maintenance detaches as usual.\n> -sane_unset GIT_TEST_MAINT_AUTO_DETACH\n> -\n>  test_lazy_prereq XMLLINT '\n>  \txmllint --version\n>  '\n> @@ -71,6 +68,7 @@ test_expect_success 'maintenance.auto config option' '\n>  \tgit init repo &&\n>  \t(\n>  \t\tcd repo &&\n> +\t\tsane_unset GIT_TEST_MAINT_AUTO_DETACH &&\n>\n>  \t\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n>  \t\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n> @@ -90,6 +88,7 @@ test_expect_success 'gc.auto config option' '\n>  \tgit init repo &&\n>  \t(\n>  \t\tcd repo &&\n> +\t\tsane_unset GIT_TEST_MAINT_AUTO_DETACH &&\n>\n>  \t\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n>  \t\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n> @@ -107,6 +106,7 @@ test_expect_success 'maintenance.auto overrides gc.auto' '\n>  \tgit init repo &&\n>  \t(\n>  \t\tcd repo &&\n> +\t\tsane_unset GIT_TEST_MAINT_AUTO_DETACH &&\n>\n>  \t\tgit config set maintenance.auto false &&\n>  \t\tgit config set gc.auto 1 &&\n>\n> --\n> 2.55.0.679.g6767b8d81c.dirty\n\nSo instead of unset everywhere we only do it selectively. Looks good.\n"},{"id":"550389","messageId":"anxF0P0KVizediDg@pks.im","threadId":"66137","inReplyTo":"CAOLa=ZTAV=JqOvE0xkE4zmHMm=xx40_3g42ob9RDBRXmw3u6_g@mail.gmail.com","subject":"Re: [PATCH 1/2] t7900: adapt some tests to use a throwaway repository","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-12T10:07:19Z","receivedAt":"2026-08-12T10:07:30Z","isPatch":true,"body":"On Wed, Aug 12, 2026 at 01:19:13AM -0700, Karthik Nayak wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> > diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\n> > index 4238569b68..6735a9e082 100755\n> > --- a/t/t7900-maintenance.sh\n> > +++ b/t/t7900-maintenance.sh\n> > @@ -67,41 +67,57 @@ test_expect_success 'run [--auto|--quiet] with gc strategy' '\n[snip]\n> >  test_expect_success 'maintenance.auto overrides gc.auto' '\n> > -\ttest_when_finished \"rm -f trace\" &&\n> > +\ttest_when_finished \"rm -rf repo\" &&\n> > +\tgit init repo &&\n> > +\t(\n> > +\t\tcd repo &&\n> >\n> > -\ttest_config maintenance.auto false &&\n> > -\ttest_config gc.auto 1 &&\n> > -\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n> > -\ttest_subcommand ! git maintenance run --auto --quiet --detach <trace &&\n> > +\t\tgit config set maintenance.auto false &&\n> > +\t\tgit config set gc.auto 1 &&\n> \n> So we change from using `test_config` to `git config`, I assume this is\n> because earlier since we used a shared folder, we had to undo any config\n> changes made. Now that's no longer needed. Nit: This is okay, but\n> would've been nicer to call out.\n\nThe issue with `test_config` is that it executes `test_when_finished`,\nand that function cannot run in subshells. So we have to use `git config\nset` instead, but because it's a throw-away repository it doesn't\nmatter.\n\nPatrick\n"},{"id":"550390","messageId":"anxGUVck4I30Jw2M@pks.im","threadId":"66137","inReplyTo":"CAOLa=ZSW+Ta5ktauamTUvp+fmjC4HHDpKOQ0sri+pBfLGq6mOg@mail.gmail.com","subject":"Re: [PATCH 2/2] t7900: fix flaky \"maintenance.strategy\" test","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-12T10:09:21Z","receivedAt":"2026-08-12T10:09:28Z","isPatch":true,"body":"On Wed, Aug 12, 2026 at 01:46:14AM -0700, Karthik Nayak wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > One of our tests for whether \"maintenance.strategy\" is being respected\n> > in t7900 is flaky in our CI systems:\n> >\n> >     + GIT_TRACE2_EVENT=/tmp/test-output/trash directory.t7900-maintenance/repo/trace2.txt git -c maintenance.strategy=incremental maintenance run --quiet\n> >     + test_maintenance_tasks trace2.txt\n> >     + cat\n> >     + sed -ne s/.*\"region_enter\".*\"category\":\"maintenance\\([^\"]*\\)\".*\"label\":\"\\([^\"][^\"]*\\)\".*/\\2\\1/p trace2.txt\n> >     + test_cmp expect actual\n> >     + test 2 -ne 2\n> >     + eval /usr/bin/diff -u \"$@\"\n> >     + /usr/bin/diff -u expect actual\n> >     --- expect\t2026-08-07 06:20:51.388322602 +0000\n> >     +++ actual\t2026-08-07 06:20:51.388322602 +0000\n> >     @@ -1,2 +0,0 @@\n> >     -gc foreground\n> >     -gc\n> >\n> > When running with the \"incremental\" strategy, we expect two git-gc(1)\n> > tasks to have been executed, but sometimes the test simply doesn't\n> > execute any of those tasks.\n> >\n> > A first hunch may be that maybe the disk-state is sometimes different\n> > and thus we decide not to run maintenance. But git-maintenance(1)\n> > doesn't run with the \"--auto\" switch, so we should execute those tasks\n> > regardless of the on-disk state.\n> >\n> > But there's a second condition that may cause us to not execute tasks,\n> > namely when the \"maintenance.lock\" file exists due to a concurrently\n> \n> Nit: s/a//\n> \n> > running tasks. We usually disable auto-maintenance from detaching in our\n> > test suite to avoid exactly these kinds of race conditions, but in t7900\n> > we unset \"GIT_TEST_MAINT_AUTO_DETACH\" and thus enable the auto-detach\n> > logic. The intent of this is to exercise git-maintenance(1) closer to\n> > how it would run in a real-world scenario, but it does cause us to race\n> > when the detached maintenance job that was triggered by `test_commit()`\n> > lives long enough.\n> \n> GIT_TEST_MAINT_AUTO_DETACH when set to true enables auto-detach, but\n> also the default value when unset is true. That's why unsetting it\n> enables auto-detach. That's a bit confusing.\n\nI'll reword this paragraph a bit. Thanks!\n\nPatrick\n"},{"id":"550391","messageId":"20260812-pks-t7900-fix-flaky-test-v2-0-9ea0e1ac0edd@pks.im","threadId":"66137","inReplyTo":"20260807-pks-t7900-fix-flaky-test-v1-0-08d0ea0fbbc5@pks.im","subject":"[PATCH v2 0/2] t7900: fix flaky \"maintenance.strategy\" test","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-12T10:11:45Z","receivedAt":"2026-08-12T10:11:59Z","isPatch":true,"body":"Hi,\n\nI've recently noticed that t7900 is flaky, see for example [1].\nThe root cause of the flake is the auto-detaching logic of\ngit-maintenance(1), which sometimes causes us to skip maintenance\naltogether when the foreground process is racing with background\nmaintenance.\n\nChanges in v2:\n  - Perform some word smithing on commit messages.\n  - Link to v1: https://patch.msgid.link/20260807-pks-t7900-fix-flaky-test-v1-0-08d0ea0fbbc5@pks.im\n\nThanks!\n\nPatrick\n\n[1]: https://gitlab.com/gitlab-org/git/-/jobs/15762975482\n\n---\nPatrick Steinhardt (2):\n      t7900: adapt some tests to use a throwaway repository\n      t7900: fix flaky \"maintenance.strategy\" test\n\n t/t7900-maintenance.sh | 76 ++++++++++++++++++++++++++++++--------------------\n 1 file changed, 46 insertions(+), 30 deletions(-)\n\nRange-diff versus v1:\n\n1:  10521f07ad ! 1:  1f3f8aa538 t7900: adapt some tests to use a throwaway repository\n    @@ Commit message\n         tests more neatly self-contained and allows us to trivially modify the\n         environment in the next commit.\n     \n    +    Note that we adapt calls to `test_config ()` to use git-config(1)\n    +    instead. This is because on the one hand we don't need the auto-revert\n    +    logic of `test_config ()` as we're using a throwaway repository anyway.\n    +    On the other hand it's not possible to use `test_config ()` as it uses\n    +    `test_when_finished ()`, which errors out when we run it in a subshell.\n    +\n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n      ## t/t7900-maintenance.sh ##\n2:  71cb84a4a7 ! 2:  ba1fbb27f9 t7900: fix flaky \"maintenance.strategy\" test\n    @@ Commit message\n     \n         But there's a second condition that may cause us to not execute tasks,\n         namely when the \"maintenance.lock\" file exists due to a concurrently\n    -    running tasks. We usually disable auto-maintenance from detaching in our\n    -    test suite to avoid exactly these kinds of race conditions, but in t7900\n    +    running git-maintenance(1) process. We usually disable auto-maintenance\n    +    from detaching in our test suite to avoid exactly these kinds of race\n    +    conditions by exporting `GIT_TEST_MAINT_AUTO_DETACH=false`. But in t7900\n         we unset \"GIT_TEST_MAINT_AUTO_DETACH\" and thus enable the auto-detach\n         logic. The intent of this is to exercise git-maintenance(1) closer to\n         how it would run in a real-world scenario, but it does cause us to race\n\n---\nbase-commit: 2c78326f810173a4f3aefd8021f1e07575412481\nchange-id: 20260807-pks-t7900-fix-flaky-test-160abfcef65a\n\n"},{"id":"550392","messageId":"20260812-pks-t7900-fix-flaky-test-v2-1-9ea0e1ac0edd@pks.im","threadId":"66137","inReplyTo":"20260812-pks-t7900-fix-flaky-test-v2-0-9ea0e1ac0edd@pks.im","subject":"[PATCH v2 1/2] t7900: adapt some tests to use a throwaway repository","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-12T10:11:46Z","receivedAt":"2026-08-12T10:12:01Z","isPatch":true,"body":"Many of the tests in t7900 operate inside the main trash repository\nthat's set up by default by our test suite. This is overall quite\nfragile as we're exercising repository maintenance in those tests, and\nmaintenance is of course intricately tied towards the on-disk state of a\nrepository. Consequently, the tests can easily impact one another.\n\nFurthermore, in the next commit we'll have to modify the environment in\na handful of those tests. As tests don't run in a subshell, doing so\nwould impact all subsequent tests by default, as well.\n\nAdapt exactly those tests to use a throwaway repository. This makes the\ntests more neatly self-contained and allows us to trivially modify the\nenvironment in the next commit.\n\nNote that we adapt calls to `test_config ()` to use git-config(1)\ninstead. This is because on the one hand we don't need the auto-revert\nlogic of `test_config ()` as we're using a throwaway repository anyway.\nOn the other hand it's not possible to use `test_config ()` as it uses\n`test_when_finished ()`, which errors out when we run it in a subshell.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/t7900-maintenance.sh | 70 +++++++++++++++++++++++++++++++-------------------\n 1 file changed, 43 insertions(+), 27 deletions(-)\n\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 4238569b68..6735a9e082 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -67,41 +67,57 @@ test_expect_success 'run [--auto|--quiet] with gc strategy' '\n '\n \n test_expect_success 'maintenance.auto config option' '\n-\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n-\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n-\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n-\t\tgit -c maintenance.auto=true \\\n-\t\tcommit --quiet --allow-empty -m 2 &&\n-\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n-\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n-\t\tgit -c maintenance.auto=false \\\n-\t\tcommit --quiet --allow-empty -m 3 &&\n-\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n+\t\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n+\t\t\tgit -c maintenance.auto=true \\\n+\t\t\tcommit --quiet --allow-empty -m 2 &&\n+\t\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n+\t\t\tgit -c maintenance.auto=false \\\n+\t\t\tcommit --quiet --allow-empty -m 3 &&\n+\t\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n+\t)\n '\n \n test_expect_success 'gc.auto config option' '\n-\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n-\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n-\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n-\t\tgit -c gc.auto=1 commit --quiet --allow-empty -m 2 &&\n-\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n-\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n-\t\tgit -c gc.auto=0 commit --quiet --allow-empty -m 3 &&\n-\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n+\t\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/true\" \\\n+\t\t\tgit -c gc.auto=1 commit --quiet --allow-empty -m 2 &&\n+\t\ttest_subcommand git maintenance run --auto --quiet --detach <true &&\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/false\" \\\n+\t\t\tgit -c gc.auto=0 commit --quiet --allow-empty -m 3 &&\n+\t\ttest_subcommand ! git maintenance run --auto --quiet --detach <false\n+\t)\n '\n \n test_expect_success 'maintenance.auto overrides gc.auto' '\n-\ttest_when_finished \"rm -f trace\" &&\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n \n-\ttest_config maintenance.auto false &&\n-\ttest_config gc.auto 1 &&\n-\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n-\ttest_subcommand ! git maintenance run --auto --quiet --detach <trace &&\n+\t\tgit config set maintenance.auto false &&\n+\t\tgit config set gc.auto 1 &&\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n+\t\ttest_subcommand ! git maintenance run --auto --quiet --detach <trace &&\n \n-\ttest_config maintenance.auto true &&\n-\ttest_config gc.auto 0 &&\n-\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n-\ttest_subcommand git maintenance run --auto --quiet --detach <trace\n+\t\tgit config set maintenance.auto true &&\n+\t\tgit config set gc.auto 0 &&\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n+\t\ttest_subcommand git maintenance run --auto --quiet --detach <trace\n+\t)\n '\n \n for cfg in maintenance.autoDetach gc.autoDetach\n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"550393","messageId":"20260812-pks-t7900-fix-flaky-test-v2-2-9ea0e1ac0edd@pks.im","threadId":"66137","inReplyTo":"20260812-pks-t7900-fix-flaky-test-v2-0-9ea0e1ac0edd@pks.im","subject":"[PATCH v2 2/2] t7900: fix flaky \"maintenance.strategy\" test","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-12T10:11:47Z","receivedAt":"2026-08-12T10:12:05Z","isPatch":true,"body":"One of our tests for whether \"maintenance.strategy\" is being respected\nin t7900 is flaky in our CI systems:\n\n    + GIT_TRACE2_EVENT=/tmp/test-output/trash directory.t7900-maintenance/repo/trace2.txt git -c maintenance.strategy=incremental maintenance run --quiet\n    + test_maintenance_tasks trace2.txt\n    + cat\n    + sed -ne s/.*\"region_enter\".*\"category\":\"maintenance\\([^\"]*\\)\".*\"label\":\"\\([^\"][^\"]*\\)\".*/\\2\\1/p trace2.txt\n    + test_cmp expect actual\n    + test 2 -ne 2\n    + eval /usr/bin/diff -u \"$@\"\n    + /usr/bin/diff -u expect actual\n    --- expect\t2026-08-07 06:20:51.388322602 +0000\n    +++ actual\t2026-08-07 06:20:51.388322602 +0000\n    @@ -1,2 +0,0 @@\n    -gc foreground\n    -gc\n\nWhen running with the \"incremental\" strategy, we expect two git-gc(1)\ntasks to have been executed, but sometimes the test simply doesn't\nexecute any of those tasks.\n\nA first hunch may be that maybe the disk-state is sometimes different\nand thus we decide not to run maintenance. But git-maintenance(1)\ndoesn't run with the \"--auto\" switch, so we should execute those tasks\nregardless of the on-disk state.\n\nBut there's a second condition that may cause us to not execute tasks,\nnamely when the \"maintenance.lock\" file exists due to a concurrently\nrunning git-maintenance(1) process. We usually disable auto-maintenance\nfrom detaching in our test suite to avoid exactly these kinds of race\nconditions by exporting `GIT_TEST_MAINT_AUTO_DETACH=false`. But in t7900\nwe unset \"GIT_TEST_MAINT_AUTO_DETACH\" and thus enable the auto-detach\nlogic. The intent of this is to exercise git-maintenance(1) closer to\nhow it would run in a real-world scenario, but it does cause us to race\nwhen the detached maintenance job that was triggered by `test_commit()`\nlives long enough.\n\nWe could trivially fix this race by disabling auto-maintenance for this\nspecific test. But that doesn't fix this class of races in this test\nsuite: while I haven't seen any of the other tests fail in the same way,\na bunch of them have this race, as well.\n\nInstead, let's retain \"GIT_TEST_MAINT_AUTO_DETACH\" and only unset it as\nrequired.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n t/t7900-maintenance.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex 6735a9e082..5fbb16f0f0 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -7,9 +7,6 @@ test_description='git maintenance builtin'\n GIT_TEST_COMMIT_GRAPH=0\n GIT_TEST_MULTI_PACK_INDEX=0\n \n-# Ensure that auto-maintenance detaches as usual.\n-sane_unset GIT_TEST_MAINT_AUTO_DETACH\n-\n test_lazy_prereq XMLLINT '\n \txmllint --version\n '\n@@ -71,6 +68,7 @@ test_expect_success 'maintenance.auto config option' '\n \tgit init repo &&\n \t(\n \t\tcd repo &&\n+\t\tsane_unset GIT_TEST_MAINT_AUTO_DETACH &&\n \n \t\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n \t\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n@@ -90,6 +88,7 @@ test_expect_success 'gc.auto config option' '\n \tgit init repo &&\n \t(\n \t\tcd repo &&\n+\t\tsane_unset GIT_TEST_MAINT_AUTO_DETACH &&\n \n \t\tGIT_TRACE2_EVENT=\"$(pwd)/default\" git commit --quiet --allow-empty -m 1 &&\n \t\ttest_subcommand git maintenance run --auto --quiet --detach <default &&\n@@ -107,6 +106,7 @@ test_expect_success 'maintenance.auto overrides gc.auto' '\n \tgit init repo &&\n \t(\n \t\tcd repo &&\n+\t\tsane_unset GIT_TEST_MAINT_AUTO_DETACH &&\n \n \t\tgit config set maintenance.auto false &&\n \t\tgit config set gc.auto 1 &&\n\n-- \n2.55.0.679.g6767b8d81c.dirty\n\n"},{"id":"550499","messageId":"CAOLa=ZRXoqtYfDYhTatXZt9ojP2_5WrtJY7exR_TEPHZVqEE2A@mail.gmail.com","threadId":"66137","inReplyTo":"anxF0P0KVizediDg@pks.im","subject":"Re: [PATCH 1/2] t7900: adapt some tests to use a throwaway repository","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-13T12:07:23Z","receivedAt":"2026-08-13T12:07:24Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Wed, Aug 12, 2026 at 01:19:13AM -0700, Karthik Nayak wrote:\n>> Patrick Steinhardt <ps@pks.im> writes:\n>> > diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\n>> > index 4238569b68..6735a9e082 100755\n>> > --- a/t/t7900-maintenance.sh\n>> > +++ b/t/t7900-maintenance.sh\n>> > @@ -67,41 +67,57 @@ test_expect_success 'run [--auto|--quiet] with gc strategy' '\n> [snip]\n>> >  test_expect_success 'maintenance.auto overrides gc.auto' '\n>> > -\ttest_when_finished \"rm -f trace\" &&\n>> > +\ttest_when_finished \"rm -rf repo\" &&\n>> > +\tgit init repo &&\n>> > +\t(\n>> > +\t\tcd repo &&\n>> >\n>> > -\ttest_config maintenance.auto false &&\n>> > -\ttest_config gc.auto 1 &&\n>> > -\tGIT_TRACE2_EVENT=\"$(pwd)/trace\" git commit --quiet --allow-empty -m 1 &&\n>> > -\ttest_subcommand ! git maintenance run --auto --quiet --detach <trace &&\n>> > +\t\tgit config set maintenance.auto false &&\n>> > +\t\tgit config set gc.auto 1 &&\n>>\n>> So we change from using `test_config` to `git config`, I assume this is\n>> because earlier since we used a shared folder, we had to undo any config\n>> changes made. Now that's no longer needed. Nit: This is okay, but\n>> would've been nicer to call out.\n>\n> The issue with `test_config` is that it executes `test_when_finished`,\n> and that function cannot run in subshells. So we have to use `git config\n> set` instead, but because it's a throw-away repository it doesn't\n> matter.\n>\n> Patrick\n\nRight, that slipped my mind entirely.\n"},{"id":"550500","messageId":"CAOLa=ZQmZ0spmdPOzCZe36i24nQh+o7d4fSz5dcJS7+O3p2skg@mail.gmail.com","threadId":"66137","inReplyTo":"20260812-pks-t7900-fix-flaky-test-v2-0-9ea0e1ac0edd@pks.im","subject":"Re: [PATCH v2 0/2] t7900: fix flaky \"maintenance.strategy\" test","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-08-13T12:08:12Z","receivedAt":"2026-08-13T12:08:13Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Hi,\n>\n> I've recently noticed that t7900 is flaky, see for example [1].\n> The root cause of the flake is the auto-detaching logic of\n> git-maintenance(1), which sometimes causes us to skip maintenance\n> altogether when the foreground process is racing with background\n> maintenance.\n>\n> Changes in v2:\n>   - Perform some word smithing on commit messages.\n>   - Link to v1: https://patch.msgid.link/20260807-pks-t7900-fix-flaky-test-v1-0-08d0ea0fbbc5@pks.im\n>\n> Thanks!\n>\n> Patrick\n>\n> [1]: https://gitlab.com/gitlab-org/git/-/jobs/15762975482\n>\n> ---\n> Patrick Steinhardt (2):\n>       t7900: adapt some tests to use a throwaway repository\n>       t7900: fix flaky \"maintenance.strategy\" test\n>\n>  t/t7900-maintenance.sh | 76 ++++++++++++++++++++++++++++++--------------------\n>  1 file changed, 46 insertions(+), 30 deletions(-)\n>\n> Range-diff versus v1:\n>\n> 1:  10521f07ad ! 1:  1f3f8aa538 t7900: adapt some tests to use a throwaway repository\n>     @@ Commit message\n>          tests more neatly self-contained and allows us to trivially modify the\n>          environment in the next commit.\n>\n>     +    Note that we adapt calls to `test_config ()` to use git-config(1)\n>     +    instead. This is because on the one hand we don't need the auto-revert\n>     +    logic of `test_config ()` as we're using a throwaway repository anyway.\n>     +    On the other hand it's not possible to use `test_config ()` as it uses\n>     +    `test_when_finished ()`, which errors out when we run it in a subshell.\n>     +\n>          Signed-off-by: Patrick Steinhardt <ps@pks.im>\n>\n>       ## t/t7900-maintenance.sh ##\n> 2:  71cb84a4a7 ! 2:  ba1fbb27f9 t7900: fix flaky \"maintenance.strategy\" test\n>     @@ Commit message\n>\n>          But there's a second condition that may cause us to not execute tasks,\n>          namely when the \"maintenance.lock\" file exists due to a concurrently\n>     -    running tasks. We usually disable auto-maintenance from detaching in our\n>     -    test suite to avoid exactly these kinds of race conditions, but in t7900\n>     +    running git-maintenance(1) process. We usually disable auto-maintenance\n>     +    from detaching in our test suite to avoid exactly these kinds of race\n>     +    conditions by exporting `GIT_TEST_MAINT_AUTO_DETACH=false`. But in t7900\n>          we unset \"GIT_TEST_MAINT_AUTO_DETACH\" and thus enable the auto-detach\n>          logic. The intent of this is to exercise git-maintenance(1) closer to\n>          how it would run in a real-world scenario, but it does cause us to race\n>\n> ---\n> base-commit: 2c78326f810173a4f3aefd8021f1e07575412481\n> change-id: 20260807-pks-t7900-fix-flaky-test-160abfcef65a\n\nThe range diff and this version looks good. Thanks!\n"}]}