On 06/20/2014 04:43 PM, Ronnie Sahlberg wrote:
> Change delete_ref to use a ref transaction for the deletion. At the same time
> since we no longer have any callers of repack_without_ref we can now delete
> this function.
>
> Change delete_ref to return 0 on success and 1 on failure instead of the
> previous 0 on success either 1 or -1 on failure.
>
> Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
> Signed-off-by: Ronnie Sahlberg <sahlberg@xxxxxxxxxx>
> ---
> refs.c | 34 +++++++++++++---------------------
> 1 file changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 3d070d5..92a06d4 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2544,11 +2544,6 @@ int repack_without_refs(const char **refnames, int n, struct strbuf *err)
> return ret;
> }
>
> -static int repack_without_ref(const char *refname)
> -{
> - return repack_without_refs(&refname, 1, NULL);
> -}
> -
> static int delete_ref_loose(struct ref_lock *lock, int flag)
> {
> if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) {
> @@ -2566,24 +2561,21 @@ static int delete_ref_loose(struct ref_lock *lock, int flag)
>
> int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
> {
> - struct ref_lock *lock;
> - int ret = 0, flag = 0;
> + struct ref_transaction *transaction;
> + struct strbuf err = STRBUF_INIT;
>
> - lock = lock_ref_sha1_basic(refname, sha1, delopt, &flag);
The old code checked that the old value of refname was sha1, regardless
of whether sha1 was null_sha1. Presumably callers never set sha1 to
null_sha1...
> - if (!lock)
> + transaction = ref_transaction_begin(&err);
> + if (!transaction ||
> + ref_transaction_delete(transaction, refname, sha1, delopt,
> + sha1 && !is_null_sha1(sha1), &err) ||
...But the new code explicitly skips the check if sha1 is null_sha1.
This shouldn't make a practical difference, because presumably callers
never set sha1 to null_sha1. But given that the new policy elsewhere
for "delete" updates is that it is an error for old_sha1 to equal
null_sha1, it seems to me that this extra check shouldn't be here. So I
think this should be changed to
ref_transaction_delete(transaction, refname, sha1, delopt,
!!sha1, &err) ||
> [...]
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html