In _commit, ENOTDIR can happen in the call to lock_ref_sha1_basic, either when
we lstat the new refname and it returns ENOTDIR or if the name checking
function reports that the same type of conflict happened. In both cases it
means that we can not create the new ref due to a name conflict.

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.

Signed-off-by: Ronnie Sahlberg <sahlb...@google.com>
---
 refs.c | 22 +++++++++++++++-------
 refs.h |  6 ++++++
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/refs.c b/refs.c
index 0e29fb4..362ff1b 100644
--- a/refs.c
+++ b/refs.c
@@ -3580,7 +3580,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;
+       int ret = 0, delnum = 0, i, df_conflict = 0;
        const char **delnames;
        int n = transaction->nr;
        struct ref_update **updates = transaction->updates;
@@ -3598,9 +3598,10 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
 
        /* Copy, sort, and reject duplicate refs */
        qsort(updates, n, sizeof(*updates), ref_update_compare);
-       ret = ref_update_reject_duplicates(updates, n, err);
-       if (ret)
+       if (ref_update_reject_duplicates(updates, n, err)) {
+               ret = -1;
                goto cleanup;
+       }
 
        /* Acquire all locks while verifying old values */
        for (i = 0; i < n; i++) {
@@ -3614,10 +3615,12 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
                                                   &update->type,
                                                   delnames, delnum);
                if (!update->lock) {
+                       if (errno == ENOTDIR)
+                               df_conflict = 1;
                        if (err)
                                strbuf_addf(err, "Cannot lock the ref '%s'.",
                                            update->refname);
-                       ret = 1;
+                       ret = -1;
                        goto cleanup;
                }
        }
@@ -3635,6 +3638,7 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
 
                                if (err)
                                        strbuf_addf(err, str, update->refname);
+                               ret = -1;
                                goto cleanup;
                        }
                }
@@ -3645,14 +3649,16 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
                struct ref_update *update = updates[i];
 
                if (update->lock) {
-                       ret |= delete_ref_loose(update->lock, update->type,
-                                               err);
+                       if (delete_ref_loose(update->lock, update->type, err))
+                               ret = -1;
+
                        if (!(update->flags & REF_ISPRUNING))
                                delnames[delnum++] = update->lock->ref_name;
                }
        }
 
-       ret |= repack_without_refs(delnames, delnum, err);
+       if (repack_without_refs(delnames, delnum, err))
+               ret = -1;
        for (i = 0; i < delnum; i++)
                unlink_or_warn(git_path("logs/%s", delnames[i]));
        clear_loose_ref_cache(&ref_cache);
@@ -3665,6 +3671,8 @@ cleanup:
                if (updates[i]->lock)
                        unlock_ref(updates[i]->lock);
        free(delnames);
+       if (df_conflict)
+               ret = -2;
        return ret;
 }
 
diff --git a/refs.h b/refs.h
index f24b2c1..5c0543d 100644
--- a/refs.h
+++ b/refs.h
@@ -333,7 +333,13 @@ int ref_transaction_delete(struct ref_transaction 
*transaction,
  * Commit all of the changes that have been queued in transaction, as
  * atomically as possible.  Return a nonzero value if there is a
  * 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
+ * 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);
 
-- 
2.0.0.438.gec92e5c

--
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