git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Alexey Muranov <alexey.muranov@gmail.com>
Subject: [PATCH 1/3] retain reflogs for deleted refs
Date: Thu, 19 Jul 2012 17:33:11 -0400	[thread overview]
Message-ID: <20120719213311.GA20385@sigill.intra.peff.net> (raw)
In-Reply-To: <20120719213225.GA20311@sigill.intra.peff.net>

When a ref is deleted, we completely delete its reflog on
the spot, leaving very little help for the user to reverse
the action. One can sometimes reconstruct the missing
entries based on the HEAD reflog, but not always; the
deleted entries may not have ever been on HEAD (for example,
in the case of a refs/remotes branch that was pruned). That
leaves "git fsck --lost-found", which can be quite tedious.

Instead, let's keep the reflogs for deleted refs around
until their entries naturally expire according to the
regular reflog expiration rules.

This cannot be done by simply leaving the reflog files in
place. The ref namespace does not allow D/F conflicts, so a
ref "foo" would block the creation of another ref "foo/bar",
and vice versa. This limitation is acceptable for two refs
to exist simultaneously, but should not have an impact if
one of the refs is deleted.

This patch moves reflog entries into a special "graveyard"
namespace, and appends a tilde (~) character, which is
not allowed in a valid ref name. This means that the deleted
reflogs of these refs:

   refs/heads/a
   refs/heads/a/b
   refs/heads/a/b/c

will be stored in:

   logs/graveyard/refs/heads/a~
   logs/graveyard/refs/heads/a/b~
   logs/graveyard/refs/heads/a/b/c~

Putting them in the graveyard namespace ensures they will
not conflict with live refs, and the tilde prevents D/F
conflicts within the graveyard namespace.

The implementation is fairly straightforward, but it's worth
noting a few things:

  1. Updates to "logs/graveyard/refs/heads/foo~" happen
     under the ref-lock for "refs/heads/foo". So deletion
     still takes a single lock, and anyone touching the
     reflog directly needs to reverse the transformation to
     find the correct lockfile.

  2. We append entries to the graveyard reflog rather than
     simply renaming the file into place. This means that
     if you create and delete a branch repeatedly, the
     graveyard will contain the concatenation of all
     iterations.

  3. We do not resurrect dead entries when a new ref is
     created with the same name. However, it would be
     possible to build an "undelete" feature on top of this
     if one was so inclined.

  4. The for_each_reflog code has been loosened to allow
     reflogs that do not have a matching ref. In this case,
     the callback is passed the null_sha1, and callers must
     be prepared to handle this case (the only caller that
     cares is the reflog expiration code, which is updated
     here).

Only one test needed to be updated; t7701 tries to create
unreachable objects by deleting branches. Of course that no
longer works, which is the intent of this patch. The test
now works around it by removing the graveyard logs.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/reflog.c                     |  9 +++--
 refs.c                               | 69 +++++++++++++++++++++++++++++++++---
 refs.h                               |  3 ++
 t/t7701-repack-unpack-unreachable.sh |  5 ++-
 4 files changed, 79 insertions(+), 7 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index b3c9e27..e79a2ca 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -359,6 +359,7 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
 	struct commit *tip_commit;
 	struct commit_list *tips;
 	int status = 0;
+	int updateref = cmd->updateref && !is_null_sha1(sha1);
 
 	memset(&cb, 0, sizeof(cb));
 
@@ -367,6 +368,10 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
 	 * getting updated.
 	 */
 	lock = lock_any_ref_for_update(ref, sha1, 0);
+	if (!lock && is_null_sha1(sha1))
+		lock = lock_any_ref_for_update(
+				graveyard_reflog_to_refname(ref),
+				sha1, 0);
 	if (!lock)
 		return error("cannot lock ref '%s'", ref);
 	log_file = git_pathdup("logs/%s", ref);
@@ -426,7 +431,7 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
 			status |= error("%s: %s", strerror(errno),
 					newlog_path);
 			unlink(newlog_path);
