Re: [PATCH] refs.c: clean up write_ref_sha1 returns

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 27, 2015, 03:22 UTC
Message-ID: <xmqq8ugonc0h.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1422306638-23785-1-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

>     I can redo the atomic-push-fix series with this cleanup merged
>     into the appropriate patches or you could just queue it on top 
>     of said series.

Yeah, I do not think we are expecting to fast track these two series through 'next' to 'master' before 2.3 final, so I think it would be better to use this patch _only_ to see if the final shape of the code this patch represents makes sense, so that we can expedite the final submission in the next development cycle, at which time we will have a chance to refresh 'next', hence a chance to clean-up atomic-push series in place.

Thanks.

Show 23 quoted lines

>  refs.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 2594b23..c8fd4a4 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2817,9 +2817,6 @@ static int close_ref(struct ref_lock *lock)
>  
>  static int commit_ref(struct ref_lock *lock, const unsigned char *sha1)
>  {
> -	if (!lock->force_write && !hashcmp(lock->old_sha1, sha1))
> -		return 0;
> -
>  	if (commit_lock_file(lock->lk))
>  		return -1;
>  	return 0;
> @@ -2880,7 +2877,6 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
>  		error("unable to lock %s for update", newrefname);
>  		goto rollback;
>  	}
> -	lock->force_write = 1;
>  	hashcpy(lock->old_sha1, orig_sha1);

Is this hashcpy() still necessary?

>  	if (write_ref_sha1(lock, orig_sha1, logmsg)
>  	    || commit_ref(lock, orig_sha1)) {