git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ronnie Sahlberg <sahlberg@google.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, Ronnie Sahlberg <sahlberg@google.com>
Subject: [PATCH 09/12] refs.c: propagate any errno==ENOTDIR from _commit back to the callers
Date: Wed, 16 Jul 2014 15:23:09 -0700	[thread overview]
Message-ID: <1405549392-27306-10-git-send-email-sahlberg@google.com> (raw)
In-Reply-To: <1405549392-27306-1-git-send-email-sahlberg@google.com>

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 <sahlberg@google.com>
---
 refs.c | 22 +++++++++++++++-------
 refs.h |  6 ++++++
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/refs.c b/refs.c
index a115478..69cbca5 100644
--- a/refs.c
+++ b/refs.c
@@ -3559,7 +3559,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;
@@ -3577,9 +3577,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++) {
@@ -3593,10 +3594,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;
 		}
 	}
@@ -3614,6 +3617,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 
 				if (err)
 					strbuf_addf(err, str, update->refname);
+				ret = -1;
 				goto cleanup;
 			}
 		}
@@ -3624,14 +3628,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);
@@ -3644,6 +3650,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 6a4d1a7..fc7942c 100644
--- a/refs.h
+++ b/refs.h
@@ -326,7 +326,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.1.527.gc6b782e

  parent reply	other threads:[~2014-07-16 22:32 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-16 22:23 [PATCH 00/12] Use ref transactions part 3 Ronnie Sahlberg
2014-07-16 22:23 ` [PATCH 01/12] wrapper.c: simplify warn_if_unremovable Ronnie Sahlberg
2014-07-18 22:21   ` Junio C Hamano
2014-07-16 22:23 ` [PATCH 02/12] wrapper.c: add a new function unlink_or_msg Ronnie Sahlberg
2014-07-18 22:25   ` Junio C Hamano
2014-07-18 22:59     ` Junio C Hamano
2014-07-22 17:42       ` Ronnie Sahlberg
2014-07-22 17:56         ` Junio C Hamano
2014-07-16 22:23 ` [PATCH 03/12] refs.c: add an err argument to delete_ref_loose Ronnie Sahlberg
2014-07-16 22:23 ` [PATCH 04/12] refs.c: pass the ref log message to _create/delete/update instead of _commit Ronnie Sahlberg
2014-07-16 22:23 ` [PATCH 05/12] refs.c: pass NULL as *flags to read_ref_full Ronnie Sahlberg
2014-07-18 22:31   ` Junio C Hamano
2014-07-22 18:19     ` Ronnie Sahlberg
2014-07-22 21:44       ` Ronnie Sahlberg
2014-07-16 22:23 ` [PATCH 06/12] refs.c: move the check for valid refname to lock_ref_sha1_basic Ronnie Sahlberg
2014-07-18 22:37   ` Junio C Hamano
2014-07-16 22:23 ` [PATCH 07/12] refs.c: call lock_ref_sha1_basic directly from commit Ronnie Sahlberg
2014-07-16 22:23 ` [PATCH 08/12] refs.c: pass a skip list to name_conflict_fn Ronnie Sahlberg
2014-07-16 22:23 ` Ronnie Sahlberg [this message]
2014-07-16 22:23 ` [PATCH 10/12] fetch.c: change s_update_ref to use a ref transaction Ronnie Sahlberg
2014-07-16 22:23 ` [PATCH 11/12] refs.c: make write_ref_sha1 static Ronnie Sahlberg
2014-07-16 22:23 ` [PATCH 12/12] refs.c: fix handling of badly named refs Ronnie Sahlberg
2014-07-22 20:41   ` Junio C Hamano
2014-07-22 21:30     ` Ronnie Sahlberg
2014-07-22 21:36       ` Ronnie Sahlberg
2014-07-22 21:46       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1405549392-27306-10-git-send-email-sahlberg@google.com \
    --to=sahlberg@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).