Re: [PATCH 2/7] Refactor of RefUpdate force to call common updateImpl instead of duplication

From: Shawn O. Pearce <spearce@spearce.org>
Date: Aug 14, 2008, 22:23 UTC
Message-ID: <20080814222345.GU3782@spearce.org>
In-Reply-To: <1218708829-8175-3-git-send-email-charleso@charleso.org>

Charles O'Farrell <charleso@charleso.org> wrote:

> @@ -266,31 +266,8 @@ public class RefUpdate {
>  	 *             an unexpected IO error occurred while writing changes.
>  	 */
>  	public Result forceUpdate() throws IOException {

[clipped uinteresting deletions to show only the postimage]

Show 25 quoted lines

> +		force = true;
> +		return update();
>  	}
>  
>  	/**
> @@ -347,6 +324,12 @@ public class RefUpdate {
>  			if (oldValue == null)
>  				return store(lock, Result.NEW);
>  
> +			if (isForceUpdate()) {
> +				if (oldValue.equals(newValue))
> +					return Result.NO_CHANGE;
> +				return store(lock, Result.FORCED);
> +			}
> +
>  			newObj = walk.parseAny(newValue);
>  			oldObj = walk.parseAny(oldValue);
>  			if (newObj == oldObj)
> @@ -355,13 +338,8 @@ public class RefUpdate {
>  			if (newObj instanceof RevCommit && oldObj instanceof RevCommit) {
>  				if (walk.isMergedInto((RevCommit) oldObj, (RevCommit) newObj))
>  					return store(lock, Result.FAST_FORWARD);
> -				if (isForceUpdate())
> -					return store(lock, Result.FORCED);
> -				return Result.REJECTED;

The problem with this change is that calls to just update() which have isForceUpdate() true but actually turn out to be FAST_FORWARD types of updates now report FORCED as the result. We don't want to always report FORCED if the application is using just update().

Its nice to know that the remote side didn't rewind since we last saw them (for example).

-- 
Shawn.