git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Clemens Buchacher <drizzd@aon.at>
To: Jakub Narebski <jnareb@gmail.com>
Cc: Jonathan Nieder <jrnieder@gmail.com>,
	git@vger.kernel.org, Nicolas Sebrecht <nicolas.s.dev@gmx.fr>,
	Nanako Shiraishi <nanako3@lavabit.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: Question about 'branch -d' safety
Date: Sun, 11 Jul 2010 15:37:30 +0200	[thread overview]
Message-ID: <20100711133730.GA10338@localhost> (raw)
In-Reply-To: <201007110916.29567.jnareb@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 5518 bytes --]

On Sun, Jul 11, 2010 at 09:16:26AM +0200, Jakub Narebski wrote:
> 
> The problem is, that when you have 'foo/bar' branch, then you have
> 'foo/bar' reflog.  When you delete branch 'foo/bar', but do not delete
> 'foo/bar' reflog (only add to it branch deletion event), and then you
> want to create 'foo' branch, git wouldn't be able to create reflog
> fo 'foo' because of directory / file (D/F) conflict: there is 'foo/'
> directory preventing file 'foo' from being created.

Right, of course. So how about this?

Clemens

-->o--
Date: Sun, 11 Jul 2010 12:37:06 +0200
Subject: [PATCH/RFC] graveyard for reflogs

Instead of removing reflogs together with refs, keep them around
and reuse them as a starting point, if a ref of the same name is
created again.

When a ref is deleted, a ~ is appended to each directory of the
corresponding reflog entry, e.g., refs/heads/branch is renamed to
refs~/heads~/branch. Since ~ must not be used for ref names,
directory/file conflicts are thus avoided.

Known issues:

 - The reflog cannot be accessed while the ref does not exist.

 - Older git versions will not resurrect the reflog, and therefore
   leave the renamed reflog behind.

 - Breaks t7701, because git-expire tries to lock log entries,
   which fails because ~ is an illegal character for refs.

 - Breaks t9300.

Signed-off-by: Clemens Buchacher <drizzd@aon.at>
---
 reflog-walk.c   |    9 +++++--
 refs.c          |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 t/t1450-fsck.sh |    1 +
 3 files changed, 73 insertions(+), 4 deletions(-)

diff --git a/reflog-walk.c b/reflog-walk.c
index 4879615..9415ac8 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -228,15 +228,18 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
 		return;
 	}
 
-	reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
 	info->last_commit_reflog = commit_reflog;
 	commit_reflog->recno--;
-	commit_info->commit = (struct commit *)parse_object(reflog->osha1);
-	if (!commit_info->commit) {
+	if (commit_reflog->recno < 0) {
 		commit->parents = NULL;
 		return;
 	}
 
+	reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
+	commit_info->commit = (struct commit *)parse_object(reflog->nsha1);
+	if (!commit_info->commit)
+		die("Invalid reflog entry");
+
 	commit->parents = xcalloc(sizeof(struct commit_list), 1);
 	commit->parents->item = commit_info->commit;
 	commit->object.flags &= ~(ADDED | SEEN | SHOWN);
diff --git a/refs.c b/refs.c
index b540067..78c48ad 100644
--- a/refs.c
+++ b/refs.c
@@ -1052,6 +1052,70 @@ static int repack_without_ref(const char *refname)
 	return commit_lock_file(&packlock);
 }
 
+static void pronounce_dead(struct strbuf *dead_ref, const char *ref)
+{
+	const char *p = ref;
+	while (*p) {
+		if (*p == '/') {
+			int len = p - ref;
+			strbuf_add(dead_ref, ref, len);
+			if (len > 0)
+				strbuf_addstr(dead_ref, "~/");
+			ref = p + 1;
+		}
+		p++;
+	}
+	strbuf_addstr(dead_ref, ref);
+}
+
+static int bury_log(const char *ref)
+{
+	struct strbuf dead_ref = STRBUF_INIT;
+	struct stat loginfo;
+
+	if (lstat(git_path("logs/%s", ref), &loginfo))
+		return 0;
+
+	pronounce_dead(&dead_ref, ref);
+
+	if (S_ISLNK(loginfo.st_mode))
+		return error("reflog for %s is a symlink", ref);
+
+	if (safe_create_leading_directories(git_path("logs/%s", dead_ref.buf)))
+		return error("unable to create directory for %s", dead_ref.buf);
+
+	if (rename(git_path("logs/%s", ref), git_path("logs/%s", dead_ref.buf)))
+		return error("unable to move logfile to logs/%s: %s",
+			dead_ref.buf, strerror(errno));
+
+	strbuf_release(&dead_ref);
+	return 0;
+}
+
+static int resurrect_log(const char *ref)
+{
+	struct strbuf dead_ref = STRBUF_INIT;
+	struct stat loginfo;
+
+	pronounce_dead(&dead_ref, ref);
+
+	if (lstat(git_path("logs/%s", dead_ref.buf), &loginfo))
+		return 0;
+
+	if (S_ISLNK(loginfo.st_mode))
+		return error("reflog for %s is a symlink", dead_ref.buf);
+
+	if (safe_create_leading_directories(git_path("logs/%s", ref)))
+		return error("unable to create directory for %s", ref);
+
+	if (rename(git_path("logs/%s", dead_ref.buf), git_path("logs/%s", ref)))
+		return error("unable to move logfile to logs/%s: %s",
+			ref, strerror(errno));
+
+	strbuf_release(&dead_ref);
+	return 0;
+}
+
 int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 {
 	struct ref_lock *lock;
@@ -1084,7 +1148,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));
+	bury_log(lock->ref_name);
 	invalidate_cached_refs();
 	unlock_ref(lock);
 	return ret;
