Re: [PATCH 3/3] gc: do not return error for prior errors in daemonized mode

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

>> Perhaps we should exit with 2 (not 0) in that "previous error" case
>> by default, and then have a configuration knob to turn that 2 into 0
>> for those who cannot easily modify the calling script?  That way, we
>> by default will *not* break those who have been paying attention to
>> zero-ness of the exit status, we allow those who want to treat this
>> "prior error" case as if there were no error with just a knob, and
>> then those who are willing to update their script can tell two cases
>> by the exit status and act differently.
>
> I think we have been exiting non-zero with "previous errors" for some
> time with the daemonizing code. It was just spelled "-1" instead of "2".
> So just jumping right there does not mean any regression from the
> current state, I don't think (but it also does not fix existing scripts
> like "repo" that check the code).

Correct.  That is why I suggested that default setting.  

However.

If we change it so that by default we exit with 0 but allow those
who care to see 2 with a knob, that would be a regression to those
who actually were acting on the non-zero exit, but they no longer
have to scrape the log/warning output if they choose to set the
knob, so the "convenience" factor might argue in favor of the "by
default we return 0 but you can choose to get the 'sensible exit
codes'" approach.  I do not have a strong opinion either way wrt
which one should be the default.

> I agree the config you suggest would give people the tools to make that
> case work. But it somehow rubs me the wrong way. Can you imagine the
> perspective of a user who is told "oh, your script breaks? Just try
> setting this option to ignore error codes in this one particular
> situation". It feels like a weird hack, because it is.

Well, the message is "your script is broken (and has always been
when 'gc --auto' saw that the previous round failed), but now we
have two ways to help you correct.  An easier but broken way is to
turn this knob to turn 2 to 0 to hide the issue under the rug; or
you can notice the difference between $?==1 and $?==2 and act
differently".  "turn 2 to 0" is a weird hack, but they are also
given the option of doing the right thing, so...

> It's also still inconsistent in the daemonize case. The run that yields
> the error won't return a non-zero exit. But the next run will exit with
> "2".

I do not see this particular "inconsistency" a problem.  The run
that first discovers the problem by definition cannot return with
non-zero; not waiting until finding out the outcome is the whole
point of running in the background and giving control back early.