Hi, Jeff King wrote: > On Mon, Jul 16, 2018 at 11:22:07AM -0700, Jonathan Nieder wrote: >> Jeff King wrote:
>>> So while I completely agree that it's not a great thing to encourage >>> users to blindly run "git prune", I think it _is_ actionable. >> >> To flesh this out a little more: what user action do you suggest? Could >> we carry out that action automatically? > > 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. Can you convince me I'm wrong? [...] >> I mentally make a distinction between this warning from "git gc >> --auto" and the warning from commands like "git gui". [...] > I don't think those warnings are the same. The warning from "git gui" > is: you may benefit from running gc. > > The warning that is deleted by this patch is: you _just_ ran gc, and hey > we even did it automatically for you, but we're still in a funky state > afterwards. You might need to clean up this state. 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. But there's a simpler explanation for what "git gc --auto" was trying to say, which Jonathan described. [...] >> Yes, this is a real problem, and it would affect any other warning >> (or even GIT_TRACE=1 output) produced by gc --auto as well. I think we >> should consider it a serious bug, separate from the symptom Jonathan is >> fixing. >> >> Unfortunately I don't have a great idea about how to fix it. Should >> we avoid writing warnings to gc.log in daemon mode? That would >> prevent the user from ever seeing the warnings, but because of the >> nature of a warning, that might be reasonable. > > If you do that without anything further, then it will break the > protection against repeatedly running auto-gc, as I described in the > previous email. Do you mean ratelimiting for the message, or do you actually mean repeatedly running auto-gc itself? If we suppress warnings, there would still be a gc.log while "git gc --auto" is running, just as though there had been no warnings at all. I believe this is close to the intended behavior; it's the same as what you'd get without daemon mode, except that you lose the warning. [...] >> Should we put warnings >> in a separate log file with different semantics? Should we change the >> format of gc.log to allow more structured information (include 'gc' >> exit code) and perform a two-stage migration to the new format (first >> learn to read the new format, then switch to writing it)? > > Any of those would work similarly to the "just detect warnings" I > suggested earlier, with respect to keeping the "1 day" expiration > intact, so I'd be OK with them. In theory they'd be more robust than > scraping the "warning:" prefix. 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. Thanks, Jonathan
