{"thread":{"id":"49519","subject":"git svn clone/fetch hits issues with gc --auto","startedAt":"2018-10-09T22:51:15Z","lastAt":"2018-10-11T00:38:35Z","messageCount":26,"participants":["Martin Langhoff","Eric Wong","Junio C Hamano","Ævar Arnfjörð Bjarmason","Jonathan Nieder","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"359957","messageId":"CACPiFCKQq--xrMf1nF=1MmC+eESE_aKms3yogoRwCY=YxcOWXA@mail.gmail.com","threadId":"49519","inReplyTo":"CACPiFCJZ83sqE7Gaj2pa12APkBF5tau-C6t4_GrXBWDwcMnJHg@mail.gmail.com","subject":"git svn clone/fetch hits issues with gc --auto","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2018-10-09T22:51:01Z","receivedAt":"2018-10-09T22:51:15Z","isPatch":false,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"Hi folks,\n\nLong time no see! Importing a 3GB (~25K revs, tons of files) SVN repo\nI hit the gc error:\n\nwarning: There are too many unreachable loose objects; run 'git prune'\nto remove them.\ngc --auto: command returned error: 255\n\nI don't seem to be the only one --\nhttps://stackoverflow.com/questions/35738680/avoiding-warning-there-are-too-many-unreachable-loose-objects-during-git-svn\n\nLooking at code history, it dropped the ability to pass options to git\nrepack when it was converted it to using git gc.\n\nExperimentally I find that tweaking it to run git gc --auto\n--prune=5.minutes.ago works well, while --prune=now breaks it.\nAttempts to commit fail with missing objects.\n\n- Why does --prune=now break it? Perhaps \"gc\" runs in the background,\nand races with the commit being prepared?\n\n- Would it be safe, sane to apply --prune=some.value on _clone_?\n\n- During _fetch_, --prune=some.value seems risky. In a checkout being\nactively used for development or merging it'd risk pruning objects\nusers expect to be there for recovery. Would there be a safe, sane\nway?\n\n- Is there a safer, saner value than 5 minutes?\n\ncheers,\n\n\nm\n-- \n martin.langhoff@gmail.com\n - ask interesting questions  ~  http://linkedin.com/in/martinlanghoff\n - don't be distracted        ~  http://github.com/martin-langhoff\n   by shiny stuff\n\n\n-- \n martin.langhoff@gmail.com\n - ask interesting questions  ~  http://linkedin.com/in/martinlanghoff\n - don't be distracted        ~  http://github.com/martin-langhoff\n   by shiny stuff\n"},{"id":"359964","messageId":"20181009234502.oxzfwirjcew2sxrm@dcvr","threadId":"49519","inReplyTo":"CACPiFCKQq--xrMf1nF=1MmC+eESE_aKms3yogoRwCY=YxcOWXA@mail.gmail.com","subject":"Re: git svn clone/fetch hits issues with gc --auto","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2018-10-09T23:45:02Z","receivedAt":"2018-10-09T23:53:41Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Martin Langhoff <martin.langhoff@gmail.com> wrote:\n> Hi folks,\n> \n> Long time no see! Importing a 3GB (~25K revs, tons of files) SVN repo\n> I hit the gc error:\n> \n> warning: There are too many unreachable loose objects; run 'git prune'\n> to remove them.\n> gc --auto: command returned error: 255\n\nGC can be annoying when that happens... For git-svn, perhaps\nthis can be appropriate to at least allow the import to continue:\n\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex 76b2965905..9b0caa3d47 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -999,7 +999,7 @@ sub restore_commit_header_env {\n }\n \n sub gc {\n-\tcommand_noisy('gc', '--auto');\n+\teval { command_noisy('gc', '--auto') };\n };\n \n sub do_git_commit {\n\n\nBut yeah, somebody else who works on git regularly could\nprobably stop repack from writing thousands of loose\nobjects (and instead write a self-contained pack with\nthose objects, instead).  I haven't followed git closely\nlately, myself.\n"},{"id":"359978","messageId":"xmqqd0sims6s.fsf@gitster-ct.c.googlers.com","threadId":"49519","inReplyTo":"20181009234502.oxzfwirjcew2sxrm@dcvr","subject":"Re: git svn clone/fetch hits issues with gc --auto","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-10T02:49:47Z","receivedAt":"2018-10-10T02:49:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Forwarding to Jonathan, as I think this is an interesting supporting\nvote for the topic that we were stuck on.\n\nEric Wong <e@80x24.org> writes:\n\n> Martin Langhoff <martin.langhoff@gmail.com> wrote:\n>> Hi folks,\n>> \n>> Long time no see! Importing a 3GB (~25K revs, tons of files) SVN repo\n>> I hit the gc error:\n>> \n>> warning: There are too many unreachable loose objects; run 'git prune'\n>> to remove them.\n>> gc --auto: command returned error: 255\n>\n> GC can be annoying when that happens... For git-svn, perhaps\n> this can be appropriate to at least allow the import to continue:\n>\n> diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\n> index 76b2965905..9b0caa3d47 100644\n> --- a/perl/Git/SVN.pm\n> +++ b/perl/Git/SVN.pm\n> @@ -999,7 +999,7 @@ sub restore_commit_header_env {\n>  }\n>  \n>  sub gc {\n> -\tcommand_noisy('gc', '--auto');\n> +\teval { command_noisy('gc', '--auto') };\n>  };\n>  \n>  sub do_git_commit {\n>\n>\n> But yeah, somebody else who works on git regularly could\n> probably stop repack from writing thousands of loose\n> objects (and instead write a self-contained pack with\n> those objects, instead).  I haven't followed git closely\n> lately, myself.\n"},{"id":"359981","messageId":"87d0sifcsm.fsf@evledraar.gmail.com","threadId":"49519","inReplyTo":"CACPiFCKQq--xrMf1nF=1MmC+eESE_aKms3yogoRwCY=YxcOWXA@mail.gmail.com","subject":"Re: git svn clone/fetch hits issues with gc --auto","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-10T08:04:09Z","receivedAt":"2018-10-10T08:04:13Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Oct 09 2018, Martin Langhoff wrote:\n\n> Hi folks,\n>\n> Long time no see! Importing a 3GB (~25K revs, tons of files) SVN repo\n> I hit the gc error:\n>\n> warning: There are too many unreachable loose objects; run 'git prune'\n> to remove them.\n> gc --auto: command returned error: 255\n>\n> I don't seem to be the only one --\n> https://stackoverflow.com/questions/35738680/avoiding-warning-there-are-too-many-unreachable-loose-objects-during-git-svn\n>\n> Looking at code history, it dropped the ability to pass options to git\n> repack when it was converted it to using git gc.\n>\n> Experimentally I find that tweaking it to run git gc --auto\n> --prune=5.minutes.ago works well, while --prune=now breaks it.\n> Attempts to commit fail with missing objects.\n>\n> - Why does --prune=now break it? Perhaps \"gc\" runs in the background,\n> and races with the commit being prepared?\n>\n> - Would it be safe, sane to apply --prune=some.value on _clone_?\n>\n> - During _fetch_, --prune=some.value seems risky. In a checkout being\n> actively used for development or merging it'd risk pruning objects\n> users expect to be there for recovery. Would there be a safe, sane\n> way?\n>\n> - Is there a safer, saner value than 5 minutes?\n\nWhat you've found is the least sucky way to work around this right now,\nbut see my\nhttps://public-inbox.org/git/87inc89j38.fsf@evledraar.gmail.com/ and\nhttps://public-inbox.org/git/87d0vmck55.fsf@evledraar.gmail.com/ for\nsome prior (and recent) discussion of this problem on-list.\n\nFWIW this has nothing to do with git-svn per-se, and also e.g. happens\nto me when I do a 'git fetch --all' sometimes on git.git.\n"},{"id":"360000","messageId":"CACPiFCL0oTjN+-aYgKEDtKC0gYwkv6RLMwakdJV85PJ5XQej6g@mail.gmail.com","threadId":"49519","inReplyTo":"xmqqd0sims6s.fsf@gitster-ct.c.googlers.com","subject":"Re: git svn clone/fetch hits issues with gc --auto","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2018-10-10T11:01:43Z","receivedAt":"2018-10-10T11:01:57Z","isPatch":false,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"Looking around, Jonathan Tan's \"[PATCH] gc: do not warn about too many\nloose objects\" makes sense to me.\n\n- remove unactionable warning\n- as the warning is gone, no gc.log is produced\n- subsequent gc runs don't exit due to gc.log\n\nMy very humble +1 on that.\n\nAs for downsides... if we have truly tons of _recent_ loose objects,\nit'll ... take disk space? I'm fine with that.\n\nFor more aggressive gc options, thoughts:\n\n - Do we always consider git gc --prune=now \"safe\" in a \"won't delete\nstuff the user is likely to want\" sense? For example -- are the\nreferences from reflogs enough safety?\n\n - Even if we don't, for some commands it should be safe to run git gc\n--prune=now at the end of the process, for example an import that\ngenerates a new git repo (git svn clone).\n\ncheers,\n\n\nm\nOn Tue, Oct 9, 2018 at 10:49 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Forwarding to Jonathan, as I think this is an interesting supporting\n> vote for the topic that we were stuck on.\n>\n> Eric Wong <e@80x24.org> writes:\n>\n> > Martin Langhoff <martin.langhoff@gmail.com> wrote:\n> >> Hi folks,\n> >>\n> >> Long time no see! Importing a 3GB (~25K revs, tons of files) SVN repo\n> >> I hit the gc error:\n> >>\n> >> warning: There are too many unreachable loose objects; run 'git prune'\n> >> to remove them.\n> >> gc --auto: command returned error: 255\n> >\n> > GC can be annoying when that happens... For git-svn, perhaps\n> > this can be appropriate to at least allow the import to continue:\n> >\n> > diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\n> > index 76b2965905..9b0caa3d47 100644\n> > --- a/perl/Git/SVN.pm\n> > +++ b/perl/Git/SVN.pm\n> > @@ -999,7 +999,7 @@ sub restore_commit_header_env {\n> >  }\n> >\n> >  sub gc {\n> > -     command_noisy('gc', '--auto');\n> > +     eval { command_noisy('gc', '--auto') };\n> >  };\n> >\n> >  sub do_git_commit {\n> >\n> >\n> > But yeah, somebody else who works on git regularly could\n> > probably stop repack from writing thousands of loose\n> > objects (and instead write a self-contained pack with\n> > those objects, instead).  I haven't followed git closely\n> > lately, myself.\n\n\n\n-- \n martin.langhoff@gmail.com\n - ask interesting questions  ~  http://linkedin.com/in/martinlanghoff\n - don't be distracted        ~  http://github.com/martin-langhoff\n   by shiny stuff\n"},{"id":"360005","messageId":"878t36f3ed.fsf@evledraar.gmail.com","threadId":"49519","inReplyTo":"CACPiFCL0oTjN+-aYgKEDtKC0gYwkv6RLMwakdJV85PJ5XQej6g@mail.gmail.com","subject":"Re: git svn clone/fetch hits issues with gc --auto","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-10T11:27:06Z","receivedAt":"2018-10-10T11:27:12Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 10 2018, Martin Langhoff wrote:\n\n> Looking around, Jonathan Tan's \"[PATCH] gc: do not warn about too many\n> loose objects\" makes sense to me.\n>\n> - remove unactionable warning\n> - as the warning is gone, no gc.log is produced\n> - subsequent gc runs don't exit due to gc.log\n>\n> My very humble +1 on that.\n>\n> As for downsides... if we have truly tons of _recent_ loose objects,\n> it'll ... take disk space? I'm fine with that.\n\nAs Jeff's\nhttps://public-inbox.org/git/20180716175103.GB18636@sigill.intra.peff.net/\nand my https://public-inbox.org/git/878t69dgvx.fsf@evledraar.gmail.com/\nnote it's a bit more complex than that.\n\nI.e.:\n\n - The warning is actionable, you can decide to up your expiration\n   policy.\n\n - We use this warning as a proxy for \"let's not run for a day\",\n   otherwise we'll just grind on gc --auto trying to consolidate\n   possibly many hundreds of K of loose objects only to find none of\n   them can be pruned because the run into the expiry policy. With the\n   warning we retry that once per day, which sucks less.\n\n - This conflation of the user-visible warning and the policy is an\n   emergent effect of how the different gc pieces interact, which as I\n   note in the linked thread(s) sucks.\n\n   But we can't just yank one piece away (as Jonathan's patch does)\n   without throwing the baby out with the bathwater.\n\n   It will mean that e.g. if you have 10k loose objects in your git.git,\n   and created them just now, that every time you run anything that runs\n   \"gc --auto\" we'll fork to the background, peg a core at 100% CPU for\n   2-3 minutes or whatever it is, only do get nowhere and do the same\n   thing again in ~3 minutes when you run your next command.\n\n - I think you may be underestimating some of the cases where this ends\n   up taking a huge amount of disk space (and now we'll issue at least\n   *some*) warning. See my\n   https://public-inbox.org/git/87fu6bmr0j.fsf@evledraar.gmail.com/\n   where a repo's .git went from 2.5G to 30G due to being stuck in this\n   mode.\n\n> For more aggressive gc options, thoughts:\n>\n>  - Do we always consider git gc --prune=now \"safe\" in a \"won't delete\n> stuff the user is likely to want\" sense? For example -- are the\n> references from reflogs enough safety?\n\nThe --prune=now command is not generally safe for the reasons noted in\nthe \"NOTES\" section in \"git help gc\".\n\n>  - Even if we don't, for some commands it should be safe to run git gc\n> --prune=now at the end of the process, for example an import that\n> generates a new git repo (git svn clone).\n\nYeah I don't see a problem with that, I didn't know about this\ninteresting use-case, i.e. that \"git svn clone\" will create a lot of\nloose objects.\n\nAs seen in my\nhttps://public-inbox.org/git/87tvm3go42.fsf@evledraar.gmail.com/ I'm\nworking on making \"gc --auto\" run at the end of clone for unrelated\nreasons, i.e. so we generate the commit-graph, seems like \"git svn\nclone\" could do something similar.\n\nSo it's creating a lot of garbage during its cloning process that can\njust be immediately thrown away? What is it doing? Using the object\nstore as a scratch pad for its own temporary state?\n\n> m\n> On Tue, Oct 9, 2018 at 10:49 PM Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> Forwarding to Jonathan, as I think this is an interesting supporting\n>> vote for the topic that we were stuck on.\n>>\n>> Eric Wong <e@80x24.org> writes:\n>>\n>> > Martin Langhoff <martin.langhoff@gmail.com> wrote:\n>> >> Hi folks,\n>> >>\n>> >> Long time no see! Importing a 3GB (~25K revs, tons of files) SVN repo\n>> >> I hit the gc error:\n>> >>\n>> >> warning: There are too many unreachable loose objects; run 'git prune'\n>> >> to remove them.\n>> >> gc --auto: command returned error: 255\n>> >\n>> > GC can be annoying when that happens... For git-svn, perhaps\n>> > this can be appropriate to at least allow the import to continue:\n>> >\n>> > diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\n>> > index 76b2965905..9b0caa3d47 100644\n>> > --- a/perl/Git/SVN.pm\n>> > +++ b/perl/Git/SVN.pm\n>> > @@ -999,7 +999,7 @@ sub restore_commit_header_env {\n>> >  }\n>> >\n>> >  sub gc {\n>> > -     command_noisy('gc', '--auto');\n>> > +     eval { command_noisy('gc', '--auto') };\n>> >  };\n>> >\n>> >  sub do_git_commit {\n>> >\n>> >\n>> > But yeah, somebody else who works on git regularly could\n>> > probably stop repack from writing thousands of loose\n>> > objects (and instead write a self-contained pack with\n>> > those objects, instead).  I haven't followed git closely\n>> > lately, myself.\n"},{"id":"360006","messageId":"CACPiFCKMF2di=waQ5reRtjUFEjuE6DkxxLcN-YnF-SqgE_m=_Q@mail.gmail.com","threadId":"49519","inReplyTo":"878t36f3ed.fsf@evledraar.gmail.com","subject":"Re: git svn clone/fetch hits issues with gc --auto","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2018-10-10T11:41:25Z","receivedAt":"2018-10-10T11:41:39Z","isPatch":false,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On Wed, Oct 10, 2018 at 7:27 AM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> As Jeff's\n> https://public-inbox.org/git/20180716175103.GB18636@sigill.intra.peff.net/\n> and my https://public-inbox.org/git/878t69dgvx.fsf@evledraar.gmail.com/\n> note it's a bit more complex than that.\n\nOk, my bad for not reading the whole thread :-) thanks for the kind explanation.\n\n>  - The warning is actionable, you can decide to up your expiration\n>    policy.\n\nA newbie-ish user shouldn't need to know git's internal store model\n_and the nuances of its special cases_ got get through.\n\n\n>  - We use this warning as a proxy for \"let's not run for a day\"\n\nOh, so _that's_ the trick with creating gc.log? I then understand the\nidea of changing to exit 0.\n\nBut it's far from clear, and a clear _flag_, and not spitting again\nthe same warning, or differently-worded warning would be better.\n\n\"We won't try running gc, a recent run was deemed pointless until some\ntime passes. Nothing to worry about.\"\n\n>  - This conflation of the user-visible warning and the policy is an\n>    emergent effect of how the different gc pieces interact, which as I\n>    note in the linked thread(s) sucks.\n\nIt sure does, and that aspect should be easy to fix...(?)\n\n> So it's creating a lot of garbage during its cloning process that can\n> just be immediately thrown away? What is it doing? Using the object\n> store as a scratch pad for its own temporary state?\n\nYeah, thats suspicious and I don't know why. I've worked on other\nimporters and while those needed 'gc' to generate packs, they didn't\ngenerate garbage objects. After gc, the repo was \"clean\".\n\ncheers,\n\n\n\nm\n-- \n martin.langhoff@gmail.com\n - ask interesting questions  ~  http://linkedin.com/in/martinlanghoff\n - don't be distracted        ~  http://github.com/martin-langhoff\n   by shiny stuff\n"},{"id":"360007","messageId":"877eiqf2nk.fsf@evledraar.gmail.com","threadId":"49519","inReplyTo":"878t36f3ed.fsf@evledraar.gmail.com","subject":"Re: git svn clone/fetch hits issues with gc --auto","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-10T11:43:11Z","receivedAt":"2018-10-10T11:43:17Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 10 2018, Ævar Arnfjörð Bjarmason wrote:\n\n> On Wed, Oct 10 2018, Martin Langhoff wrote:\n>\n>> Looking around, Jonathan Tan's \"[PATCH] gc: do not warn about too many\n>> loose objects\" makes sense to me.\n>>\n>> - remove unactionable warning\n>> - as the warning is gone, no gc.log is produced\n>> - subsequent gc runs don't exit due to gc.log\n>>\n>> My very humble +1 on that.\n>>\n>> As for downsides... if we have truly tons of _recent_ loose objects,\n>> it'll ... take disk space? I'm fine with that.\n>\n> As Jeff's\n> https://public-inbox.org/git/20180716175103.GB18636@sigill.intra.peff.net/\n> and my https://public-inbox.org/git/878t69dgvx.fsf@evledraar.gmail.com/\n> note it's a bit more complex than that.\n>\n> I.e.:\n>\n>  - The warning is actionable, you can decide to up your expiration\n>    policy.\n>\n>  - We use this warning as a proxy for \"let's not run for a day\",\n>    otherwise we'll just grind on gc --auto trying to consolidate\n>    possibly many hundreds of K of loose objects only to find none of\n>    them can be pruned because the run into the expiry policy. With the\n>    warning we retry that once per day, which sucks less.\n>\n>  - This conflation of the user-visible warning and the policy is an\n>    emergent effect of how the different gc pieces interact, which as I\n>    note in the linked thread(s) sucks.\n>\n>    But we can't just yank one piece away (as Jonathan's patch does)\n>    without throwing the baby out with the bathwater.\n>\n>    It will mean that e.g. if you have 10k loose objects in your git.git,\n>    and created them just now, that every time you run anything that runs\n>    \"gc --auto\" we'll fork to the background, peg a core at 100% CPU for\n>    2-3 minutes or whatever it is, only do get nowhere and do the same\n>    thing again in ~3 minutes when you run your next command.\n>\n>  - I think you may be underestimating some of the cases where this ends\n>    up taking a huge amount of disk space (and now we'll issue at least\n>    *some*) warning. See my\n>    https://public-inbox.org/git/87fu6bmr0j.fsf@evledraar.gmail.com/\n>    where a repo's .git went from 2.5G to 30G due to being stuck in this\n>    mode.\n>\n>> For more aggressive gc options, thoughts:\n>>\n>>  - Do we always consider git gc --prune=now \"safe\" in a \"won't delete\n>> stuff the user is likely to want\" sense? For example -- are the\n>> references from reflogs enough safety?\n>\n> The --prune=now command is not generally safe for the reasons noted in\n> the \"NOTES\" section in \"git help gc\".\n>\n>>  - Even if we don't, for some commands it should be safe to run git gc\n>> --prune=now at the end of the process, for example an import that\n>> generates a new git repo (git svn clone).\n>\n> Yeah I don't see a problem with that, I didn't know about this\n> interesting use-case, i.e. that \"git svn clone\" will create a lot of\n> loose objects.\n>\n> As seen in my\n> https://public-inbox.org/git/87tvm3go42.fsf@evledraar.gmail.com/ I'm\n> working on making \"gc --auto\" run at the end of clone for unrelated\n> reasons, i.e. so we generate the commit-graph, seems like \"git svn\n> clone\" could do something similar.\n>\n> So it's creating a lot of garbage during its cloning process that can\n> just be immediately thrown away? What is it doing? Using the object\n> store as a scratch pad for its own temporary state?\n\nTo answer my own question (which was based on a thinko) it's continually\ncreating loose objects during import, i.e. packs are not involved (don't\nknow why I thought that), so yeah, because all of those have <2wks\nexpiry we end up warning as gc --auto is run.\n\nBut I actually think the git-svn import is revealing an entirely\ndifferent problem.\n\nI.e. when I clone I seem to be getting a refs/remotes/git-svn branch\nthat's kept up-to-date, and when I \"gc\" everything's consolidated into a\npack, we don't have any loose objects that are meant for expiry.\n\nBut the reason git-svn is whining is because we're doing this in gc\n(simplified for the sake af discussion):\n\n    if (too_many_loose()) {\n        expire();\n        repack();\n        if (too_many_loose())\n            die(\"oh noes too many loose that don't match our expiry policy!\");\n    }\n\nBut they don't fall under our expiry policy at all, we're just assuming\nthat a crapload of loose objects haven't been added in the interim from\nwhen we ran expire() + repack() until when we check too_many_loose()\nagain.\n\nThat's a logic error which we could just solve at some expense by seeing\n*which* objects are loose and candidates for expiry at the beginning,\nand not warning if at the end we have *different* loose objects that\nshould be consolidated, that just means we genuinely should run gc\nagain.\n\nOr is this just wrong? I don't really know. If the above is true I'm\nmissing how tweaking gc.pruneExpire=5.minutes.ago is helping. Surely\nwe'd either just end up with the same set of loose objects (since the\nclone is still running), or alternatively if git-svn hadn't gotten\naround to updating refs create a corrupt repo.\n\n\n\n\n>> m\n>> On Tue, Oct 9, 2018 at 10:49 PM Junio C Hamano <gitster@pobox.com> wrote:\n>>>\n>>> Forwarding to Jonathan, as I think this is an interesting supporting\n>>> vote for the topic that we were stuck on.\n>>>\n>>> Eric Wong <e@80x24.org> writes:\n>>>\n>>> > Martin Langhoff <martin.langhoff@gmail.com> wrote:\n>>> >> Hi folks,\n>>> >>\n>>> >> Long time no see! Importing a 3GB (~25K revs, tons of files) SVN repo\n>>> >> I hit the gc error:\n>>> >>\n>>> >> warning: There are too many unreachable loose objects; run 'git prune'\n>>> >> to remove them.\n>>> >> gc --auto: command returned error: 255\n>>> >\n>>> > GC can be annoying when that happens... For git-svn, perhaps\n>>> > this can be appropriate to at least allow the import to continue:\n>>> >\n>>> > diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\n>>> > index 76b2965905..9b0caa3d47 100644\n>>> > --- a/perl/Git/SVN.pm\n>>> > +++ b/perl/Git/SVN.pm\n>>> > @@ -999,7 +999,7 @@ sub restore_commit_header_env {\n>>> >  }\n>>> >\n>>> >  sub gc {\n>>> > -     command_noisy('gc', '--auto');\n>>> > +     eval { command_noisy('gc', '--auto') };\n>>> >  };\n>>> >\n>>> >  sub do_git_commit {\n>>> >\n>>> >\n>>> > But yeah, somebody else who works on git regularly could\n>>> > probably stop repack from writing thousands of loose\n>>> > objects (and instead write a self-contained pack with\n>>> > those objects, instead).  I haven't followed git closely\n>>> > lately, myself.\n"},{"id":"360009","messageId":"875zyaf2f1.fsf@evledraar.gmail.com","threadId":"49519","inReplyTo":"CACPiFCKMF2di=waQ5reRtjUFEjuE6DkxxLcN-YnF-SqgE_m=_Q@mail.gmail.com","subject":"Re: git svn clone/fetch hits issues with gc --auto","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-10T11:48:18Z","receivedAt":"2018-10-10T11:48:24Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 10 2018, Martin Langhoff wrote:\n\n> On Wed, Oct 10, 2018 at 7:27 AM Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n>> As Jeff's\n>> https://public-inbox.org/git/20180716175103.GB18636@sigill.intra.peff.net/\n>> and my https://public-inbox.org/git/878t69dgvx.fsf@evledraar.gmail.com/\n>> note it's a bit more complex than that.\n>\n> Ok, my bad for not reading the whole thread :-) thanks for the kind explanation.\n>\n>>  - The warning is actionable, you can decide to up your expiration\n>>    policy.\n>\n> A newbie-ish user shouldn't need to know git's internal store model\n> _and the nuances of its special cases_ got get through.\n\nOh yeah, don't get me wrong. I think this whole thing sucks, and as the\nlinked threads show I've run into various sucky edge cases of this.\n\nI'm just saying it's hard in this case to remove one piece without the\nwhole Jenga tower collapsing, and it's probably a good idea in some of\nthese cases to pester the user about what he wants, but probably not via\ngc --auto emitting the same warning every time, e.g. in one of these\nthreads I suggested maybe \"git status\" should emit this.\n\n>\n>>  - We use this warning as a proxy for \"let's not run for a day\"\n>\n> Oh, so _that's_ the trick with creating gc.log? I then understand the\n> idea of changing to exit 0.\n>\n> But it's far from clear, and a clear _flag_, and not spitting again\n> the same warning, or differently-worded warning would be better.\n>\n> \"We won't try running gc, a recent run was deemed pointless until some\n> time passes. Nothing to worry about.\"\n\nYup. That would be better. Right now we don't write anything\nmachine-readable to the log, and we'd need to start doing that. E.g. we\ncould just as well be reporting that gc --auto is segfaulting and that's\nwhy you have all this garbage. We just \"cat\" it.\n\n>>  - This conflation of the user-visible warning and the policy is an\n>>    emergent effect of how the different gc pieces interact, which as I\n>>    note in the linked thread(s) sucks.\n>\n> It sure does, and that aspect should be easy to fix...(?)\n>\n>> So it's creating a lot of garbage during its cloning process that can\n>> just be immediately thrown away? What is it doing? Using the object\n>> store as a scratch pad for its own temporary state?\n>\n> Yeah, thats suspicious and I don't know why. I've worked on other\n> importers and while those needed 'gc' to generate packs, they didn't\n> generate garbage objects. After gc, the repo was \"clean\".\n\nI tried to find this out in my reply-to-myself in\nhttps://public-inbox.org/git/877eiqf2nk.fsf@evledraar.gmail.com/\n\nBut as noted just looked at this briefly, and I don't use git-svn for\nyears now, so I don't know and might be missing something.\n"},{"id":"360010","messageId":"xmqqzhvmkn4z.fsf@gitster-ct.c.googlers.com","threadId":"49519","inReplyTo":"878t36f3ed.fsf@evledraar.gmail.com","subject":"Re: git svn clone/fetch hits issues with gc --auto","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-10T12:21:48Z","receivedAt":"2018-10-10T12:21:55Z","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>  - We use this warning as a proxy for \"let's not run for a day\",\n>    otherwise we'll just grind on gc --auto trying to consolidate\n>    possibly many hundreds of K of loose objects only to find none of\n>    them can be pruned because the run into the expiry policy. With the\n>    warning we retry that once per day, which sucks less.\n>\n>  - This conflation of the user-visible warning and the policy is an\n>    emergent effect of how the different gc pieces interact, which as I\n>    note in the linked thread(s) sucks.\n>\n>    But we can't just yank one piece away (as Jonathan's patch does)\n>    without throwing the baby out with the bathwater.\n>\n>    It will mean that e.g. if you have 10k loose objects in your git.git,\n>    and created them just now, that every time you run anything that runs\n>    \"gc --auto\" we'll fork to the background, peg a core at 100% CPU for\n>    2-3 minutes or whatever it is, only do get nowhere and do the same\n>    thing again in ~3 minutes when you run your next command.\n\nWe probably can keep the \"let's not run for a day\" safety while\npretending that \"git gc -auto\" succeeded for callers like \"git svn\"\nso that these callers do not hae to do \"eval { ... }\" to hide our\nexit code, no?\n\nI think that is what Jonathan's patch (jn/gc-auto) does.\n\nFrom: Jonathan Nieder <jrnieder@gmail.com>\nDate: Mon, 16 Jul 2018 23:57:40 -0700\nSubject: [PATCH] gc: do not return error for prior errors in daemonized mode\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 95c8afd07b..ce8a663a01 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -438,9 +438,15 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)\n \treturn NULL;\n }\n \n-static void report_last_gc_error(void)\n+/*\n+ * Returns 0 if there was no previous error and gc can proceed, 1 if\n+ * gc should not proceed due to an error in the last run. Prints a\n+ * message and returns -1 if an error occured while reading gc.log\n+ */\n+static int report_last_gc_error(void)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n+\tint ret = 0;\n...\n \tif (len < 0)\n+\t\tret = error_errno(_(\"cannot read '%s'\"), gc_log_path);\n+\telse if (len > 0) {\n+\t\t/*\n+\t\t * A previous gc failed.  Report the error, and don't\n+\t\t * bother with an automatic gc run since it is likely\n+\t\t * to fail in the same way.\n+\t\t */\n+\t\twarning(_(\"The last gc run reported the following. \"\n \t\t\t       \"Please correct the root cause\\n\"\n \t\t\t       \"and remove %s.\\n\"\n \t\t\t       \"Automatic cleanup will not be performed \"\n \t\t\t       \"until the file is removed.\\n\\n\"\n \t\t\t       \"%s\"),\n \t\t\t    gc_log_path, sb.buf);\n+\t\tret = 1;\n+\t}\n \tstrbuf_release(&sb);\n done:\n \tfree(gc_log_path);\n+\treturn ret;\n }\n \nI.e. report_last_gc_error() returns 1 when finds that the previous\nattempt to \"gc --auto\" failed.  And then\n\n@@ -561,7 +576,13 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t\tfprintf(stderr, _(\"See \\\"git help gc\\\" for manual housekeeping.\\n\"));\n \t\t}\n \t\tif (detach_auto) {\n-\t\t\treport_last_gc_error(); /* dies on error */\n+\t\t\tint ret = report_last_gc_error();\n+\t\t\tif (ret < 0)\n+\t\t\t\t/* an I/O error occured, already reported */\n+\t\t\t\texit(128);\n+\t\t\tif (ret == 1)\n+\t\t\t\t/* Last gc --auto failed. Skip this one. */\n+\t\t\t\treturn 0;\n\n... it exits with 0 without bothering to rerun \"gc\".\n\nSo it won't get stuck for 3 minutes; the repository after \"gc\n--auto\" punts will stay to be suboptimal for a day, and the user\nkill not get an \"actionable\" error notice (due to this hiding of\nprevious error), hence cannot make changes that may help like\nshortening expiry period, though.\n\n"},{"id":"360012","messageId":"874lduf05o.fsf@evledraar.gmail.com","threadId":"49519","inReplyTo":"xmqqzhvmkn4z.fsf@gitster-ct.c.googlers.com","subject":"Re: git svn clone/fetch hits issues with gc --auto","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-10T12:37:07Z","receivedAt":"2018-10-10T12:37:13Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 10 2018, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>\n>>  - We use this warning as a proxy for \"let's not run for a day\",\n>>    otherwise we'll just grind on gc --auto trying to consolidate\n>>    possibly many hundreds of K of loose objects only to find none of\n>>    them can be pruned because the run into the expiry policy. With the\n>>    warning we retry that once per day, which sucks less.\n>>\n>>  - This conflation of the user-visible warning and the policy is an\n>>    emergent effect of how the different gc pieces interact, which as I\n>>    note in the linked thread(s) sucks.\n>>\n>>    But we can't just yank one piece away (as Jonathan's patch does)\n>>    without throwing the baby out with the bathwater.\n>>\n>>    It will mean that e.g. if you have 10k loose objects in your git.git,\n>>    and created them just now, that every time you run anything that runs\n>>    \"gc --auto\" we'll fork to the background, peg a core at 100% CPU for\n>>    2-3 minutes or whatever it is, only do get nowhere and do the same\n>>    thing again in ~3 minutes when you run your next command.\n>\n> We probably can keep the \"let's not run for a day\" safety while\n> pretending that \"git gc -auto\" succeeded for callers like \"git svn\"\n> so that these callers do not hae to do \"eval { ... }\" to hide our\n> exit code, no?\n>\n> I think that is what Jonathan's patch (jn/gc-auto) does.\n\nYeah we could take that patch to skip the eval {} suggested upthread.\n\nAs noted when it was discussed I'm *mildly* negative on hiding a IMO\nmeaningful exit code like that, but maybe sprinkling eval {} or other\n\"run but ignore exit code\" in stuff running \"gc --auto\" is worth it, and\nwe could just document that you may want to check gc.log.\n\n> From: Jonathan Nieder <jrnieder@gmail.com>\n> Date: Mon, 16 Jul 2018 23:57:40 -0700\n> Subject: [PATCH] gc: do not return error for prior errors in daemonized mode\n>\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 95c8afd07b..ce8a663a01 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -438,9 +438,15 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)\n>  \treturn NULL;\n>  }\n>\n> -static void report_last_gc_error(void)\n> +/*\n> + * Returns 0 if there was no previous error and gc can proceed, 1 if\n> + * gc should not proceed due to an error in the last run. Prints a\n> + * message and returns -1 if an error occured while reading gc.log\n> + */\n> +static int report_last_gc_error(void)\n>  {\n>  \tstruct strbuf sb = STRBUF_INIT;\n> +\tint ret = 0;\n> ...\n>  \tif (len < 0)\n> +\t\tret = error_errno(_(\"cannot read '%s'\"), gc_log_path);\n> +\telse if (len > 0) {\n> +\t\t/*\n> +\t\t * A previous gc failed.  Report the error, and don't\n> +\t\t * bother with an automatic gc run since it is likely\n> +\t\t * to fail in the same way.\n> +\t\t */\n> +\t\twarning(_(\"The last gc run reported the following. \"\n>  \t\t\t       \"Please correct the root cause\\n\"\n>  \t\t\t       \"and remove %s.\\n\"\n>  \t\t\t       \"Automatic cleanup will not be performed \"\n>  \t\t\t       \"until the file is removed.\\n\\n\"\n>  \t\t\t       \"%s\"),\n>  \t\t\t    gc_log_path, sb.buf);\n> +\t\tret = 1;\n> +\t}\n>  \tstrbuf_release(&sb);\n>  done:\n>  \tfree(gc_log_path);\n> +\treturn ret;\n>  }\n>\n> I.e. report_last_gc_error() returns 1 when finds that the previous\n> attempt to \"gc --auto\" failed.  And then\n>\n> @@ -561,7 +576,13 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n>  \t\t\tfprintf(stderr, _(\"See \\\"git help gc\\\" for manual housekeeping.\\n\"));\n>  \t\t}\n>  \t\tif (detach_auto) {\n> -\t\t\treport_last_gc_error(); /* dies on error */\n> +\t\t\tint ret = report_last_gc_error();\n> +\t\t\tif (ret < 0)\n> +\t\t\t\t/* an I/O error occured, already reported */\n> +\t\t\t\texit(128);\n> +\t\t\tif (ret == 1)\n> +\t\t\t\t/* Last gc --auto failed. Skip this one. */\n> +\t\t\t\treturn 0;\n>\n> ... it exits with 0 without bothering to rerun \"gc\".\n>\n> So it won't get stuck for 3 minutes; the repository after \"gc\n> --auto\" punts will stay to be suboptimal for a day, and the user\n> kill not get an \"actionable\" error notice (due to this hiding of\n> previous error), hence cannot make changes that may help like\n> shortening expiry period, though.\n\nRight, because it still writes the gc.log, but we'll still be yelling at\nthe user on every commit/fetch etc. that we discovered such-and-such an\nissue on the last gc for that full day.\n\nThat 3 minute comment was in reference to if we'd apply Jonathan Tan's\n\"[PATCH] gc: do not warn about too many loose objects without any other\nchanges. Then we'd just keep returning true on too_many_loose_objects()\neven though gc wouldn't help to resolve it.\n"},{"id":"360047","messageId":"CACPiFC+sPTSa6WRp8iy3e_qKVD46Z9VsXA-UnMhv-ygpZbUCaw@mail.gmail.com","threadId":"49519","inReplyTo":"xmqqzhvmkn4z.fsf@gitster-ct.c.googlers.com","subject":"Re: git svn clone/fetch hits issues with gc --auto","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2018-10-10T16:38:00Z","receivedAt":"2018-10-10T16:38:14Z","isPatch":false,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On Wed, Oct 10, 2018 at 8:21 AM Junio C Hamano <gitster@pobox.com> wrote:\n> We probably can keep the \"let's not run for a day\" safety while\n> pretending that \"git gc -auto\" succeeded for callers like \"git svn\"\n> so that these callers do not hae to do \"eval { ... }\" to hide our\n> exit code, no?\n>\n> I think that is what Jonathan's patch (jn/gc-auto) does.\n\n+1\n\n`--auto` means \"DTRT, but remember you're running as part of a larger\nprocess; don't error out unless it's critical\".\n\ncheers,\n\n\nm\n-- \n martin.langhoff@gmail.com\n - ask interesting questions  ~  http://linkedin.com/in/martinlanghoff\n - don't be distracted        ~  http://github.com/martin-langhoff\n   by shiny stuff\n"},{"id":"360048","messageId":"20181010165152.GA180779@aiede.svl.corp.google.com","threadId":"49519","inReplyTo":"875zyaf2f1.fsf@evledraar.gmail.com","subject":"Re: git svn clone/fetch hits issues with gc --auto","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-10-10T16:51:52Z","receivedAt":"2018-10-10T16:51:57Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nÆvar Arnfjörð Bjarmason wrote:\n\n> I'm just saying it's hard in this case to remove one piece without the\n> whole Jenga tower collapsing, and it's probably a good idea in some of\n> these cases to pester the user about what he wants, but probably not via\n> gc --auto emitting the same warning every time, e.g. in one of these\n> threads I suggested maybe \"git status\" should emit this.\n\nI have to say, I don't have a lot of sympathy for this.\n\nI've been running with the patches I sent before for a while now, and\nthe behavior that they create is great.  I think we can make further\nrefinements on top.  To put it another way, I haven't actually\nexperienced any bad knock-on effects, and I think other feature\nrequests can be addressed separately.\n\nI do have sympathy for some wishes for changes to \"git gc --auto\"\nbehavior (I think it should be synchronous regardless of config and\nthe asynchrony should move to being requested explicitly through a\ncommand line option by the callers within Git) but I don't understand\nwhy this holds up a change that IMHO is wholly positive for users.\n\nTo put it another way, I am getting the feeling that the objections to\nthat series were theoretical, while the practical benefits of the\npatch are pretty immediate and real.  I'm happy to help anyone who\nwants to polish it but time has shown no one is working on that, so...\n\nThanks,\nJonathan\n"},{"id":"360049","messageId":"20181010174624.GC8786@sigill.intra.peff.net","threadId":"49519","inReplyTo":"20181010165152.GA180779@aiede.svl.corp.google.com","subject":"Re: git svn clone/fetch hits issues with gc --auto","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-10T17:46:24Z","receivedAt":"2018-10-10T17:46:28Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 10, 2018 at 09:51:52AM -0700, Jonathan Nieder wrote:\n\n> Ævar Arnfjörð Bjarmason wrote:\n> \n> > I'm just saying it's hard in this case to remove one piece without the\n> > whole Jenga tower collapsing, and it's probably a good idea in some of\n> > these cases to pester the user about what he wants, but probably not via\n> > gc --auto emitting the same warning every time, e.g. in one of these\n> > threads I suggested maybe \"git status\" should emit this.\n> \n> I have to say, I don't have a lot of sympathy for this.\n> \n> I've been running with the patches I sent before for a while now, and\n> the behavior that they create is great.  I think we can make further\n> refinements on top.  To put it another way, I haven't actually\n> experienced any bad knock-on effects, and I think other feature\n> requests can be addressed separately.\n\nI think there may be some miscommunication here. The Jenga tower above\nis referring (I think) to Jonathan Tan's original patch to drop the\nwarning entirely, which does have some unwanted side effects.\n\nYour patches are much less controversial, I think, and are in next and\nmarked as \"will merge to master\" in the last \"what's cooking\".\n\n-Peff\n"},{"id":"360053","messageId":"87va69ejfk.fsf@evledraar.gmail.com","threadId":"49519","inReplyTo":"20181010165152.GA180779@aiede.svl.corp.google.com","subject":"Re: git svn clone/fetch hits issues with gc --auto","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-10T18:38:23Z","receivedAt":"2018-10-10T18:38:29Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 10 2018, Jonathan Nieder wrote:\n\n> Hi,\n>\n> Ævar Arnfjörð Bjarmason wrote:\n>\n>> I'm just saying it's hard in this case to remove one piece without the\n>> whole Jenga tower collapsing, and it's probably a good idea in some of\n>> these cases to pester the user about what he wants, but probably not via\n>> gc --auto emitting the same warning every time, e.g. in one of these\n>> threads I suggested maybe \"git status\" should emit this.\n>\n> I have to say, I don't have a lot of sympathy for this.\n>\n> I've been running with the patches I sent before for a while now, and\n> the behavior that they create is great.  I think we can make further\n> refinements on top.  To put it another way, I haven't actually\n> experienced any bad knock-on effects, and I think other feature\n> requests can be addressed separately.\n>\n> I do have sympathy for some wishes for changes to \"git gc --auto\"\n> behavior (I think it should be synchronous regardless of config and\n> the asynchrony should move to being requested explicitly through a\n> command line option by the callers within Git) but I don't understand\n> why this holds up a change that IMHO is wholly positive for users.\n>\n> To put it another way, I am getting the feeling that the objections to\n> that series were theoretical, while the practical benefits of the\n> patch are pretty immediate and real.  I'm happy to help anyone who\n> wants to polish it but time has shown no one is working on that, so...\n\n[I wrote this before seeing Jeff's reply, but just to bo clear...]\n\nYes, like Jeff says I'm not referring to your gitster/jn/gc-auto with\nthis \"Jenga tower\" comment.\n\nRe that patch: I've said what I think about tools printing error\nmessages saying \"I can't do stuff\" while not returning a non-zero exit\ncode, so I won't repeat that here. But whatever anyone thinks of that\nit's ultimately a rather trivial detail, and doesn't have any knock-on\neffects on the rest of git-gc behavior.\n\nI'm talking about the \"gc: do not warn about too many loose objects\"\npatch and similar approaches. FWIW what I'm describing in\n<878t36f3ed.fsf@evledraar.gmail.com> isn't some theoretical concern. In\nsome large repositories at work that experience a lot of branch churn\nand have fetch.prune / fetch.pruneTags turned on active checkouts very\nquickly get to the default 6700 limit.\n\nI've currently found that gc.pruneExpire=4.days.ago is close to a sweet\nspot of avoiding that issue for now, while not e.g. gc-ing a loose\nobject someone committed on Friday before the same time on Monday, but\nbefore I tweaked that, but with the default of 2.weeks we'd much more\nregularly see the problem described in [1].\n\nBut as noted in the various GC threads linked from this one that sort of\nsolution within the confines of the current implementation and\nconfiguration promises we've made, which lead to all sorts of stupidity.\n\n1. https://public-inbox.org/git/87inc89j38.fsf@evledraar.gmail.com/\n"},{"id":"360060","messageId":"20181010192732.13918-1-avarab@gmail.com","threadId":"49519","inReplyTo":"20181010174624.GC8786@sigill.intra.peff.net","subject":"[PATCH] gc: introduce an --auto-exit-code option for undoing 3029970275","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-10T19:27:32Z","receivedAt":"2018-10-10T19:27:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add an --auto-exit-code variable and a corresponding 'gc.autoExitCode'\nconfiguration option to optionally bring back the 'git gc --auto' exit\ncode behavior as it existed between 2.6.3..2.19.0 (inclusive).\n\nThis was changed in 3029970275 (\"gc: do not return error for prior\nerrors in daemonized mode\", 2018-07-16). The motivation for that patch\nwas to appease 3rd party tools whose treatment of the 'git gc --auto'\nexit code is different from that of git core where it has always been\nignored.\n\nThat means that out of the three modes gc --auto will operate in:\n\n 1. gc --auto has nothing to do\n 2. gc --auto has something to do, will fork and try to do it\n 3. gc --auto has something to do, but notices that gc has been failing\n    before when forked and can't do anything now.\n\nWe started exiting with zero in the case of #3, instead of\nnon-zero (see [1] for more details). As noted by the docs being added\nhere the #3 case is relatively rare, so I think it's fine to change\nthis as the default with the assumption that the use-case for tools\nlike the \"repo\" tool noted in commit 3029970275 above are more common\nthan not.\n\nBut it left us without any option to have \"git gc --auto\" tell us\nabout this failure except by starting to either parse its output, or\nfor the caller to start breaking the encapsulation and starting to\ncheck for .git/gc.log themselves.\n\nLet's instead provide an option to exit with non-zero when we have\nerrors to tell the user about, and provide a configuration option so\nthat it can be dropped in-place in anticipation of upgrading to Git\nversion 2.20 without having to make using --auto-exit-code conditional\non the git version in use.\n\n1. https://public-inbox.org/git/878t69dgvx.fsf@evledraar.gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\n> On Wed, Oct 10, 2018 at 09:51:52AM -0700, Jonathan Nieder wrote:\n>\n>> Ævar Arnfjörð Bjarmason wrote:\n>> \n>> > I'm just saying it's hard in this case to remove one piece without the\n>> > whole Jenga tower collapsing, and it's probably a good idea in some of\n>> > these cases to pester the user about what he wants, but probably not via\n>> > gc --auto emitting the same warning every time, e.g. in one of these\n>> > threads I suggested maybe \"git status\" should emit this.\n>> \n>> I have to say, I don't have a lot of sympathy for this.\n>> \n>> I've been running with the patches I sent before for a while now, and\n>> the behavior that they create is great.  I think we can make further\n>> refinements on top.  To put it another way, I haven't actually\n>> experienced any bad knock-on effects, and I think other feature\n>> requests can be addressed separately.\n>\n> I think there may be some miscommunication here. The Jenga tower above\n> is referring (I think) to Jonathan Tan's original patch to drop the\n> warning entirely, which does have some unwanted side effects.\n>\n> Your patches are much less controversial, I think, and are in next and\n> marked as \"will merge to master\" in the last \"what's cooking\".\n\n[Junio: This goes on top of gitster/jn/gc-auto]\n\nI thought the jn/gc-auto topic was still in \"pu\", my fault for not\npaying attention. It seems the general consensus is against my notion\nof what should be the default (which is fine), but since (as noted in\nthe patch) it seems yucky to start breaking the encapsulation of\ngc.log, especially as it's looking more and more likely that it'll be\nan implementation detail we might drop, here's a patch on top of\njn/gc-auto to optionally restore the old behavior.\n\n Documentation/config.txt | 28 ++++++++++++++++++++++++++++\n Documentation/git-gc.txt |  7 +++++++\n builtin/gc.c             |  7 ++++++-\n t/t6500-gc.sh            |  2 ++\n 4 files changed, 43 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 5b72684999..e37a463bf8 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1635,6 +1635,34 @@ gc.autoDetach::\n \tMake `git gc --auto` return immediately and run in background\n \tif the system supports it. Default is true.\n \n+gc.autoExitCode::\n+\tMake `git gc --auto` return non-zero if it would have\n+\tdemonized itself (see `gc.autoDetach`) due to a needed gc, but\n+\ta 'gc.log' is found within the `gc.logExpiry` with an error\n+\tfrom a previous run.\n++\n+When 'git gc' is run with the default of `gc.autoDetach=true` a\n+failure might have been noted in the 'gc.log' from a previously\n+detached `--auto` run. If the failure is within the time configured in\n+`gc.logExpiry` the `--auto` run will abort early and report the error\n+in the 'gc.log'.\n++\n+From version 2.6.3 to version 2.19 of Git encountering this error\n+would cause 'git gc' to exit with non-zero, but this was deemed to be\n+a hassle for third-party tools to handle since it rarely happens, and\n+they usually don't assume that 'git gc --auto' can fail. Therefore\n+since version 2.20 of Git 'git gc --auto' will always exit with zero\n+if it would have demonized itself, even when encountering such an\n+error.\n++\n+Supplying this option will make 'git gc' exit with non-zero in that\n+case, which allows for detecting cases where a repository is in a\n+state where git 'gc --auto' is refusing to demonize due to previously\n+encountered errors.\n++\n+This option can also be turned on as a one-off with the\n+`--auto-exit-code` option, see linkgit:git-gc[1].\n+\n gc.bigPackThreshold::\n \tIf non-zero, all packs larger than this limit are kept when\n \t`git gc` is run. This is very similar to `--keep-base-pack`\ndiff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt\nindex 24b2dd44fe..adf53fc959 100644\n--- a/Documentation/git-gc.txt\n+++ b/Documentation/git-gc.txt\n@@ -71,6 +71,13 @@ If houskeeping is required due to many loose objects or packs, all\n other housekeeping tasks (e.g. rerere, working trees, reflog...) will\n be performed as well.\n \n+--auto-exit-code::\n+\tMake `git gc --auto` return non-zero if it would have\n+\tdemonized itself (see `gc.autoDetach`) due to a needed gc, but\n+\ta 'gc.log' is found within the time period of `gc.logExpiry`\n+\twith an error from a previous run. See linkgit:git-config[1]\n+\tfor more details about this option which can also be\n+\tconfigured with the `gc.autoExitCode` boolean variable.\n \n --prune=<date>::\n \tPrune loose objects older than date (default is 2 weeks ago,\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex ce8a663a01..69c0dedd8c 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -41,6 +41,7 @@ static int aggressive_window = 250;\n static int gc_auto_threshold = 6700;\n static int gc_auto_pack_limit = 50;\n static int detach_auto = 1;\n+static int gc_auto_exit_code = 0;\n static timestamp_t gc_log_expire_time;\n static const char *gc_log_expire = \"1.day.ago\";\n static const char *prune_expire = \"2.weeks.ago\";\n@@ -130,6 +131,7 @@ static void gc_config(void)\n \tgit_config_get_int(\"gc.auto\", &gc_auto_threshold);\n \tgit_config_get_int(\"gc.autopacklimit\", &gc_auto_pack_limit);\n \tgit_config_get_bool(\"gc.autodetach\", &detach_auto);\n+\tgit_config_get_bool(\"gc.autoexitcode\", &gc_auto_exit_code);\n \tgit_config_get_expiry(\"gc.pruneexpire\", &prune_expire);\n \tgit_config_get_expiry(\"gc.worktreepruneexpire\", &prune_worktrees_expire);\n \tgit_config_get_expiry(\"gc.logexpiry\", &gc_log_expire);\n@@ -518,6 +520,9 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"aggressive\", &aggressive, N_(\"be more thorough (increased runtime)\")),\n \t\tOPT_BOOL_F(0, \"auto\", &auto_gc, N_(\"enable auto-gc mode\"),\n \t\t\t   PARSE_OPT_NOCOMPLETE),\n+\t\tOPT_BOOL_F(0, \"auto-exit-code\", &gc_auto_exit_code,\n+\t\t\t   N_(\"exit with non-zero if an error is encountered before gc is daemonized\"),\n+\t\t\t   PARSE_OPT_NOCOMPLETE),\n \t\tOPT_BOOL_F(0, \"force\", &force,\n \t\t\t   N_(\"force running gc even if there may be another gc running\"),\n \t\t\t   PARSE_OPT_NOCOMPLETE),\n@@ -582,7 +587,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t\t\texit(128);\n \t\t\tif (ret == 1)\n \t\t\t\t/* Last gc --auto failed. Skip this one. */\n-\t\t\t\treturn 0;\n+\t\t\t\treturn gc_auto_exit_code;\n \n \t\t\tif (lock_repo_for_gc(force, &pid))\n \t\t\t\treturn 0;\ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex a222efdbe1..25bce70316 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -121,6 +121,8 @@ test_expect_success 'background auto gc does not run if gc.log is present and re\n \ttest_config gc.logexpiry 5.days &&\n \ttest-tool chmtime =-345600 .git/gc.log &&\n \tgit gc --auto &&\n+\ttest_must_fail git gc --auto --auto-exit-code &&\n+\ttest_must_fail git -c gc.autoExitCode=true gc --auto &&\n \ttest_config gc.logexpiry 2.days &&\n \trun_and_wait_for_auto_gc &&\n \tls .git/objects/pack/pack-*.pack >packs &&\n-- \n2.19.1.390.gf3a00b506f\n\n"},{"id":"360064","messageId":"20181010203531.GA12949@sigill.intra.peff.net","threadId":"49519","inReplyTo":"20181010192732.13918-1-avarab@gmail.com","subject":"Re: [PATCH] gc: introduce an --auto-exit-code option for undoing 3029970275","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-10T20:35:32Z","receivedAt":"2018-10-10T20:35:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 10, 2018 at 07:27:32PM +0000, Ævar Arnfjörð Bjarmason wrote:\n\n> Add an --auto-exit-code variable and a corresponding 'gc.autoExitCode'\n> configuration option to optionally bring back the 'git gc --auto' exit\n> code behavior as it existed between 2.6.3..2.19.0 (inclusive).\n> \n> This was changed in 3029970275 (\"gc: do not return error for prior\n> errors in daemonized mode\", 2018-07-16). The motivation for that patch\n> was to appease 3rd party tools whose treatment of the 'git gc --auto'\n> exit code is different from that of git core where it has always been\n> ignored.\n\nOK. I wouldn't want to use this myself, but I think you've made clear\nwhy you find it useful. So I don't mind making it an optional behavior\n(and it probably beats you trying to poke at the logfile yourself).\n\nI'm not sure if the config is going to actually help that much, though.\nThe callers within Git will generally ignore the exit code anyway. So\nfor those cases, setting it will at best do nothing, and at worst it may\nconfuse the few stragglers (e.g., the git-svn one under recent\ndiscussion).\n\nCallers who _are_ prepared to act on the exit code probably ought to\njust use --auto-exit-code in their invocation.\n\nThat said, I'm not entirely opposed to the matching config. There's\nenough history here that somebody might want a sledgehammer setting to\ngo back to the old behavior.\n\n-Peff\n"},{"id":"360070","messageId":"20181010205611.GA195252@aiede.svl.corp.google.com","threadId":"49519","inReplyTo":"20181010192732.13918-1-avarab@gmail.com","subject":"Re: [PATCH] gc: introduce an --auto-exit-code option for undoing 3029970275","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-10-10T20:56:11Z","receivedAt":"2018-10-10T20:56:15Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nÆvar Arnfjörð Bjarmason wrote:\n\n> Add an --auto-exit-code variable and a corresponding 'gc.autoExitCode'\n> configuration option to optionally bring back the 'git gc --auto' exit\n> code behavior as it existed between 2.6.3..2.19.0 (inclusive).\n\nHm.  Can you tell me more about the use case where this would be\nhelpful to you?  That would help us come up with a better name for it.\n\nThanks,\nJonathan\n"},{"id":"360072","messageId":"87tvltecvy.fsf@evledraar.gmail.com","threadId":"49519","inReplyTo":"20181010203531.GA12949@sigill.intra.peff.net","subject":"Re: [PATCH] gc: introduce an --auto-exit-code option for undoing 3029970275","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-10T20:59:45Z","receivedAt":"2018-10-10T20:59:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 10 2018, Jeff King wrote:\n\n> On Wed, Oct 10, 2018 at 07:27:32PM +0000, Ævar Arnfjörð Bjarmason wrote:\n>\n>> Add an --auto-exit-code variable and a corresponding 'gc.autoExitCode'\n>> configuration option to optionally bring back the 'git gc --auto' exit\n>> code behavior as it existed between 2.6.3..2.19.0 (inclusive).\n>>\n>> This was changed in 3029970275 (\"gc: do not return error for prior\n>> errors in daemonized mode\", 2018-07-16). The motivation for that patch\n>> was to appease 3rd party tools whose treatment of the 'git gc --auto'\n>> exit code is different from that of git core where it has always been\n>> ignored.\n>\n> OK. I wouldn't want to use this myself, but I think you've made clear\n> why you find it useful. So I don't mind making it an optional behavior\n> (and it probably beats you trying to poke at the logfile yourself).\n\n[...]\n\n> I'm not sure if the config is going to actually help that much, though.\n> The callers within Git will generally ignore the exit code anyway. So\n> for those cases, setting it will at best do nothing, and at worst it may\n> confuse the few stragglers (e.g., the git-svn one under recent\n> discussion).\n\nYeah git internals don't care, but we've never advertised the\ncombination of --auto and gc.autoDetach=true as being something\ninternal-only, so e.g. I wrote stuff expecting errors, and one might run\n\"git gc --auto\" in a repo whose .git/objects state is uncertain to see\nif it needed repack (and have a shell integration that reports\nfailures...).\n\n> Callers who _are_ prepared to act on the exit code probably ought to\n> just use --auto-exit-code in their invocation.\n>\n> That said, I'm not entirely opposed to the matching config. There's\n> enough history here that somebody might want a sledgehammer setting to\n> go back to the old behavior.\n\nIf it's not a config option then as git is upgraded I'll need to change\nmy across-server invocation to be some variant of checking git version,\nthen etiher using the --auto-exit-code option or not (which'll error on\nolder gits). Easier to be able to just drop in a config setting before\nthe upgrade.\n"},{"id":"360073","messageId":"87sh1declw.fsf@evledraar.gmail.com","threadId":"49519","inReplyTo":"20181010205611.GA195252@aiede.svl.corp.google.com","subject":"Re: [PATCH] gc: introduce an --auto-exit-code option for undoing 3029970275","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-10T21:05:47Z","receivedAt":"2018-10-10T21:05:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 10 2018, Jonathan Nieder wrote:\n\n> Hi,\n>\n> Ævar Arnfjörð Bjarmason wrote:\n>\n>> Add an --auto-exit-code variable and a corresponding 'gc.autoExitCode'\n>> configuration option to optionally bring back the 'git gc --auto' exit\n>> code behavior as it existed between 2.6.3..2.19.0 (inclusive).\n>\n> Hm.  Can you tell me more about the use case where this would be\n> helpful to you?  That would help us come up with a better name for it.\n\nFrom the E-Mail linked from the commit message[1] (I opted not to put\nthis in, because it was getting a bit long:\n\n    Right. I know. What I mean is now I can (and do) use it to run 'git gc\n    --auto' across our server fleet and see whether I have any of #3, or\n    whether it's all #1 or #2. If there's nothing to do in #1 that's fine,\n    and it just so happens that I'll run gc due to #2 that's also fine, but\n    I'd like to see if gc really is stuck.\n\n    This of course relies on them having other users / scripts doing normal\n    git commands which would trigger previous 'git gc --auto' runs.\n\nI.e. with your change that command:\n\n    git gc --auto\n\nWould change to something like:\n\n    git gc --auto && ! test -e .git/gc.log\n\nWhich, as noted is a bit of a nasty breaker of the encapsulation, so\nnow:\n\n    git gc --auto --auto-exit-code\n\nOr just a variant of that which will have dropped the config in-place in\n/etc/gitconfig, and then as before:\n\n    git gc --auto\n\n1. https://public-inbox.org/git/878t69dgvx.fsf@evledraar.gmail.com/\n"},{"id":"360074","messageId":"20181010211428.GA231512@aiede.svl.corp.google.com","threadId":"49519","inReplyTo":"87sh1declw.fsf@evledraar.gmail.com","subject":"Re: [PATCH] gc: introduce an --auto-exit-code option for undoing 3029970275","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-10-10T21:14:28Z","receivedAt":"2018-10-10T21:14:34Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n>     Right. I know. What I mean is now I can (and do) use it to run 'git gc\n>     --auto' across our server fleet and see whether I have any of #3, or\n>     whether it's all #1 or #2. If there's nothing to do in #1 that's fine,\n>     and it just so happens that I'll run gc due to #2 that's also fine, but\n>     I'd like to see if gc really is stuck.\n>\n>     This of course relies on them having other users / scripts doing normal\n>     git commands which would trigger previous 'git gc --auto' runs.\n>\n> I.e. with your change that command:\n>\n>     git gc --auto\n>\n> Would change to something like:\n>\n>     git gc --auto && ! test -e .git/gc.log\n>\n> Which, as noted is a bit of a nasty breaker of the encapsulation\n\nThat helps.  What if we package up the \"test -e .git/gc.log\" bit\n*without* having the side effect of running git gc --auto, so that you\ncan run\n\n\tif ! git gc --detached-exit-code\n\tthen\n\t\t... handle the error ...\n\tfi\n\tgit gc --auto; # perhaps also with --detach\n\n?\n\nI'm not great at naming options, so the --detached-exit-code name is\nbikesheddable.  What I really mean to ask about is: what if the status\nreporting goes in a separate command from running gc --auto?\n\nPerhaps this reporting could also print the message from a previous\nrun, so you could write:\n\n\tgit gc --detached-status || exit\n\tgit gc --auto; # perhaps also passing --detach\n\n(Names still open for bikeshedding.)\n\nThanks and hope that helps,\nJonathan\n"},{"id":"360077","messageId":"xmqqin29lc0s.fsf@gitster-ct.c.googlers.com","threadId":"49519","inReplyTo":"20181010211428.GA231512@aiede.svl.corp.google.com","subject":"Re: [PATCH] gc: introduce an --auto-exit-code option for undoing 3029970275","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-10T21:36:35Z","receivedAt":"2018-10-10T21:36:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Perhaps this reporting could also print the message from a previous\n> run, so you could write:\n>\n> \tgit gc --detached-status || exit\n> \tgit gc --auto; # perhaps also passing --detach\n>\n> (Names still open for bikeshedding.)\n\nWhen the command is given --detached-exit-code/status option, what\ndoes it do?  Does it perform the \"did an earlier run left gc.log?\"\nand report the result and nothing else?  In other words, is it a\npure replacement for \"test -e .git/gc.log\"?  Or does it do some of\nthe \"auto-gc\" prep logic like guestimating loose object count and\nhave that also in its exit status (e.g. \"from the gc.log left\nbehind, we know that we failed to reduce loose object count down\nsufficiently after finding there are more than 6700 earlier, but now\nwe do not have that many loose object, so there is nothing to\ncomplain about the presence of gc.log\")?\n\nI am bad at naming myself, but worse at guessing what others meant\nwith a new thing that was given a new name whose name is fuzzy,\nso... ;-)\n"},{"id":"360089","messageId":"20181010215143.GB231512@aiede.svl.corp.google.com","threadId":"49519","inReplyTo":"xmqqin29lc0s.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] gc: introduce an --auto-exit-code option for undoing 3029970275","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-10-10T21:51:43Z","receivedAt":"2018-10-10T21:51:48Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> Perhaps this reporting could also print the message from a previous\n>> run, so you could write:\n>>\n>> \tgit gc --detached-status || exit\n>> \tgit gc --auto; # perhaps also passing --detach\n>>\n>> (Names still open for bikeshedding.)\n>\n> When the command is given --detached-exit-code/status option, what\n> does it do?  Does it perform the \"did an earlier run left gc.log?\"\n> and report the result and nothing else?  In other words, is it a\n> pure replacement for \"test -e .git/gc.log\"?\n\nMy intent was the latter.  In other words, in the idiom\n\n\tdo_something_async &\n\t... a lot of time passes ...\n\twait\n\nit is something like the replacement for \"wait\".\n\nMore precisely,\n\n\tgit gc --detached-status || exit\n\nwould mean something like\n\n\tif test -e .git/gc.log\t# Error from previous gc --detach?\n\tthen\n\t\tcat >&2 .git/gc.log\t# Report the error.\n\t\texit 1\n\tfi\n\n>                                              Or does it do some of\n> the \"auto-gc\" prep logic like guestimating loose object count and\n> have that also in its exit status (e.g. \"from the gc.log left\n> behind, we know that we failed to reduce loose object count down\n> sufficiently after finding there are more than 6700 earlier, but now\n> we do not have that many loose object, so there is nothing to\n> complain about the presence of gc.log\")?\n\nDepending on the use case, a user might want to avoid losing\ninformation about the results of a previous \"git gc --detach\" run,\neven if they no longer apply.  For example, a user might want to\ncollect the error message for monitoring or later log analysis, to\ntrack down intermittent gc errors that go away on their own.\n\nA separate possible use case might be a\n\n\tgit gc --needs-auto-gc\n\ncommand that detects whether an auto gc is needed.  With that, a\ncaller that only wants to learn about errors if auto gc is needed\ncould run\n\n\tif git gc --needs-auto-gc\n\tthen\n\t\tgit gc --detached-status || exit\n\tfi\n\n> I am bad at naming myself, but worse at guessing what others meant\n> with a new thing that was given a new name whose name is fuzzy,\n> so... ;-)\n\nNo problem.  I'm mostly trying to tease out more details about the use\ncase.\n\nThanks,\nJonathan\n"},{"id":"360094","messageId":"87o9c1e9br.fsf@evledraar.gmail.com","threadId":"49519","inReplyTo":"20181010215143.GB231512@aiede.svl.corp.google.com","subject":"Re: [PATCH] gc: introduce an --auto-exit-code option for undoing 3029970275","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-10-10T22:16:40Z","receivedAt":"2018-10-10T22:16:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 10 2018, Jonathan Nieder wrote:\n\n> Junio C Hamano wrote:\n>> Jonathan Nieder <jrnieder@gmail.com> writes:\n>\n>>> Perhaps this reporting could also print the message from a previous\n>>> run, so you could write:\n>>>\n>>> \tgit gc --detached-status || exit\n>>> \tgit gc --auto; # perhaps also passing --detach\n>>>\n>>> (Names still open for bikeshedding.)\n>>\n>> When the command is given --detached-exit-code/status option, what\n>> does it do?  Does it perform the \"did an earlier run left gc.log?\"\n>> and report the result and nothing else?  In other words, is it a\n>> pure replacement for \"test -e .git/gc.log\"?\n>\n> My intent was the latter.  In other words, in the idiom\n>\n> \tdo_something_async &\n> \t... a lot of time passes ...\n> \twait\n>\n> it is something like the replacement for \"wait\".\n>\n> More precisely,\n>\n> \tgit gc --detached-status || exit\n>\n> would mean something like\n>\n> \tif test -e .git/gc.log\t# Error from previous gc --detach?\n> \tthen\n> \t\tcat >&2 .git/gc.log\t# Report the error.\n> \t\texit 1\n> \tfi\n>\n>>                                              Or does it do some of\n>> the \"auto-gc\" prep logic like guestimating loose object count and\n>> have that also in its exit status (e.g. \"from the gc.log left\n>> behind, we know that we failed to reduce loose object count down\n>> sufficiently after finding there are more than 6700 earlier, but now\n>> we do not have that many loose object, so there is nothing to\n>> complain about the presence of gc.log\")?\n>\n> Depending on the use case, a user might want to avoid losing\n> information about the results of a previous \"git gc --detach\" run,\n> even if they no longer apply.  For example, a user might want to\n> collect the error message for monitoring or later log analysis, to\n> track down intermittent gc errors that go away on their own.\n>\n> A separate possible use case might be a\n>\n> \tgit gc --needs-auto-gc\n>\n> command that detects whether an auto gc is needed.  With that, a\n> caller that only wants to learn about errors if auto gc is needed\n> could run\n>\n> \tif git gc --needs-auto-gc\n> \tthen\n> \t\tgit gc --detached-status || exit\n> \tfi\n>\n>> I am bad at naming myself, but worse at guessing what others meant\n>> with a new thing that was given a new name whose name is fuzzy,\n>> so... ;-)\n>\n> No problem.  I'm mostly trying to tease out more details about the use\n> case.\n\nLikewise, so don't take the following as an assertion of fact, but more\nof a fact-finding mission:\n\nWe could add something like this --detached-status / --needs-auto-gc,\nbut I don't need it, and frankly I can't think of a reason for why\nanyone would want to use these.\n\nThe entire point of having gc --auto in the first place is that you\ndon't care when exactly GC happens, you're happy with whenever git\ndecides it's needed.\n\nSo why would anyone need a --needs-auto-gc? If your criteria for doing\nGC exactly matches that of gc --auto then ... you just run gc --auto, if\nit isn't (e.g. if you're using Microsoft's Windows repo) you're not\nusing gc --auto in the first place, and neither --needs-auto-gc nor\n--auto is useful to you.\n\nSo maybe I'm missing something here, but a --needs-auto-gc just seems\nlike a gratuitous exposure of an internal implementation detail whose\nonly actionable result is doing what we're doing with \"gc --auto\" now,\ni.e. just run gc.\n\nWhich is what I'm doing by running \"gc --auto\" across a set of servers\nand looking at the exit code. If it's been failing I get an error, if\nthere's no need to gc nothing happens, and if it hasn't been failing and\nit just so happens that it's time to GC then fine, now was as good a\ntime as any.\n\nSo if we assume that for the sake of argument there's no point in a\n--detached-status either. My only reason for ever caring about that\nstatus is when I run \"gc --auto\" and it says it can't fork() itself so\nit fails. Since I'm using \"gc --auto\" I have zero reason to even ask\nthat question unless I'm OK with kicking off a gc run as a side-effect,\nso why split up the two? It just introduces a race condition for no\nbenefit.\n"},{"id":"360097","messageId":"20181010222526.GC231512@aiede.svl.corp.google.com","threadId":"49519","inReplyTo":"87o9c1e9br.fsf@evledraar.gmail.com","subject":"Re: [PATCH] gc: introduce an --auto-exit-code option for undoing 3029970275","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-10-10T22:25:26Z","receivedAt":"2018-10-10T22:25:31Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> Which is what I'm doing by running \"gc --auto\" across a set of servers\n> and looking at the exit code. If it's been failing I get an error, if\n> there's no need to gc nothing happens, and if it hasn't been failing and\n> it just so happens that it's time to GC then fine, now was as good a\n> time as any.\n\nFor this, a simple \"git gc --detached-status\" would work.  It sounds\nlike the bonus gc --auto run was a side effect instead of being a\nrequirement.\n\n> So if we assume that for the sake of argument there's no point in a\n> --detached-status either. My only reason for ever caring about that\n> status is when I run \"gc --auto\" and it says it can't fork() itself so\n> it fails. Since I'm using \"gc --auto\" I have zero reason to even ask\n> that question unless I'm OK with kicking off a gc run as a side-effect,\n> so why split up the two? It just introduces a race condition for no\n> benefit.\n\nWhat I am trying to do is design an interface which is simple to\nexplain in the manual.  The existing \"git gc --auto\" interface since\ndetaching was introduced is super confusing to me, so I'm going\nthrough the thought exercise of \"If we were starting over, what would\nwe build instead?\"\n\nPart of the answer to that question might include a\n\n\t--report-from-the-last-time-you-detached\n\noption.  I'm still failing to come up of a case where the answer to\nthat question would include a\n\n\t--report-from-the-last-time-you-detached-and-if-it-went-okay\\\n\t-then-run-another-detached-gc\n\noption.\n\nIn other words, I think our disconnect is that you are describing\nthings in terms of \"I have been happy with the existing git gc --auto\ndetaching behavior, so how can we maintain something as close as\npossible to that\"?  And I am trying to describe things in terms of\n\"What is the simplest, most maintainable, and easiest to explain way\nto keep Ævar's servers working well\"?\n\nJonathan\n"},{"id":"360119","messageId":"20181011003832.GE13853@sigill.intra.peff.net","threadId":"49519","inReplyTo":"87tvltecvy.fsf@evledraar.gmail.com","subject":"Re: [PATCH] gc: introduce an --auto-exit-code option for undoing 3029970275","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-11T00:38:32Z","receivedAt":"2018-10-11T00:38:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 10, 2018 at 10:59:45PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > Callers who _are_ prepared to act on the exit code probably ought to\n> > just use --auto-exit-code in their invocation.\n> >\n> > That said, I'm not entirely opposed to the matching config. There's\n> > enough history here that somebody might want a sledgehammer setting to\n> > go back to the old behavior.\n> \n> If it's not a config option then as git is upgraded I'll need to change\n> my across-server invocation to be some variant of checking git version,\n> then etiher using the --auto-exit-code option or not (which'll error on\n> older gits). Easier to be able to just drop in a config setting before\n> the upgrade.\n\nYeah, that's the \"there's enough history here\" that I was referring to,\nbut I hadn't quite thought through a concrete example. That makes sense.\n\n(Though I also think the other part of the thread is reasonable, too,\nwhere we'd just have a command to abstract away \"cat .git/gc.log\" into\n\"git gc --show-detached-log\" or something).\n\n-Peff\n"}]}