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

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

From
Ronnie Sahlberg <sahlberg@google.com>
Date
May 27, 2014, 20:25 UTC
Message-ID
<1401222360-21175-7-git-send-email-sahlberg@google.com>
In-Reply-To
<1401222360-21175-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_strbuf 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..48548d6 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_strbuf(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..9e04b43 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_strbuf(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_strbuf(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_strbuf(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..2da62ac 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_strbuf(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.g0203784
Previous: Ronnie SahlbergNext: Jonathan Nieder
Message 8 of 89 in “Use ref transactions”
  1. 00/41 Use ref transactionsRonnie Sahlberg, May 27, 2014
  2. 01/41 refs.c: remove ref_transaction_rollbackRonnie Sahlberg, May 27, 2014
  3. 02/41 refs.c: ref_transaction_commit should not free the transactionRonnie Sahlberg, May 27, 2014
  4. Jonathan NiederMay 27, 2014
  5. 03/41 refs.c: constify the sha arguments for ref_transaction_create|delete|updateRonnie Sahlberg, May 27, 2014
  6. 04/41 refs.c: allow passing NULL to ref_transaction_freeRonnie Sahlberg, May 27, 2014
  7. 05/41 refs.c: add a strbuf argument to ref_transaction_commit for error loggingRonnie Sahlberg, May 27, 2014
  8. 06/41 refs.c: add an err argument to repack_without_refsRonnie Sahlberg, May 27, 2014
  9. Jonathan NiederMay 28, 2014
  10. Ronnie SahlbergMay 28, 2014
  11. Jonathan NiederMay 28, 2014
  12. 07/41 refs.c: make ref_update_reject_duplicates take a strbuf argument for errorsRonnie Sahlberg, May 27, 2014
  13. 08/41 refs.c: add an err argument to delete_ref_looseRonnie Sahlberg, May 27, 2014
  14. Jonathan NiederMay 28, 2014
  15. Ronnie SahlbergMay 28, 2014
  16. 09/41 refs.c: make update_ref_write update a strbuf on failureRonnie Sahlberg, May 27, 2014
  17. 10/41 update-ref.c: log transaction error from the update_refRonnie Sahlberg, May 27, 2014
  18. Jonathan NiederMay 28, 2014
  19. Ronnie SahlbergMay 28, 2014
  20. Jonathan NiederMay 28, 2014
  21. 11/41 refs.c: remove the onerr argument to ref_transaction_commitRonnie Sahlberg, May 27, 2014
  22. 12/41 refs.c: change ref_transaction_update() to do error checking and return statusRonnie Sahlberg, May 27, 2014
  23. 13/41 refs.c: change ref_transaction_create to do error checking and return statusRonnie Sahlberg, May 27, 2014
  24. Jonathan NiederMay 28, 2014
  25. Ronnie SahlbergMay 28, 2014
  26. Jonathan NiederMay 28, 2014
  27. Ronnie SahlbergMay 28, 2014
  28. Jonathan NiederMay 28, 2014
  29. Ronnie SahlbergMay 28, 2014
  30. Jonathan NiederMay 28, 2014
  31. 14/41 refs.c: update ref_transaction_delete to check for error and return statusRonnie Sahlberg, May 27, 2014
  32. Jonathan NiederMay 28, 2014
  33. 15/41 refs.c: make ref_transaction_begin take an err argumentRonnie Sahlberg, May 27, 2014
  34. 16/41 refs.c: add transaction.status and track OPEN/CLOSED/ERRORRonnie Sahlberg, May 27, 2014
  35. Jonathan NiederMay 28, 2014
  36. Ronnie SahlbergMay 28, 2014
  37. 17/41 tag.c: use ref transactions when doing updatesRonnie Sahlberg, May 27, 2014
  38. 18/41 replace.c: use the ref transaction functions for updatesRonnie Sahlberg, May 27, 2014
  39. 19/41 commit.c: use ref transactions for updatesRonnie Sahlberg, May 27, 2014
  40. 20/41 sequencer.c: use ref transactions for all ref updatesRonnie Sahlberg, May 27, 2014
  41. Jonathan NiederMay 28, 2014
  42. 21/41 fast-import.c: change update_branch to use ref transactionsRonnie Sahlberg, May 27, 2014
  43. Jonathan NiederMay 28, 2014
  44. 22/41 branch.c: use ref transaction for all ref updatesRonnie Sahlberg, May 27, 2014
  45. 23/41 refs.c: change update_ref to use a transactionRonnie Sahlberg, May 27, 2014
  46. Jonathan NiederMay 28, 2014
  47. Ronnie SahlbergMay 28, 2014
  48. 24/41 receive-pack.c: use a reference transaction for updating the refsRonnie Sahlberg, May 27, 2014
  49. 25/41 fast-import.c: use a ref transaction when dumping tagsRonnie Sahlberg, May 27, 2014
  50. Jonathan NiederMay 28, 2014
  51. Ronnie SahlbergMay 28, 2014
  52. Jonathan NiederMay 28, 2014
  53. Ronnie SahlbergMay 28, 2014
  54. Jonathan NiederMay 28, 2014
  55. Ronnie SahlbergMay 29, 2014
  56. Jonathan NiederMay 29, 2014
  57. Ronnie SahlbergJun 3, 2014
  58. 26/41 walker.c: use ref transaction for ref updatesRonnie Sahlberg, May 27, 2014
  59. Jonathan NiederMay 28, 2014
  60. Ronnie SahlbergMay 28, 2014
  61. 27/41 refs.c: make lock_ref_sha1 staticRonnie Sahlberg, May 27, 2014
  62. 28/41 refs.c: remove the update_ref_lock functionRonnie Sahlberg, May 27, 2014
  63. 29/41 refs.c: remove the update_ref_write functionRonnie Sahlberg, May 27, 2014
  64. Jonathan NiederMay 28, 2014
  65. 30/41 refs.c: remove lock_ref_sha1Ronnie Sahlberg, May 27, 2014
  66. 31/41 refs.c: make prune_ref use a transaction to delete the refRonnie Sahlberg, May 27, 2014
  67. Jonathan NiederMay 28, 2014
  68. Ronnie SahlbergMay 28, 2014
  69. 32/41 refs.c: make delete_ref use a transactionRonnie Sahlberg, May 27, 2014
  70. Jonathan NiederMay 30, 2014
  71. Ronnie SahlbergJun 3, 2014
  72. 33/41 refs.c: pass the ref log message to _create/delete/update instead of _commitRonnie Sahlberg, May 27, 2014
  73. Jonathan NiederMay 30, 2014
  74. Ronnie SahlbergJun 3, 2014
  75. 34/41 refs.c: pass NULL as *flags to read_ref_fullRonnie Sahlberg, May 27, 2014
  76. 35/41 refs.c: pack all refs before we start to rename a refRonnie Sahlberg, May 27, 2014
  77. Jonathan NiederMay 30, 2014
  78. Ronnie SahlbergJun 3, 2014
  79. 36/41 refs.c: move the check for valid refname to lock_ref_sha1_basicRonnie Sahlberg, May 27, 2014
  80. Jonathan NiederMay 30, 2014
  81. Ronnie SahlbergJun 3, 2014
  82. 37/41 refs.c: call lock_ref_sha1_basic directly from commitRonnie Sahlberg, May 27, 2014
  83. 38/41 refs.c: pass a skip list to name_conflict_fnRonnie Sahlberg, May 27, 2014
  84. 39/41 refs.c: propagate any errno==ENOTDIR from _commit back to the callersRonnie Sahlberg, May 27, 2014
  85. Jonathan NiederMay 31, 2014
  86. Ronnie SahlbergJun 3, 2014
  87. 40/41 fetch.c: change s_update_ref to use a ref transactionRonnie Sahlberg, May 27, 2014
  88. 41/41 refs.c: make write_ref_sha1 staticRonnie Sahlberg, May 27, 2014
  89. Jonathan NiederJun 3, 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.