Re: [PATCH] Add test for symlinked .git/packed-refs

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] Add test for symlinked .git/packed-refs

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:05

Peter Baumann [off-list ref] writes:
Signed-off-by: Peter Baumann <redacted>
---
On Wed, Apr 18, 2007 at 02:31:29PM -0700, Junio C Hamano wrote:
quoted
Oh, I never questioned that you made that basic case work.  I
was worried about not making sure the symlink we are looking at
really is the case we are willing to handle, and not erroring
out if that is not the case, perhaps like the attached patch on
top of yours.

An additional test or two in t/t3210 would be nice to accompany
this change.
Something like this?
That's a good start, but I expected to see at least tests for
two cases: a case in which .git/packed-refs symlink points at an
actual file (i.e. the original repository has run pack-refs) and
another case in which .git/packed-refs symlink is dangling
(i.e. the original repository hasn't run pack-refs).  I
understand that the borrower "worktree" can have .git/packed-refs
symlink pointing at the repositories .git/packed-refs yet to be
born.

[PATCH] pack-refs: dereference .git/packed-refs if it is a symlink

From: Peter Baumann <hidden>
Date: 2016-06-15 22:43:06

git-new-workdir creates a new working directory where everything
necessary, including .git/packed-refs, is symlinked to your master repo.
But git-pack-refs breaks the symlink, so you could accidentally loose some
refs.

This fixes git-pack-refs to first dereference .git/packed-refs if it is a
symlink. While we are it, add some tests to prevent this from happening
again.

Signed-off-by: Peter Baumann <redacted>
---
On Wed, Apr 18, 2007 at 11:06:45PM -0700, Junio C Hamano wrote:
Peter Baumann [off-list ref] writes:
quoted
Signed-off-by: Peter Baumann <redacted>
---
On Wed, Apr 18, 2007 at 02:31:29PM -0700, Junio C Hamano wrote:
quoted
An additional test or two in t/t3210 would be nice to accompany
this change.
Something like this?
That's a good start, but I expected to see at least tests for
two cases: a case in which .git/packed-refs symlink points at an
actual file (i.e. the original repository has run pack-refs) and
another case in which .git/packed-refs symlink is dangling
(i.e. the original repository hasn't run pack-refs).  I
understand that the borrower "worktree" can have .git/packed-refs
symlink pointing at the repositories .git/packed-refs yet to be
born.
builtin-pack-refs.c  |   18 +++++++++++++++++-
As I couldn't find anything related to this in your repo, I added a test
for a danling symklink and integrated your little fix to check for
doubly symlinked files for easier handling and to not mess up the
history with all does tiny "fixes"

Greetings,
  Peter

 t/t3210-pack-refs.sh |   15 +++++++++++++++
 2 files changed, 32 insertions(+), 1 deletions(-)
diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index d080e30..1ce4f55 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -89,6 +89,8 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
 {
 	int fd, i;
 	struct pack_refs_cb_data cbdata;
+	struct stat st;
+	char *ref_file_name;
 
 	memset(&cbdata, 0, sizeof(cbdata));
 
@@ -113,7 +115,21 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
 	if (i != argc)
 		usage(builtin_pack_refs_usage);
 
-	fd = hold_lock_file_for_update(&packed, git_path("packed-refs"), 1);
+	ref_file_name = git_path("packed-refs");
+	if (!lstat(ref_file_name, &st) && S_ISLNK(st.st_mode)) {
+		char *buf = xmalloc(st.st_size + 1);
+		if (readlink(ref_file_name, buf, st.st_size + 1) != st.st_size) {
+			free(buf);
+			die("readlink failed\n");
+		}
+		buf[st.st_size] = '\0';
+		if (!lstat(buf, &st) && S_ISLNK(st.st_mode))
+			die("cannot have doubly symlinked packed-refs file: %s",
+			    ref_file_name);
+		ref_file_name = buf;
+	}
+
+	fd = hold_lock_file_for_update(&packed, ref_file_name, 1);
 	cbdata.refs_file = fdopen(fd, "w");
 	if (!cbdata.refs_file)
 		die("unable to create ref-pack file structure (%s)",
diff --git a/t/t3210-pack-refs.sh b/t/t3210-pack-refs.sh
index f0c7e22..5756304 100755
--- a/t/t3210-pack-refs.sh
+++ b/t/t3210-pack-refs.sh
@@ -105,4 +105,19 @@ test_expect_success 'pack, prune and repack' '
 	diff all-of-them again
 '
 
+test_expect_success \
+	'derefence symlinks for packed-refs' \
+	'mv -f .git/packed-refs .git/real_packed-refs &&
+	ln -s `pwd`/.git/real_packed-refs .git/packed-refs &&
+	git-tag z &&
+	git-pack-refs --prune &&
+	diff .git/real_packed-refs .git/packed-refs'
+
+test_expect_success \
+	'derefence dangling symlinks for packed-refs' \
+	'git branch dangling_symlink &&
+	rm .git/real_packed-refs
+	git-pack-refs --all --prune &&
+	diff .git/real_packed-refs .git/packed-refs'
+
 test_done
-- 
1.5.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help