git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v12 06/41] refs.c: add an err argument to repack_without_refs

From
Ronnie Sahlberg <sahlberg@google.com>
Date
May 29, 2014, 16:07 UTC
Message-ID
<1401379676-9307-2-git-send-email-sahlberg@google.com>
In-Reply-To
<1401379676-9307-1-git-send-email-sahlberg@google.com>

Update repack_without_refs to take an err argument and update it if there is a failure. Pass the err variable from ref_transaction_commit to this function so that callers can print a meaningful error message if _commit fails due to a problem in repack_without_refs.

Add a new function unable_to_lock_message that takes a strbuf argument and fills in the reason for the failure.

In commit_packed_refs, make sure that we propagate any errno that commit_lock_file might have set back to our caller.

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
---
 cache.h    |  2 ++
 lockfile.c | 21 ++++++++++++---------
 refs.c     | 25 +++++++++++++++++++------
 3 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/cache.h b/cache.h
index 8c6cdc2..5858da8 100644
--- a/cache.h
+++ b/cache.h
@@ -559,6 +559,8 @@ struct lock_file {
 #define LOCK_DIE_ON_ERROR 1
 #define LOCK_NODEREF 2
 extern int unable_to_lock_error(const char *path, int err);
+extern void unable_to_lock_message(const char *path, int err,
+				   struct strbuf *buf);
 extern NORETURN void unable_to_lock_index_die(const char *path, int err);
 extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
 extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);
diff --git a/lockfile.c b/lockfile.c
index 8fbcb6a..92e0397 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -157,33 +157,36 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
 	return lk->fd;
 }
 
-static char *unable_to_lock_message(const char *path, int err)
+void unable_to_lock_message(const char *path, int err, struct strbuf *buf)
 {
-	struct strbuf buf = STRBUF_INIT;
 
 	if (err == EEXIST) {
-		strbuf_addf(&buf, "Unable to create '%s.lock': %s.\n\n"
+		strbuf_addf(buf, "Unable to create '%s.lock': %s.\n\n"
 		    "If no other git process is currently running, this probably means a\n"
 		    "git process crashed in this repository earlier. Make sure no other git\n"
 		    "process is running and remove the file manually to continue.",
 			    absolute_path(path), strerror(err));
 	} else
-		strbuf_addf(&buf, "Unable to create '%s.lock': %s",
+		strbuf_addf(buf, "Unable to create '%s.lock': %s",
 			    absolute_path(path), strerror(err));
-	return strbuf_detach(&buf, NULL);
 }
 
 int unable_to_lock_error(const char *path, int err)
 {
-	char *msg = unable_to_lock_message(path, err);
-	error("%s", msg);
-	free(msg);
+	struct strbuf buf = STRBUF_INIT;
+
+	unable_to_lock_message(path, err, &buf);
+	error("%s", buf.buf);
+	strbuf_release(&buf);
 	return -1;
 }
 
 NORETURN void unable_to_lock_index_die(const char *path, int err)
 {
-	die("%s", unable_to_lock_message(path, err));
+	struct strbuf buf = STRBUF_INIT;
+
+	unable_to_lock_message(path, err, &buf);
+	die("%s", buf.buf);
 }
 
 int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)
diff --git a/refs.c b/refs.c
index 25c3a93..6e28841 100644
--- a/refs.c
+++ b/refs.c
@@ -2208,6 +2208,7 @@ int commit_packed_refs(void)
 	struct packed_ref_cache *packed_ref_cache =
 		get_packed_ref_cache(&ref_cache);
 	int error = 0;
+	int save_errno = 0;
 
 	if (!packed_ref_cache->lock)
 		die("internal error: packed-refs not locked");
@@ -2217,10 +2218,13 @@ int commit_packed_refs(void)
 	do_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),
 				 0, write_packed_entry_fn,
 				 &packed_ref_cache->lock->fd);
-	if (commit_lock_file(packed_ref_cache->lock))
+	if (commit_lock_file(packed_ref_cache->lock)) {
+		save_errno = errno;
 		error = -1;
+	}
 	packed_ref_cache->lock = NULL;
 	release_packed_ref_cache(packed_ref_cache);
