Allow multiple "git_path()" uses

Allow multiple "git_path()" uses

[Linus Torvalds]

This allows you to maintain a few filesystem pathnames concurrently, by 
simply replacing the single static "pathname" buffer with a LRU of four 
buffers.

We did exactly the same thing with sha1_to_hex(), for pretty much exactly 
the same reason. Sometimes you want to use two pathnames, and while it's 
easy enough to xstrdup() them, why not just do the LU buffer thing.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---

[ This actually came up when I was playing with git_path("refs-packed") 
  when following refs, so while it doesn't hit us right now, it's for some 
  future work.. ]

diff --git a/path.c b/path.c
index db8905f..bb89fb0 100644
--- a/path.c
+++ b/path.c
@@ -13,9 +13,15 @@
 #include "cache.h"
 #include <pwd.h>
 
-static char pathname[PATH_MAX];
 static char bad_path[] = "/bad-path/";
 
+static char *get_pathname(void)
+{
+	static char pathname_array[4][PATH_MAX];
+	static int index;
+	return pathname_array[3 & ++index];
+}
+
 static char *cleanup_path(char *path)
 {
 	/* Clean it up */
@@ -31,6 +37,7 @@ char *mkpath(const char *fmt, ...)
 {
 	va_list args;
 	unsigned len;
+	char *pathname = get_pathname();
 
 	va_start(args, fmt);
 	len = vsnprintf(pathname, PATH_MAX, fmt, args);
@@ -43,6 +50,7 @@ char *mkpath(const char *fmt, ...)
 char *git_path(const char *fmt, ...)
 {
 	const char *git_dir = get_git_dir();
+	char *pathname = get_pathname();
 	va_list args;
 	unsigned len;
 

Start handling references internally as a sorted in-memory list

[Linus Torvalds]

This also adds some very rudimentary support for the notion of packed
refs.  HOWEVER! At this point it isn't used to actually look up a ref
yet, only for listing them (ie "for_each_ref()" and friends see the
packed refs, but none of the other single-ref lookup routines).

Note how we keep two separate lists: one for the loose refs, and one for 
the packed refs we read. That's so that we can easily keep the two apart, 
and read only one set or the other (and still always make sure that the 
loose refs take precedence).

[ From this, it's not actually obvious why we'd keep the two separate 
  lists, but it's important to have the packed refs on their own list 
  later on, when I add support for looking up a single loose one.

  For that case, we will want to read _just_ the packed refs in case the 
  single-ref lookup fails, yet we may end up needing the other list at 
  some point in the future, so keeping them separated is important ]

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
----

This passes all the tests, but largely because it doesn't yet actually use 
and test the ref-packing code. The code exists, but it doesn't remove old 
refs yet. It will be easy enough to do with this background code, though: 
I think this is all set up to be reasonably efficient.

And yeah, I know that the "sorting" code is O(n**2) thanks to doing an 
insertion sort into a simple linked list. Tough. I didn't care enough to 
do it well. With "n" usually being a few hundred at most, we really don't 
care, and if we ever do, we _can_ fix it later on to use a heap or 
something.

diff --git a/Makefile b/Makefile
index 7b3114f..5a2e946 100644
--- a/Makefile
+++ b/Makefile
@@ -295,7 +295,8 @@ BUILTIN_OBJS = \
 	builtin-upload-tar.o \
 	builtin-verify-pack.o \
 	builtin-write-tree.o \
-	builtin-zip-tree.o
+	builtin-zip-tree.o \
+	builtin-pack-refs.o
 
 GITLIBS = $(LIB_FILE) $(XDIFF_LIB)
 LIBS = $(GITLIBS) -lz
diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
new file mode 100644
index 0000000..0f5d827
--- /dev/null
+++ b/builtin-pack-refs.c
@@ -0,0 +1,41 @@
+#include "cache.h"
+#include "refs.h"
+
+static FILE *refs_file;
+static const char *result_path, *lock_path;
+
+static void remove_lock_file(void)
+{
+	if (lock_path)
+		unlink(lock_path);
+}
+
+static int handle_one_ref(const char *path, const unsigned char *sha1)
+{
+	fprintf(refs_file, "%s %s\n", sha1_to_hex(sha1), path);
+	return 0;
+}
+
+int cmd_pack_refs(int argc, const char **argv, const char *prefix)
+{
+	int fd;
+
+	result_path = xstrdup(git_path("packed-refs"));
+	lock_path = xstrdup(mkpath("%s.lock", result_path));
+
+	fd = open(lock_path, O_CREAT | O_EXCL | O_WRONLY, 0666);
+	if (fd < 0)
+		die("unable to create new ref-pack file (%s)", strerror(errno));
+	atexit(remove_lock_file);
+
+	refs_file = fdopen(fd, "w");
+	if (!refs_file)
+		die("unable to create ref-pack file structure (%s)", strerror(errno));
+	for_each_ref(handle_one_ref);
+	fsync(fd);
+	fclose(refs_file);
+	if (rename(lock_path, result_path) < 0)
+		die("unable to overwrite old ref-pack file (%s)", strerror(errno));
+	lock_path = NULL;
+	return 0;
+}
diff --git a/builtin.h b/builtin.h
index 25431d7..34ed7b9 100644
--- a/builtin.h
+++ b/builtin.h
@@ -61,5 +61,6 @@ extern int cmd_version(int argc, const c
 extern int cmd_whatchanged(int argc, const char **argv, const char *prefix);
 extern int cmd_write_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_verify_pack(int argc, const char **argv, const char *prefix);
+extern int cmd_pack_refs(int argc, const char **argv, const char *prefix);
 
 #endif
diff --git a/git.c b/git.c
index 335f405..63d1ba3 100644
--- a/git.c
+++ b/git.c
@@ -266,6 +266,7 @@ static void handle_internal_command(int 
 		{ "whatchanged", cmd_whatchanged, RUN_SETUP | USE_PAGER },
 		{ "write-tree", cmd_write_tree, RUN_SETUP },
 		{ "verify-pack", cmd_verify_pack },
+		{ "pack-refs", cmd_pack_refs, RUN_SETUP },
 	};
 	int i;
 
diff --git a/refs.c b/refs.c
index 5e65314..5f80a68 100644
--- a/refs.c
+++ b/refs.c
@@ -3,6 +3,145 @@ #include "cache.h"
 
 #include <errno.h>
 
+struct ref_list {
+	struct ref_list *next;
+	unsigned char sha1[20];
+	char name[FLEX_ARRAY];
+};
+
+static const char *parse_ref_line(char *line, unsigned char *sha1)
+{
+	/*
+	 * 42: the answer to everything.
+	 *
+	 * In this case, it happens to be the answer to
+	 *  40 (length of sha1 hex representation)
+	 *  +1 (space in between hex and name)
+	 *  +1 (newline at the end of the line)
+	 */
+	int len = strlen(line) - 42;
+
+	if (len <= 0)
+		return NULL;
+	if (get_sha1_hex(line, sha1) < 0)
+		return NULL;
+	if (!isspace(line[40]))
+		return NULL;
+	line += 41;
+	if (line[len] != '\n')
+		return NULL;
+	line[len] = 0;
+	return line;
+}
+
+static struct ref_list *add_ref(const char *name, const unsigned char *sha1, struct ref_list *list)
+{
+	int len;
+	struct ref_list **p = &list, *entry;
+
+	/* Find the place to insert the ref into.. */
+	while ((entry = *p) != NULL) {
+		int cmp = strcmp(entry->name, name);
+		if (cmp > 0)
+			break;
+
+		/* Same as existing entry? */
+		if (!cmp)
+			return list;
+		p = &entry->next;
+	}
+
+	/* Allocate it and add it in.. */
+	len = strlen(name) + 1;
+	entry = xmalloc(sizeof(struct ref_list) + len);
+	hashcpy(entry->sha1, sha1);
+	memcpy(entry->name, name, len);
+	entry->next = *p;
+	*p = entry;
+	return list;
+}
+
+static struct ref_list *get_packed_refs(void)
+{
+	static int did_refs = 0;
+	static struct ref_list *refs = NULL;
+
+	if (!did_refs) {
+		FILE *f = fopen(git_path("packed-refs"), "r");
+		if (f) {
+			struct ref_list *list = NULL;
+			char refline[PATH_MAX];
+			while (fgets(refline, sizeof(refline), f)) {
+				unsigned char sha1[20];
+				const char *name = parse_ref_line(refline, sha1);
+				if (!name)
+					continue;
+				list = add_ref(name, sha1, list);
+			}
+			fclose(f);
+			refs = list;
+		}
+		did_refs = 1;
+	}
+	return refs;
+}
+
+static struct ref_list *get_ref_dir(const char *base, struct ref_list *list)
+{
+	DIR *dir = opendir(git_path("%s", base));
+
+	if (dir) {
+		struct dirent *de;
+		int baselen = strlen(base);
+		char *path = xmalloc(baselen + 257);
+
+		memcpy(path, base, baselen);
+		if (baselen && base[baselen-1] != '/')
+			path[baselen++] = '/';
+
+		while ((de = readdir(dir)) != NULL) {
+			unsigned char sha1[20];
+			struct stat st;
+			int namelen;
+
+			if (de->d_name[0] == '.')
+				continue;
+			namelen = strlen(de->d_name);
+			if (namelen > 255)
+				continue;
+			if (has_extension(de->d_name, ".lock"))
+				continue;
+			memcpy(path + baselen, de->d_name, namelen+1);
+			if (stat(git_path("%s", path), &st) < 0)
+				continue;
+			if (S_ISDIR(st.st_mode)) {
+				list = get_ref_dir(path, list);
+				continue;
+			}
+			if (read_ref(git_path("%s", path), sha1) < 0) {
+				error("%s points nowhere!", path);
+				continue;
+			}
+			list = add_ref(path, sha1, list);
+		}
+		free(path);
+		closedir(dir);
+	}
+	return list;
+}
+
+static struct ref_list *get_loose_refs(void)
+{
+	static int did_refs = 0;
+	static struct ref_list *refs = NULL;
+
+	if (!did_refs) {
+		refs = get_ref_dir("refs", NULL);
+		did_refs = 1;
+	}
+	return refs;
+}
+
 /* We allow "recursive" symbolic refs. Only within reason, though */
 #define MAXDEPTH 5
 
@@ -121,60 +260,41 @@ int read_ref(const char *filename, unsig
 
 static int do_for_each_ref(const char *base, int (*fn)(const char *path, const unsigned char *sha1), int trim)
 {
-	int retval = 0;
-	DIR *dir = opendir(git_path("%s", base));
-
-	if (dir) {
-		struct dirent *de;
-		int baselen = strlen(base);
-		char *path = xmalloc(baselen + 257);
-
-		if (!strncmp(base, "./", 2)) {
-			base += 2;
-			baselen -= 2;
+	int retval;
+	struct ref_list *packed = get_packed_refs();
+	struct ref_list *loose = get_loose_refs();
+
+	while (packed && loose) {
+		struct ref_list *entry;
+		int cmp = strcmp(packed->name, loose->name);
+		if (!cmp) {
+			packed = packed->next;
+			continue;
 		}
-		memcpy(path, base, baselen);
-		if (baselen && base[baselen-1] != '/')
-			path[baselen++] = '/';
-
-		while ((de = readdir(dir)) != NULL) {
-			unsigned char sha1[20];
-			struct stat st;
-			int namelen;
+		if (cmp > 0) {
+			entry = loose;
+			loose = loose->next;
+		} else {
+			entry = packed;
+			packed = packed->next;
+		}
+		if (strncmp(base, entry->name, trim))
+			continue;
+		retval = fn(entry->name + trim, entry->sha1);
+		if (retval)
+			return retval;
+	}
 
-			if (de->d_name[0] == '.')
-				continue;
-			namelen = strlen(de->d_name);
-			if (namelen > 255)
-				continue;
-			if (has_extension(de->d_name, ".lock"))
-				continue;
-			memcpy(path + baselen, de->d_name, namelen+1);
-			if (stat(git_path("%s", path), &st) < 0)
-				continue;
-			if (S_ISDIR(st.st_mode)) {
-				retval = do_for_each_ref(path, fn, trim);
-				if (retval)
-					break;
-				continue;
-			}
-			if (read_ref(git_path("%s", path), sha1) < 0) {
-				error("%s points nowhere!", path);
-				continue;
-			}
-			if (!has_sha1_file(sha1)) {
-				error("%s does not point to a valid "
-				      "commit object!", path);
-				continue;
-			}
-			retval = fn(path + trim, sha1);
+	packed = packed ? packed : loose;
+	while (packed) {
+		if (!strncmp(base, packed->name, trim)) {
+			retval = fn(packed->name + trim, packed->sha1);
 			if (retval)
-				break;
+				return retval;
 		}
-		free(path);
-		closedir(dir);
+		packed = packed->next;
 	}
-	return retval;
+	return 0;
 }
 
 int head_ref(int (*fn)(const char *path, const unsigned char *sha1))
@@ -187,22 +307,22 @@ int head_ref(int (*fn)(const char *path,
 
 int for_each_ref(int (*fn)(const char *path, const unsigned char *sha1))
 {
-	return do_for_each_ref("refs", fn, 0);
+	return do_for_each_ref("refs/", fn, 0);
 }
 
 int for_each_tag_ref(int (*fn)(const char *path, const unsigned char *sha1))
 {
-	return do_for_each_ref("refs/tags", fn, 10);
+	return do_for_each_ref("refs/tags/", fn, 10);
 }
 
 int for_each_branch_ref(int (*fn)(const char *path, const unsigned char *sha1))
 {
-	return do_for_each_ref("refs/heads", fn, 11);
+	return do_for_each_ref("refs/heads/", fn, 11);
 }
 
 int for_each_remote_ref(int (*fn)(const char *path, const unsigned char *sha1))
 {
-	return do_for_each_ref("refs/remotes", fn, 13);
+	return do_for_each_ref("refs/remotes/", fn, 13);
 }
 
 int get_ref_sha1(const char *ref, unsigned char *sha1)

Re: Start handling references internally as a sorted in-memory list

[Linus Torvalds]

On Mon, 11 Sep 2006, Linus Torvalds wrote:
> 
> And yeah, I know that the "sorting" code is O(n**2) thanks to doing an 
> insertion sort into a simple linked list. Tough. I didn't care enough to 
> do it well. With "n" usually being a few hundred at most, we really don't 
> care, and if we ever do, we _can_ fix it later on to use a heap or 
> something.

Btw, to expand on that - one of the nice things from this whole re-org of 
how we handle refs is that when you do

	git-rev-parse --all

it now sorts them in a reliable order - by name. Before, they came out in 
some random order that totally depended on the filesystem that the 
references were on.

Use the "--symbolic --all" to see this in a more obvious way, before and 
after.

		Linus

Re: Start handling references internally as a sorted in-memory list

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> This also adds some very rudimentary support for the notion of packed
> And yeah, I know that the "sorting" code is O(n**2) thanks to doing an 
> insertion sort into a simple linked list. Tough. I didn't care enough to 
> do it well. With "n" usually being a few hundred at most, we really don't 
> care, and if we ever do, we _can_ fix it later on to use a heap or 
> something.

I thought what triggered the restructuring of this part was N
actually being large enough to cause pain to some people, so I
suspect later might need to be reasonably soon ;-).

> +static const char *parse_ref_line(char *line, unsigned char *sha1)
> +{
> +	/*
> +	 * 42: the answer to everything.
> +	 *
> +	 * In this case, it happens to be the answer to
>...

Rof,l.

I've been wondering what happens if you pack "refs/heads/foo",
delete it, and create "refs/heads/foo/bar" with your proposal.
I'll find the answer (which is _not_ "42") in the updated
do_for_each_ref(), I guess.

Re: Start handling references internally as a sorted in-memory list

[Linus Torvalds]

On Mon, 11 Sep 2006, Junio C Hamano wrote:
> 
> I thought what triggered the restructuring of this part was N
> actually being large enough to cause pain to some people, so I
> suspect later might need to be reasonably soon ;-).

Well, the current problems is actually related to the _IO_ load of O(n) 
(and to some degree the disk usage), and that is indeed fairly high. The 
new O(n**2) is all for a (very) simple CPU loop, so I suspect "n" has to 
be in the millions before it even approaches the IO load problem ;)

> I've been wondering what happens if you pack "refs/heads/foo",
> delete it, and create "refs/heads/foo/bar" with your proposal.
> I'll find the answer (which is _not_ "42") in the updated
> do_for_each_ref(), I guess.

Right now, the code in question only contains some preliminary support for 
refs at all, and it's technically incorrect. If you ever have a 
"packed-refs" file that contains _anything_ outside of the actual loose 
refs, it won't do the right thing.

But I'd rather have this as a base, than send one huge patch that does 
everything.

For example, right now the code doesn't support negative refs (even the 
simple kind) at all, so you don't need to even have the "file turned into 
a directory" case for it to "not work".

So right now it's basically just scaffolding. It's hopefully _good_ 
scaffolding, so that when I send the next few patches, it actually starts 
working.. ;)

		Linus

Re: Start handling references internally as a sorted in-memory list

[Chris Wedgwood]

On Mon, Sep 11, 2006 at 04:37:32PM -0700, Linus Torvalds wrote:

> This also adds some very rudimentary support for the notion of packed
> refs.

Neat.  Although git makes some repos nice and small the refs take a
lot of space when people use a lot of them:

3.5M  glibc/glibc-cvs.git/.git/refs/
11M   gcc-git/gcc.git/.git/refs/

Add support for negative refs

[Linus Torvalds]

You can remove a ref that is packed two different ways: either simply
repack all the refs without that one, or create a loose ref that has the
magic all-zero SHA1.

This also adds back the test that a ref actually has the object it
points to.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---

This one is trivial, and obviously depends on the previous series.

I'm sending it just because I have a much bigger cleanup that I'd _really_ 
like to go in, and I did this trivial one before that much more 
interesting one.

diff --git a/refs.c b/refs.c
index 5f80a68..72e2283 100644
--- a/refs.c
+++ b/refs.c
@@ -280,6 +280,12 @@ static int do_for_each_ref(const char *b
 		}
 		if (strncmp(base, entry->name, trim))
 			continue;
+		if (is_null_sha1(entry->sha1))
+			continue;
+		if (!has_sha1_file(entry->sha1)) {
+			error("%s does not point to a valid object!", entry->name);
+			continue;
+		}
 		retval = fn(entry->name + trim, entry->sha1);
 		if (retval)
 			return retval;

Make ref resolution saner

[Linus Torvalds]

The old code used to totally mix up the notion of a ref-name and the path 
that that ref was associated with.  That was not only horribly ugly (a 
number of users got the path, and then wanted to try to turn it back into 
a ref-name again), but it fundamnetally doesn't work at all once we do any 
setup where a ref doesn't have a 1:1 relationship with a particular 
pathname.

This fixes things up so that we use the ref-name throughout, and only
turn it into a pathname once we actually look it up in the filesystem.
That makes a lot of things much clearer and more straightforward.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---

This patch is a bit scary, but look at the first two hunks to 
builtin-fmt-merge-msg.c to get an idea of what kind of *crap* it gets rid 
of.

There were several places where we used "resolve_ref()" to figure out what 
the branch it was on, but then to actually figure out what the branch-name 
of that branch was (rather than the cwd-relative _filename_ is), it then 
did strange things with the string length of the original ref compared 
with the string length of the _filename_ of the original ref etc etc.

Making all the ref-related functions just use the ref-name itself 
throughout just gets rid of the crap, but more importantly, it's 
absolutely required for just about _any_ ref-packing scheme.

That said, it _is_ a scary patch. It removes more lines that it adds:

 builtin-fmt-merge-msg.c |    7 +----
 builtin-init-db.c       |    4 +--
 builtin-show-branch.c   |   46 +++++++++++++------------------
 builtin-symbolic-ref.c  |   14 +++-------
 cache.h                 |    4 +--
 refs.c                  |   69 +++++++++++++++++++++++++----------------------
 sha1_name.c             |   14 +++++-----
 7 files changed, 74 insertions(+), 84 deletions(-)

but if I missed some place where we used a ref as a pathname, or added one 
"git_path()" translation too many, it results in problems.

The good news is that our test-suite for this seems to be reasonably 
complete. It sure caught a lot of places I had missed the first time 
around. So this should be ok.

I'd _really_ like for this to go in, even regardless of the other patches 
I have sent (although I think it depends on them in mostly fairly trivial 
ways, since the previous patches changed how we do the recursive 
readdir() on the refs/ directory).

It's really needed.


diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index c407c03..b93c17c 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -249,7 +249,7 @@ int cmd_fmt_merge_msg(int argc, const ch
 	FILE *in = stdin;
 	const char *sep = "";
 	unsigned char head_sha1[20];
-	const char *head, *current_branch;
+	const char *current_branch;
 
 	git_config(fmt_merge_msg_config);
 
@@ -277,10 +277,7 @@ int cmd_fmt_merge_msg(int argc, const ch
 		usage(fmt_merge_msg_usage);
 
 	/* get current branch */
-	head = xstrdup(git_path("HEAD"));
-	current_branch = resolve_ref(head, head_sha1, 1);
-	current_branch += strlen(head) - 4;
-	free((char *)head);
+	current_branch = resolve_ref("HEAD", head_sha1, 1);
 	if (!strncmp(current_branch, "refs/heads/", 11))
 		current_branch += 11;
 
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 5085018..23b7714 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -218,8 +218,8 @@ static void create_default_files(const c
 	 * branch, if it does not exist yet.
 	 */
 	strcpy(path + len, "HEAD");
-	if (read_ref(path, sha1) < 0) {
-		if (create_symref(path, "refs/heads/master") < 0)
+	if (read_ref("HEAD", sha1) < 0) {
+		if (create_symref("HEAD", "refs/heads/master") < 0)
 			exit(1);
 	}
 
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 578c9fa..4d8db0c 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -437,21 +437,13 @@ static void snarf_refs(int head, int tag
 	}
 }
 
-static int rev_is_head(char *head_path, int headlen, char *name,
+static int rev_is_head(char *head, int headlen, char *name,
 		       unsigned char *head_sha1, unsigned char *sha1)
 {
-	int namelen;
-	if ((!head_path[0]) ||
+	if ((!head[0]) ||
 	    (head_sha1 && sha1 && hashcmp(head_sha1, sha1)))
 		return 0;
-	namelen = strlen(name);
-	if ((headlen < namelen) ||
-	    memcmp(head_path + headlen - namelen, name, namelen))
-		return 0;
-	if (headlen == namelen ||
-	    head_path[headlen - namelen - 1] == '/')
-		return 1;
-	return 0;
+	return !strcmp(head, name);
 }
 
 static int show_merge_base(struct commit_list *seen, int num_rev)
@@ -559,9 +551,9 @@ int cmd_show_branch(int ac, const char *
 	int all_heads = 0, all_tags = 0;
 	int all_mask, all_revs;
 	int lifo = 1;
-	char head_path[128];
-	const char *head_path_p;
-	int head_path_len;
+	char head[128];
+	const char *head_p;
+	int head_len;
 	unsigned char head_sha1[20];
 	int merge_base = 0;
 	int independent = 0;
@@ -638,31 +630,31 @@ int cmd_show_branch(int ac, const char *
 		ac--; av++;
 	}
 
-	head_path_p = resolve_ref(git_path("HEAD"), head_sha1, 1);
-	if (head_path_p) {
-		head_path_len = strlen(head_path_p);
-		memcpy(head_path, head_path_p, head_path_len + 1);
+	head_p = resolve_ref("HEAD", head_sha1, 1);
+	if (head_p) {
+		head_len = strlen(head_p);
+		memcpy(head, head_p, head_len + 1);
 	}
 	else {
-		head_path_len = 0;
-		head_path[0] = 0;
+		head_len = 0;
+		head[0] = 0;
 	}
 
-	if (with_current_branch && head_path_p) {
+	if (with_current_branch && head_p) {
 		int has_head = 0;
 		for (i = 0; !has_head && i < ref_name_cnt; i++) {
 			/* We are only interested in adding the branch
 			 * HEAD points at.
 			 */
-			if (rev_is_head(head_path,
-					head_path_len,
+			if (rev_is_head(head,
+					head_len,
 					ref_name[i],
 					head_sha1, NULL))
 				has_head++;
 		}
 		if (!has_head) {
-			int pfxlen = strlen(git_path("refs/heads/"));
-			append_one_rev(head_path + pfxlen);
+			int pfxlen = strlen("refs/heads/");
+			append_one_rev(head + pfxlen);
 		}
 	}
 
@@ -713,8 +705,8 @@ int cmd_show_branch(int ac, const char *
 	if (1 < num_rev || extra < 0) {
 		for (i = 0; i < num_rev; i++) {
 			int j;
-			int is_head = rev_is_head(head_path,
-						  head_path_len,
+			int is_head = rev_is_head(head,
+						  head_len,
 						  ref_name[i],
 						  head_sha1,
 						  rev[i]->object.sha1);
diff --git a/builtin-symbolic-ref.c b/builtin-symbolic-ref.c
index 1d3a5e2..6f18db8 100644
--- a/builtin-symbolic-ref.c
+++ b/builtin-symbolic-ref.c
@@ -7,15 +7,11 @@ static const char git_symbolic_ref_usage
 static void check_symref(const char *HEAD)
 {
 	unsigned char sha1[20];
-	const char *git_HEAD = xstrdup(git_path("%s", HEAD));
-	const char *git_refs_heads_master = resolve_ref(git_HEAD, sha1, 0);
-	if (git_refs_heads_master) {
-		/* we want to strip the .git/ part */
-		int pfxlen = strlen(git_HEAD) - strlen(HEAD);
-		puts(git_refs_heads_master + pfxlen);
-	}
-	else
+	const char *refs_heads_master = resolve_ref("HEAD", sha1, 0);
+
+	if (!refs_heads_master)
 		die("No such ref: %s", HEAD);
+	puts(refs_heads_master);
 }
 
 int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
@@ -26,7 +22,7 @@ int cmd_symbolic_ref(int argc, const cha
 		check_symref(argv[1]);
 		break;
 	case 3:
-		create_symref(xstrdup(git_path("%s", argv[1])), argv[2]);
+		create_symref(argv[1], argv[2]);
 		break;
 	default:
 		usage(git_symbolic_ref_usage);
diff --git a/cache.h b/cache.h
index a53204f..5d6c7ee 100644
--- a/cache.h
+++ b/cache.h
@@ -287,8 +287,8 @@ extern int get_sha1_hex(const char *hex,
 extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
 extern int read_ref(const char *filename, unsigned char *sha1);
 extern const char *resolve_ref(const char *path, unsigned char *sha1, int);
-extern int create_symref(const char *git_HEAD, const char *refs_heads_master);
-extern int validate_symref(const char *git_HEAD);
+extern int create_symref(const char *ref, const char *refs_heads_master);
+extern int validate_symref(const char *ref);
 
 extern int base_name_compare(const char *name1, int len1, int mode1, const char *name2, int len2, int mode2);
 extern int cache_name_compare(const char *name1, int len1, const char *name2, int len2);
diff --git a/refs.c b/refs.c
index 72e2283..50c25d3 100644
--- a/refs.c
+++ b/refs.c
@@ -93,11 +93,11 @@ static struct ref_list *get_ref_dir(cons
 	if (dir) {
 		struct dirent *de;
 		int baselen = strlen(base);
-		char *path = xmalloc(baselen + 257);
+		char *ref = xmalloc(baselen + 257);
 
-		memcpy(path, base, baselen);
+		memcpy(ref, base, baselen);
 		if (baselen && base[baselen-1] != '/')
-			path[baselen++] = '/';
+			ref[baselen++] = '/';
 
 		while ((de = readdir(dir)) != NULL) {
 			unsigned char sha1[20];
@@ -111,20 +111,20 @@ static struct ref_list *get_ref_dir(cons
 				continue;
 			if (has_extension(de->d_name, ".lock"))
 				continue;
-			memcpy(path + baselen, de->d_name, namelen+1);
-			if (stat(git_path("%s", path), &st) < 0)
+			memcpy(ref + baselen, de->d_name, namelen+1);
+			if (stat(git_path("%s", ref), &st) < 0)
 				continue;
 			if (S_ISDIR(st.st_mode)) {
-				list = get_ref_dir(path, list);
+				list = get_ref_dir(ref, list);
 				continue;
 			}
-			if (read_ref(git_path("%s", path), sha1) < 0) {
-				error("%s points nowhere!", path);
+			if (read_ref(ref, sha1) < 0) {
+				error("%s points nowhere!", ref);
 				continue;
 			}
-			list = add_ref(path, sha1, list);
+			list = add_ref(ref, sha1, list);
 		}
-		free(path);
+		free(ref);
 		closedir(dir);
 	}
 	return list;
@@ -145,12 +145,14 @@ static struct ref_list *get_loose_refs(v
 /* We allow "recursive" symbolic refs. Only within reason, though */
 #define MAXDEPTH 5
 
-const char *resolve_ref(const char *path, unsigned char *sha1, int reading)
+const char *resolve_ref(const char *ref, unsigned char *sha1, int reading)
 {
 	int depth = MAXDEPTH, len;
 	char buffer[256];
+	static char ref_buffer[256];
 
 	for (;;) {
+		const char *path = git_path("%s", ref);
 		struct stat st;
 		char *buf;
 		int fd;
@@ -169,14 +171,16 @@ const char *resolve_ref(const char *path
 			if (reading || errno != ENOENT)
 				return NULL;
 			hashclr(sha1);
-			return path;
+			return ref;
 		}
 
 		/* Follow "normalized" - ie "refs/.." symlinks by hand */
 		if (S_ISLNK(st.st_mode)) {
 			len = readlink(path, buffer, sizeof(buffer)-1);
 			if (len >= 5 && !memcmp("refs/", buffer, 5)) {
-				path = git_path("%.*s", len, buffer);
+				buffer[len] = 0;
+				strcpy(ref_buffer, buffer);
+				ref = ref_buffer;
 				continue;
 			}
 		}
@@ -201,19 +205,22 @@ const char *resolve_ref(const char *path
 		while (len && isspace(*buf))
 			buf++, len--;
 		while (len && isspace(buf[len-1]))
-			buf[--len] = 0;
-		path = git_path("%.*s", len, buf);
+			len--;
+		buf[len] = 0;
+		memcpy(ref_buffer, buf, len + 1);
+		ref = ref_buffer;
 	}
 	if (len < 40 || get_sha1_hex(buffer, sha1))
 		return NULL;
-	return path;
+	return ref;
 }
 
-int create_symref(const char *git_HEAD, const char *refs_heads_master)
+int create_symref(const char *ref_target, const char *refs_heads_master)
 {
 	const char *lockpath;
 	char ref[1000];
 	int fd, len, written;
+	const char *git_HEAD = git_path("%s", ref_target);
 
 #ifndef NO_SYMLINK_HEAD
 	if (prefer_symlink_refs) {
@@ -251,9 +258,9 @@ #endif
 	return 0;
 }
 
-int read_ref(const char *filename, unsigned char *sha1)
+int read_ref(const char *ref, unsigned char *sha1)
 {
-	if (resolve_ref(filename, sha1, 1))
+	if (resolve_ref(ref, sha1, 1))
 		return 0;
 	return -1;
 }
@@ -306,7 +313,7 @@ static int do_for_each_ref(const char *b
 int head_ref(int (*fn)(const char *path, const unsigned char *sha1))
 {
 	unsigned char sha1[20];
-	if (!read_ref(git_path("HEAD"), sha1))
+	if (!read_ref("HEAD", sha1))
 		return fn("HEAD", sha1);
 	return 0;
 }
@@ -335,7 +342,7 @@ int get_ref_sha1(const char *ref, unsign
 {
 	if (check_ref_format(ref))
 		return -1;
-	return read_ref(git_path("refs/%s", ref), sha1);
+	return read_ref(mkpath("refs/%s", ref), sha1);
 }
 
 /*
@@ -416,31 +423,30 @@ static struct ref_lock *verify_lock(stru
 	return lock;
 }
 
-static struct ref_lock *lock_ref_sha1_basic(const char *path,
+static struct ref_lock *lock_ref_sha1_basic(const char *ref,
 	int plen,
 	const unsigned char *old_sha1, int mustexist)
 {
-	const char *orig_path = path;
+	const char *orig_ref = ref;
 	struct ref_lock *lock;
 	struct stat st;
 
 	lock = xcalloc(1, sizeof(struct ref_lock));
 	lock->lock_fd = -1;
 
-	plen = strlen(path) - plen;
-	path = resolve_ref(path, lock->old_sha1, mustexist);
-	if (!path) {
+	ref = resolve_ref(ref, lock->old_sha1, mustexist);
+	if (!ref) {
 		int last_errno = errno;
 		error("unable to resolve reference %s: %s",
-			orig_path, strerror(errno));
+			orig_ref, strerror(errno));
 		unlock_ref(lock);
 		errno = last_errno;
 		return NULL;
 	}
 	lock->lk = xcalloc(1, sizeof(struct lock_file));
 
-	lock->ref_file = xstrdup(path);
-	lock->log_file = xstrdup(git_path("logs/%s", lock->ref_file + plen));
+	lock->ref_file = xstrdup(git_path("%s", ref));
+	lock->log_file = xstrdup(git_path("logs/%s", ref));
 	lock->force_write = lstat(lock->ref_file, &st) && errno == ENOENT;
 
 	if (safe_create_leading_directories(lock->ref_file))
@@ -455,15 +461,14 @@ struct ref_lock *lock_ref_sha1(const cha
 {
 	if (check_ref_format(ref))
 		return NULL;
-	return lock_ref_sha1_basic(git_path("refs/%s", ref),
+	return lock_ref_sha1_basic(mkpath("refs/%s", ref),
 		5 + strlen(ref), old_sha1, mustexist);
 }
 
 struct ref_lock *lock_any_ref_for_update(const char *ref,
 	const unsigned char *old_sha1, int mustexist)
 {
-	return lock_ref_sha1_basic(git_path("%s", ref),
-		strlen(ref), old_sha1, mustexist);
+	return lock_ref_sha1_basic(ref, strlen(ref), old_sha1, mustexist);
 }
 
 void unlock_ref(struct ref_lock *lock)
diff --git a/sha1_name.c b/sha1_name.c
index 1fbc443..b497528 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -247,8 +247,8 @@ static int get_sha1_basic(const char *st
 		NULL
 	};
 	static const char *warning = "warning: refname '%.*s' is ambiguous.\n";
-	const char **p, *pathname;
-	char *real_path = NULL;
+	const char **p, *ref;
+	char *real_ref = NULL;
 	int refs_found = 0, am;
 	unsigned long at_time = (unsigned long)-1;
 	unsigned char *this_result;
@@ -276,10 +276,10 @@ static int get_sha1_basic(const char *st
 
 	for (p = fmt; *p; p++) {
 		this_result = refs_found ? sha1_from_ref : sha1;
-		pathname = resolve_ref(git_path(*p, len, str), this_result, 1);
-		if (pathname) {
+		ref = resolve_ref(mkpath(*p, len, str), this_result, 1);
+		if (ref) {
 			if (!refs_found++)
-				real_path = xstrdup(pathname);
+				real_ref = xstrdup(ref);
 			if (!warn_ambiguous_refs)
 				break;
 		}
@@ -293,12 +293,12 @@ static int get_sha1_basic(const char *st
 
 	if (at_time != (unsigned long)-1) {
 		read_ref_at(
-			real_path + strlen(git_path(".")) - 1,
+			real_ref,
 			at_time,
 			sha1);
 	}
 
-	free(real_path);
+	free(real_ref);
 	return 0;
 }
 

Re: Make ref resolution saner

[Jeff King]

On Mon, Sep 11, 2006 at 08:17:35PM -0700, Linus Torvalds wrote:

> The old code used to totally mix up the notion of a ref-name and the path 
> that that ref was associated with.  That was not only horribly ugly (a

I assume your patch is against master; it looks like there's exactly one
call to resolve_ref that's in next but not master. One-liner fix below.

-Peff

-- >8 --
wt-status: use simplified resolve_ref to find current branch

---
 wt-status.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index ec2c728..e2f49c7 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -41,7 +41,7 @@ void wt_status_prepare(struct wt_status 
 
 	s->is_initial = get_sha1("HEAD", sha1) ? 1 : 0;
 
-	head = resolve_ref(git_path("HEAD"), sha1, 0);
+	head = resolve_ref("HEAD", sha1, 0);
 	s->branch = head ?
 		    strdup(head + strlen(get_git_dir()) + 1) :
 		    NULL;
-- 
1.4.2.g39f1

Re: Make ref resolution saner

[Linus Torvalds]

On Tue, 12 Sep 2006, Jeff King wrote:
> 
> I assume your patch is against master;

Yeah. Well, master plus my previous patches.

> it looks like there's exactly one call to resolve_ref that's in next but 
> not master. One-liner fix below.

Not quite enough. It's the same thing: wt-status.c plays games with the 
return value (which _used_ to be a path) in order to turn it back into a 
ref. But now that it's all about refs, the games are unnecessary:

> diff --git a/wt-status.c b/wt-status.c
> index ec2c728..e2f49c7 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -41,7 +41,7 @@ void wt_status_prepare(struct wt_status 
>  
>  	s->is_initial = get_sha1("HEAD", sha1) ? 1 : 0;
>  
> -	head = resolve_ref(git_path("HEAD"), sha1, 0);
> +	head = resolve_ref("HEAD", sha1, 0);
>  	s->branch = head ?
>  		    strdup(head + strlen(get_git_dir()) + 1) :
>  		    NULL;

So that "strdup(head + strlen(get_git_dir()) + 1)" should now be just 
"strdup(head)".

		Linus

[RFC] git-pack-refs --prune

[Junio C Hamano]

"git pack-refs --prune", after successfully packing the existing
refs, removes the loose ref files.  It tries to protect against
race by doing the usual lock_ref_sha1() which makes sure the
contents of the ref has not changed since we last looked at.

I am not sure I got the locking right, hence this RFC.

We would probably need to perform some sort of 'sync' after
closing and renaming the lockfile to its final location before
pruning.  Is there a way cheaper than sync(2) to make sure the
effect of rename(2) hits the disk platter?

---
diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index 0f5d827..2b3a483 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -3,6 +3,15 @@ #include "refs.h"
 
 static FILE *refs_file;
 static const char *result_path, *lock_path;
+static const char builtin_pack_refs_usage[] =
+"git-pack-refs [--prune]";
+
+static int prune;
+struct keepref {
+	struct keepref *next;
+	unsigned char sha1[20];
+	char name[FLEX_ARRAY];
+} *keepref;
 
 static void remove_lock_file(void)
 {
@@ -13,12 +22,50 @@ static void remove_lock_file(void)
 static int handle_one_ref(const char *path, const unsigned char *sha1)
 {
 	fprintf(refs_file, "%s %s\n", sha1_to_hex(sha1), path);
+	if (prune) {
+		int namelen = strlen(path) + 1;
+		struct keepref *n = xcalloc(1, sizeof(*n) + namelen);
+		hashcpy(n->sha1, sha1);
+		strcpy(n->name, path);
+		n->next = keepref;
+		keepref = n;
+	}
 	return 0;
 }
 
+/* make sure nobody touched the ref, and unlink */
+static void prune_ref(struct keepref *r)
+{
+	struct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1, 1);
+
+	if (lock) {
+		unlink(git_path(r->name));
+		unlock_ref(lock);
+	}
+}
+
+static void prune_refs(void)
+{
+	struct keepref *r;
+	for (r = keepref; r; r = r->next)
+		prune_ref(r);
+}
+
 int cmd_pack_refs(int argc, const char **argv, const char *prefix)
 {
-	int fd;
+	int fd, i;
+
+	for (i = 1; i < argc; i++) {
+		const char *arg = argv[i];
+		if (!strcmp(arg, "--prune")) {
+			prune = 1;
+			continue;
+		}
+		/* perhaps other parameters later... */
+		break;
+	}
+	if (i != argc)
+		usage(builtin_pack_refs_usage);
 
 	result_path = xstrdup(git_path("packed-refs"));
 	lock_path = xstrdup(mkpath("%s.lock", result_path));
@@ -37,5 +84,7 @@ int cmd_pack_refs(int argc, const char *
 	if (rename(lock_path, result_path) < 0)
 		die("unable to overwrite old ref-pack file (%s)", strerror(errno));
 	lock_path = NULL;
+	if (prune)
+		prune_refs();
 	return 0;
 }

Re: [RFC] git-pack-refs --prune

[Linus Torvalds]

On Mon, 18 Sep 2006, Junio C Hamano wrote:
> 
> I am not sure I got the locking right, hence this RFC.

It looks correct (the important part to check is that the SHA1 of the ref 
you remove still matches the SHA1 of the object you packed).

That said, we should fix it up a bit, notably

 - we should _not_ prune refs that are indirect.

   Right now, if we have a symbolic link, we _incorrectly_ pack it as 
   unlinked. The packed format doesn't have any "link" format.

   This isn't a problem in practice, because the only link we ever use is 
   the HEAD link, but it's incorrect. As long as we don't prune, it wasn't 
   an issue - a unpacked head will always override a packed one, so 
   packing the thing didn't really matter.

 - we should probably avoid even trying to prune stuff that was already 
   packed.

The way to fix both these problems at once would be to add a flag to the 
"for_each_ref()", which says whether it followed a link, or whether it was 
already packed, so that we wouldn't pack symlinks at all, and we wouldn't 
add already-packed refs to the "keeprefs" list.

But that requires a sligh semantic extension to "do_for_each_ref()" (and 
"struct ref_list" needs a flag to say whether it was looked up through a 
symlink).

I was thinking that the easy way to solve it is to just _pack_ everything 
(the way we do now - incorrectly for symrefs), but never prune a symref.

		Linus

Re: [RFC] git-pack-refs --prune

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> The way to fix both these problems at once would be to add a flag to the 
> "for_each_ref()", which says whether it followed a link, or whether it was 
> already packed, so that we wouldn't pack symlinks at all, and we wouldn't 
> add already-packed refs to the "keeprefs" list.
>
> But that requires a sligh semantic extension to "do_for_each_ref()" (and 
> "struct ref_list" needs a flag to say whether it was looked up through a 
> symlink).
>
> I was thinking that the easy way to solve it is to just _pack_ everything 
> (the way we do now - incorrectly for symrefs), but never prune a symref.

I see.  Thanks for pointing out the issue with symrefs.  I think
clone with --use-separate-remote creates remote/$that_repo/HEAD
that points at the branch the remote side's HEAD points at (to
be precise, the one it guessed the remote side's HEAD points
at), so this is a real issue already.

I wanted to update for_each_ref() anyway for other reasons (it
really should take callback data -- the way the current users
use global variables to work this around is eyesore), so
hopefully I'll find time to take a look at it.

Rough outline:

 - for_each_ref() and friends become:

   typedef int each_ref_fn(const char *refname,
			   const unsigned char *sha1,
   #define REF_IS_SYMREF 01
   #define REF_IS_PACKED 02
                           int flags, /* above bits or'ed */ 
                           void *cb_data);
   int for_each_ref(each_ref_fn fn, void *cb_data);

 - handle_one_ref notices a symref and ignores it; it remembers
   refs that are not symref and are still loose for later
   pruning under --prune.

We might want to update the initial handshake of upload-pack
protocol so that peek-remote and fetch-pack can tell which one
is a symref pointing at what.  Do the usual server_capabilities
discovery in connect.c::get_remote_heads(), and if an extension
"symref" is supported than ask for symref information (typically
we would only get "HEAD points at refs/heads/foo" and nothing
else).  Then git-clone.sh does not have to make a guess.  But
that is a separate topic.



 

Re: [RFC] git-pack-refs --prune

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> On Mon, 18 Sep 2006, Junio C Hamano wrote:
>> 
>> I am not sure I got the locking right, hence this RFC.
>
> It looks correct (the important part to check is that the SHA1 of the ref 
> you remove still matches the SHA1 of the object you packed).
>
> That said, we should fix it up a bit, notably
>
>  - we should _not_ prune refs that are indirect.
>
>  - we should probably avoid even trying to prune stuff that was already 
>    packed.
>
> The way to fix both these problems at once would be to add a flag to the 
> "for_each_ref()", which says whether it followed a link, or whether it was 
> already packed, so that we wouldn't pack symlinks at all, and we wouldn't 
> add already-packed refs to the "keeprefs" list.
>
> But that requires a sligh semantic extension to "do_for_each_ref()" (and 
> "struct ref_list" needs a flag to say whether it was looked up through a 
> symlink).

Ok, so I did these and the result is a 4-patch series.

[PATCH 1/5] symbolit-ref: fix resolve_ref conversion.

[Junio C Hamano]

An earlier conversion accidentally hardcoded "HEAD" to be passed to
resolve_ref(), thereby causing git-symbolic-ref command to always
report where the HEAD points at, ignoring the command line parameter.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-symbolic-ref.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-symbolic-ref.c b/builtin-symbolic-ref.c
index 6f18db8..13163ba 100644
--- a/builtin-symbolic-ref.c
+++ b/builtin-symbolic-ref.c
@@ -7,7 +7,7 @@ static const char git_symbolic_ref_usage
 static void check_symref(const char *HEAD)
 {
 	unsigned char sha1[20];
-	const char *refs_heads_master = resolve_ref("HEAD", sha1, 0);
+	const char *refs_heads_master = resolve_ref(HEAD, sha1, 0);
 
 	if (!refs_heads_master)
 		die("No such ref: %s", HEAD);
-- 
1.4.2.1.g4dc7

[PATCH 2/5] Add callback data to for_each_ref() family.

[Junio C Hamano]

This is a long overdue fix to the API for for_each_ref() family
of functions.  It allows the callers to specify a callback data
pointer, so that the caller does not have to use static
variables to communicate with the callback funciton.

The updated for_each_ref() family takes a function of type

	int (*fn)(const char *, const unsigned char *, void *)

and a void pointer as parameters, and calls the function with
the name of the ref and its SHA-1 with the caller-supplied void
pointer as parameters.

The commit updates two callers, builtin-name-rev.c and
builtin-pack-refs.c as an example.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-name-rev.c    |    8 ++++----
 builtin-pack-refs.c   |    8 +++++---
 builtin-prune.c       |    4 ++--
 builtin-push.c        |    4 ++--
 builtin-rev-parse.c   |   10 +++++-----
 builtin-show-branch.c |   26 +++++++++++++-------------
 describe.c            |    4 ++--
 fetch-pack.c          |    8 ++++----
 fetch.c               |    4 ++--
 fsck-objects.c        |    4 ++--
 http-push.c           |    4 ++--
 receive-pack.c        |    6 +++---
 refs.c                |   31 ++++++++++++++++++-------------
 refs.h                |   11 ++++++-----
 revision.c            |    4 ++--
 send-pack.c           |    4 ++--
 server-info.c         |    4 ++--
 upload-pack.c         |    6 +++---
 18 files changed, 79 insertions(+), 71 deletions(-)

diff --git a/builtin-name-rev.c b/builtin-name-rev.c
index 52886b6..9e3e537 100644
--- a/builtin-name-rev.c
+++ b/builtin-name-rev.c
@@ -75,11 +75,10 @@ copy_data:
 	}
 }
 
-static int tags_only;
-
-static int name_ref(const char *path, const unsigned char *sha1)
+static int name_ref(const char *path, const unsigned char *sha1, void *cb_data)
 {
 	struct object *o = parse_object(sha1);
+	int tags_only = *(int*)cb_data;
 	int deref = 0;
 
 	if (tags_only && strncmp(path, "refs/tags/", 10))
@@ -131,6 +130,7 @@ int cmd_name_rev(int argc, const char **
 {
 	struct object_array revs = { 0, 0, NULL };
 	int as_is = 0, all = 0, transform_stdin = 0;
+	int tags_only = 0;
 
 	git_config(git_default_config);
 
@@ -186,7 +186,7 @@ int cmd_name_rev(int argc, const char **
 		add_object_array((struct object *)commit, *argv, &revs);
 	}
 
-	for_each_ref(name_ref);
+	for_each_ref(name_ref, &tags_only);
 
 	if (transform_stdin) {
 		char buffer[2048];
diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index 0f5d827..b3d5470 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -1,7 +1,6 @@
 #include "cache.h"
 #include "refs.h"
 
-static FILE *refs_file;
 static const char *result_path, *lock_path;
 
 static void remove_lock_file(void)
@@ -10,8 +9,10 @@ static void remove_lock_file(void)
 		unlink(lock_path);
 }
 
-static int handle_one_ref(const char *path, const unsigned char *sha1)
+static int handle_one_ref(const char *path, const unsigned char *sha1, void *cb_data)
 {
+	FILE *refs_file = cb_data;
+
 	fprintf(refs_file, "%s %s\n", sha1_to_hex(sha1), path);
 	return 0;
 }
@@ -19,6 +20,7 @@ static int handle_one_ref(const char *pa
 int cmd_pack_refs(int argc, const char **argv, const char *prefix)
 {
 	int fd;
+	FILE *refs_file;
 
 	result_path = xstrdup(git_path("packed-refs"));
 	lock_path = xstrdup(mkpath("%s.lock", result_path));
@@ -31,7 +33,7 @@ int cmd_pack_refs(int argc, const char *
 	refs_file = fdopen(fd, "w");
 	if (!refs_file)
 		die("unable to create ref-pack file structure (%s)", strerror(errno));
-	for_each_ref(handle_one_ref);
+	for_each_ref(handle_one_ref, refs_file);
 	fsync(fd);
 	fclose(refs_file);
 	if (rename(lock_path, result_path) < 0)
diff --git a/builtin-prune.c b/builtin-prune.c
index 6228c79..e21c29b 100644
--- a/builtin-prune.c
+++ b/builtin-prune.c
@@ -174,7 +174,7 @@ static void walk_commit_list(struct rev_
 	}
 }
 
-static int add_one_ref(const char *path, const unsigned char *sha1)
+static int add_one_ref(const char *path, const unsigned char *sha1, void *cb_data)
 {
 	struct object *object = parse_object(sha1);
 	if (!object)
@@ -240,7 +240,7 @@ int cmd_prune(int argc, const char **arg
 	revs.tree_objects = 1;
 
 	/* Add all external refs */
-	for_each_ref(add_one_ref);
+	for_each_ref(add_one_ref, NULL);
 
 	/* Add all refs from the index file */
 	add_cache_refs();
diff --git a/builtin-push.c b/builtin-push.c
index c43f256..88fc8e2 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -27,7 +27,7 @@ static void add_refspec(const char *ref)
 	refspec_nr = nr;
 }
 
-static int expand_one_ref(const char *ref, const unsigned char *sha1)
+static int expand_one_ref(const char *ref, const unsigned char *sha1, void *cb_data)
 {
 	/* Ignore the "refs/" at the beginning of the refname */
 	ref += 5;
@@ -51,7 +51,7 @@ static void expand_refspecs(void)
 	}
 	if (!tags)
 		return;
-	for_each_ref(expand_one_ref);
+	for_each_ref(expand_one_ref, NULL);
 }
 
 static void set_refspecs(const char **refs, int nr)
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index fd3ccc8..c771274 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -137,7 +137,7 @@ static void show_default(void)
 	}
 }
 
-static int show_reference(const char *refname, const unsigned char *sha1)
+static int show_reference(const char *refname, const unsigned char *sha1, void *cb_data)
 {
 	show_rev(NORMAL, sha1, refname);
 	return 0;
@@ -299,19 +299,19 @@ int cmd_rev_parse(int argc, const char *
 				continue;
 			}
 			if (!strcmp(arg, "--all")) {
-				for_each_ref(show_reference);
+				for_each_ref(show_reference, NULL);
 				continue;
 			}
 			if (!strcmp(arg, "--branches")) {
-				for_each_branch_ref(show_reference);
+				for_each_branch_ref(show_reference, NULL);
 				continue;
 			}
 			if (!strcmp(arg, "--tags")) {
-				for_each_tag_ref(show_reference);
+				for_each_tag_ref(show_reference, NULL);
 				continue;
 			}
 			if (!strcmp(arg, "--remotes")) {
-				for_each_remote_ref(show_reference);
+				for_each_remote_ref(show_reference, NULL);
 				continue;
 			}
 			if (!strcmp(arg, "--show-prefix")) {
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 4d8db0c..b3548ae 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -346,7 +346,7 @@ static void sort_ref_range(int bottom, i
 	      compare_ref_name);
 }
 
-static int append_ref(const char *refname, const unsigned char *sha1)
+static int append_ref(const char *refname, const unsigned char *sha1, void *cb_data)
 {
 	struct commit *commit = lookup_commit_reference_gently(sha1, 1);
 	int i;
@@ -369,7 +369,7 @@ static int append_ref(const char *refnam
 	return 0;
 }
 
-static int append_head_ref(const char *refname, const unsigned char *sha1)
+static int append_head_ref(const char *refname, const unsigned char *sha1, void *cb_data)
 {
 	unsigned char tmp[20];
 	int ofs = 11;
@@ -380,14 +380,14 @@ static int append_head_ref(const char *r
 	 */
 	if (get_sha1(refname + ofs, tmp) || hashcmp(tmp, sha1))
 		ofs = 5;
-	return append_ref(refname + ofs, sha1);
+	return append_ref(refname + ofs, sha1, cb_data);
 }
 
-static int append_tag_ref(const char *refname, const unsigned char *sha1)
+static int append_tag_ref(const char *refname, const unsigned char *sha1, void *cb_data)
 {
 	if (strncmp(refname, "refs/tags/", 10))
 		return 0;
-	return append_ref(refname + 5, sha1);
+	return append_ref(refname + 5, sha1, cb_data);
 }
 
 static const char *match_ref_pattern = NULL;
@@ -401,7 +401,7 @@ static int count_slash(const char *s)
 	return cnt;
 }
 
-static int append_matching_ref(const char *refname, const unsigned char *sha1)
+static int append_matching_ref(const char *refname, const unsigned char *sha1, void *cb_data)
 {
 	/* we want to allow pattern hold/<asterisk> to show all
 	 * branches under refs/heads/hold/, and v0.99.9? to show
@@ -417,22 +417,22 @@ static int append_matching_ref(const cha
 	if (fnmatch(match_ref_pattern, tail, 0))
 		return 0;
 	if (!strncmp("refs/heads/", refname, 11))
-		return append_head_ref(refname, sha1);
+		return append_head_ref(refname, sha1, cb_data);
 	if (!strncmp("refs/tags/", refname, 10))
-		return append_tag_ref(refname, sha1);
-	return append_ref(refname, sha1);
+		return append_tag_ref(refname, sha1, cb_data);
+	return append_ref(refname, sha1, cb_data);
 }
 
 static void snarf_refs(int head, int tag)
 {
 	if (head) {
 		int orig_cnt = ref_name_cnt;
-		for_each_ref(append_head_ref);
+		for_each_ref(append_head_ref, NULL);
 		sort_ref_range(orig_cnt, ref_name_cnt);
 	}
 	if (tag) {
 		int orig_cnt = ref_name_cnt;
-		for_each_ref(append_tag_ref);
+		for_each_ref(append_tag_ref, NULL);
 		sort_ref_range(orig_cnt, ref_name_cnt);
 	}
 }
@@ -487,7 +487,7 @@ static void append_one_rev(const char *a
 {
 	unsigned char revkey[20];
 	if (!get_sha1(av, revkey)) {
-		append_ref(av, revkey);
+		append_ref(av, revkey, NULL);
 		return;
 	}
 	if (strchr(av, '*') || strchr(av, '?') || strchr(av, '[')) {
@@ -495,7 +495,7 @@ static void append_one_rev(const char *a
 		int saved_matches = ref_name_cnt;
 		match_ref_pattern = av;
 		match_ref_slash = count_slash(av);
-		for_each_ref(append_matching_ref);
+		for_each_ref(append_matching_ref, NULL);
 		if (saved_matches == ref_name_cnt &&
 		    ref_name_cnt < MAX_REVS)
 			error("no matching refs with %s", av);
diff --git a/describe.c b/describe.c
index ab192f8..ea0f2ce 100644
--- a/describe.c
+++ b/describe.c
@@ -53,7 +53,7 @@ static void add_to_known_names(const cha
 	names = ++idx;
 }
 
-static int get_name(const char *path, const unsigned char *sha1)
+static int get_name(const char *path, const unsigned char *sha1, void *cb_data)
 {
 	struct commit *commit = lookup_commit_reference_gently(sha1, 1);
 	struct object *object;
@@ -113,7 +113,7 @@ static void describe(const char *arg, in
 
 	if (!initialized) {
 		initialized = 1;
-		for_each_ref(get_name);
+		for_each_ref(get_name, NULL);
 		qsort(name_array, names, sizeof(*name_array), compare_names);
 	}
 
diff --git a/fetch-pack.c b/fetch-pack.c
index e8708aa..6264ea1 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -42,7 +42,7 @@ static void rev_list_push(struct commit 
 	}
 }
 
-static int rev_list_insert_ref(const char *path, const unsigned char *sha1)
+static int rev_list_insert_ref(const char *path, const unsigned char *sha1, void *cb_data)
 {
 	struct object *o = deref_tag(parse_object(sha1), path, 0);
 
@@ -143,7 +143,7 @@ static int find_common(int fd[2], unsign
 	unsigned in_vain = 0;
 	int got_continue = 0;
 
-	for_each_ref(rev_list_insert_ref);
+	for_each_ref(rev_list_insert_ref, NULL);
 
 	fetching = 0;
 	for ( ; refs ; refs = refs->next) {
@@ -253,7 +253,7 @@ done:
 
 static struct commit_list *complete;
 
-static int mark_complete(const char *path, const unsigned char *sha1)
+static int mark_complete(const char *path, const unsigned char *sha1, void *cb_data)
 {
 	struct object *o = parse_object(sha1);
 
@@ -365,7 +365,7 @@ static int everything_local(struct ref *
 		}
 	}
 
-	for_each_ref(mark_complete);
+	for_each_ref(mark_complete, NULL);
 	if (cutoff)
 		mark_recent_complete_commits(cutoff);
 
diff --git a/fetch.c b/fetch.c
index 34df8d3..36d1e76 100644
--- a/fetch.c
+++ b/fetch.c
@@ -201,7 +201,7 @@ static int interpret_target(char *target
 	return -1;
 }
 
-static int mark_complete(const char *path, const unsigned char *sha1)
+static int mark_complete(const char *path, const unsigned char *sha1, void *cb_data)
 {
 	struct commit *commit = lookup_commit_reference_gently(sha1, 1);
 	if (commit) {
@@ -274,7 +274,7 @@ int pull(int targets, char **target, con
 	}
 
 	if (!get_recover)
-		for_each_ref(mark_complete);
+		for_each_ref(mark_complete, NULL);
 
 	for (i = 0; i < targets; i++) {
 		if (interpret_target(target[i], &sha1[20 * i])) {
diff --git a/fsck-objects.c b/fsck-objects.c
index 456c17e..bb0c94e 100644
--- a/fsck-objects.c
+++ b/fsck-objects.c
@@ -402,7 +402,7 @@ static void fsck_dir(int i, char *path)
 
 static int default_refs;
 
-static int fsck_handle_ref(const char *refname, const unsigned char *sha1)
+static int fsck_handle_ref(const char *refname, const unsigned char *sha1, void *cb_data)
 {
 	struct object *obj;
 
@@ -424,7 +424,7 @@ static int fsck_handle_ref(const char *r
 
 static void get_default_heads(void)
 {
-	for_each_ref(fsck_handle_ref);
+	for_each_ref(fsck_handle_ref, NULL);
 
 	/*
 	 * Not having any default heads isn't really fatal, but
diff --git a/http-push.c b/http-push.c
index 670ff00..460c9be 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1864,7 +1864,7 @@ static int update_remote(unsigned char *
 static struct ref *local_refs, **local_tail;
 static struct ref *remote_refs, **remote_tail;
 
-static int one_local_ref(const char *refname, const unsigned char *sha1)
+static int one_local_ref(const char *refname, const unsigned char *sha1, void *cb_data)
 {
 	struct ref *ref;
 	int len = strlen(refname) + 1;
@@ -1913,7 +1913,7 @@ static void one_remote_ref(char *refname
 static void get_local_heads(void)
 {
 	local_tail = &local_refs;
-	for_each_ref(one_local_ref);
+	for_each_ref(one_local_ref, NULL);
 }
 
 static void get_dav_remote_heads(void)
diff --git a/receive-pack.c b/receive-pack.c
index 78f75da..7abc921 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -12,7 +12,7 @@ static int report_status;
 static char capabilities[] = "report-status";
 static int capabilities_sent;
 
-static int show_ref(const char *path, const unsigned char *sha1)
+static int show_ref(const char *path, const unsigned char *sha1, void *cb_data)
 {
 	if (capabilities_sent)
 		packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
@@ -25,9 +25,9 @@ static int show_ref(const char *path, co
 
 static void write_head_info(void)
 {
-	for_each_ref(show_ref);
+	for_each_ref(show_ref, NULL);
 	if (!capabilities_sent)
-		show_ref("capabilities^{}", null_sha1);
+		show_ref("capabilities^{}", null_sha1, NULL);
 
 }
 
diff --git a/refs.c b/refs.c
index 7bd36e4..85564f0 100644
--- a/refs.c
+++ b/refs.c
@@ -275,7 +275,7 @@ int read_ref(const char *ref, unsigned c
 	return -1;
 }
 
-static int do_for_each_ref(const char *base, int (*fn)(const char *path, const unsigned char *sha1), int trim)
+static int do_for_each_ref(const char *base, each_ref_fn fn, int trim, void *cb_data)
 {
 	int retval;
 	struct ref_list *packed = get_packed_refs();
@@ -303,7 +303,7 @@ static int do_for_each_ref(const char *b
 			error("%s does not point to a valid object!", entry->name);
 			continue;
 		}
-		retval = fn(entry->name + trim, entry->sha1);
+		retval = fn(entry->name + trim, entry->sha1, cb_data);
 		if (retval)
 			return retval;
 	}
@@ -311,7 +311,7 @@ static int do_for_each_ref(const char *b
 	packed = packed ? packed : loose;
 	while (packed) {
 		if (!strncmp(base, packed->name, trim)) {
-			retval = fn(packed->name + trim, packed->sha1);
+			retval = fn(packed->name + trim, packed->sha1, cb_data);
 			if (retval)
 				return retval;
 		}
@@ -320,34 +320,39 @@ static int do_for_each_ref(const char *b
 	return 0;
 }
 
-int head_ref(int (*fn)(const char *path, const unsigned char *sha1))
+int head_ref(each_ref_fn fn, void *cb_data)
 {
 	unsigned char sha1[20];
 	if (!read_ref("HEAD", sha1))
-		return fn("HEAD", sha1);
+		return fn("HEAD", sha1, cb_data);
 	return 0;
 }
 
-int for_each_ref(int (*fn)(const char *path, const unsigned char *sha1))
+int for_each_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref("refs/", fn, 0);
+	return do_for_each_ref("refs/", fn, 0, cb_data);
 }
 
-int for_each_tag_ref(int (*fn)(const char *path, const unsigned char *sha1))
+int for_each_tag_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref("refs/tags/", fn, 10);
+	return do_for_each_ref("refs/tags/", fn, 10, cb_data);
 }
 
-int for_each_branch_ref(int (*fn)(const char *path, const unsigned char *sha1))
+int for_each_branch_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref("refs/heads/", fn, 11);
+	return do_for_each_ref("refs/heads/", fn, 11, cb_data);
 }
 
-int for_each_remote_ref(int (*fn)(const char *path, const unsigned char *sha1))
+int for_each_remote_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref("refs/remotes/", fn, 13);
+	return do_for_each_ref("refs/remotes/", fn, 13, cb_data);
 }
 
+/* NEEDSWORK: This is only used by ssh-upload and it should go; the
+ * caller should do resolve_ref or read_ref like everybody else.  Or
+ * maybe everybody else should use get_ref_sha1() instead of doing
+ * read_ref().
+ */
 int get_ref_sha1(const char *ref, unsigned char *sha1)
 {
 	if (check_ref_format(ref))
diff --git a/refs.h b/refs.h
index af347e6..886c857 100644
--- a/refs.h
+++ b/refs.h
@@ -14,11 +14,12 @@ struct ref_lock {
  * Calls the specified function for each ref file until it returns nonzero,
  * and returns the value
  */
-extern int head_ref(int (*fn)(const char *path, const unsigned char *sha1));
-extern int for_each_ref(int (*fn)(const char *path, const unsigned char *sha1));
-extern int for_each_tag_ref(int (*fn)(const char *path, const unsigned char *sha1));
-extern int for_each_branch_ref(int (*fn)(const char *path, const unsigned char *sha1));
-extern int for_each_remote_ref(int (*fn)(const char *path, const unsigned char *sha1));
+typedef int each_ref_fn(const char *refname, const unsigned char *sha1, void *cb_data);
+extern int head_ref(each_ref_fn, void *);
+extern int for_each_ref(each_ref_fn, void *);
+extern int for_each_tag_ref(each_ref_fn, void *);
+extern int for_each_branch_ref(each_ref_fn, void *);
+extern int for_each_remote_ref(each_ref_fn, void *);
 
 /** Reads the refs file specified into sha1 **/
 extern int get_ref_sha1(const char *ref, unsigned char *sha1);
diff --git a/revision.c b/revision.c
index 6a2539b..0e84b8a 100644
--- a/revision.c
+++ b/revision.c
@@ -466,7 +466,7 @@ static void limit_list(struct rev_info *
 static int all_flags;
 static struct rev_info *all_revs;
 
-static int handle_one_ref(const char *path, const unsigned char *sha1)
+static int handle_one_ref(const char *path, const unsigned char *sha1, void *cb_data)
 {
 	struct object *object = get_reference(all_revs, path, sha1, all_flags);
 	add_pending_object(all_revs, object, "");
@@ -477,7 +477,7 @@ static void handle_all(struct rev_info *
 {
 	all_revs = revs;
 	all_flags = flags;
-	for_each_ref(handle_one_ref);
+	for_each_ref(handle_one_ref, NULL);
 }
 
 static int add_parents_only(struct rev_info *revs, const char *arg, int flags)
diff --git a/send-pack.c b/send-pack.c
index 5bb123a..ee13093 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -215,7 +215,7 @@ static int ref_newer(const unsigned char
 static struct ref *local_refs, **local_tail;
 static struct ref *remote_refs, **remote_tail;
 
-static int one_local_ref(const char *refname, const unsigned char *sha1)
+static int one_local_ref(const char *refname, const unsigned char *sha1, void *cb_data)
 {
 	struct ref *ref;
 	int len = strlen(refname) + 1;
@@ -230,7 +230,7 @@ static int one_local_ref(const char *ref
 static void get_local_heads(void)
 {
 	local_tail = &local_refs;
-	for_each_ref(one_local_ref);
+	for_each_ref(one_local_ref, NULL);
 }
 
 static int receive_status(int in)
diff --git a/server-info.c b/server-info.c
index 2fb8f57..7667b41 100644
--- a/server-info.c
+++ b/server-info.c
@@ -7,7 +7,7 @@ #include "tag.h"
 /* refs */
 static FILE *info_ref_fp;
 
-static int add_info_ref(const char *path, const unsigned char *sha1)
+static int add_info_ref(const char *path, const unsigned char *sha1, void *cb_data)
 {
 	struct object *o = parse_object(sha1);
 
@@ -34,7 +34,7 @@ static int update_info_refs(int force)
 	info_ref_fp = fopen(path1, "w");
 	if (!info_ref_fp)
 		return error("unable to update %s", path0);
-	for_each_ref(add_info_ref);
+	for_each_ref(add_info_ref, NULL);
 	fclose(info_ref_fp);
 	rename(path1, path0);
 	free(path0);
diff --git a/upload-pack.c b/upload-pack.c
index 189b239..10237eb 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -416,7 +416,7 @@ static void receive_needs(void)
 	}
 }
 
-static int send_ref(const char *refname, const unsigned char *sha1)
+static int send_ref(const char *refname, const unsigned char *sha1, void *cb_data)
 {
 	static const char *capabilities = "multi_ack thin-pack side-band side-band-64k";
 	struct object *o = parse_object(sha1);
@@ -444,8 +444,8 @@ static int send_ref(const char *refname,
 static void upload_pack(void)
 {
 	reset_timeout();
-	head_ref(send_ref);
-	for_each_ref(send_ref);
+	head_ref(send_ref, NULL);
+	for_each_ref(send_ref, NULL);
 	packet_flush(1);
 	receive_needs();
 	if (want_obj.nr) {
-- 
1.4.2.1.g4dc7

[PATCH 3/5] Tell between packed, unpacked and symbolic refs.

[Junio C Hamano]

This adds a "int *flag" parameter to resolve_ref() and makes
for_each_ref() family to call callback function with an extra
"int flag" parameter.  They are used to give two bits of
information (REF_ISSYMREF and REF_ISPACKED) about the ref.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-fmt-merge-msg.c |    2 +-
 builtin-name-rev.c      |    2 +-
 builtin-pack-refs.c     |    3 ++-
 builtin-prune.c         |    2 +-
 builtin-push.c          |    2 +-
 builtin-rev-parse.c     |    2 +-
 builtin-show-branch.c   |   22 +++++++++++-----------
 builtin-symbolic-ref.c  |    6 +++++-
 cache.h                 |    2 +-
 describe.c              |    2 +-
 fetch-pack.c            |    4 ++--
 fetch.c                 |    2 +-
 fsck-objects.c          |    7 ++++---
 http-push.c             |    2 +-
 receive-pack.c          |    4 ++--
 refs.c                  |   44 +++++++++++++++++++++++++++++++-------------
 refs.h                  |    4 +++-
 revision.c              |    2 +-
 send-pack.c             |    2 +-
 server-info.c           |    2 +-
 sha1_name.c             |    2 +-
 upload-pack.c           |    2 +-
 wt-status.c             |    2 +-
 23 files changed, 75 insertions(+), 49 deletions(-)

diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index b93c17c..3d3097d 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -277,7 +277,7 @@ int cmd_fmt_merge_msg(int argc, const ch
 		usage(fmt_merge_msg_usage);
 
 	/* get current branch */
-	current_branch = resolve_ref("HEAD", head_sha1, 1);
+	current_branch = resolve_ref("HEAD", head_sha1, 1, NULL);
 	if (!strncmp(current_branch, "refs/heads/", 11))
 		current_branch += 11;
 
diff --git a/builtin-name-rev.c b/builtin-name-rev.c
index 9e3e537..618aa31 100644
--- a/builtin-name-rev.c
+++ b/builtin-name-rev.c
@@ -75,7 +75,7 @@ copy_data:
 	}
 }
 
-static int name_ref(const char *path, const unsigned char *sha1, void *cb_data)
+static int name_ref(const char *path, const unsigned char *sha1, int flags, void *cb_data)
 {
 	struct object *o = parse_object(sha1);
 	int tags_only = *(int*)cb_data;
diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index b3d5470..9871089 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -9,7 +9,8 @@ static void remove_lock_file(void)
 		unlink(lock_path);
 }
 
-static int handle_one_ref(const char *path, const unsigned char *sha1, void *cb_data)
+static int handle_one_ref(const char *path, const unsigned char *sha1,
+			  int flags, void *cb_data)
 {
 	FILE *refs_file = cb_data;
 
diff --git a/builtin-prune.c b/builtin-prune.c
index e21c29b..e79b515 100644
--- a/builtin-prune.c
+++ b/builtin-prune.c
@@ -174,7 +174,7 @@ static void walk_commit_list(struct rev_
 	}
 }
 
-static int add_one_ref(const char *path, const unsigned char *sha1, void *cb_data)
+static int add_one_ref(const char *path, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct object *object = parse_object(sha1);
 	if (!object)
diff --git a/builtin-push.c b/builtin-push.c
index 88fc8e2..581c44b 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -27,7 +27,7 @@ static void add_refspec(const char *ref)
 	refspec_nr = nr;
 }
 
-static int expand_one_ref(const char *ref, const unsigned char *sha1, void *cb_data)
+static int expand_one_ref(const char *ref, const unsigned char *sha1, int flag, void *cb_data)
 {
 	/* Ignore the "refs/" at the beginning of the refname */
 	ref += 5;
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index c771274..3b716fb 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -137,7 +137,7 @@ static void show_default(void)
 	}
 }
 
-static int show_reference(const char *refname, const unsigned char *sha1, void *cb_data)
+static int show_reference(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	show_rev(NORMAL, sha1, refname);
 	return 0;
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index b3548ae..5d6ce56 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -346,7 +346,7 @@ static void sort_ref_range(int bottom, i
 	      compare_ref_name);
 }
 
-static int append_ref(const char *refname, const unsigned char *sha1, void *cb_data)
+static int append_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct commit *commit = lookup_commit_reference_gently(sha1, 1);
 	int i;
@@ -369,7 +369,7 @@ static int append_ref(const char *refnam
 	return 0;
 }
 
-static int append_head_ref(const char *refname, const unsigned char *sha1, void *cb_data)
+static int append_head_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	unsigned char tmp[20];
 	int ofs = 11;
@@ -380,14 +380,14 @@ static int append_head_ref(const char *r
 	 */
 	if (get_sha1(refname + ofs, tmp) || hashcmp(tmp, sha1))
 		ofs = 5;
-	return append_ref(refname + ofs, sha1, cb_data);
+	return append_ref(refname + ofs, sha1, flag, cb_data);
 }
 
-static int append_tag_ref(const char *refname, const unsigned char *sha1, void *cb_data)
+static int append_tag_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	if (strncmp(refname, "refs/tags/", 10))
 		return 0;
-	return append_ref(refname + 5, sha1, cb_data);
+	return append_ref(refname + 5, sha1, flag, cb_data);
 }
 
 static const char *match_ref_pattern = NULL;
@@ -401,7 +401,7 @@ static int count_slash(const char *s)
 	return cnt;
 }
 
-static int append_matching_ref(const char *refname, const unsigned char *sha1, void *cb_data)
+static int append_matching_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	/* we want to allow pattern hold/<asterisk> to show all
 	 * branches under refs/heads/hold/, and v0.99.9? to show
@@ -417,10 +417,10 @@ static int append_matching_ref(const cha
 	if (fnmatch(match_ref_pattern, tail, 0))
 		return 0;
 	if (!strncmp("refs/heads/", refname, 11))
-		return append_head_ref(refname, sha1, cb_data);
+		return append_head_ref(refname, sha1, flag, cb_data);
 	if (!strncmp("refs/tags/", refname, 10))
-		return append_tag_ref(refname, sha1, cb_data);
-	return append_ref(refname, sha1, cb_data);
+		return append_tag_ref(refname, sha1, flag, cb_data);
+	return append_ref(refname, sha1, flag, cb_data);
 }
 
 static void snarf_refs(int head, int tag)
@@ -487,7 +487,7 @@ static void append_one_rev(const char *a
 {
 	unsigned char revkey[20];
 	if (!get_sha1(av, revkey)) {
-		append_ref(av, revkey, NULL);
+		append_ref(av, revkey, 0, NULL);
 		return;
 	}
 	if (strchr(av, '*') || strchr(av, '?') || strchr(av, '[')) {
@@ -630,7 +630,7 @@ int cmd_show_branch(int ac, const char *
 		ac--; av++;
 	}
 
-	head_p = resolve_ref("HEAD", head_sha1, 1);
+	head_p = resolve_ref("HEAD", head_sha1, 1, NULL);
 	if (head_p) {
 		head_len = strlen(head_p);
 		memcpy(head, head_p, head_len + 1);
diff --git a/builtin-symbolic-ref.c b/builtin-symbolic-ref.c
index 13163ba..d8be052 100644
--- a/builtin-symbolic-ref.c
+++ b/builtin-symbolic-ref.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "cache.h"
+#include "refs.h"
 
 static const char git_symbolic_ref_usage[] =
 "git-symbolic-ref name [ref]";
@@ -7,10 +8,13 @@ static const char git_symbolic_ref_usage
 static void check_symref(const char *HEAD)
 {
 	unsigned char sha1[20];
-	const char *refs_heads_master = resolve_ref(HEAD, sha1, 0);
+	int flag;
+	const char *refs_heads_master = resolve_ref(HEAD, sha1, 0, &flag);
 
 	if (!refs_heads_master)
 		die("No such ref: %s", HEAD);
+	else if (!(flag & REF_ISSYMREF))
+		die("ref %s is not a symbolic ref", HEAD);
 	puts(refs_heads_master);
 }
 
diff --git a/cache.h b/cache.h
index 282eed6..6def155 100644
--- a/cache.h
+++ b/cache.h
@@ -286,7 +286,7 @@ extern int get_sha1(const char *str, uns
 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
 extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
 extern int read_ref(const char *filename, unsigned char *sha1);
-extern const char *resolve_ref(const char *path, unsigned char *sha1, int);
+extern const char *resolve_ref(const char *path, unsigned char *sha1, int, int *);
 extern int create_symref(const char *ref, const char *refs_heads_master);
 extern int validate_symref(const char *ref);
 
diff --git a/describe.c b/describe.c
index ea0f2ce..f4029ee 100644
--- a/describe.c
+++ b/describe.c
@@ -53,7 +53,7 @@ static void add_to_known_names(const cha
 	names = ++idx;
 }
 
-static int get_name(const char *path, const unsigned char *sha1, void *cb_data)
+static int get_name(const char *path, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct commit *commit = lookup_commit_reference_gently(sha1, 1);
 	struct object *object;
diff --git a/fetch-pack.c b/fetch-pack.c
index 6264ea1..99ac08b 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -42,7 +42,7 @@ static void rev_list_push(struct commit 
 	}
 }
 
-static int rev_list_insert_ref(const char *path, const unsigned char *sha1, void *cb_data)
+static int rev_list_insert_ref(const char *path, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct object *o = deref_tag(parse_object(sha1), path, 0);
 
@@ -253,7 +253,7 @@ done:
 
 static struct commit_list *complete;
 
-static int mark_complete(const char *path, const unsigned char *sha1, void *cb_data)
+static int mark_complete(const char *path, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct object *o = parse_object(sha1);
 
diff --git a/fetch.c b/fetch.c
index 36d1e76..a2cbdfb 100644
--- a/fetch.c
+++ b/fetch.c
@@ -201,7 +201,7 @@ static int interpret_target(char *target
 	return -1;
 }
 
-static int mark_complete(const char *path, const unsigned char *sha1, void *cb_data)
+static int mark_complete(const char *path, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct commit *commit = lookup_commit_reference_gently(sha1, 1);
 	if (commit) {
diff --git a/fsck-objects.c b/fsck-objects.c
index bb0c94e..46b628c 100644
--- a/fsck-objects.c
+++ b/fsck-objects.c
@@ -402,7 +402,7 @@ static void fsck_dir(int i, char *path)
 
 static int default_refs;
 
-static int fsck_handle_ref(const char *refname, const unsigned char *sha1, void *cb_data)
+static int fsck_handle_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct object *obj;
 
@@ -458,9 +458,10 @@ static void fsck_object_dir(const char *
 static int fsck_head_link(void)
 {
 	unsigned char sha1[20];
-	const char *head_points_at = resolve_ref("HEAD", sha1, 1);
+	int flag;
+	const char *head_points_at = resolve_ref("HEAD", sha1, 1, &flag);
 
-	if (!head_points_at)
+	if (!head_points_at || !(flag & REF_ISSYMREF))
 		return error("HEAD is not a symbolic ref");
 	if (strncmp(head_points_at, "refs/heads/", 11))
 		return error("HEAD points to something strange (%s)",
diff --git a/http-push.c b/http-push.c
index 460c9be..ecefdfd 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1864,7 +1864,7 @@ static int update_remote(unsigned char *
 static struct ref *local_refs, **local_tail;
 static struct ref *remote_refs, **remote_tail;
 
-static int one_local_ref(const char *refname, const unsigned char *sha1, void *cb_data)
+static int one_local_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct ref *ref;
 	int len = strlen(refname) + 1;
diff --git a/receive-pack.c b/receive-pack.c
index 7abc921..abbcb6a 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -12,7 +12,7 @@ static int report_status;
 static char capabilities[] = "report-status";
 static int capabilities_sent;
 
-static int show_ref(const char *path, const unsigned char *sha1, void *cb_data)
+static int show_ref(const char *path, const unsigned char *sha1, int flag, void *cb_data)
 {
 	if (capabilities_sent)
 		packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
@@ -27,7 +27,7 @@ static void write_head_info(void)
 {
 	for_each_ref(show_ref, NULL);
 	if (!capabilities_sent)
-		show_ref("capabilities^{}", null_sha1, NULL);
+		show_ref("capabilities^{}", null_sha1, 0, NULL);
 
 }
 
diff --git a/refs.c b/refs.c
index 85564f0..40f16af 100644
--- a/refs.c
+++ b/refs.c
@@ -5,6 +5,7 @@ #include <errno.h>
 
 struct ref_list {
 	struct ref_list *next;
+	unsigned char flag; /* ISSYMREF? ISPACKED? */
 	unsigned char sha1[20];
 	char name[FLEX_ARRAY];
 };
@@ -36,7 +37,8 @@ static const char *parse_ref_line(char *
 	return line;
 }
 
-static struct ref_list *add_ref(const char *name, const unsigned char *sha1, struct ref_list *list)
+static struct ref_list *add_ref(const char *name, const unsigned char *sha1,
+				int flag, struct ref_list *list)
 {
 	int len;
 	struct ref_list **p = &list, *entry;
@@ -58,6 +60,7 @@ static struct ref_list *add_ref(const ch
 	entry = xmalloc(sizeof(struct ref_list) + len);
 	hashcpy(entry->sha1, sha1);
 	memcpy(entry->name, name, len);
+	entry->flag = flag;
 	entry->next = *p;
 	*p = entry;
 	return list;
@@ -78,7 +81,7 @@ static struct ref_list *get_packed_refs(
 				const char *name = parse_ref_line(refline, sha1);
 				if (!name)
 					continue;
-				list = add_ref(name, sha1, list);
+				list = add_ref(name, sha1, REF_ISPACKED, list);
 			}
 			fclose(f);
 			refs = list;
@@ -104,6 +107,7 @@ static struct ref_list *get_ref_dir(cons
 		while ((de = readdir(dir)) != NULL) {
 			unsigned char sha1[20];
 			struct stat st;
+			int flag;
 			int namelen;
 
 			if (de->d_name[0] == '.')
@@ -120,11 +124,11 @@ static struct ref_list *get_ref_dir(cons
 				list = get_ref_dir(ref, list);
 				continue;
 			}
-			if (read_ref(ref, sha1) < 0) {
+			if (!resolve_ref(ref, sha1, 1, &flag)) {
 				error("%s points nowhere!", ref);
 				continue;
 			}
-			list = add_ref(ref, sha1, list);
+			list = add_ref(ref, sha1, flag, list);
 		}
 		free(ref);
 		closedir(dir);
@@ -147,12 +151,15 @@ static struct ref_list *get_loose_refs(v
 /* We allow "recursive" symbolic refs. Only within reason, though */
 #define MAXDEPTH 5
 
-const char *resolve_ref(const char *ref, unsigned char *sha1, int reading)
+const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *flag)
 {
 	int depth = MAXDEPTH, len;
 	char buffer[256];
 	static char ref_buffer[256];
 
+	if (flag)
+		*flag = 0;
+
 	for (;;) {
 		const char *path = git_path("%s", ref);
 		struct stat st;
@@ -174,6 +181,8 @@ const char *resolve_ref(const char *ref,
 			while (list) {
 				if (!strcmp(ref, list->name)) {
 					hashcpy(sha1, list->sha1);
+					if (flag)
+						*flag |= REF_ISPACKED;
 					return ref;
 				}
 				list = list->next;
@@ -191,6 +200,8 @@ const char *resolve_ref(const char *ref,
 				buffer[len] = 0;
 				strcpy(ref_buffer, buffer);
 				ref = ref_buffer;
+				if (flag)
+					*flag |= REF_ISSYMREF;
 				continue;
 			}
 		}
@@ -219,6 +230,8 @@ const char *resolve_ref(const char *ref,
 		buf[len] = 0;
 		memcpy(ref_buffer, buf, len + 1);
 		ref = ref_buffer;
+		if (flag)
+			*flag |= REF_ISSYMREF;
 	}
 	if (len < 40 || get_sha1_hex(buffer, sha1))
 		return NULL;
@@ -270,12 +283,13 @@ #endif
 
 int read_ref(const char *ref, unsigned char *sha1)
 {
-	if (resolve_ref(ref, sha1, 1))
+	if (resolve_ref(ref, sha1, 1, NULL))
 		return 0;
 	return -1;
 }
 
-static int do_for_each_ref(const char *base, each_ref_fn fn, int trim, void *cb_data)
+static int do_for_each_ref(const char *base, each_ref_fn fn, int trim,
+			   void *cb_data)
 {
 	int retval;
 	struct ref_list *packed = get_packed_refs();
@@ -303,7 +317,8 @@ static int do_for_each_ref(const char *b
 			error("%s does not point to a valid object!", entry->name);
 			continue;
 		}
-		retval = fn(entry->name + trim, entry->sha1, cb_data);
+		retval = fn(entry->name + trim, entry->sha1,
+			    entry->flag, cb_data);
 		if (retval)
 			return retval;
 	}
@@ -311,7 +326,8 @@ static int do_for_each_ref(const char *b
 	packed = packed ? packed : loose;
 	while (packed) {
 		if (!strncmp(base, packed->name, trim)) {
-			retval = fn(packed->name + trim, packed->sha1, cb_data);
+			retval = fn(packed->name + trim, packed->sha1,
+				    packed->flag, cb_data);
 			if (retval)
 				return retval;
 		}
@@ -323,8 +339,10 @@ static int do_for_each_ref(const char *b
 int head_ref(each_ref_fn fn, void *cb_data)
 {
 	unsigned char sha1[20];
-	if (!read_ref("HEAD", sha1))
-		return fn("HEAD", sha1, cb_data);
+	int flag;
+
+	if (resolve_ref("HEAD", sha1, 1, &flag))
+		return fn("HEAD", sha1, flag, cb_data);
 	return 0;
 }
 
@@ -415,7 +433,7 @@ int check_ref_format(const char *ref)
 static struct ref_lock *verify_lock(struct ref_lock *lock,
 	const unsigned char *old_sha1, int mustexist)
 {
-	if (!resolve_ref(lock->ref_name, lock->old_sha1, mustexist)) {
+	if (!resolve_ref(lock->ref_name, lock->old_sha1, mustexist, NULL)) {
 		error("Can't verify ref %s", lock->ref_name);
 		unlock_ref(lock);
 		return NULL;
@@ -441,7 +459,7 @@ static struct ref_lock *lock_ref_sha1_ba
 	lock = xcalloc(1, sizeof(struct ref_lock));
 	lock->lock_fd = -1;
 
-	ref = resolve_ref(ref, lock->old_sha1, mustexist);
+	ref = resolve_ref(ref, lock->old_sha1, mustexist, NULL);
 	if (!ref) {
 		int last_errno = errno;
 		error("unable to resolve reference %s: %s",
diff --git a/refs.h b/refs.h
index 886c857..305d408 100644
--- a/refs.h
+++ b/refs.h
@@ -14,7 +14,9 @@ struct ref_lock {
  * Calls the specified function for each ref file until it returns nonzero,
  * and returns the value
  */
-typedef int each_ref_fn(const char *refname, const unsigned char *sha1, void *cb_data);
+#define REF_ISSYMREF 01
+#define REF_ISPACKED 02
+typedef int each_ref_fn(const char *refname, const unsigned char *sha1, int flags, void *cb_data);
 extern int head_ref(each_ref_fn, void *);
 extern int for_each_ref(each_ref_fn, void *);
 extern int for_each_tag_ref(each_ref_fn, void *);
diff --git a/revision.c b/revision.c
index 0e84b8a..cb13b90 100644
--- a/revision.c
+++ b/revision.c
@@ -466,7 +466,7 @@ static void limit_list(struct rev_info *
 static int all_flags;
 static struct rev_info *all_revs;
 
-static int handle_one_ref(const char *path, const unsigned char *sha1, void *cb_data)
+static int handle_one_ref(const char *path, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct object *object = get_reference(all_revs, path, sha1, all_flags);
 	add_pending_object(all_revs, object, "");
diff --git a/send-pack.c b/send-pack.c
index ee13093..fbd792c 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -215,7 +215,7 @@ static int ref_newer(const unsigned char
 static struct ref *local_refs, **local_tail;
 static struct ref *remote_refs, **remote_tail;
 
-static int one_local_ref(const char *refname, const unsigned char *sha1, void *cb_data)
+static int one_local_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct ref *ref;
 	int len = strlen(refname) + 1;
diff --git a/server-info.c b/server-info.c
index 7667b41..6cd38be 100644
--- a/server-info.c
+++ b/server-info.c
@@ -7,7 +7,7 @@ #include "tag.h"
 /* refs */
 static FILE *info_ref_fp;
 
-static int add_info_ref(const char *path, const unsigned char *sha1, void *cb_data)
+static int add_info_ref(const char *path, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct object *o = parse_object(sha1);
 
diff --git a/sha1_name.c b/sha1_name.c
index b497528..84d24c6 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -276,7 +276,7 @@ static int get_sha1_basic(const char *st
 
 	for (p = fmt; *p; p++) {
 		this_result = refs_found ? sha1_from_ref : sha1;
-		ref = resolve_ref(mkpath(*p, len, str), this_result, 1);
+		ref = resolve_ref(mkpath(*p, len, str), this_result, 1, NULL);
 		if (ref) {
 			if (!refs_found++)
 				real_ref = xstrdup(ref);
diff --git a/upload-pack.c b/upload-pack.c
index 10237eb..9412a9b 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -416,7 +416,7 @@ static void receive_needs(void)
 	}
 }
 
-static int send_ref(const char *refname, const unsigned char *sha1, void *cb_data)
+static int send_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	static const char *capabilities = "multi_ack thin-pack side-band side-band-64k";
 	struct object *o = parse_object(sha1);
diff --git a/wt-status.c b/wt-status.c
index 050922d..d8e284c 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -41,7 +41,7 @@ void wt_status_prepare(struct wt_status 
 
 	s->is_initial = get_sha1("HEAD", sha1) ? 1 : 0;
 
-	head = resolve_ref("HEAD", sha1, 0);
+	head = resolve_ref("HEAD", sha1, 0, NULL);
 	s->branch = head ? xstrdup(head) : NULL;
 
 	s->reference = "HEAD";
-- 
1.4.2.1.g4dc7

[PATCH 4/5] pack-refs: do not pack symbolic refs.

[Junio C Hamano]

Now we can tell which one is symbolic and which one is not, it
is easy to do so.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-pack-refs.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index 9871089..0fc8a55 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -14,7 +14,9 @@ static int handle_one_ref(const char *pa
 {
 	FILE *refs_file = cb_data;
 
-	fprintf(refs_file, "%s %s\n", sha1_to_hex(sha1), path);
+	/* Do not pack the symbolic refs */
+	if (!(flags & REF_ISSYMREF))
+		fprintf(refs_file, "%s %s\n", sha1_to_hex(sha1), path);
 	return 0;
 }
 
-- 
1.4.2.1.g4dc7

[PATCH 5/5] git-pack-refs --prune

[Junio C Hamano]

"git pack-refs --prune", after successfully packing the existing
refs, removes the loose ref files.  It tries to protect against
race by doing the usual lock_ref_sha1() which makes sure the
contents of the ref has not changed since we last looked at.

Also we do not bother trying to prune what was already packed, and
we do not try pruning symbolic refs.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-pack-refs.c |   84 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 75 insertions(+), 9 deletions(-)

diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index 0fc8a55..246dd63 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -2,6 +2,20 @@ #include "cache.h"
 #include "refs.h"
 
 static const char *result_path, *lock_path;
+static const char builtin_pack_refs_usage[] =
+"git-pack-refs [--prune]";
+
+struct ref_to_prune {
+	struct ref_to_prune *next;
+	unsigned char sha1[20];
+	char name[FLEX_ARRAY];
+};
+
+struct pack_refs_cb_data {
+	int prune;
+	struct ref_to_prune *ref_to_prune;
+	FILE *refs_file;
+};
 
 static void remove_lock_file(void)
 {
@@ -9,21 +23,70 @@ static void remove_lock_file(void)
 		unlink(lock_path);
 }
 
+static int do_not_prune(int flags)
+{
+	/* If it is already packed or if it is a symref,
+	 * do not prune it.
+	 */
+	return (flags & (REF_ISSYMREF|REF_ISPACKED));
+}
+
 static int handle_one_ref(const char *path, const unsigned char *sha1,
 			  int flags, void *cb_data)
 {
-	FILE *refs_file = cb_data;
+	struct pack_refs_cb_data *cb = cb_data;
 
 	/* Do not pack the symbolic refs */
 	if (!(flags & REF_ISSYMREF))
-		fprintf(refs_file, "%s %s\n", sha1_to_hex(sha1), path);
+		fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), path);
+	if (cb->prune && !do_not_prune(flags)) {
+		int namelen = strlen(path) + 1;
+		struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);
+		hashcpy(n->sha1, sha1);
+		strcpy(n->name, path);
+		n->next = cb->ref_to_prune;
+		cb->ref_to_prune = n;
+	}
 	return 0;
 }
 
+/* make sure nobody touched the ref, and unlink */
+static void prune_ref(struct ref_to_prune *r)
+{
+	struct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1, 1);
+
+	if (lock) {
+		unlink(git_path(r->name));
+		unlock_ref(lock);
+	}
+}
+
+static void prune_refs(struct ref_to_prune *r)
+{
+	while (r) {
+		prune_ref(r);
+		r = r->next;
+	}
+}
+
 int cmd_pack_refs(int argc, const char **argv, const char *prefix)
 {
-	int fd;
-	FILE *refs_file;
+	int fd, i;
+	struct pack_refs_cb_data cbdata;
+
+	memset(&cbdata, 0, sizeof(cbdata));
+
+	for (i = 1; i < argc; i++) {
+		const char *arg = argv[i];
+		if (!strcmp(arg, "--prune")) {
+			cbdata.prune = 1;
+			continue;
+		}
+		/* perhaps other parameters later... */
+		break;
+	}
+	if (i != argc)
+		usage(builtin_pack_refs_usage);
 
 	result_path = xstrdup(git_path("packed-refs"));
 	lock_path = xstrdup(mkpath("%s.lock", result_path));
@@ -33,14 +96,17 @@ int cmd_pack_refs(int argc, const char *
 		die("unable to create new ref-pack file (%s)", strerror(errno));
 	atexit(remove_lock_file);
 
-	refs_file = fdopen(fd, "w");
-	if (!refs_file)
-		die("unable to create ref-pack file structure (%s)", strerror(errno));
-	for_each_ref(handle_one_ref, refs_file);
+	cbdata.refs_file = fdopen(fd, "w");
+	if (!cbdata.refs_file)
+		die("unable to create ref-pack file structure (%s)",
+		    strerror(errno));
+	for_each_ref(handle_one_ref, &cbdata);
 	fsync(fd);
-	fclose(refs_file);
+	fclose(cbdata.refs_file);
 	if (rename(lock_path, result_path) < 0)
 		die("unable to overwrite old ref-pack file (%s)", strerror(errno));
 	lock_path = NULL;
+	if (cbdata.prune)
+		prune_refs(cbdata.ref_to_prune);
 	return 0;
 }
-- 
1.4.2.1.g4dc7

Re: [RFC] git-pack-refs --prune

[Linus Torvalds]

On Thu, 21 Sep 2006, Junio C Hamano wrote:
> 
> Ok, so I did these and the result is a 4-patch series.

Looks good to me. I would have combined 2/3, since the bulk of them is the 
calling conversion change, and they both add a new argument to the same 
function, so combining them would make just one patch that isn't even 
noticeably larger than either of the two originals, but that's just a 
small nitpick.

Having callback data is clearly good, and the "flags" may end up being 
useful for other things in the future too (ie if we add attributes to 
branches, we could at some point have "hidden" and "read-only" etc flags)

		Linus

Re: [RFC] git-pack-refs --prune

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> On Thu, 21 Sep 2006, Junio C Hamano wrote:
>> 
>> Ok, so I did these and the result is a 4-patch series.
>
> Looks good to me. I would have combined 2/3, since the bulk of them is the 
> calling conversion change, and they both add a new argument to the same 
> function, so combining them would make just one patch that isn't even 
> noticeably larger than either of the two originals, but that's just a 
> small nitpick.

Actually I did these two as a single change and then later split
them.  Unfortunately cherry-picking only 3 without 2 would not
have worked anyway in this particular case with git, but an
ideal SCM would have done a better job.  It's just taste and
principle and splitting them into two would not have any
practical advantage in the real world.

Side note to interested bystanders.

Patch 2 changes function signature of for_each_ref() and
friends and adds one parameter, void *cbdata, to them and the
called-back functions.  Typical changes looked like these:

+typedef int each_ref_fn(const char *refname, const unsigned char *sha1, void *cb_data);
-static int name_ref(const char *path, const unsigned char *sha1)
+static int name_ref(const char *path, const unsigned char *sha1, void *cb_data)
-	for_each_ref(name_ref);
+	for_each_ref(name_ref, &tags_only);

Then patch 3 adds another parameter, int flags, to the same functions involved:

-typedef int each_ref_fn(const char *refname, const unsigned char *sha1, void *cb_data);
+#define REF_ISSYMREF 01
+#define REF_ISPACKED 02
+typedef int each_ref_fn(const char *refname, const unsigned char *sha1, int flags, void *cb_data);
-static int name_ref(const char *path, const unsigned char *sha1, void *cb_data)
+static int name_ref(const char *path, const unsigned char *sha1, int flags, void *cb_data)

An ideal SCM should help the user who wants to cherry pick patch
3 without patch 2, effectively applying a patch like this:

* Type definition of for_each_ref() callback function (with
  flags but without callback data):

+#define REF_ISSYMREF 01
+#define REF_ISPACKED 02
+typedef int each_ref_fn(const char *refname, const unsigned char *sha1, int flags);

* An actual callback function:

-static int name_ref(const char *path, const unsigned char *sha1)
+static int name_ref(const char *path, const unsigned char *sha1, int flags)

* The call-site of for_each_ref (note patch2 and patch2+patch3
  have the same effect):

-	for_each_ref(name_ref);
+	for_each_ref(name_ref, &tags_only);