From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 04 Sep 2007 17:52:44 GMT
Subject: Re: [PATCH] Functions for updating refs.
Message-ID: <7v642qnwr7.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <46DD6020.4050401@gmail.com>

Carlos Rica <jasampler@gmail.com> writes:

> diff --git a/refs.c b/refs.c
> index 09a2c87..4fd5065 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1455,3 +1455,35 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
>  {
>  	return do_for_each_reflog("", fn, cb_data);
>  }
> +
> +int update_ref_or_die(const char *action, const char *refname,
> +				const unsigned char *sha1,
> +				const unsigned char *oldval, int flags)
> +{
> +	static struct ref_lock *lock;
> +	lock = lock_any_ref_for_update(refname, oldval, flags);
> +	if (!lock)
> +		die("Cannot lock the ref '%s'.", refname);
> +	if (write_ref_sha1(lock, sha1, action) < 0)
> +		die("Cannot update the ref '%s'.", refname);
> +	return 0;
> +}
> +
> +int update_ref_or_error(const char *action, const char *refname,
> +				const unsigned char *sha1,
> +				const unsigned char *oldval, int quiet)
> +{
> +	static struct ref_lock *lock;
> +	lock = lock_any_ref_for_update(refname, oldval, 0);
> +	if (!lock) {
> +		if (!quiet)
> +			error("Cannot lock the ref '%s'.", refname);
> +		return 1;
> +	}
> +	if (write_ref_sha1(lock, sha1, action) < 0) {
> +		if (!quiet)
> +			error("Cannot update the ref '%s'.", refname);
> +		return 1;
> +	}
> +	return 0;
> +}

This makes me wonder three things:

 - Why doesn't "or_error" side allow "flags" as "or_die" one?
   Could the 'quiet' option become part of "flags" perhaps?

 - They look quite similar.  Is it a good idea to refactor them
   further, or they are so small it does not matter?

 - Why isn't lock released with unlock_ref()?