@@ -1384,6 +1448,7 @@ int write_ref_sha1(struct ref_lock *lock,
 		unlock_ref(lock);
 		return -1;
 	}
+	resurrect_log(lock->ref_name);
 	invalidate_cached_refs();
 	if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
 	    (strcmp(lock->ref_name, lock->orig_ref_name) &&
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 759cf12..65f160e 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -55,6 +55,7 @@ test_expect_success 'object with bad sha1' '
 	grep "$sha.*corrupt" out &&
 	rm -f .git/objects/$new &&
 	git update-ref -d refs/heads/bogus &&
+	rm -f .git/logs/refs~/heads~/bogus &&
 	git read-tree -u --reset HEAD
 '
 
-- 
1.7.1.571.gba4d01


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

  parent reply	other threads:[~2010-07-11 13:39 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-29 21:54 Question about 'branch -d' safety Nanako Shiraishi
2009-12-29 22:31 ` Nicolas Sebrecht
2009-12-30  3:12   ` Nanako Shiraishi
2009-12-30  6:43     ` Junio C Hamano
2009-12-30 21:08     ` Nicolas Sebrecht
2010-07-10  6:55     ` Clemens Buchacher
2010-07-10 21:40       ` Jonathan Nieder
2010-07-10 21:57         ` Jakub Narebski
2010-07-10 22:17           ` Jonathan Nieder
2010-07-11  6:55           ` Clemens Buchacher
2010-07-11  7:16             ` Jakub Narebski
2010-07-11  8:48               ` Julian Phillips
2010-07-11 13:37               ` Clemens Buchacher [this message]
2010-07-11 18:41                 ` Junio C Hamano
2010-07-11 19:05                   ` Jakub Narebski
2010-07-11 22:02                   ` Will Palmer
2010-07-12 18:47                   ` Clemens Buchacher
2010-07-12 23:50                     ` Junio C Hamano
2010-07-13  7:13                       ` Clemens Buchacher
2010-07-13  8:00                         ` Will Palmer
2010-07-13  8:30                           ` Johannes Sixt
2010-07-13  9:00                             ` Will Palmer
2010-07-13 22:21                           ` Clemens Buchacher
2010-07-17  9:30                 ` Clemens Buchacher
2010-07-18  0:43                   ` Jonathan Nieder
2010-07-18 11:55                     ` Jakub Narebski
2010-07-18 20:27                       ` Will Palmer
2010-07-18 23:19                         ` Jakub Narebski
2010-07-19  7:12                           ` Will Palmer
2010-07-19 11:01                             ` Jakub Narebski
2010-07-19 17:16                             ` Joshua Jensen
2010-07-19 19:34                               ` Clemens Buchacher
2010-07-19 19:45                               ` Will Palmer
2010-07-19 20:40                                 ` Jakub Narebski
2010-07-20  3:05                                 ` Joshua Jensen
2010-07-20  6:31                                   ` Will Palmer
2010-07-19 20:36                               ` Jakub Narebski
2010-07-19 18:06                   ` Junio C Hamano
2010-07-19 19:22                     ` Clemens Buchacher
2010-07-19 20:49                     ` Jakub Narebski
2010-07-20 13:19                     ` Ævar Arnfjörð Bjarmason
2010-07-20 13:34                       ` Matthieu Moy

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=20100711133730.GA10338@localhost \
    --to=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jnareb@gmail.com \
    --cc=jrnieder@gmail.com \
    --cc=nanako3@lavabit.com \
    --cc=nicolas.s.dev@gmx.fr \
    /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).