Re: [PATCH] Make repack less likely to corrupt repository
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 10, 2009, 23:56 UTC
- Message-ID
- <7vd4dpkfr2.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <200902110051.20975.robin.rosenberg.lists@dewire.com>
Robin Rosenberg <robin.rosenberg.lists@dewire.com> writes:
Show 55 quoted lines
> tisdag 10 februari 2009 21:16:31 skrev Junio C Hamano:
>> Robin Rosenberg <robin.rosenberg@dewire.com> writes:
>>
>> >> I was not talking about any loss. The result would be a funny mixture of
>> >> permutations of {old-,}pack-*.{pack,idx} the user needs to match up after
>> >
>> > We don't leave old-files around unless we go very very wrong and only in
>> > that case would be leave "old"-files for one pack around and only if gc wants
>> > to replace a pack with the same name. That would not be fatal and the
>> > user can continue repacking to get rid of the redundant stuff once the cause
>> > of them problem is fixed.
>>
>> You can succeed for the first name and then fail for the second name, for
>> example, and can end up with old-pack-* and pack-* with the same name. I
>> found that potentially confusing. Since you are trying to improve the
>> area, it would be nicer to make it less prone to fail and easier to
>> recover.
>>
>> Here is another attempt to rewrite it, which is closer to what you are
>> doing in your patch, but hopefully easier to understand what is going on
>> and more atomic.
>
> Almost perfect.
>
>> +# If renaming failed for any of them, roll the ones we have
>> +# already renamed back to their original names.
>> +if test -n "$failed"
>> +then
>> + rollback_failure=
>> + for file in $rollback
>> + do
>> + mv "$PACKDIR/old-$file" "$PACKDIR/$file" ||
>> + rollback_failure="$rollback_failure $file"
>> + done
>> + if test -n "$rollback_failure"
>> + then
>> + echo >&2 "WARNING: Some packs in use have been renamed by"
>> + echo >&2 "WARNING: prefixing old- to their name, in order to"
>> + echo >&2 "WARNING: replace them with the new version of the"
>> + echo >&2 "WARNING: file. But the operation failed, and"
>> + echo >&2 "WARNING: attempt to rename them back to their"
>> + echo >&2 "WARNING: original names also failed."
>> + echo >&2 "WARNING: Please rename them in $PACKDIR manually:"
>> + for file in $rollback_failure
>> + do
>> + echo >&2 "WARNING: old-$file -> $file"
>> + done
>
> Exit 1 here. We did not succeed in rolling back
>
>> + fi
>> + exit 1
>
> But here we should exit 0 because we succeeded in rolling back the changes,
> so we do not need to scare the user.We failed to honor what the end user wanted: to repack. Why should we exit 0 here?