On Fri, May 30, 2014 at 5:22 PM, Jonathan Nieder <[email protected]> wrote:
> Hi,
>
> Ronnie Sahlberg wrote:
>
>> For these cases, save the errno value and abort and make sure that the caller
>> can see errno==ENOTDIR.
>>
>> Also start defining specific return codes for _commit, assign -1 as a generic
>> error and -2 as the error that refers to a name conflict. Callers can (and
>> should) use that return value inspecting errno directly.
>
> Heh. Here's a patch on top to hopefully finish that part of the job.
Thanks. Updated.
>
> Unfortunately, the value of errno after calling lock_ref_sha1_basic is
> not reliable.
I have changed the patch for lock_ref_sha1_basic so it now does :
if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
errno = EINVAL;
return NULL;
}
so this function should no longer fail without changing errno to a,
hopefully, sane value.
> http://thread.gmane.org/gmane.comp.version-control.git/249159/focus=249407
> lists the error paths that are broken (marked with "[!]" in that
> message).
>
> diff --git i/builtin/fetch.c w/builtin/fetch.c
> index b13e8f9..0a01cda 100644
> --- i/builtin/fetch.c
> +++ w/builtin/fetch.c
> @@ -377,6 +377,7 @@ static int s_update_ref(const char *action,
> char *rla = getenv("GIT_REFLOG_ACTION");
> struct ref_transaction *transaction;
> struct strbuf err = STRBUF_INIT;
> + int ret, df_conflict = 0;
>
> if (dry_run)
> return 0;
> @@ -387,16 +388,22 @@ static int s_update_ref(const char *action,
> transaction = ref_transaction_begin(&err);
> if (!transaction ||
> ref_transaction_update(transaction, ref->name, ref->new_sha1,
> - ref->old_sha1, 0, check_old, msg, &err) ||
> - ref_transaction_commit(transaction, &err)) {
> - ref_transaction_free(transaction);
> - error("%s", err.buf);
> - strbuf_release(&err);
> - return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
> - STORE_REF_ERROR_OTHER;
> - }
> + ref->old_sha1, 0, check_old, msg, &err))
> + goto fail;
> +
> + ret = ref_transaction_commit(transaction, &err);
> + if (ret == UPDATE_REFS_NAME_CONFLICT)
> + df_conflict = 1;
> + if (ret)
> + goto fail;
> ref_transaction_free(transaction);
> return 0;
> +fail:
> + ref_transaction_free(transaction);
> + error("%s", err.buf);
> + strbuf_release(&err);
> + return df_conflict ? STORE_REF_ERROR_DF_CONFLICT
> + : STORE_REF_ERROR_OTHER;
> }
>
> #define REFCOL_WIDTH 10
> diff --git i/refs.c w/refs.c
> index dbaabba..b22b99b 100644
> --- i/refs.c
> +++ w/refs.c
> @@ -3499,7 +3499,7 @@ static int ref_update_reject_duplicates(struct
> ref_update **updates, int n,
> int ref_transaction_commit(struct ref_transaction *transaction,
> struct strbuf *err)
> {
> - int ret = 0, delnum = 0, i, save_errno = 0;
> + int ret = 0, delnum = 0, i, df_conflict = 0;
> const char **delnames;
> int n = transaction->nr;
> struct ref_update **updates = transaction->updates;
> @@ -3535,7 +3535,7 @@ int ref_transaction_commit(struct ref_transaction
> *transaction,
> delnames, delnum);
> if (!update->lock) {
> if (errno == ENOTDIR)
> - save_errno = errno;
> + df_conflict = 1;
> if (err)
> strbuf_addf(err, "Cannot lock the ref '%s'.",
> update->refname);
> @@ -3590,8 +3590,7 @@ cleanup:
> if (updates[i]->lock)
> unlock_ref(updates[i]->lock);
> free(delnames);
> - errno = save_errno;
> - if (save_errno == ENOTDIR)
> + if (df_conflict)
> ret = -2;
> return ret;
> }
> diff --git i/refs.h w/refs.h
> index 88732a1..1583097 100644
> --- i/refs.h
> +++ w/refs.h
> @@ -325,9 +325,11 @@ int ref_transaction_delete(struct ref_transaction
> *transaction,
> * problem.
> * If the transaction is already in failed state this function will return
> * an error.
> - * Function returns 0 on success, -1 for generic failures and -2 if the
> - * failure was due to a name collision (ENOTDIR).
> + * Function returns 0 on success, -1 for generic failures and
> + * UPDATE_REFS_NAME_CONFLICT (-2) if the failure was due to a name
> + * collision (ENOTDIR).
> */
> +#define UPDATE_REFS_NAME_CONFLICT -2
> int ref_transaction_commit(struct ref_transaction *transaction,
> struct strbuf *err);
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
