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. Unfortunately, the value of errno after calling lock_ref_sha1_basic is not reliable. 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html