Re: [PATCH] Make repack less likely to corrupt repository

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

Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:

> söndag 15 februari 2009 17:46:59 skrev Johannes Schindelin:
>> Hi,
>> 
>> On Sun, 15 Feb 2009, Robin Rosenberg wrote:
>> > -	exit 1
>> > +	echo >&2 "INFO: We recovered from the repack error, but your repo"
>> > +	echo >&2 "INFO: is probably suboptimally packed. You may try to repack"
>> > +	echo >&2 "INFO: later. A common reason for repacking failure is that"
>> > +	echo >&2 "INFO: a Windows program was locking one of the old pack files."
>> > +	echo >&2 "INFO: To repack successfully you may have to close that program"
>> > +	echo >&2 "INFO: before repacking."
>> 
>> cat >&2 << EOF?
>
> Yeah, but I followed the pattern from the warnings just prior to these messages.
>
>> > +	exit
>> 
>> You lose the error condition here, but I cannot find a convincing argument 
>> about that in the commit message.
>
> I was thinking of my patch as an ammendment to Junios patch. In that context.
>
> "Exit success if rollback fails after failing to rename old packs." Doesn't this
> count?

No, it does not count, because that sentence is just a statement about
_what_ your change does, and does not explain _why_ it is better to report
success, other than an unstated "because I think that is more appropriate."

I think your "INFO:" lines are good to give additional useful cue for
the human operators, though.

If we wanted to move away old ones to replace them with new ones, and we
failed to comply with what the user wished for because the preparatory
step of moving-them-away failed, it _is_ a failure, and for this reason,
it should be reported as such.  That would be my counter-statement that
explains _why_.

If you were arguing for using a different but still non-zero exit status
to signal the "you asked us to repack but I refused to do so because I
couldn't move the in-use packs away; by the way I did not corrupt your
repository because I successfully rolled back everything I did, so do not
worry to much about it" case, I agree that such a change would be better
than what we have.  It would allow an automated process to tell a more
grave repository error and "I need to kill the git object server that is
pinning some of the packfiles open and re-run the repack" situation apart.