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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to