git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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?

Previous: Robin RosenbergNext: Robin Rosenberg
Message 8 of 20 in “Make repack less likely to corrupt repository”
  1. Make repack less likely to corrupt repositoryRobin Rosenberg, Feb 9, 2009
  2. Junio C HamanoFeb 9, 2009
  3. Robin RosenbergFeb 10, 2009
  4. Junio C HamanoFeb 10, 2009
  5. Robin RosenbergFeb 10, 2009
  6. Junio C HamanoFeb 10, 2009
  7. Robin RosenbergFeb 10, 2009
  8. Junio C HamanoFeb 10, 2009
  9. Robin RosenbergFeb 11, 2009
  10. Junio C HamanoFeb 11, 2009
  11. Robin RosenbergFeb 11, 2009
  12. Robin RosenbergFeb 15, 2009
  13. Johannes SchindelinFeb 15, 2009
  14. Robin RosenbergFeb 15, 2009
  15. Junio C HamanoFeb 15, 2009
  16. Robin RosenbergFeb 16, 2009
  17. Robin RosenbergFeb 19, 2009
  18. Johannes SchindelinFeb 19, 2009
  19. Junio C HamanoFeb 20, 2009
  20. Junio C HamanoFeb 20, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.