Re: [PATCH] gc: do not warn about too many loose objects

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 16, 2018, 19:54 UTC
Message-ID: <20180716195431.GD11513@aiede.svl.corp.google.com>
In-Reply-To: <20180716194136.GA25189@sigill.intra.peff.net>

Jeff King wrote:

> On Mon, Jul 16, 2018 at 12:09:49PM -0700, Jonathan Nieder wrote:
>> Jeff King wrote:

Show 8 quoted lines

>>> Er, the action is to run "git prune", like the warning says. :)
>>
>> I don't think we want to recommend that, especially when "git gc --auto"
>> does the right thing automatically.
>
> But that's the point. This warning is written literally after running
> "git gc --auto" _didn't_ do the right thing. Yes, it would be nicer if
> it could do the right thing. But it doesn't yet know how to.

I think we fundamentally disagree, and I would like your help getting past this impasse.

Even restricting attention to the warning, not the exit code, I think you are saying the current behavior is acceptable. I do not believe it is. I think you are saying Jonathan's patch makes the behavior worse, and I'm not seeing it. Can you describe an example user interaction where the current behavior would be better than the behavior after Jonathan's patch? That might help make this discussion more concrete.

[...]

> See the thread I linked earlier on putting unreachable objects into a
> pack, which I think is the real solution.

Have you looked over the discussion in "Loose objects and unreachable objects" in Documentation/technical/hash-function-transition.txt? Do you have thoughts on it (preferrably in a separate thread)?

[...]

Show 6 quoted lines

>> This sounds awful.  It sounds to me like you're saying "git gc --auto"
>> is saying "I just did the wrong thing, and here is how you can clean
>> up after me".  That's not how I want a program to behave.
>
> Sure, it would be nice if it did the right thing. Nobody has written
> that yet. Until they do, we have to deal with the fallout.

"git gc --auto" is already doing the right thing.

[...]

> I mean that if you do not write a persistent log, then "gc --auto" will
> do an unproductive gc every time it is invoked. I.e., it will see "oh,
> there are too many loose objects", and then waste a bunch of CPU every
> time you commit.

If so, then this would already be the behavior in non daemon mode. Are you saying this accidental effect of daemon mode is in fact intentional?

[...]

Show 21 quoted lines

>>>                                 But in practice, I think you have to
>>> resort to scraping anyway, if you are interested in treating warnings
>>> from sub-processes the same way.
>>
>> Can you say more about this for me?  I am not understanding what
>> you're saying necessitates scraping the output.  I would strongly
>> prefer to avoid scraping the output.
>
> A daemonized git-gc runs a bunch of sub-commands (e.g., "git prune")
> with their stderr redirected into the logfile. If you want to have
> warnings go somewhere else, then either:
>
>   - you need some way to tell those sub-commands to send the warnings
>     elsewhere (i.e., _not_ stderr)
>
>     or
>
>   - you have to post-process the output they send to separate warnings
>     from other errors. Right now that means scraping. If you are
>     proposing a system of machine-readable output, it would need to work
>     not just for git-gc, but for every sub-command it runs.

or

    - you can simply record and trust the exit code

A simple way to do that without changing formats is to truncate the file when exiting with status 0.

Thanks, Jonathan