refs.c: ref_transaction_commit: distinguish name conflicts from other errors

In _commit, ENOTDIR can happen in the call to lock_ref_sha1_basic, either
when we lstat the new refname 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.

Start defining specific return codes for _commit.  TRANSACTION_NAME_CONFLICT
refers to a failure to create a ref due to a name conflict with another ref.
TRANSACTION_GENERIC_ERROR is for all other errors.

When "git fetch" is creating refs, name conflicts differ from other errors in
that they are likely to be resolved by running "git remote prune <remote>".
"git fetch" currently inspects errno to decide whether to give that advice.
Once it switches to the transaction API, it can check for
TRANSACTION_NAME_CONFLICT instead.

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs.c b/refs.c
index a007cf32573740637f16ddc8e5faf47c0df7dbe0..9d9bbebfb9b410614ac9e7565a89ae273534421d 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -3637,9 +3637,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 = TRANSACTION_GENERIC_ERROR;
                goto cleanup;
+       }
 
        /* Acquire all locks while verifying old values */
        for (i = 0; i < n; i++) {
@@ -3653,10 +3654,12 @@ int ref_transaction_commit(struct ref_transaction *transaction,
                                                   update->flags,
                                                   &update->type);
                if (!update->lock) {
+                       ret = (errno == ENOTDIR)
+                               ? TRANSACTION_NAME_CONFLICT
+                               : TRANSACTION_GENERIC_ERROR;
                        if (err)
                                strbuf_addf(err, "Cannot lock the ref '%s'.",
                                            update->refname);
-                       ret = 1;
                        goto cleanup;
                }
        }
@@ -3666,15 +3669,16 @@ int ref_transaction_commit(struct ref_transaction *transaction,
                struct ref_update *update = updates[i];
 
                if (!is_null_sha1(update->new_sha1)) {
-                       ret = write_ref_sha1(update->lock, update->new_sha1,
-                                            update->msg);
-                       update->lock = NULL; /* freed by write_ref_sha1 */
-                       if (ret) {
+                       if (write_ref_sha1(update->lock, update->new_sha1,
+                                          update->msg)) {
+                               update->lock = NULL; /* freed by write_ref_sha1 */
                                if (err)
                                        strbuf_addf(err, "Cannot update the ref '%s'.",
                                                    update->refname);
+                               ret = TRANSACTION_GENERIC_ERROR;
                                goto cleanup;
                        }
+                       update->lock = NULL; /* freed by write_ref_sha1 */
                }
        }
 
@@ -3683,14 +3687,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 = TRANSACTION_GENERIC_ERROR;
+
                        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 = TRANSACTION_GENERIC_ERROR;
        for (i = 0; i < delnum; i++)
                unlink_or_warn(git_path("logs/%s", delnames[i]));
        clear_loose_ref_cache(&ref_cache);

diff --git a/refs.h b/refs.h
index 1a55cabb4012a8557b21ce7bae604b3ec66ea8cf..50b115acc13a31e3d26886366cdf64aabfa17ceb 100644 (file)
--- a/refs.h
+++ b/refs.h
@@ -326,9 +326,14 @@ 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.
+ * atomically as possible.
+ *
+ * Returns 0 for success, or one of the below error codes for errors.
  */
+/* Naming conflict (for example, the ref names A and A/B conflict). */
+#define TRANSACTION_NAME_CONFLICT -1
+/* All other errors. */
+#define TRANSACTION_GENERIC_ERROR -2
 int ref_transaction_commit(struct ref_transaction *transaction,
                           struct strbuf *err);