From: Ronnie Sahlberg <sahlb...@google.com>
Date: Fri, 16 May 2014 14:14:38 -0700

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 <sahlb...@google.com>
Signed-off-by: Jonathan Nieder <jrnie...@gmail.com>
---
Since v21:
- clarified commit message and updated to match code
- small code cleanups
- clarified API doc, introduced TRANSACTION_GENERIC_ERROR so both error
  codes have names

 refs.c | 26 ++++++++++++++++----------
 refs.h |  9 +++++++--
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/refs.c b/refs.c
index 6820c93..623a1ae 100644
--- a/refs.c
+++ b/refs.c
@@ -3583,9 +3583,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++) {
@@ -3599,10 +3600,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;
                }
        }
@@ -3612,15 +3615,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 */
                }
        }
 
@@ -3629,14 +3633,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 aded545..fd63b47 100644
--- a/refs.h
+++ b/refs.h
@@ -323,9 +323,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);
 
-- 
2.1.0.rc2.206.gedb03e5

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