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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Feb 19, 2009, 22:44 UTC
Message-ID: <alpine.DEB.1.00.0902192337040.6223@intel-tinevez-2-302>
In-Reply-To: <200902192321.17802.robin.rosenberg.lists@dewire.com>

Hi,

On Thu, 19 Feb 2009, Robin Rosenberg wrote:

> Here is a new version on my ammendments to Junios patch.

s/on/of/

Show 10 quoted lines

> 
> -- robin
> 
> From 67347a63ce5ba324a750eb2c1ed7b9b0260d966a Mon Sep 17 00:00:00 2001
> From: Robin Rosenberg <robin.rosenberg@dewire.com>
> Date: Thu, 19 Feb 2009 23:18:59 +0100
> Subject: [PATCH] Recover from some known repack failure situations
> 
> For the know problems that we can recover from we exit
> with code 2 instead of 1.

s/know/known/

> Also removed the old packs when repack succeeds

s/removed/remove/  s/$/./

> 
> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> ---

I'd suggest a complete rewording of the commit message, though:

	repack: exit with status 2 for non-fatal errors

	Under certain circumstances, e.g. on Windows, when a pack file 
	is still in use, deleting files can fail.

	This is a non-fatal error condition, as the repository is not 
	corrupted as a consequence; however, it is an error nevertheless, 
	as $GIT_DIR takes more space than it should.

	We indicate that special condition with an exit status 2.

Show 13 quoted lines

> diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
> index aaa8852..e5ecd66 100644
> --- a/Documentation/git-repack.txt
> +++ b/Documentation/git-repack.txt
> @@ -24,6 +24,12 @@ associated index file.
>  Packs are used to reduce the load on mirror systems, backup
>  engines, disk storage, etc.
>  
> +On Windows repacking may fail because packs that should be replaced
> +are locked by other programs. In this case the program will recover
> +from the situation with an exit code of 2. The user does not have
> +to take any action to recover. For repacking to succeed the user
> +mustc stop the offending program or wait for it to close the packs.

s/The user does not have to take any action to recover. //

(It is not true)

s/mustc/must/

Show 20 quoted lines

> +
>  OPTIONS
>  -------
>  
> diff --git a/git-repack.sh b/git-repack.sh
> index 0f13043..519c83a 100755
> --- a/git-repack.sh
> +++ b/git-repack.sh
> @@ -136,8 +136,15 @@ then
>  		do
>  			echo >&2 "WARNING:   old-$file -> $file"
>  		done
> +		exit 1
>  	fi
> -	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"

Two long lines.

Show 10 quoted lines

> +	echo >&2 "INFO: before repacking."
> +	exit 2
>  fi
>  
>  # Now the ones with the same name are out of the way...
> @@ -152,6 +159,15 @@ do
>  	exit
>  done
>  
> +# Remove the "old-" files

s/old-/old-pack-/

Show 7 quoted lines

> +for name in $names
> +do
> +	rm -f "$PACKDIR/old-pack-$name.idx"
> +	rm -f "$PACKDIR/old-pack-$name.pack"
> +done
> +
> +# End of pack replacement.

Is this comment really needed here?

Ciao, Dscho