{"thread":{"id":"56629","subject":"p2000 failure due to empty reflog","startedAt":"2021-10-02T17:37:37Z","lastAt":"2021-10-11T16:32:52Z","messageCount":10,"participants":["René Scharfe","Derrick Stolee","Ævar Arnfjörð Bjarmason","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"437771","messageId":"b25ac1cc-8e77-17e6-602a-b289c1e1cfeb@web.de","threadId":"56629","inReplyTo":null,"subject":"p2000 failure due to empty reflog","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2021-10-02T17:37:33Z","receivedAt":"2021-10-02T17:37:37Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"p2000 fails for me and reports:\n\n   perf 18 - git checkout -f - (full-v3):\n   running:\n   \t\t\t(\n   \t\t\t\tcd full-v3 &&\n   \t\t\t\techo >>f2/f4/a &&\n   \t\t\t\tgit checkout -f -\n   \t\t\t)\n\n   error: pathspec '-' did not match any file(s) known to git\n\ncheckout fails because the reflog is empty, so the \"-\" can't be\nresolved.  The pathspec error message is confusing, though.\n\nThe patch below adds a reflog entry and allows the script to\nsucceed.\n\nBefore the \"test_perf_on_all git commit -a -m A\", there are two\nreflog entries in each of the five clones, after it there are\nnone.  How is that even possible?\n\n\n---\n t/perf/p2000-sparse-operations.sh | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\nindex 597626276f..8529e3d295 100755\n--- a/t/perf/p2000-sparse-operations.sh\n+++ b/t/perf/p2000-sparse-operations.sh\n@@ -25,6 +25,7 @@ test_expect_success 'setup repo and indexes' '\n \tgit commit -m \"level 0\" &&\n \tBLOB=$(git rev-parse HEAD:a) &&\n \tOLD_COMMIT=$(git rev-parse HEAD) &&\n+\ttest_export OLD_COMMIT &&\n \tOLD_TREE=$(git rev-parse HEAD^{tree}) &&\n\n \tfor i in $(test_seq 1 3)\n@@ -109,6 +110,14 @@ test_perf_on_all git status\n test_perf_on_all git add -A\n test_perf_on_all git add .\n test_perf_on_all git commit -a -m A\n+\n+test_expect_success 'add reflog entry' '\n+\tfor repo in full-v3 full-v4 sparse-v3 sparse-v4\n+\tdo\n+\t\tgit -C $repo checkout $OLD_COMMIT\n+\tdone\n+'\n+\n test_perf_on_all git checkout -f -\n\n test_done\n--\n2.33.0\n"},{"id":"437938","messageId":"30df5e39-3f2a-00d8-541b-347c43f36b38@gmail.com","threadId":"56629","inReplyTo":"b25ac1cc-8e77-17e6-602a-b289c1e1cfeb@web.de","subject":"Re: p2000 failure due to empty reflog","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-10-04T19:55:41Z","receivedAt":"2021-10-04T19:55:45Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/2/2021 1:37 PM, René Scharfe wrote:\n> p2000 fails for me and reports:\n> \n>    perf 18 - git checkout -f - (full-v3):\n>    running:\n>    \t\t\t(\n>    \t\t\t\tcd full-v3 &&\n>    \t\t\t\techo >>f2/f4/a &&\n>    \t\t\t\tgit checkout -f -\n>    \t\t\t)\n> \n>    error: pathspec '-' did not match any file(s) known to git\n> \n> checkout fails because the reflog is empty, so the \"-\" can't be\n> resolved.  The pathspec error message is confusing, though.\n> \n> The patch below adds a reflog entry and allows the script to\n> succeed.\n> \n> Before the \"test_perf_on_all git commit -a -m A\", there are two\n> reflog entries in each of the five clones, after it there are\n> none.  How is that even possible?\n\nThat is certainly confusing. Is there something about your global\n(or local to your test repo) GC settings that cause an auto-GC to\nprune the reflog aggressively?\n\n> @@ -109,6 +110,14 @@ test_perf_on_all git status\n>  test_perf_on_all git add -A\n>  test_perf_on_all git add .\n>  test_perf_on_all git commit -a -m A\n> +\n> +test_expect_success 'add reflog entry' '\n> +\tfor repo in full-v3 full-v4 sparse-v3 sparse-v4\n> +\tdo\n> +\t\tgit -C $repo checkout $OLD_COMMIT\n> +\tdone\n> +'\n> +\n>  test_perf_on_all git checkout -f -\n\nWhile I believe this will fix the situation, it might only be\na band-aid on the real problem of losing the reflog during the\ntest.\n\nThanks,\n-Stolee\n"},{"id":"438000","messageId":"581663a7-9b16-e464-ada7-368f20c99ff1@web.de","threadId":"56629","inReplyTo":"30df5e39-3f2a-00d8-541b-347c43f36b38@gmail.com","subject":"Re: p2000 failure due to empty reflog","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2021-10-05T20:28:40Z","receivedAt":"2021-10-05T20:28:44Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 04.10.21 um 21:55 schrieb Derrick Stolee:\n> On 10/2/2021 1:37 PM, René Scharfe wrote:\n>> p2000 fails for me and reports:\n>>\n>>    perf 18 - git checkout -f - (full-v3):\n>>    running:\n>>    \t\t\t(\n>>    \t\t\t\tcd full-v3 &&\n>>    \t\t\t\techo >>f2/f4/a &&\n>>    \t\t\t\tgit checkout -f -\n>>    \t\t\t)\n>>\n>>    error: pathspec '-' did not match any file(s) known to git\n>>\n>> checkout fails because the reflog is empty, so the \"-\" can't be\n>> resolved.  The pathspec error message is confusing, though.\n>>\n>> The patch below adds a reflog entry and allows the script to\n>> succeed.\n>>\n>> Before the \"test_perf_on_all git commit -a -m A\", there are two\n>> reflog entries in each of the five clones, after it there are\n>> none.  How is that even possible?\n>\n> That is certainly confusing. Is there something about your global\n> (or local to your test repo) GC settings that cause an auto-GC to\n> prune the reflog aggressively?\n\nGood point.  I have gc.auto=0 in my config, but the tests use their own,\nempty config.  A trace shows that \"git gc --auto --no-quiet\" is started.\nThe following patch turns that off and allows the tests to succeed.\n\nNot doing maintenance in parallel to a performance test is a good idea\nanyway, but I still don't understand why it would empty the reflog --\nthat seems excessive, dangerous even.\n\nOne of the maintenance commands from the trace is \"git reflog expire\n--all\".  If I put that in before the \"checkout -\" test (on top of the\npatch below) then the reflog is emptied again and the test fails.\n\nRené\n\n---\n t/perf/p2000-sparse-operations.sh | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\nindex 597626276f..9a06904247 100755\n--- a/t/perf/p2000-sparse-operations.sh\n+++ b/t/perf/p2000-sparse-operations.sh\n@@ -105,6 +105,13 @@ test_perf_on_all () {\n \tdone\n }\n\n+test_expect_success 'disable housekeeping' '\n+\tfor repo in full-v3 full-v4 sparse-v3 sparse-v4\n+\tdo\n+\t\tgit -C $repo config gc.auto 0\n+\tdone\n+'\n+\n test_perf_on_all git status\n test_perf_on_all git add -A\n test_perf_on_all git add .\n--\n2.33.0\n"},{"id":"438011","messageId":"8735pfjg47.fsf@evledraar.gmail.com","threadId":"56629","inReplyTo":"30df5e39-3f2a-00d8-541b-347c43f36b38@gmail.com","subject":"Re: p2000 failure due to empty reflog","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-05T21:38:09Z","receivedAt":"2021-10-05T21:50:38Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Oct 04 2021, Derrick Stolee wrote:\n\n> On 10/2/2021 1:37 PM, René Scharfe wrote:\n>> p2000 fails for me and reports:\n>> \n>>    perf 18 - git checkout -f - (full-v3):\n>>    running:\n>>    \t\t\t(\n>>    \t\t\t\tcd full-v3 &&\n>>    \t\t\t\techo >>f2/f4/a &&\n>>    \t\t\t\tgit checkout -f -\n>>    \t\t\t)\n>> \n>>    error: pathspec '-' did not match any file(s) known to git\n>> \n>> checkout fails because the reflog is empty, so the \"-\" can't be\n>> resolved.  The pathspec error message is confusing, though.\n>> \n>> The patch below adds a reflog entry and allows the script to\n>> succeed.\n>> \n>> Before the \"test_perf_on_all git commit -a -m A\", there are two\n>> reflog entries in each of the five clones, after it there are\n>> none.  How is that even possible?\n>\n> That is certainly confusing. Is there something about your global\n> (or local to your test repo) GC settings that cause an auto-GC to\n> prune the reflog aggressively?\n\nPerhaps something to do with certain parts of the code doing a\ntime(NULL), and other parts faking up times to be 2005-something?\n\nI have some WIP patches that run concurrent \"git gc --auto\" with a\ndetach across the whole test suite, and IIRC I had to search/replace all\ntime(NULL) with my own time() that faked up (looks it up) an epoch of\n1113465166 to avoid this exact issue.\n\nIf you try doing that across our tests I think you'll find that the\n@{-1} syntax (reflog-powered) will start failing.\n\nBut that's from vague memory, but perhaps it'll point you in the right\ndirection...\n"},{"id":"438079","messageId":"xmqqsfxersvy.fsf@gitster.g","threadId":"56629","inReplyTo":"581663a7-9b16-e464-ada7-368f20c99ff1@web.de","subject":"Re: p2000 failure due to empty reflog","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-06T16:59:45Z","receivedAt":"2021-10-06T16:59:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> Good point.  I have gc.auto=0 in my config, but the tests use their own,\n> empty config.  A trace shows that \"git gc --auto --no-quiet\" is started.\n> The following patch turns that off and allows the tests to succeed.\n>\n> Not doing maintenance in parallel to a performance test is a good idea\n> anyway,\n\nSounds like a sensible analysis, and I agree that it is a good idea\nto by default disable the maintenance tasks in t/perf (possibly at a\nmore centralized place).\n\n> but I still don't understand why it would empty the reflog --\n> that seems excessive, dangerous even.\n>\n> One of the maintenance commands from the trace is \"git reflog expire\n> --all\".  If I put that in before the \"checkout -\" test (on top of the\n> patch below) then the reflog is emptied again and the test fails.\n>\n> René\n>\n> ---\n>  t/perf/p2000-sparse-operations.sh | 7 +++++++\n>  1 file changed, 7 insertions(+)\n>\n> diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\n> index 597626276f..9a06904247 100755\n> --- a/t/perf/p2000-sparse-operations.sh\n> +++ b/t/perf/p2000-sparse-operations.sh\n> @@ -105,6 +105,13 @@ test_perf_on_all () {\n>  \tdone\n>  }\n>\n> +test_expect_success 'disable housekeeping' '\n> +\tfor repo in full-v3 full-v4 sparse-v3 sparse-v4\n> +\tdo\n> +\t\tgit -C $repo config gc.auto 0\n> +\tdone\n> +'\n> +\n>  test_perf_on_all git status\n>  test_perf_on_all git add -A\n>  test_perf_on_all git add .\n> --\n> 2.33.0\n"},{"id":"438408","messageId":"23cfe67d-5a7c-6525-cb50-396cbd1121c6@web.de","threadId":"56629","inReplyTo":"8735pfjg47.fsf@evledraar.gmail.com","subject":"Re: p2000 failure due to empty reflog","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2021-10-09T14:39:26Z","receivedAt":"2021-10-09T14:39:30Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 05.10.21 um 23:38 schrieb Ævar Arnfjörð Bjarmason:\n>\n> On Mon, Oct 04 2021, Derrick Stolee wrote:\n>\n>> On 10/2/2021 1:37 PM, René Scharfe wrote:\n>>> p2000 fails for me and reports:\n>>>\n>>>    perf 18 - git checkout -f - (full-v3):\n>>>    running:\n>>>    \t\t\t(\n>>>    \t\t\t\tcd full-v3 &&\n>>>    \t\t\t\techo >>f2/f4/a &&\n>>>    \t\t\t\tgit checkout -f -\n>>>    \t\t\t)\n>>>\n>>>    error: pathspec '-' did not match any file(s) known to git\n>>>\n>>> checkout fails because the reflog is empty, so the \"-\" can't be\n>>> resolved.  The pathspec error message is confusing, though.\n>>>\n>>> The patch below adds a reflog entry and allows the script to\n>>> succeed.\n>>>\n>>> Before the \"test_perf_on_all git commit -a -m A\", there are two\n>>> reflog entries in each of the five clones, after it there are\n>>> none.  How is that even possible?\n>>\n>> That is certainly confusing. Is there something about your global\n>> (or local to your test repo) GC settings that cause an auto-GC to\n>> prune the reflog aggressively?\n>\n> Perhaps something to do with certain parts of the code doing a\n> time(NULL), and other parts faking up times to be 2005-something?\n>\n> I have some WIP patches that run concurrent \"git gc --auto\" with a\n> detach across the whole test suite, and IIRC I had to search/replace all\n> time(NULL) with my own time() that faked up (looks it up) an epoch of\n> 1113465166 to avoid this exact issue.\n>\n> If you try doing that across our tests I think you'll find that the\n> @{-1} syntax (reflog-powered) will start failing.\n>\n> But that's from vague memory, but perhaps it'll point you in the right\n> direction...\nYes, that helped.  The reflog entries have fake dates in 2005, and \"git\nreflog expire --all\" called by \"git gc --auto\" deletes entries older\nthan 90 days based on the actual current time.\n\nRené\n"},{"id":"438409","messageId":"ba3e16f7-bf9c-c5f3-4c0d-8288db6f44c7@web.de","threadId":"56629","inReplyTo":"xmqqsfxersvy.fsf@gitster.g","subject":"[PATCH] perf: disable automatic housekeeping","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2021-10-09T14:39:24Z","receivedAt":"2021-10-09T14:39:31Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Turn off automatic background maintenance for perf tests by default to\navoid interference with performance measurements.  Do that by using the\nnew file t/perf/config and using it as the system config file for perf\ntests.  Future tests intended to measure gc performance can override\nthe setting locally or call \"git gc\" explicitly.\n\nThis fixes a breakage in p2000 caused by gc automatically emptying the\nreflog due its fake dates from 2005 being older than 90 days.\n\nHelped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n t/perf/config      | 2 ++\n t/perf/perf-lib.sh | 4 ++++\n 2 files changed, 6 insertions(+)\n create mode 100644 t/perf/config\n\ndiff --git a/t/perf/config b/t/perf/config\nnew file mode 100644\nindex 0000000000..b92768b039\n--- /dev/null\n+++ b/t/perf/config\n@@ -0,0 +1,2 @@\n+[gc]\n+\tauto = 0\ndiff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh\nindex f74cfd35d6..4c4c568a37 100644\n--- a/t/perf/perf-lib.sh\n+++ b/t/perf/perf-lib.sh\n@@ -27,6 +27,10 @@ TEST_NO_MALLOC_CHECK=t\n\n . ../test-lib.sh\n\n+unset GIT_CONFIG_NOSYSTEM\n+GIT_CONFIG_SYSTEM=\"$TEST_DIRECTORY/perf/config\"\n+export GIT_CONFIG_SYSTEM\n+\n if test -n \"$GIT_TEST_INSTALLED\" -a -z \"$PERF_SET_GIT_TEST_INSTALLED\"\n then\n \terror \"Do not use GIT_TEST_INSTALLED with the perf tests.\n--\n2.33.0\n"},{"id":"438410","messageId":"87wnmmkzaa.fsf@evledraar.gmail.com","threadId":"56629","inReplyTo":"ba3e16f7-bf9c-c5f3-4c0d-8288db6f44c7@web.de","subject":"\"git reflog expire\" blindly trusting timestamps in reflogs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-09T14:57:20Z","receivedAt":"2021-10-09T15:12:34Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\n[Changed subject]\n\nOn Sat, Oct 09 2021, René Scharfe wrote:\n\n> Turn off automatic background maintenance for perf tests by default to\n> avoid interference with performance measurements.\n\nTurning off background GC during the perf tests seems like a good idea\nin general, so I think this patch should go in. Even with the WIP\n(haven't picked it up again in a while) test mode I menitoned in[1] it\nstill wouldn't make any sense to run detached background GC in t/perf.\n\nBecause first of all we take the repo as-is (hardlinks and all), so\nchanging it is a bug in itself.\n\nBut...\n\n> Do that by using the\n> new file t/perf/config and using it as the system config file for perf\n> tests.  Future tests intended to measure gc performance can override\n> the setting locally or call \"git gc\" explicitly.\n>\n> This fixes a breakage in p2000 caused by gc automatically emptying the\n> reflog due its fake dates from 2005 being older than 90 days.\n\nI think the behavior of git-reflog is probably just broken here and\nneeds fixing in general.\n\nI.e. we don't actually change the ctime the relevant reflog files, so\nsurely this is pointing to deeper edge cases where e.g. someone might\nimport old history, only to see it wiped away due to the commits being\n\"old\", but the mtime is just seconds/minutes old...\n\nBut I never had the time/chance to dig into that, maybe there's a good\nreason for it.\n\nOf course we can't rely on *just* the mtime since the whole point is to\nexpire older entries, we need to walk the file and trust the timestamps\ntherein. See the show_one_reflog_ent().\n\nBut my recollection & reading of [2] is that we nuke the whole\nfile. Surely we should have a \"uh oh?\" safeguard when we encounter a\nfile whose tip timestamp_t is from 2005, it's 2021, and our lstat()\nmtime says it was changed seconds ago.\n\nI.e. shouldn't reflog expiry at least have the invariant that nothing\nshould expire in a given file if the tip commit is old enough to be\nexpired, *AND* the mtime of the file is more recent than the expiry\nepoch, because such a scenario points to either git's own test suite\n(and we should just fake up the time there), or that we're about to\ncorrupt some user's repository because they're either doing similar\nimport work, or their clock was screwy?\n\n> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> Signed-off-by: René Scharfe <l.s.r@web.de>\n> ---\n>  t/perf/config      | 2 ++\n>  t/perf/perf-lib.sh | 4 ++++\n>  2 files changed, 6 insertions(+)\n>  create mode 100644 t/perf/config\n>\n> diff --git a/t/perf/config b/t/perf/config\n> new file mode 100644\n> index 0000000000..b92768b039\n> --- /dev/null\n> +++ b/t/perf/config\n> @@ -0,0 +1,2 @@\n> +[gc]\n> +\tauto = 0\n> diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh\n> index f74cfd35d6..4c4c568a37 100644\n> --- a/t/perf/perf-lib.sh\n> +++ b/t/perf/perf-lib.sh\n> @@ -27,6 +27,10 @@ TEST_NO_MALLOC_CHECK=t\n>\n>  . ../test-lib.sh\n>\n> +unset GIT_CONFIG_NOSYSTEM\n> +GIT_CONFIG_SYSTEM=\"$TEST_DIRECTORY/perf/config\"\n> +export GIT_CONFIG_SYSTEM\n> +\n>  if test -n \"$GIT_TEST_INSTALLED\" -a -z \"$PERF_SET_GIT_TEST_INSTALLED\"\n>  then\n>  \terror \"Do not use GIT_TEST_INSTALLED with the perf tests.\n\n1. https://lore.kernel.org/git/8735pfjg47.fsf@evledraar.gmail.com/\n2. https://lore.kernel.org/git/b25ac1cc-8e77-17e6-602a-b289c1e1cfeb@web.de/\n"},{"id":"438413","messageId":"xmqqa6jif5pg.fsf@gitster.g","threadId":"56629","inReplyTo":"87wnmmkzaa.fsf@evledraar.gmail.com","subject":"Re: \"git reflog expire\" blindly trusting timestamps in reflogs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-09T17:50:19Z","receivedAt":"2021-10-09T17:50:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> I.e. shouldn't reflog expiry at least have the invariant that nothing\n> should expire in a given file if the tip commit is old enough to be\n> expired, *AND* the mtime of the file is more recent than the expiry\n> epoch, because such a scenario points to either git's own test suite\n> (and we should just fake up the time there), or that we're about to\n> corrupt some user's repository because they're either doing similar\n> import work, or their clock was screwy?\n\nA reflog whose entries (and possibly the containing file) have\ninconsistent timestamps might need some care when pruning.  If the\ntime @{1} has is older than @{2}, there may be something fishy going\non.  Expiring @{1}, keep @{2}, and expire @{3} in such a case where\n@{2} is exceptionally new but all others are older than expiry limit\nis one possibility, but \"entries out of order detected---do you want\nto keep the one that is sandwiched between much older ones [y/N]?\"\nmight be worth asking.  In any case, a reflog with such unordered\nentries may not answer time-based @{1.week.ago} question correctly.\n\nI do not think, at least by default, we should care about file's\nmtime to the same degree, because it can easily be modified by\naccident and because the timestamps on entries and mtime can come\nfrom different time sources, and the local clock may be faster than\nthe fileserver's clock, resulting in the tip entry having a more\nrecent timestamp than the file's mtime.\n"},{"id":"438478","messageId":"YWRnMbAOQ1t2DVHb@coredump.intra.peff.net","threadId":"56629","inReplyTo":"87wnmmkzaa.fsf@evledraar.gmail.com","subject":"Re: \"git reflog expire\" blindly trusting timestamps in reflogs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-11T16:32:49Z","receivedAt":"2021-10-11T16:32:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 09, 2021 at 04:57:20PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> On Sat, Oct 09 2021, René Scharfe wrote:\n> \n> > Turn off automatic background maintenance for perf tests by default to\n> > avoid interference with performance measurements.\n> \n> Turning off background GC during the perf tests seems like a good idea\n> in general, so I think this patch should go in. Even with the WIP\n> (haven't picked it up again in a while) test mode I menitoned in[1] it\n> still wouldn't make any sense to run detached background GC in t/perf.\n> \n> Because first of all we take the repo as-is (hardlinks and all), so\n> changing it is a bug in itself.\n\nLots of perf tests modify the repository. This is generally OK as Git\ntends to write and rename tempfiles (breaking the hardlink) rather than\nmodifying anything in place. We also only do the hardlink thing for the\nobjects/ directory, so the scope is limited.\n\nIt does make me a little nervous, but to my knowledge we've never had a\nperf test which hurt the original donor repo in this way. And there are\nmore subtle ways to screw things up with various filesystem references;\nsee 36e834abc1 (t/perf: avoid copying worktree files from test repo,\n2021-02-26).\n\nThat's all just an aside; I agree that we should avoid background gc\njust because it confuses performance tests.\n\n-Peff\n"}]}