Re: [BUG] git-new-workdir doesn't understand packed refs

Peter Baumann [off-list ref] writes:

The problem is, when I created the new workdir, I don't have a file
.git/packed-refs, so a new workdir was created with a dangling symlink,
e.g.  workdir/.git/packed-refs -> repo/.git/packed-refs (but the last one
doesn't exist). As it seems, git gc removes the dangling symlink and
replaces it with a file.

Yes, packed-refs file is creat-to-temp-and-then-rename, and we
will lose the sharing if it is run in the symlink-shared work
tree.

We can do one of two things.  I am not sure which one is better.

 (0) The effect of 'git gc' by definition in the symlink-shared
     work tree should be the same as in the original repository
     as the former is to share all the refspace and object
     database.  So we _could_ declare that running 'git gc' in
     symlink-shared work tree is insane and educate people to
     run that in the original repository.  This is _not_ doing
     anything.

 (1) We could by convention declare a worktree whose .git/refs
     is a symlink, and have git-gc and friends check for it, and
     either refuse to run or automatically chdir and run there.

     If we were to do this, we probably should check more than
     just .git/refs but some other symlinks under .git/ as well.

 (2) We could dereference .git/packed-refs, when it is a
     symlink, by hand, just like we dereference a symlink HEAD
     by hand (see resolve_ref() in refs.c), and run the
     creat-to-temp-and-then-rename sequence to update the real
     file that is pointed at by it.


     

On Wed, Apr 18, 2007 at 12:40:10AM -0700, Junio C Hamano wrote:

Peter Baumann [off-list ref] writes:

quoted

The problem is, when I created the new workdir, I don't have a file
.git/packed-refs, so a new workdir was created with a dangling symlink,
e.g.  workdir/.git/packed-refs -> repo/.git/packed-refs (but the last one
doesn't exist). As it seems, git gc removes the dangling symlink and
replaces it with a file.

Yes, packed-refs file is creat-to-temp-and-then-rename, and we
will lose the sharing if it is run in the symlink-shared work
tree.

We can do one of two things.  I am not sure which one is better.

 (0) The effect of 'git gc' by definition in the symlink-shared
     work tree should be the same as in the original repository
     as the former is to share all the refspace and object
     database.  So we _could_ declare that running 'git gc' in
     symlink-shared work tree is insane and educate people to
     run that in the original repository.  This is _not_ doing
     anything.

 (1) We could by convention declare a worktree whose .git/refs
     is a symlink, and have git-gc and friends check for it, and
     either refuse to run or automatically chdir and run there.

     If we were to do this, we probably should check more than
     just .git/refs but some other symlinks under .git/ as well.

 (2) We could dereference .git/packed-refs, when it is a
     symlink, by hand, just like we dereference a symlink HEAD
     by hand (see resolve_ref() in refs.c), and run the
     creat-to-temp-and-then-rename sequence to update the real
     file that is pointed at by it.

Its not all the clear which one is the best, but (2) sounds as the most
promosing aproach. Hopefully, I'll have time to cook up a patch this
evening.

-Peter

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 it to first dereference .git/packed-refs if it is a
symlink.

Signed-off-by: Peter Baumann <redacted>
---
 builtin-pack-refs.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index d080e30..afa9b5a 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,18 @@ 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';
+		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)",

-- 
1.5.1

On Wed, 18 Apr 2007, Peter Baumann wrote:

On Wed, Apr 18, 2007 at 12:40:10AM -0700, Junio C Hamano wrote:

quoted

We can do one of two things.  I am not sure which one is better.

 (0) The effect of 'git gc' by definition in the symlink-shared
     work tree should be the same as in the original repository
     as the former is to share all the refspace and object
     database.  So we _could_ declare that running 'git gc' in
     symlink-shared work tree is insane and educate people to
     run that in the original repository.  This is _not_ doing
     anything.

 (1) We could by convention declare a worktree whose .git/refs
     is a symlink, and have git-gc and friends check for it, and
     either refuse to run or automatically chdir and run there.

     If we were to do this, we probably should check more than
     just .git/refs but some other symlinks under .git/ as well.

 (2) We could dereference .git/packed-refs, when it is a
     symlink, by hand, just like we dereference a symlink HEAD
     by hand (see resolve_ref() in refs.c), and run the
     creat-to-temp-and-then-rename sequence to update the real
     file that is pointed at by it.

Its not all the clear which one is the best, but (2) sounds as the most
promosing aproach. Hopefully, I'll have time to cook up a patch this
evening.

Personally I think (1) might be slightly better, in the refuse to run 
form.  gc is a repository operation, not a working directory one - and by 
refusing to run in a workdir this is made clear.  You could print out a 
message that includes the location of the actual repo to be more friendly 
though.

But whatever solution you go for, you can't use _any_ workdir that points 
at a repo that is having gc run on, either directly or indirectly, without 
risky odd behaviour.

-- 
Julian

  ---
Q:	How many supply-siders does it take to change a light bulb?
A:	None.  The darkness will cause the light bulb to change by itself.

On Wed, 18 Apr 2007, Peter Baumann wrote:

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 it to first dereference .git/packed-refs if it is a
symlink.

Wouldn't it be nicer to instead make "git gc" *notice* the fact that we're 
in a workdir, and just "cd" to the main git repository instead?

		Linus

On Wed, Apr 18, 2007 at 09:09:13AM -0700, Linus Torvalds wrote:

On Wed, 18 Apr 2007, Peter Baumann wrote:

quoted

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 it to first dereference .git/packed-refs if it is a
symlink.

Wouldn't it be nicer to instead make "git gc" *notice* the fact that we're 
in a workdir, and just "cd" to the main git repository instead?

		Linus

Don't think so. Because then all the low level tools aren't aware of this.
And restricting a WorkDir to use only porcelanish commands isn't what I
want. And teaching every tool about symklinked workdirs doesn't sound right
to me.

-Peter