-		} else if (cmd->updateref &&
+		} else if (updateref &&
 			(write_in_full(lock->lock_fd,
 				sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
 			 write_str_in_full(lock->lock_fd, "\n") != 1 ||
@@ -438,7 +443,7 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
 			status |= error("cannot rename %s to %s",
 					newlog_path, log_file);
 			unlink(newlog_path);
-		} else if (cmd->updateref && commit_ref(lock)) {
+		} else if (updateref && commit_ref(lock)) {
 			status |= error("Couldn't set %s", lock->ref_name);
 		} else {
 			adjust_shared_perm(log_file);
diff --git a/refs.c b/refs.c
index da74a2b..553de77 100644
--- a/refs.c
+++ b/refs.c
@@ -4,6 +4,8 @@
 #include "tag.h"
 #include "dir.h"
 
+static void mark_reflog_deleted(struct ref_lock *lock);
+
 /*
  * Make sure "ref" is something reasonable to have under ".git/refs/";
  * We do not like it if:
@@ -1780,7 +1782,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 	 */
 	ret |= repack_without_ref(refname);
 
-	unlink_or_warn(git_path("logs/%s", lock->ref_name));
+	mark_reflog_deleted(lock);
 	invalidate_ref_cache(NULL);
 	unlock_ref(lock);
 	return ret;
@@ -2385,9 +2387,8 @@ static int do_for_each_reflog(struct strbuf *name, each_ref_fn fn, void *cb_data
 			} else {
 				unsigned char sha1[20];
 				if (read_ref_full(name->buf, sha1, 0, NULL))
-					retval = error("bad ref for %s", name->buf);
-				else
-					retval = fn(name->buf, sha1, 0, cb_data);
+					hashcpy(sha1, null_sha1);
+				retval = fn(name->buf, sha1, 0, cb_data);
 			}
 			if (retval)
 				break;
@@ -2552,3 +2553,63 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
 	free(short_name);
 	return xstrdup(refname);
 }
+
+char *refname_to_graveyard_reflog(const char *ref)
+{
+	return git_path("logs/graveyard/%s~", ref);
+}
+
+char *graveyard_reflog_to_refname(const char *log)
+{
+	static struct strbuf buf = STRBUF_INIT;
+
+	if (!prefixcmp(log, "graveyard/"))
+		log += 10;
+
+	strbuf_reset(&buf);
+	strbuf_addstr(&buf, log);
+	if (buf.len > 0 && buf.buf[buf.len-1] == '~')
+		strbuf_setlen(&buf, buf.len - 1);
+
+	return buf.buf;
+}
+
+static int copy_reflog_entries(const char *dst, const char *src)
+{
+	int fdi, fdo, status;
+
+	fdi = open(src, O_RDONLY);
+	if (fdi < 0)
+		return errno == ENOENT ? 0 : -1;
+
+	fdo = open(dst, O_WRONLY | O_APPEND | O_CREAT, 0666);
+	if (fdo < 0) {
+		close(fdi);
+		return -1;
+	}
+
+	status = copy_fd(fdi, fdo);
+	if (close(fdo) < 0)
+		return -1;
+	if (status < 0 || adjust_shared_perm(dst) < 0)
+		return -1;
+	return 0;
+}
+
+static void mark_reflog_deleted(struct ref_lock *lock)
+{
+	static const char msg[] = "ref deleted";
+	const char *log = git_path("logs/%s", lock->ref_name);
+	char *grave = refname_to_graveyard_reflog(lock->ref_name);
+
+	if (log_ref_write(lock->ref_name, lock->old_sha1, null_sha1, msg) < 0)
+		warning("unable to update reflog for %s: %s",
+			lock->ref_name, strerror(errno));
+
+	if (safe_create_leading_directories(grave) < 0 ||
+	    copy_reflog_entries(grave, log) < 0)
+		warning("unable to copy reflog entries to graveyard: %s",
+			strerror(errno));
+
+	unlink_or_warn(log);
+}
diff --git a/refs.h b/refs.h
index d6c2fe2..9d14558 100644
--- a/refs.h
+++ b/refs.h
@@ -111,6 +111,9 @@ int for_each_recent_reflog_ent(const char *refname, each_reflog_ent_fn fn, long,
  */
 extern int for_each_reflog(each_ref_fn, void *);
 
+char *refname_to_graveyard_reflog(const char *ref);
+char *graveyard_reflog_to_refname(const char *log);
+
 #define REFNAME_ALLOW_ONELEVEL 1
 #define REFNAME_REFSPEC_PATTERN 2
 #define REFNAME_DOT_COMPONENT 4
diff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh
index b8d4cde..c06b715 100755
--- a/t/t7701-repack-unpack-unreachable.sh
+++ b/t/t7701-repack-unpack-unreachable.sh
@@ -38,7 +38,9 @@ test_expect_success '-A with -d option leaves unreachable objects unpacked' '
 	git show $csha1 &&
 	git show $tsha1 &&
 	# now expire the reflog, while keeping reachable ones but expiring
-	# unreachables immediately
+	# unreachables immediately; also remove any graveyard reflogs
+	# from deleted branches that would keep things reachable
+	rm -rf .git/logs/graveyard &&
 	test_tick &&
 	sometimeago=$(( $test_tick - 10000 )) &&
 	git reflog expire --expire=$sometimeago --expire-unreachable=$test_tick --all &&
@@ -76,6 +78,7 @@ test_expect_success '-A without -d option leaves unreachable objects packed' '
 	test 1 = $(ls -1 .git/objects/pack/pack-*.pack | wc -l) &&
 	packfile=$(ls .git/objects/pack/pack-*.pack) &&
 	git branch -D transient_branch &&
+	rm -rf .git/logs/graveyard &&
 	test_tick &&
 	git repack -A -l &&
 	test ! -f "$fsha1path" &&
-- 
1.7.10.5.40.g059818d

  reply	other threads:[~2012-07-19 21:33 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-19  7:30 Feature request: fetch --prune by default Alexey Muranov
2012-07-19 11:55 ` Jeff King
2012-07-19 14:03   ` Dan Johnson
2012-07-19 15:11     ` Stefan Haller
2012-08-16 23:22     ` Junio C Hamano
2012-08-21  6:51       ` Jeff King
2013-06-20 19:22     ` Sam Roberts
2012-07-19 16:21   ` Alexey Muranov
2012-07-19 17:34     ` Konstantin Khomoutov
2012-07-19 21:20       ` Alexey Muranov
2012-07-19 21:57         ` Alexey Muranov
2012-07-20  7:11         ` Johannes Sixt
2012-07-20  7:28           ` Alexey Muranov
2012-08-16 23:27             ` Junio C Hamano
2012-07-19 16:40   ` Alexey Muranov
2012-07-19 16:48     ` Dan Johnson
2012-07-19 16:51       ` Alexey Muranov
2012-07-19 21:32   ` [RFC/PATCH 0/3] reflog graveyard Jeff King
2012-07-19 21:33     ` Jeff King [this message]
2012-07-19 22:23       ` [PATCH 1/3] retain reflogs for deleted refs Alexey Muranov
2012-07-20 14:26         ` Jeff King
2012-07-20 14:32           ` Alexey Muranov
2012-07-19 22:36       ` Junio C Hamano
2012-07-20 14:43         ` Jeff King
2012-07-20 15:07           ` Jeff King
2012-07-20 15:39             ` Junio C Hamano
2012-07-20 15:42           ` Junio C Hamano
2012-07-20 15:50             ` Jeff King
2012-08-16 23:29         ` Junio C Hamano
2012-07-20  9:49       ` Michael Haggerty
2012-07-20 15:44         ` Jeff King
2012-07-20 16:37           ` Johannes Sixt
2012-07-20 17:09             ` Jeff King
2012-07-22 11:03               ` Alexey Muranov
2012-07-26 12:47               ` Nguyen Thai Ngoc Duy
2012-07-26 16:26                 ` Alexey Muranov
2012-07-26 16:41                   ` Matthieu Moy
2012-07-26 16:59                     ` Jeff King
2012-07-26 17:24                       ` Alexey Muranov
2012-07-26 17:46               ` Junio C Hamano
2012-07-26 17:52                 ` Jeff King
2012-07-22 11:10           ` Alexey Muranov
2012-07-22 11:12             ` Alexey Muranov
2012-07-22 13:14             ` Jeff King
2012-07-22 14:40               ` Alexey Muranov
2012-07-22 15:50                 ` Jeff King
2012-07-20 16:32         ` Johannes Sixt
2012-08-18 17:14       ` [RFC 0/3] Reflogs for deleted refs: fix breakage and suggest namespace change mhagger
2012-08-18 17:14         ` [RFC 1/3] t9300: format test in modern style prior to modifying it mhagger
2012-08-18 17:14         ` [RFC 2/3] Delete reflogs for dead references to allow pruning mhagger
2012-08-21  8:33           ` Jeff King
2012-08-18 17:14         ` [RFC 3/3] Change naming convention for the reflog graveyard mhagger
2012-08-18 20:39         ` [RFC 0/3] Reflogs for deleted refs: fix breakage and suggest namespace change Junio C Hamano
2012-08-18 21:11           ` Alexey Muranov
2012-08-19  0:02             ` Junio C Hamano
2012-08-19  7:07               ` Alexey Muranov
2012-08-19  7:15               ` Alexey Muranov
2012-08-19 11:28               ` Alexey Muranov
2012-08-19 17:38                 ` Junio C Hamano
2012-08-19 22:09                   ` Alexey Muranov
2012-08-20  0:26                     ` Junio C Hamano
2012-08-20  1:01                       ` Junio C Hamano
2012-08-20 11:32                       ` Alexey Muranov
2012-08-20 11:57                         ` Alexey Muranov
2012-08-19 13:19           ` Michael Haggerty
2012-08-19 16:27             ` Junio C Hamano
2012-08-21  8:27           ` Jeff King
2012-08-21 17:56             ` Junio C Hamano
2012-07-19 21:33     ` [PATCH 2/3] teach sha1_name to look in graveyard reflogs Jeff King
2012-07-19 22:39       ` Junio C Hamano
2012-07-20 15:53         ` Jeff King
2012-07-22 20:53           ` Junio C Hamano
2012-07-19 21:33     ` [PATCH 3/3] add tests for reflogs of deleted refs Jeff King

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20120719213311.GA20385@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=alexey.muranov@gmail.com \
    --cc=git@vger.kernel.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).