+	errno = save_errno;
 	return error;
 }
 
@@ -2427,12 +2431,12 @@ static int curate_packed_ref_fn(struct ref_entry *entry, void *cb_data)
 	return 0;
 }
 
-static int repack_without_refs(const char **refnames, int n)
+static int repack_without_refs(const char **refnames, int n, struct strbuf *err)
 {
 	struct ref_dir *packed;
 	struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
 	struct string_list_item *ref_to_delete;
-	int i, removed = 0;
+	int i, ret, removed = 0;
 
 	/* Look for a packed ref */
 	for (i = 0; i < n; i++)
@@ -2444,6 +2448,11 @@ static int repack_without_refs(const char **refnames, int n)
 		return 0; /* no refname exists in packed refs */
 
 	if (lock_packed_refs(0)) {
+		if (err) {
+			unable_to_lock_message(git_path("packed-refs"), errno,
+					       err);
+			return -1;
+		}
 		unable_to_lock_error(git_path("packed-refs"), errno);
 		return error("cannot delete '%s' from packed refs", refnames[i]);
 	}
@@ -2470,12 +2479,16 @@ static int repack_without_refs(const char **refnames, int n)
 	}
 
 	/* Write what remains */
-	return commit_packed_refs();
+	ret = commit_packed_refs();
+	if (ret && err)
+		strbuf_addf(err, "unable to overwrite old ref-pack file: %s",
+			    strerror(errno));
+	return ret;
 }
 
 static int repack_without_ref(const char *refname)
 {
-	return repack_without_refs(&refname, 1);
+	return repack_without_refs(&refname, 1, NULL);
 }
 
 static int delete_ref_loose(struct ref_lock *lock, int flag)
@@ -3481,7 +3494,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 		}
 	}
 
-	ret |= repack_without_refs(delnames, delnum);
+	ret |= repack_without_refs(delnames, delnum, err);
 	for (i = 0; i < delnum; i++)
 		unlink_or_warn(git_path("logs/%s", delnames[i]));
 	clear_loose_ref_cache(&ref_cache);

-- 
2.0.0.rc3.474.g3833130
Previous: Ronnie SahlbergNext: Jonathan Nieder
Message 2 of 18 in “Use ref transactions for all ref updates”
  1. 00/44 Use ref transactions for all ref updatesRonnie Sahlberg, May 29, 2014
  2. 06/41 refs.c: add an err argument to repack_without_refsRonnie Sahlberg, May 29, 2014
  3. Jonathan NiederMay 29, 2014
  4. Ronnie SahlbergJun 3, 2014
  5. 08/41 refs.c: add an err argument to delete_ref_looseRonnie Sahlberg, May 29, 2014
  6. 16/41 refs.c: add transaction.status and track OPEN/CLOSED/ERRORRonnie Sahlberg, May 29, 2014
  7. 24/41 receive-pack.c: use a reference transaction for updating the refsRonnie Sahlberg, May 29, 2014
  8. 25/41 fast-import.c: use a ref transaction when dumping tagsRonnie Sahlberg, May 29, 2014
  9. 26/41 walker.c: use ref transaction for ref updatesRonnie Sahlberg, May 29, 2014
  10. 31/41 refs.c: make prune_ref use a transaction to delete the refRonnie Sahlberg, May 29, 2014
  11. 32/41 refs.c: make delete_ref use a transactionRonnie Sahlberg, May 29, 2014
  12. 33/41 refs.c: pass the ref log message to _create/delete/update instead of _commitRonnie Sahlberg, May 29, 2014
  13. 36/41 refs.c: move the check for valid refname to lock_ref_sha1_basicRonnie Sahlberg, May 29, 2014
  14. 38/41 refs.c: pass a skip list to name_conflict_fnRonnie Sahlberg, May 29, 2014
  15. 39/41 refs.c: propagate any errno==ENOTDIR from _commit back to the callersRonnie Sahlberg, May 29, 2014
  16. 40/41 fetch.c: change s_update_ref to use a ref transactionRonnie Sahlberg, May 29, 2014
  17. 41/41 refs.c: make write_ref_sha1 staticRonnie Sahlberg, May 29, 2014
  18. Ronnie SahlbergMay 29, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.