git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 00/21] git remote: set-head and new show output
@ 2009-02-25  8:32 Jay Soffian
  2009-02-25  8:32 ` [PATCH 01/21] test scripts: refactor start_httpd helper Jay Soffian
                   ` (25 more replies)
  0 siblings, 26 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

This series replaces three related topics from pu:

  js/remote-set-head
  jk/head-lookup
  js/remote-display

It is based on master. I re-ordered the original commits such that all
the refactoring outside of builtin-remote is done first, followed by
some small cleanups of builtin-remote itself, and finally ending with
the new builtin-remote functionality (set-head) and changes to its
"show" output. I think it is easier to review this way and the history
will be cleaner.

The end result is largely unchanged from what is currently in pu, but
the intermediate results obviously differ to account for the
re-ordering. I've verified that each intermediate result compiles and
passes t5505-remote.sh. I also ran the full test-suite after the first
group of refactoring, and again at the end.

I've diffed each of the changed files at the end of this series against
what is currently in pu to verify I didn't miss anything and I saw
nothing of note.

So I think this series is clean, and doesn't need an extensive
re-review, but a quick look-over would be appreciated.

I signed off on Jeff's patches; please remove my SoB from those if it is
inappropriate for me to have done so.

Thanks,

j.

Jay Soffian (17):
  move duplicated get_local_heads() to remote.c
  move duplicated ref_newer() to remote.c
  move locate_head() to remote.c
  remote: simplify guess_remote_head()
  remote: let guess_remote_head() optionally return all matches
  remote: make match_refs() copy src ref before assigning to peer_ref
  remote: make match_refs() not short-circuit
  string-list: new for_each_string_list() function
  builtin-remote: refactor duplicated cleanup code
  builtin-remote: remove unused code in get_ref_states
  builtin-remote: rename variables and eliminate redundant function call
  builtin-remote: make get_remote_ref_states() always populate states.tracked
  builtin-remote: fix two inconsistencies in the output of "show <remote>"
  builtin-remote: teach show to display remote HEAD
  builtin-remote: add set-head subcommand
  builtin-remote: new show output style
  builtin-remote: new show output style for push refspecs

Jeff King (4):
  test scripts: refactor start_httpd helper
  add basic http clone/fetch tests
  refactor find_ref_by_name() to accept const list
  remote: make guess_remote_head() use exact HEAD lookup if it is available

 Documentation/git-remote.txt           |   28 ++-
 Makefile                               |    1 +
 builtin-clone.c                        |   41 +---
 builtin-remote.c                       |  563 ++++++++++++++++++++++++++------
 builtin-send-pack.c                    |   79 +-----
 cache.h                                |    2 +-
 contrib/completion/git-completion.bash |    2 +-
 http-push.c                            |   72 +----
 refs.c                                 |    4 +-
 remote.c                               |  136 ++++++++-
 remote.h                               |   12 +
 string-list.c                          |   10 +
 string-list.h                          |    5 +
 t/lib-httpd.sh                         |    9 +-
 t/t5505-remote.sh                      |  114 +++++--
 t/t5540-http-push.sh                   |    9 +-
 t/t5550-http-fetch.sh                  |   57 ++++
 17 files changed, 818 insertions(+), 326 deletions(-)
 create mode 100755 t/t5550-http-fetch.sh

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 01/21] test scripts: refactor start_httpd helper
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 02/21] add basic http clone/fetch tests Jay Soffian
                   ` (24 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

From: Jeff King <peff@peff.net>

There are some redirects and some error checking that need
to be done by the caller; let's move both into the
start_httpd function so that all callers don't have to
repeat them (there is only one caller now, but another will
follow in this series).

This doesn't violate any assumptions that aren't already
being made by lib-httpd, which is happy to say "skipping"
and call test_done for a number of other cases.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 t/lib-httpd.sh       |    9 +++++++--
 t/t5540-http-push.sh |    8 +-------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 3824020..88cfc51 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -82,13 +82,18 @@ prepare_httpd() {
 }
 
 start_httpd() {
-	prepare_httpd
+	prepare_httpd >&3 2>&4
 
 	trap 'stop_httpd; die' EXIT
 
 	"$LIB_HTTPD_PATH" -d "$HTTPD_ROOT_PATH" \
 		-f "$TEST_PATH/apache.conf" $HTTPD_PARA \
-		-c "Listen 127.0.0.1:$LIB_HTTPD_PORT" -k start
+		-c "Listen 127.0.0.1:$LIB_HTTPD_PORT" -k start \
+		>&3 2>&4
+	if ! test $? = 0; then
+		say "skipping test, web server setup failed"
+		test_done
+	fi
 }
 
 stop_httpd() {
diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index 11b3432..57a4411 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh
@@ -20,13 +20,7 @@ then
 fi
 
 . "$TEST_DIRECTORY"/lib-httpd.sh
-
-if ! start_httpd >&3 2>&4
-then
-	say "skipping test, web server setup failed"
-	test_done
-	exit
-fi
+start_httpd
 
 test_expect_success 'setup remote repository' '
 	cd "$ROOT_PATH" &&
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 02/21] add basic http clone/fetch tests
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
  2009-02-25  8:32 ` [PATCH 01/21] test scripts: refactor start_httpd helper Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 03/21] refactor find_ref_by_name() to accept const list Jay Soffian
                   ` (23 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

From: Jeff King <peff@peff.net>

This was mostly being tested implicitly by the "http push"
tests. But making a separate test script means that:

  - we will run fetch tests even when http pushing support
    is not built

  - when there are failures on fetching, they are easier to
    see and isolate, as they are not in the middle of push
    tests

This script defaults to running the webserver on port 5550,
and puts the original t5540 on port 5540, so that the two
can be run simultaneously without conflict (but both still
respect an externally set LIB_HTTPD_PORT).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 Makefile              |    1 +
 t/t5540-http-push.sh  |    1 +
 t/t5550-http-fetch.sh |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 0 deletions(-)
 create mode 100755 t/t5550-http-fetch.sh

diff --git a/Makefile b/Makefile
index b040a96..5e54c9c 100644
--- a/Makefile
+++ b/Makefile
@@ -1363,6 +1363,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS
 GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS
 	@echo SHELL_PATH=\''$(subst ','\'',$(SHELL_PATH_SQ))'\' >$@
 	@echo TAR=\''$(subst ','\'',$(subst ','\'',$(TAR)))'\' >>$@
+	@echo NO_CURL=\''$(subst ','\'',$(subst ','\'',$(NO_CURL)))'\' >>$@
 
 ### Detect Tck/Tk interpreter path changes
 ifndef NO_TCLTK
diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index 57a4411..cefab45 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh
@@ -11,6 +11,7 @@ This test runs various sanity checks on http-push.'
 
 ROOT_PATH="$PWD"
 LIB_HTTPD_DAV=t
+LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5540'}
 
 if git http-push > /dev/null 2>&1 || [ $? -eq 128 ]
 then
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
new file mode 100755
index 0000000..b6e6ec9
--- /dev/null
+++ b/t/t5550-http-fetch.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+
+test_description='test fetching over http'
+. ./test-lib.sh
+
+if test -n "$NO_CURL"; then
+	say 'skipping test, git built without http support'
+	test_done
+fi
+
+. "$TEST_DIRECTORY"/lib-httpd.sh
+LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5550'}
+start_httpd
+
+test_expect_success 'setup repository' '
+	echo content >file &&
+	git add file &&
+	git commit -m one
+'
+
+test_expect_success 'create http-accessible bare repository' '
+	mkdir "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	(cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	 git --bare init &&
+	 echo "exec git update-server-info" >hooks/post-update &&
+	 chmod +x hooks/post-update
+	) &&
+	git remote add public "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+	git push public master:master
+'
+
+test_expect_success 'clone http repository' '
+	git clone $HTTPD_URL/repo.git clone &&
+	test_cmp file clone/file
+'
+
+test_expect_success 'fetch changes via http' '
+	echo content >>file &&
+	git commit -a -m two &&
+	git push public
+	(cd clone && git pull) &&
+	test_cmp file clone/file
+'
+
+stop_httpd
+test_done
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 03/21] refactor find_ref_by_name() to accept const list
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
  2009-02-25  8:32 ` [PATCH 01/21] test scripts: refactor start_httpd helper Jay Soffian
  2009-02-25  8:32 ` [PATCH 02/21] add basic http clone/fetch tests Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 04/21] move duplicated get_local_heads() to remote.c Jay Soffian
                   ` (22 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

From: Jeff King <peff@peff.net>

Since it doesn't actually touch its argument, this makes
sense.

However, we still want to return a non-const version (which
requires a cast) so that this:

  struct ref *a, *b;
  a = find_ref_by_name(b);

works. Unfortunately, you can also silently strip the const
from a variable:

  struct ref *a;
  const struct ref *b;
  a = find_ref_by_name(b);

This is a classic C const problem because there is no way to
say "return the type with the same constness that was passed
to us"; we provide the same semantics as standard library
functions like strchr.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 cache.h |    2 +-
 refs.c  |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index 189151d..609380d 100644
--- a/cache.h
+++ b/cache.h
@@ -801,7 +801,7 @@ struct ref {
 #define REF_HEADS	(1u << 1)
 #define REF_TAGS	(1u << 2)
 
-extern struct ref *find_ref_by_name(struct ref *list, const char *name);
+extern struct ref *find_ref_by_name(const struct ref *list, const char *name);
 
 #define CONNECT_VERBOSE       (1u << 0)
 extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);
diff --git a/refs.c b/refs.c
index 6eb5f53..b2a37e1 100644
--- a/refs.c
+++ b/refs.c
@@ -1628,10 +1628,10 @@ int update_ref(const char *action, const char *refname,
 	return 0;
 }
 
-struct ref *find_ref_by_name(struct ref *list, const char *name)
+struct ref *find_ref_by_name(const struct ref *list, const char *name)
 {
 	for ( ; list; list = list->next)
 		if (!strcmp(list->name, name))
-			return list;
+			return (struct ref *)list;
 	return NULL;
 }
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 04/21] move duplicated get_local_heads() to remote.c
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (2 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 03/21] refactor find_ref_by_name() to accept const list Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 05/21] move duplicated ref_newer() " Jay Soffian
                   ` (21 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

get_local_heads() appears to have been copied from builtin-send-pack.c
to http-push.c via cut and paste. This patch moves the function and its
helper one_local_ref() to remote.c.

The two copies of one_local_ref() were not identical. I used the more
recent version from builtin-send-pack.c after confirming with Jeff King
that it was an oversight that commit 30affa1e did not update both
copies.

This is in preparation for being able to call it from builtin-remote.c

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 builtin-send-pack.c |   29 ++---------------------------
 http-push.c         |   23 ++---------------------
 remote.c            |   26 ++++++++++++++++++++++++++
 remote.h            |    1 +
 4 files changed, 31 insertions(+), 48 deletions(-)

diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index d65d019..2fbfc29 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -133,33 +133,8 @@ static int ref_newer(const unsigned char *new_sha1,
 	return found;
 }
 
-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, int flag, void *cb_data)
-{
-	struct ref *ref;
-	int len;
-
-	/* we already know it starts with refs/ to get here */
-	if (check_ref_format(refname + 5))
-		return 0;
-
-	len = strlen(refname) + 1;
-	ref = xcalloc(1, sizeof(*ref) + len);
-	hashcpy(ref->new_sha1, sha1);
-	memcpy(ref->name, refname, len);
-	*local_tail = ref;
-	local_tail = &ref->next;
-	return 0;
-}
-
-static void get_local_heads(void)
-{
-	local_tail = &local_refs;
-	for_each_ref(one_local_ref, NULL);
-}
-
 static int receive_status(int in, struct ref *refs)
 {
 	struct ref *hint;
@@ -387,7 +362,7 @@ static int refs_pushed(struct ref *ref)
 
 static int do_send_pack(int in, int out, struct remote *remote, const char *dest, int nr_refspec, const char **refspec)
 {
-	struct ref *ref;
+	struct ref *ref, *local_refs;
 	int new_refs;
 	int ask_for_status_report = 0;
 	int allow_deleting_refs = 0;
@@ -405,7 +380,7 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
 	/* No funny business with the matcher */
 	remote_tail = get_remote_heads(in, &remote_refs, 0, NULL, REF_NORMAL,
 				       &extra_have);
-	get_local_heads();
+	local_refs = get_local_heads();
 
 	/* Does the other end support the reporting? */
 	if (server_supports("report-status"))
diff --git a/http-push.c b/http-push.c
index 30d2d34..cfeed81 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1792,21 +1792,8 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)
 	return 1;
 }
 
-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, int flag, void *cb_data)
-{
-	struct ref *ref;
-	int len = strlen(refname) + 1;
-	ref = xcalloc(1, sizeof(*ref) + len);
-	hashcpy(ref->new_sha1, sha1);
-	memcpy(ref->name, refname, len);
-	*local_tail = ref;
-	local_tail = &ref->next;
-	return 0;
-}
-
 static void one_remote_ref(char *refname)
 {
 	struct ref *ref;
@@ -1839,12 +1826,6 @@ static void one_remote_ref(char *refname)
 	remote_tail = &ref->next;
 }
 
-static void get_local_heads(void)
-{
-	local_tail = &local_refs;
-	for_each_ref(one_local_ref, NULL);
-}
-
 static void get_dav_remote_heads(void)
 {
 	remote_tail = &remote_refs;
@@ -2195,7 +2176,7 @@ int main(int argc, char **argv)
 	int rc = 0;
 	int i;
 	int new_refs;
-	struct ref *ref;
+	struct ref *ref, *local_refs;
 	char *rewritten_url = NULL;
 
 	git_extract_argv0_path(argv[0]);
@@ -2302,7 +2283,7 @@ int main(int argc, char **argv)
 		fetch_indices();
 
 	/* Get a list of all local and remote heads to validate refspecs */
-	get_local_heads();
+	local_refs = get_local_heads();
 	fprintf(stderr, "Fetching remote heads...\n");
 	get_dav_remote_heads();
 
diff --git a/remote.c b/remote.c
index d7079c6..01aae77 100644
--- a/remote.c
+++ b/remote.c
@@ -1376,3 +1376,29 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 			    base, num_ours, num_theirs);
 	return 1;
 }
+
+static int one_local_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+{
+	struct ref ***local_tail = cb_data;
+	struct ref *ref;
+	int len;
+
+	/* we already know it starts with refs/ to get here */
+	if (check_ref_format(refname + 5))
+		return 0;
+
+	len = strlen(refname) + 1;
+	ref = xcalloc(1, sizeof(*ref) + len);
+	hashcpy(ref->new_sha1, sha1);
+	memcpy(ref->name, refname, len);
+	**local_tail = ref;
+	*local_tail = &ref->next;
+	return 0;
+}
+
+struct ref *get_local_heads(void)
+{
+	struct ref *local_refs, **local_tail = &local_refs;
+	for_each_ref(one_local_ref, &local_tail);
+	return local_refs;
+}
diff --git a/remote.h b/remote.h
index a46a5be..56ca8b1 100644
--- a/remote.h
+++ b/remote.h
@@ -137,4 +137,5 @@ enum match_refs_flags {
 int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs);
 int format_tracking_info(struct branch *branch, struct strbuf *sb);
 
+struct ref *get_local_heads(void);
 #endif
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 05/21] move duplicated ref_newer() to remote.c
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (3 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 04/21] move duplicated get_local_heads() to remote.c Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 06/21] move locate_head() " Jay Soffian
                   ` (20 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

ref_newer() appears to have been copied from builtin-send-pack.c to
http-push.c via cut and paste. This patch moves the function and its
helper unmark_and_free() to remote.c. There was a slight difference
between the two implementations, one used TMP_MARK for the mark, the
other used 1. Per Jeff King, I went with TMP_MARK as more correct.

This is in preparation for being able to call it from builtin-remote.c

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 builtin-send-pack.c |   50 --------------------------------------------------
 http-push.c         |   49 -------------------------------------------------
 remote.c            |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 remote.h            |    1 +
 4 files changed, 50 insertions(+), 99 deletions(-)

diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 2fbfc29..9072905 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -1,6 +1,5 @@
 #include "cache.h"
 #include "commit.h"
-#include "tag.h"
 #include "refs.h"
 #include "pkt-line.h"
 #include "run-command.h"
@@ -84,55 +83,6 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
 	return 0;
 }
 
-static void unmark_and_free(struct commit_list *list, unsigned int mark)
-{
-	while (list) {
-		struct commit_list *temp = list;
-		temp->item->object.flags &= ~mark;
-		list = temp->next;
-		free(temp);
-	}
-}
-
-static int ref_newer(const unsigned char *new_sha1,
-		     const unsigned char *old_sha1)
-{
-	struct object *o;
-	struct commit *old, *new;
-	struct commit_list *list, *used;
-	int found = 0;
-
-	/* Both new and old must be commit-ish and new is descendant of
-	 * old.  Otherwise we require --force.
-	 */
-	o = deref_tag(parse_object(old_sha1), NULL, 0);
-	if (!o || o->type != OBJ_COMMIT)
-		return 0;
-	old = (struct commit *) o;
-
-	o = deref_tag(parse_object(new_sha1), NULL, 0);
-	if (!o || o->type != OBJ_COMMIT)
-		return 0;
-	new = (struct commit *) o;
-
-	if (parse_commit(new) < 0)
-		return 0;
-
-	used = list = NULL;
-	commit_list_insert(new, &list);
-	while (list) {
-		new = pop_most_recent_commit(&list, 1);
-		commit_list_insert(new, &used);
-		if (new == old) {
-			found = 1;
-			break;
-		}
-	}
-	unmark_and_free(list, 1);
-	unmark_and_free(used, 1);
-	return found;
-}
-
 static struct ref *remote_refs, **remote_tail;
 
 static int receive_status(int in, struct ref *refs)
diff --git a/http-push.c b/http-push.c
index cfeed81..392533a 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1843,55 +1843,6 @@ static int is_zero_sha1(const unsigned char *sha1)
 	return 1;
 }
 
-static void unmark_and_free(struct commit_list *list, unsigned int mark)
-{
-	while (list) {
-		struct commit_list *temp = list;
-		temp->item->object.flags &= ~mark;
-		list = temp->next;
-		free(temp);
-	}
-}
-
-static int ref_newer(const unsigned char *new_sha1,
-		     const unsigned char *old_sha1)
-{
-	struct object *o;
-	struct commit *old, *new;
-	struct commit_list *list, *used;
-	int found = 0;
-
-	/* Both new and old must be commit-ish and new is descendant of
-	 * old.  Otherwise we require --force.
-	 */
-	o = deref_tag(parse_object(old_sha1), NULL, 0);
-	if (!o || o->type != OBJ_COMMIT)
-		return 0;
-	old = (struct commit *) o;
-
-	o = deref_tag(parse_object(new_sha1), NULL, 0);
-	if (!o || o->type != OBJ_COMMIT)
-		return 0;
-	new = (struct commit *) o;
-
-	if (parse_commit(new) < 0)
-		return 0;
-
-	used = list = NULL;
-	commit_list_insert(new, &list);
-	while (list) {
-		new = pop_most_recent_commit(&list, TMP_MARK);
-		commit_list_insert(new, &used);
-		if (new == old) {
-			found = 1;
-			break;
-		}
-	}
-	unmark_and_free(list, TMP_MARK);
-	unmark_and_free(used, TMP_MARK);
-	return found;
-}
-
 static void add_remote_info_ref(struct remote_ls_ctx *ls)
 {
 	struct strbuf *buf = (struct strbuf *)ls->userData;
diff --git a/remote.c b/remote.c
index 01aae77..c8b7ea4 100644
--- a/remote.c
+++ b/remote.c
@@ -5,6 +5,7 @@
 #include "diff.h"
 #include "revision.h"
 #include "dir.h"
+#include "tag.h"
 
 static struct refspec s_tag_refspec = {
 	0,
@@ -1269,6 +1270,54 @@ int resolve_remote_symref(struct ref *ref, struct ref *list)
 	return 1;
 }
 
+static void unmark_and_free(struct commit_list *list, unsigned int mark)
+{
+	while (list) {
+		struct commit_list *temp = list;
+		temp->item->object.flags &= ~mark;
+		list = temp->next;
+		free(temp);
+	}
+}
+
+int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1)
+{
+	struct object *o;
+	struct commit *old, *new;
+	struct commit_list *list, *used;
+	int found = 0;
+
+	/* Both new and old must be commit-ish and new is descendant of
+	 * old.  Otherwise we require --force.
+	 */
+	o = deref_tag(parse_object(old_sha1), NULL, 0);
+	if (!o || o->type != OBJ_COMMIT)
+		return 0;
+	old = (struct commit *) o;
+
+	o = deref_tag(parse_object(new_sha1), NULL, 0);
+	if (!o || o->type != OBJ_COMMIT)
+		return 0;
+	new = (struct commit *) o;
+
+	if (parse_commit(new) < 0)
+		return 0;
+
+	used = list = NULL;
+	commit_list_insert(new, &list);
+	while (list) {
+		new = pop_most_recent_commit(&list, TMP_MARK);
+		commit_list_insert(new, &used);
+		if (new == old) {
+			found = 1;
+			break;
+		}
+	}
+	unmark_and_free(list, TMP_MARK);
+	unmark_and_free(used, TMP_MARK);
+	return found;
+}
+
 /*
  * Return true if there is anything to report, otherwise false.
  */
diff --git a/remote.h b/remote.h
index 56ca8b1..c0666a0 100644
--- a/remote.h
+++ b/remote.h
@@ -74,6 +74,7 @@ int check_ref_type(const struct ref *ref, int flags);
 void free_refs(struct ref *ref);
 
 int resolve_remote_symref(struct ref *ref, struct ref *list);
+int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1);
 
 /*
  * Removes and frees any duplicate refs in the map.
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 06/21] move locate_head() to remote.c
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (4 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 05/21] move duplicated ref_newer() " Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 07/21] remote: simplify guess_remote_head() Jay Soffian
                   ` (19 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

Move locate_head() to remote.c and rename it to guess_remote_head() to
more accurately reflect what it does. This is in preparation for being
able to call it from builtin-remote.c

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 builtin-clone.c |   41 +++--------------------------------------
 remote.c        |   37 +++++++++++++++++++++++++++++++++++++
 remote.h        |    9 +++++++++
 3 files changed, 49 insertions(+), 38 deletions(-)

diff --git a/builtin-clone.c b/builtin-clone.c
index c338910..d179d1c 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -20,6 +20,7 @@
 #include "dir.h"
 #include "pack-refs.h"
 #include "sigchain.h"
+#include "remote.h"
 
 /*
  * Overall FIXMEs:
@@ -293,43 +294,6 @@ static void remove_junk_on_signal(int signo)
 	raise(signo);
 }
 
-static const struct ref *locate_head(const struct ref *refs,
-				     const struct ref *mapped_refs,
-				     const struct ref **remote_head_p)
-{
-	const struct ref *remote_head = NULL;
-	const struct ref *remote_master = NULL;
-	const struct ref *r;
-	for (r = refs; r; r = r->next)
-		if (!strcmp(r->name, "HEAD"))
-			remote_head = r;
-
-	for (r = mapped_refs; r; r = r->next)
-		if (!strcmp(r->name, "refs/heads/master"))
-			remote_master = r;
-
-	if (remote_head_p)
-		*remote_head_p = remote_head;
-
-	/* If there's no HEAD value at all, never mind. */
-	if (!remote_head)
-		return NULL;
-
-	/* If refs/heads/master could be right, it is. */
-	if (remote_master && !hashcmp(remote_master->old_sha1,
-				      remote_head->old_sha1))
-		return remote_master;
-
-	/* Look for another ref that points there */
-	for (r = mapped_refs; r; r = r->next)
-		if (r != remote_head &&
-		    !hashcmp(r->old_sha1, remote_head->old_sha1))
-			return r;
-
-	/* Nothing is the same */
-	return NULL;
-}
-
 static struct ref *write_remote_refs(const struct ref *refs,
 		struct refspec *refspec, const char *reflog)
 {
@@ -545,7 +509,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 		mapped_refs = write_remote_refs(refs, &refspec, reflog_msg.buf);
 
-		head_points_at = locate_head(refs, mapped_refs, &remote_head);
+		head_points_at = guess_remote_head(refs, mapped_refs,
+						   &remote_head);
 	}
 	else {
 		warning("You appear to have cloned an empty repository.");
diff --git a/remote.c b/remote.c
index c8b7ea4..49a183e 100644
--- a/remote.c
+++ b/remote.c
@@ -1451,3 +1451,40 @@ struct ref *get_local_heads(void)
 	for_each_ref(one_local_ref, &local_tail);
 	return local_refs;
 }
+
+const struct ref *guess_remote_head(const struct ref *refs,
+				    const struct ref *mapped_refs,
+				    const struct ref **remote_head_p)
+{
+	const struct ref *remote_head = NULL;
+	const struct ref *remote_master = NULL;
+	const struct ref *r;
+	for (r = refs; r; r = r->next)
+		if (!strcmp(r->name, "HEAD"))
+			remote_head = r;
+
+	for (r = mapped_refs; r; r = r->next)
+		if (!strcmp(r->name, "refs/heads/master"))
+			remote_master = r;
+
+	if (remote_head_p)
+		*remote_head_p = remote_head;
+
+	/* If there's no HEAD value at all, never mind. */
+	if (!remote_head)
+		return NULL;
+
+	/* If refs/heads/master could be right, it is. */
+	if (remote_master && !hashcmp(remote_master->old_sha1,
+				      remote_head->old_sha1))
+		return remote_master;
+
+	/* Look for another ref that points there */
+	for (r = mapped_refs; r; r = r->next)
+		if (r != remote_head &&
+		    !hashcmp(r->old_sha1, remote_head->old_sha1))
+			return r;
+
+	/* Nothing is the same */
+	return NULL;
+}
diff --git a/remote.h b/remote.h
index c0666a0..9605da9 100644
--- a/remote.h
+++ b/remote.h
@@ -139,4 +139,13 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs);
 int format_tracking_info(struct branch *branch, struct strbuf *sb);
 
 struct ref *get_local_heads(void);
+/*
+ * Look in refs for HEAD. Then look for a matching SHA1 in mapped_refs,
+ * first checking if refs/heads/master matches. Return NULL if nothing matches
+ * or if there is no HEAD in refs. remote_head_p is assigned HEAD if not NULL.
+ */
+const struct ref *guess_remote_head(const struct ref *refs,
+				    const struct ref *mapped_refs,
+				    const struct ref **remote_head_p);
+
 #endif
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 07/21] remote: simplify guess_remote_head()
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (5 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 06/21] move locate_head() " Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 08/21] remote: let guess_remote_head() optionally return all matches Jay Soffian
                   ` (18 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

This function had complications which made it hard to extend.

- It used to do two things: find the HEAD ref, and then find a
  matching ref, optionally returning the former via assignment to a
  passed-in pointer. Since finding HEAD is a one-liner, just have a
  caller do it themselves and pass it as an argument.

- It used to manually search through the ref list for
  refs/heads/master; this can be a one-line call to
  find_ref_by_name.

Originally contributed by Jeff King along with the next commit as a
single patch.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
I split Jeff's patch into two as it makes the diff a littler easier on
the eyes and the history is clearer this way.

 builtin-clone.c |    4 ++--
 remote.c        |   31 ++++++++-----------------------
 remote.h        |   13 ++++++-------
 3 files changed, 16 insertions(+), 32 deletions(-)

diff --git a/builtin-clone.c b/builtin-clone.c
index d179d1c..f9ce4fb 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -509,8 +509,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 		mapped_refs = write_remote_refs(refs, &refspec, reflog_msg.buf);
 
-		head_points_at = guess_remote_head(refs, mapped_refs,
-						   &remote_head);
+		remote_head = find_ref_by_name(refs, "HEAD");
+		head_points_at = guess_remote_head(remote_head, mapped_refs);
 	}
 	else {
 		warning("You appear to have cloned an empty repository.");
diff --git a/remote.c b/remote.c
index 49a183e..aed760e 100644
--- a/remote.c
+++ b/remote.c
@@ -1452,37 +1452,22 @@ struct ref *get_local_heads(void)
 	return local_refs;
 }
 
-const struct ref *guess_remote_head(const struct ref *refs,
-				    const struct ref *mapped_refs,
-				    const struct ref **remote_head_p)
+const struct ref *guess_remote_head(const struct ref *head,
+				    const struct ref *refs)
 {
-	const struct ref *remote_head = NULL;
-	const struct ref *remote_master = NULL;
 	const struct ref *r;
-	for (r = refs; r; r = r->next)
-		if (!strcmp(r->name, "HEAD"))
-			remote_head = r;
 
-	for (r = mapped_refs; r; r = r->next)
-		if (!strcmp(r->name, "refs/heads/master"))
-			remote_master = r;
-
-	if (remote_head_p)
-		*remote_head_p = remote_head;
-
-	/* If there's no HEAD value at all, never mind. */
-	if (!remote_head)
+	if (!head)
 		return NULL;
 
 	/* If refs/heads/master could be right, it is. */
-	if (remote_master && !hashcmp(remote_master->old_sha1,
-				      remote_head->old_sha1))
-		return remote_master;
+	r = find_ref_by_name(refs, "refs/heads/master");
+	if (r && !hashcmp(r->old_sha1, head->old_sha1))
+		return r;
 
 	/* Look for another ref that points there */
-	for (r = mapped_refs; r; r = r->next)
-		if (r != remote_head &&
-		    !hashcmp(r->old_sha1, remote_head->old_sha1))
+	for (r = refs; r; r = r->next)
+		if (r != head && !hashcmp(r->old_sha1, head->old_sha1))
 			return r;
 
 	/* Nothing is the same */
diff --git a/remote.h b/remote.h
index 9605da9..db49ce0 100644
--- a/remote.h
+++ b/remote.h
@@ -139,13 +139,12 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs);
 int format_tracking_info(struct branch *branch, struct strbuf *sb);
 
 struct ref *get_local_heads(void);
+
 /*
- * Look in refs for HEAD. Then look for a matching SHA1 in mapped_refs,
- * first checking if refs/heads/master matches. Return NULL if nothing matches
- * or if there is no HEAD in refs. remote_head_p is assigned HEAD if not NULL.
+ * Look for a ref in refs whose SHA1 matches head, first checking if
+ * refs/heads/master matches. Return NULL if nothing matches or if head
+ * is NULL.
  */
-const struct ref *guess_remote_head(const struct ref *refs,
-				    const struct ref *mapped_refs,
-				    const struct ref **remote_head_p);
-
+const struct ref *guess_remote_head(const struct ref *head,
+				    const struct ref *refs);
 #endif
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 08/21] remote: let guess_remote_head() optionally return all matches
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (6 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 07/21] remote: simplify guess_remote_head() Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-26 14:37   ` Jeff King
  2009-02-25  8:32 ` [PATCH 09/21] remote: make match_refs() copy src ref before assigning to peer_ref Jay Soffian
                   ` (17 subsequent siblings)
  25 siblings, 1 reply; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

Determining HEAD is ambiguous since it is done by comparing SHA1s.

In the case of multiple matches we return refs/heads/master if it
matches, else we return the first match we encounter. builtin-remote
needs all matches returned to it, so add a flag for it to request such.

To be simple and consistent, the return value is now a copy (including
peer_ref) of the matching refs.

Originally contributed by Jeff King along with the prior commit as a
single patch.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 builtin-clone.c |    2 +-
 remote.c        |   36 ++++++++++++++++++++++++++----------
 remote.h        |   14 ++++++++------
 3 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/builtin-clone.c b/builtin-clone.c
index f9ce4fb..3146ca8 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -510,7 +510,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		mapped_refs = write_remote_refs(refs, &refspec, reflog_msg.buf);
 
 		remote_head = find_ref_by_name(refs, "HEAD");
-		head_points_at = guess_remote_head(remote_head, mapped_refs);
+		head_points_at = guess_remote_head(remote_head, mapped_refs, 0);
 	}
 	else {
 		warning("You appear to have cloned an empty repository.");
diff --git a/remote.c b/remote.c
index aed760e..3940a3c 100644
--- a/remote.c
+++ b/remote.c
@@ -1452,24 +1452,40 @@ struct ref *get_local_heads(void)
 	return local_refs;
 }
 
-const struct ref *guess_remote_head(const struct ref *head,
-				    const struct ref *refs)
+struct ref *copy_ref_with_peer(const struct ref *src)
+{
+	struct ref *dst = copy_ref(src);
+	dst->peer_ref = copy_ref(src->peer_ref);
+	return dst;
+}
+
+struct ref *guess_remote_head(const struct ref *head,
+			      const struct ref *refs,
+			      int all)
 {
 	const struct ref *r;
+	struct ref *list = NULL;
+	struct ref **tail = &list;
 
 	if (!head)
 		return NULL;
 
 	/* If refs/heads/master could be right, it is. */
-	r = find_ref_by_name(refs, "refs/heads/master");
-	if (r && !hashcmp(r->old_sha1, head->old_sha1))
-		return r;
+	if (!all) {
+		r = find_ref_by_name(refs, "refs/heads/master");
+		if (r && !hashcmp(r->old_sha1, head->old_sha1))
+			return copy_ref_with_peer(r);
+	}
 
 	/* Look for another ref that points there */
-	for (r = refs; r; r = r->next)
-		if (r != head && !hashcmp(r->old_sha1, head->old_sha1))
-			return r;
+	for (r = refs; r; r = r->next) {
+		if (r != head && !hashcmp(r->old_sha1, head->old_sha1)) {
+			*tail = copy_ref_with_peer(r);
+			tail = &((*tail)->next);
+			if (!all)
+				break;
+		}
+	}
 
-	/* Nothing is the same */
-	return NULL;
+	return list;
 }
diff --git a/remote.h b/remote.h
index db49ce0..de3d21b 100644
--- a/remote.h
+++ b/remote.h
@@ -139,12 +139,14 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs);
 int format_tracking_info(struct branch *branch, struct strbuf *sb);
 
 struct ref *get_local_heads(void);
-
 /*
- * Look for a ref in refs whose SHA1 matches head, first checking if
- * refs/heads/master matches. Return NULL if nothing matches or if head
- * is NULL.
+ * Find refs from a list which are likely to be pointed to by the given HEAD
+ * ref. If 'all' is false, returns the most likely ref; otherwise, returns a
+ * list of all candidate refs. If no match is found (or 'head' is NULL),
+ * returns NULL. All returns are newly allocated and should be freed.
  */
-const struct ref *guess_remote_head(const struct ref *head,
-				    const struct ref *refs);
+struct ref *guess_remote_head(const struct ref *head,
+			      const struct ref *refs,
+			      int all);
+
 #endif
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 09/21] remote: make match_refs() copy src ref before assigning to peer_ref
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (7 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 08/21] remote: let guess_remote_head() optionally return all matches Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 10/21] remote: make match_refs() not short-circuit Jay Soffian
                   ` (16 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

In some instances, match_refs() sets the peer_ref field of refs in the
dst list such that it points to a ref in the src list. This prevents
callers from freeing both the src and dst lists, as doing so would cause
a double-free since free_refs() frees the peer_ref.

As well, the following configuration causes two refs in the dst list to
have the same peer_ref, which can also lead to a double-free:

  push = refs/heads/master:refs/heads/backup
  push = refs/heads/master:refs/heads/master

Existing callers of match_heads() call it only once and then terminate,
w/o ever bothering to free the src or dst lists, so this is not
currently a problem.

This patch modifies match_refs() to first copy any refs it plucks from
the src list before assigning them as a peer_ref. This allows
builtin-remote, a future caller, to free the src and dst lists.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 remote.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/remote.c b/remote.c
index 3940a3c..81def8b 100644
--- a/remote.c
+++ b/remote.c
@@ -928,6 +928,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
 			  struct refspec *rs)
 {
 	struct ref *matched_src, *matched_dst;
+	int copy_src;
 
 	const char *dst_value = rs->dst;
 	char *dst_guess;
@@ -938,6 +939,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
 	matched_src = matched_dst = NULL;
 	switch (count_refspec_match(rs->src, src, &matched_src)) {
 	case 1:
+		copy_src = 1;
 		break;
 	case 0:
 		/* The source could be in the get_sha1() format
@@ -947,6 +949,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
 		matched_src = try_explicit_object_name(rs->src);
 		if (!matched_src)
 			return error("src refspec %s does not match any.", rs->src);
+		copy_src = 0;
 		break;
 	default:
 		return error("src refspec %s matches more than one.", rs->src);
@@ -992,7 +995,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
 		return error("dst ref %s receives from more than one src.",
 		      matched_dst->name);
 	else {
-		matched_dst->peer_ref = matched_src;
+		matched_dst->peer_ref = copy_src ? copy_ref(matched_src) : matched_src;
 		matched_dst->force = rs->force;
 	}
 	return 0;
@@ -1100,7 +1103,7 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
 			dst_peer = make_linked_ref(dst_name, dst_tail);
 			hashcpy(dst_peer->new_sha1, src->new_sha1);
 		}
-		dst_peer->peer_ref = src;
+		dst_peer->peer_ref = copy_ref(src);
 		dst_peer->force = pat->force;
 	free_name:
 		free(dst_name);
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 10/21] remote: make match_refs() not short-circuit
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (8 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 09/21] remote: make match_refs() copy src ref before assigning to peer_ref Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 11/21] string-list: new for_each_string_list() function Jay Soffian
                   ` (15 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

match_refs() returns non-zero if there is an error in
match_explicit_refs(), without handling any remaining pattern ref specs.

Its existing callers exit upon receiving non-zero, so a partial result
is of no consequence to them; however a new caller, builtin-remote, is
interested in the complete result even if there are errors in
match_explicit_refs().

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 remote.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/remote.c b/remote.c
index 81def8b..926f842 100644
--- a/remote.c
+++ b/remote.c
@@ -1044,6 +1044,7 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
 	struct refspec *rs;
 	int send_all = flags & MATCH_REFS_ALL;
 	int send_mirror = flags & MATCH_REFS_MIRROR;
+	int errs;
 	static const char *default_refspec[] = { ":", 0 };
 
 	if (!nr_refspec) {
@@ -1051,8 +1052,7 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
 		refspec = default_refspec;
 	}
 	rs = parse_push_refspec(nr_refspec, (const char **) refspec);
-	if (match_explicit_refs(src, dst, dst_tail, rs, nr_refspec))
-		return -1;
+	errs = match_explicit_refs(src, dst, dst_tail, rs, nr_refspec);
 
 	/* pick the remainder */
 	for ( ; src; src = src->next) {
@@ -1108,6 +1108,8 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
 	free_name:
 		free(dst_name);
 	}
+	if (errs)
+		return -1;
 	return 0;
 }
 
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 11/21] string-list: new for_each_string_list() function
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (9 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 10/21] remote: make match_refs() not short-circuit Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 12/21] builtin-remote: refactor duplicated cleanup code Jay Soffian
                   ` (14 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

Add a convenience function for iterating over a string_list's items via
a callback.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 string-list.c |   10 ++++++++++
 string-list.h |    5 +++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/string-list.c b/string-list.c
index 15e14cf..1ac536e 100644
--- a/string-list.c
+++ b/string-list.c
@@ -92,6 +92,16 @@ struct string_list_item *string_list_lookup(const char *string, struct string_li
 	return list->items + i;
 }
 
+int for_each_string_list(string_list_each_func_t fn,
+			 struct string_list *list, void *cb_data)
+{
+	int i, ret = 0;
+	for (i = 0; i < list->nr; i++)
+		if ((ret = fn(&list->items[i], cb_data)))
+			break;
+	return ret;
+}
+
 void string_list_clear(struct string_list *list, int free_util)
 {
 	if (list->items) {
diff --git a/string-list.h b/string-list.h
index d32ba05..14bbc47 100644
--- a/string-list.h
+++ b/string-list.h
@@ -20,6 +20,11 @@ void string_list_clear(struct string_list *list, int free_util);
 typedef void (*string_list_clear_func_t)(void *p, const char *str);
 void string_list_clear_func(struct string_list *list, string_list_clear_func_t clearfunc);
 
+/* Use this function to iterate over each item */
+typedef int (*string_list_each_func_t)(struct string_list_item *, void *);
+int for_each_string_list(string_list_each_func_t,
+			 struct string_list *list, void *cb_data);
+
 /* Use these functions only on sorted lists: */
 int string_list_has_string(const struct string_list *list, const char *string);
 int string_list_find_insert_index(const struct string_list *list, const char *string,
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 12/21] builtin-remote: refactor duplicated cleanup code
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (10 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 11/21] string-list: new for_each_string_list() function Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 13/21] builtin-remote: remove unused code in get_ref_states Jay Soffian
                   ` (13 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

This patch moves identical lines of code into a cleanup function. The
function has two callers and is about to gain a third.

Also removed a bogus NEEDSWORK comment per Daniel Barkalow:

  Actually, the comment is wrong; "remote" comes from remote_get(),
  which returns things from a cache in remote.c; there could be a
  remote_put() to let the code know that the caller is done with the
  object, but it wouldn't presently do anything.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 builtin-remote.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index ac69d37..b89a353 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -632,6 +632,13 @@ static void show_list(const char *title, struct string_list *list,
 		printf("    %s\n", list->items[i].string);
 }
 
+static void free_remote_ref_states(struct ref_states *states)
+{
+	string_list_clear(&states->new, 0);
+	string_list_clear(&states->stale, 0);
+	string_list_clear(&states->tracked, 0);
+}
+
 static int get_remote_ref_states(const char *name,
 				 struct ref_states *states,
 				 int query)
@@ -738,10 +745,7 @@ static int show(int argc, const char **argv)
 			}
 		}
 
-		/* NEEDSWORK: free remote */
-		string_list_clear(&states.new, 0);
-		string_list_clear(&states.stale, 0);
-		string_list_clear(&states.tracked, 0);
+		free_remote_ref_states(&states);
 	}
 
 	return result;
@@ -792,10 +796,7 @@ static int prune(int argc, const char **argv)
 			warn_dangling_symref(dangling_msg, refname);
 		}
 
-		/* NEEDSWORK: free remote */
-		string_list_clear(&states.new, 0);
-		string_list_clear(&states.stale, 0);
-		string_list_clear(&states.tracked, 0);
+		free_remote_ref_states(&states);
 	}
 
 	return result;
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 13/21] builtin-remote: remove unused code in get_ref_states
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (11 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 12/21] builtin-remote: refactor duplicated cleanup code Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 14/21] builtin-remote: rename variables and eliminate redundant function call Jay Soffian
                   ` (12 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

get_ref_states() populates the util pointer of the string_list_item's
that it adds to states->new and states->tracked, but nothing ever uses
the pointer, so we can get rid of the extra code.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 builtin-remote.c |   13 +++----------
 1 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index b89a353..3e6dee4 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -250,18 +250,11 @@ static int get_ref_states(const struct ref *ref, struct ref_states *states)
 
 	states->new.strdup_strings = states->tracked.strdup_strings = 1;
 	for (ref = fetch_map; ref; ref = ref->next) {
-		struct string_list *target = &states->tracked;
 		unsigned char sha1[20];
-		void *util = NULL;
-
 		if (!ref->peer_ref || read_ref(ref->peer_ref->name, sha1))
-			target = &states->new;
-		else {
-			target = &states->tracked;
-			if (hashcmp(sha1, ref->new_sha1))
-				util = &states;
-		}
-		string_list_append(abbrev_branch(ref->name), target)->util = util;
+			string_list_append(abbrev_branch(ref->name), &states->new);
+		else
+			string_list_append(abbrev_branch(ref->name), &states->tracked);
 	}
 	free_refs(fetch_map);
 
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 14/21] builtin-remote: rename variables and eliminate redundant function call
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (12 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 13/21] builtin-remote: remove unused code in get_ref_states Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 15/21] builtin-remote: make get_remote_ref_states() always populate states.tracked Jay Soffian
                   ` (11 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

- The variable name "remote" is used as both a "char *" and as a "struct
  remote *"; this is confusing, so rename the former to remote_name.

- Consistently refer to the refs returned by transport_get_remote_refs()
  as remote_refs.

- There is no need to call "sort_string_list(&branch_list)" as
  branch_list is populated via string_list_insert(), which maintains its
  order.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
In the original series this is two commits, but after re-ordering it
made sense to squash the two together as they were both very minimal.

 builtin-remote.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index 3e6dee4..fc02e5f 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -143,7 +143,7 @@ static int add(int argc, const char **argv)
 }
 
 struct branch_info {
-	char *remote;
+	char *remote_name;
 	struct string_list merge;
 };
 
@@ -182,9 +182,9 @@ static int config_read_branches(const char *key, const char *value, void *cb)
 			item->util = xcalloc(sizeof(struct branch_info), 1);
 		info = item->util;
 		if (type == REMOTE) {
-			if (info->remote)
+			if (info->remote_name)
 				warning("more than one branch.%s", key);
-			info->remote = xstrdup(value);
+			info->remote_name = xstrdup(value);
 		} else {
 			char *space = strchr(value, ' ');
 			value = abbrev_branch(value);
@@ -206,7 +206,6 @@ static void read_branches(void)
 	if (branch_list.nr)
 		return;
 	git_config(config_read_branches, NULL);
-	sort_string_list(&branch_list);
 }
 
 struct ref_states {
@@ -238,13 +237,14 @@ static int handle_one_branch(const char *refname,
 	return 0;
 }
 
-static int get_ref_states(const struct ref *ref, struct ref_states *states)
+static int get_ref_states(const struct ref *remote_refs, struct ref_states *states)
 {
 	struct ref *fetch_map = NULL, **tail = &fetch_map;
+	struct ref *ref;
 	int i;
 
 	for (i = 0; i < states->remote->fetch_refspec_nr; i++)
-		if (get_fetch_map(ref, states->remote->fetch + i, &tail, 1))
+		if (get_fetch_map(remote_refs, states->remote->fetch + i, &tail, 1))
 			die("Could not get fetch map for refspec %s",
 				states->remote->fetch_refspec[i]);
 
@@ -459,7 +459,7 @@ static int mv(int argc, const char **argv)
 	for (i = 0; i < branch_list.nr; i++) {
 		struct string_list_item *item = branch_list.items + i;
 		struct branch_info *info = item->util;
-		if (info->remote && !strcmp(info->remote, rename.old)) {
+		if (info->remote_name && !strcmp(info->remote_name, rename.old)) {
 			strbuf_reset(&buf);
 			strbuf_addf(&buf, "branch.%s.remote", item->string);
 			if (git_config_set(buf.buf, rename.new)) {
@@ -569,7 +569,7 @@ static int rm(int argc, const char **argv)
 	for (i = 0; i < branch_list.nr; i++) {
 		struct string_list_item *item = branch_list.items + i;
 		struct branch_info *info = item->util;
-		if (info->remote && !strcmp(info->remote, remote->name)) {
+		if (info->remote_name && !strcmp(info->remote_name, remote->name)) {
 			const char *keys[] = { "remote", "merge", NULL }, **k;
 			for (k = keys; *k; k++) {
 				strbuf_reset(&buf);
@@ -637,7 +637,7 @@ static int get_remote_ref_states(const char *name,
 				 int query)
 {
 	struct transport *transport;
-	const struct ref *ref;
+	const struct ref *remote_refs;
 
 	states->remote = remote_get(name);
 	if (!states->remote)
@@ -648,10 +648,10 @@ static int get_remote_ref_states(const char *name,
 	if (query) {
 		transport = transport_get(NULL, states->remote->url_nr > 0 ?
 			states->remote->url[0] : NULL);
-		ref = transport_get_remote_refs(transport);
+		remote_refs = transport_get_remote_refs(transport);
 		transport_disconnect(transport);
 
-		get_ref_states(ref, states);
+		get_ref_states(remote_refs, states);
 	}
 
 	return 0;
@@ -701,7 +701,7 @@ static int show(int argc, const char **argv)
 			struct branch_info *info = branch->util;
 			int j;
 
-			if (!info->merge.nr || strcmp(*argv, info->remote))
+			if (!info->merge.nr || strcmp(*argv, info->remote_name))
 				continue;
 			printf("  Remote branch%s merged with 'git pull' "
 				"while on branch %s\n   ",
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 15/21] builtin-remote: make get_remote_ref_states() always populate states.tracked
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (13 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 14/21] builtin-remote: rename variables and eliminate redundant function call Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 16/21] builtin-remote: fix two inconsistencies in the output of "show <remote>" Jay Soffian
                   ` (10 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

When not querying the remote, show() was having to populate
states.tracked itself. It makes more sense for get_remote_ref_states()
to do this consistently. Since show() is the only caller of
get_remote_ref_states() with query=0, this change does not affect
other callers.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 builtin-remote.c |   33 ++++++++++++++++-----------------
 1 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index fc02e5f..1b5e8b6 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -632,6 +632,20 @@ static void free_remote_ref_states(struct ref_states *states)
 	string_list_clear(&states->tracked, 0);
 }
 
+static int append_ref_to_tracked_list(const char *refname,
+	const unsigned char *sha1, int flags, void *cb_data)
+{
+	struct ref_states *states = cb_data;
+	struct refspec refspec;
+
+	memset(&refspec, 0, sizeof(refspec));
+	refspec.dst = (char *)refname;
+	if (!remote_find_tracking(states->remote, &refspec))
+		string_list_append(abbrev_branch(refspec.src), &states->tracked);
+
+	return 0;
+}
+
 static int get_remote_ref_states(const char *name,
 				 struct ref_states *states,
 				 int query)
@@ -652,21 +666,8 @@ static int get_remote_ref_states(const char *name,
 		transport_disconnect(transport);
 
 		get_ref_states(remote_refs, states);
-	}
-
-	return 0;
-}
-
-static int append_ref_to_tracked_list(const char *refname,
-	const unsigned char *sha1, int flags, void *cb_data)
-{
-	struct ref_states *states = cb_data;
-	struct refspec refspec;
-
-	memset(&refspec, 0, sizeof(refspec));
-	refspec.dst = (char *)refname;
-	if (!remote_find_tracking(states->remote, &refspec))
-		string_list_append(abbrev_branch(refspec.src), &states->tracked);
+	} else
+		for_each_ref(append_ref_to_tracked_list, states);
 
 	return 0;
 }
@@ -720,8 +721,6 @@ static int show(int argc, const char **argv)
 				"prune')", &states.stale, "");
 		}
 
-		if (no_query)
-			for_each_ref(append_ref_to_tracked_list, &states);
 		show_list("  Tracked remote branch%s", &states.tracked, "");
 
 		if (states.remote->push_refspec_nr) {
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 16/21] builtin-remote: fix two inconsistencies in the output of "show <remote>"
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (14 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 15/21] builtin-remote: make get_remote_ref_states() always populate states.tracked Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 17/21] builtin-remote: teach show to display remote HEAD Jay Soffian
                   ` (9 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

Remote and stale branches are emitted in alphabetical order, but new and
tracked branches are not. So sort the latter to be consistent with the
former. This also lets us use more efficient string_list_has_string()
instead of unsorted_string_list_has_string().

"show <remote>" prunes symrefs, but "show <remote> -n" does not. Fix the
latter to match the former.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 builtin-remote.c  |   15 ++++++++++-----
 t/t5505-remote.sh |    2 +-
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index 1b5e8b6..963be6d 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -226,10 +226,8 @@ static int handle_one_branch(const char *refname,
 		const char *name = abbrev_branch(refspec.src);
 		/* symbolic refs pointing nowhere were handled already */
 		if ((flags & REF_ISSYMREF) ||
-				unsorted_string_list_has_string(&states->tracked,
-					name) ||
-				unsorted_string_list_has_string(&states->new,
-					name))
+		    string_list_has_string(&states->tracked, name) ||
+		    string_list_has_string(&states->new, name))
 			return 0;
 		item = string_list_append(name, &states->stale);
 		item->util = xstrdup(refname);
@@ -258,6 +256,8 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
 	}
 	free_refs(fetch_map);
 
+	sort_string_list(&states->new);
+	sort_string_list(&states->tracked);
 	for_each_ref(handle_one_branch, states);
 	sort_string_list(&states->stale);
 
@@ -638,6 +638,9 @@ static int append_ref_to_tracked_list(const char *refname,
 	struct ref_states *states = cb_data;
 	struct refspec refspec;
 
+	if (flags & REF_ISSYMREF)
+		return 0;
+
 	memset(&refspec, 0, sizeof(refspec));
 	refspec.dst = (char *)refname;
 	if (!remote_find_tracking(states->remote, &refspec))
@@ -666,8 +669,10 @@ static int get_remote_ref_states(const char *name,
 		transport_disconnect(transport);
 
 		get_ref_states(remote_refs, states);
-	} else
+	} else {
 		for_each_ref(append_ref_to_tracked_list, states);
+		sort_string_list(&states->tracked);
+	}
 
 	return 0;
 }
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index eb63718..a13d4b6 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -141,8 +141,8 @@ cat > test/expect << EOF
   New remote branch (next fetch will store in remotes/origin)
     master
   Tracked remote branches
-    side
     master
+    side
   Local branches pushed with 'git push'
     master:upstream
     +refs/tags/lastbackup
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 17/21] builtin-remote: teach show to display remote HEAD
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (15 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 16/21] builtin-remote: fix two inconsistencies in the output of "show <remote>" Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 18/21] builtin-remote: add set-head subcommand Jay Soffian
                   ` (8 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

This is in preparation for teaching remote how to set
refs/remotes/<remote>/HEAD to match what HEAD is set to at <remote>, but
is useful in its own right.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 builtin-remote.c  |   54 ++++++++++++++++++++++++++++++++++++++++++++++++----
 t/t5505-remote.sh |   12 +++++++++-
 2 files changed, 59 insertions(+), 7 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index 963be6d..4543cf0 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -18,6 +18,9 @@ static const char * const builtin_remote_usage[] = {
 	NULL
 };
 
+#define GET_REF_STATES (1<<0)
+#define GET_HEAD_NAMES (1<<1)
+
 static int verbose;
 
 static int show_all(void);
@@ -210,7 +213,7 @@ static void read_branches(void)
 
 struct ref_states {
 	struct remote *remote;
-	struct string_list new, stale, tracked;
+	struct string_list new, stale, tracked, heads;
 };
 
 static int handle_one_branch(const char *refname,
@@ -264,6 +267,28 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
 	return 0;
 }
 
+static int get_head_names(const struct ref *remote_refs, struct ref_states *states)
+{
+	struct ref *ref, *matches;
+	struct ref *fetch_map = NULL, **fetch_map_tail = &fetch_map;
+	struct refspec refspec;
+
+	refspec.force = 0;
+	refspec.pattern = 1;
+	refspec.src = refspec.dst = "refs/heads/";
+	states->heads.strdup_strings = 1;
+	get_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0);
+	matches = guess_remote_head(find_ref_by_name(remote_refs, "HEAD"),
+				    fetch_map, 1);
+	for(ref = matches; ref; ref = ref->next)
+		string_list_append(abbrev_branch(ref->name), &states->heads);
+
+	free_refs(fetch_map);
+	free_refs(matches);
+
+	return 0;
+}
+
 struct known_remote {
 	struct known_remote *next;
 	struct remote *remote;
@@ -630,6 +655,7 @@ static void free_remote_ref_states(struct ref_states *states)
 	string_list_clear(&states->new, 0);
 	string_list_clear(&states->stale, 0);
 	string_list_clear(&states->tracked, 0);
+	string_list_clear(&states->heads, 0);
 }
 
 static int append_ref_to_tracked_list(const char *refname,
@@ -668,7 +694,10 @@ static int get_remote_ref_states(const char *name,
 		remote_refs = transport_get_remote_refs(transport);
 		transport_disconnect(transport);
 
-		get_ref_states(remote_refs, states);
+		if (query & GET_REF_STATES)
+			get_ref_states(remote_refs, states);
+		if (query & GET_HEAD_NAMES)
+			get_head_names(remote_refs, states);
 	} else {
 		for_each_ref(append_ref_to_tracked_list, states);
 		sort_string_list(&states->tracked);
@@ -679,7 +708,7 @@ static int get_remote_ref_states(const char *name,
 
 static int show(int argc, const char **argv)
 {
-	int no_query = 0, result = 0;
+	int no_query = 0, result = 0, query_flag = 0;
 	struct option options[] = {
 		OPT_GROUP("show specific options"),
 		OPT_BOOLEAN('n', NULL, &no_query, "do not query remotes"),
@@ -692,15 +721,30 @@ static int show(int argc, const char **argv)
 	if (argc < 1)
 		return show_all();
 
+	if (!no_query)
+		query_flag = (GET_REF_STATES | GET_HEAD_NAMES);
+
 	memset(&states, 0, sizeof(states));
 	for (; argc; argc--, argv++) {
 		int i;
 
-		get_remote_ref_states(*argv, &states, !no_query);
+		get_remote_ref_states(*argv, &states, query_flag);
 
 		printf("* remote %s\n  URL: %s\n", *argv,
 			states.remote->url_nr > 0 ?
 				states.remote->url[0] : "(no URL)");
+		if (no_query)
+			printf("  HEAD branch: (not queried)\n");
+		else if (!states.heads.nr)
+			printf("  HEAD branch: (unknown)\n");
+		else if (states.heads.nr == 1)
+			printf("  HEAD branch: %s\n", states.heads.items[0].string);
+		else {
+			printf("  HEAD branch (remote HEAD is ambiguous,"
+			       " may be one of the following):\n");
+			for (i = 0; i < states.heads.nr; i++)
+				printf("    %s\n", states.heads.items[i].string);
+		}
 
 		for (i = 0; i < branch_list.nr; i++) {
 			struct string_list_item *branch = branch_list.items + i;
@@ -772,7 +816,7 @@ static int prune(int argc, const char **argv)
 	for (; argc; argc--, argv++) {
 		int i;
 
-		get_remote_ref_states(*argv, &states, 1);
+		get_remote_ref_states(*argv, &states, GET_REF_STATES);
 
 		if (states.stale.nr) {
 			printf("Pruning %s\n", *argv);
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index a13d4b6..91525c3 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -136,6 +136,7 @@ EOF
 cat > test/expect << EOF
 * remote origin
   URL: $(pwd)/one
+  HEAD branch: master
   Remote branch merged with 'git pull' while on branch master
     master
   New remote branch (next fetch will store in remotes/origin)
@@ -146,6 +147,11 @@ cat > test/expect << EOF
   Local branches pushed with 'git push'
     master:upstream
     +refs/tags/lastbackup
+* remote two
+  URL: ../two
+  HEAD branch (remote HEAD is ambiguous, may be one of the following):
+    another
+    master
 EOF
 
 test_expect_success 'show' '
@@ -154,6 +160,7 @@ test_expect_success 'show' '
 		refs/heads/master:refs/heads/upstream &&
 	 git fetch &&
 	 git branch -d -r origin/master &&
+	 git config --add remote.two.url ../two &&
 	 (cd ../one &&
 	  echo 1 > file &&
 	  test_tick &&
@@ -162,13 +169,14 @@ test_expect_success 'show' '
 		refs/heads/master:refs/heads/upstream &&
 	 git config --add remote.origin.push \
 		+refs/tags/lastbackup &&
-	 git remote show origin > output &&
+	 git remote show origin two > output &&
 	 test_cmp expect output)
 '
 
 cat > test/expect << EOF
 * remote origin
   URL: $(pwd)/one
+  HEAD branch: (not queried)
   Remote branch merged with 'git pull' while on branch master
     master
   Tracked remote branches
@@ -343,7 +351,7 @@ test_expect_success '"remote show" does not show symbolic refs' '
 	git clone one three &&
 	(cd three &&
 	 git remote show origin > output &&
-	 ! grep HEAD < output &&
+	 ! grep "^ *HEAD$" < output &&
 	 ! grep -i stale < output)
 
 '
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 18/21] builtin-remote: add set-head subcommand
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (16 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 17/21] builtin-remote: teach show to display remote HEAD Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 19/21] remote: make guess_remote_head() use exact HEAD lookup if it is available Jay Soffian
                   ` (7 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

Provide a porcelain command for setting and deleting
$GIT_DIR/remotes/<remote>/HEAD.

While we're at it, document what $GIT_DIR/remotes/<remote>/HEAD is all
about.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 Documentation/git-remote.txt           |   28 +++++++++++++-
 builtin-remote.c                       |   62 ++++++++++++++++++++++++++++++++
 contrib/completion/git-completion.bash |    2 +-
 t/t5505-remote.sh                      |   40 ++++++++++++++++++++
 4 files changed, 129 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index fad983e..c9c0e6f 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -13,6 +13,7 @@ SYNOPSIS
 'git remote add' [-t <branch>] [-m <master>] [-f] [--mirror] <name> <url>
 'git remote rename' <old> <new>
 'git remote rm' <name>
+'git remote set-head' <name> [-a | -d | <branch>]
 'git remote show' [-n] <name>
 'git remote prune' [-n | --dry-run] <name>
 'git remote update' [group]
@@ -53,8 +54,7 @@ is created.  You can give more than one `-t <branch>` to track
 multiple branches without grabbing all branches.
 +
 With `-m <master>` option, `$GIT_DIR/remotes/<name>/HEAD` is set
-up to point at remote's `<master>` branch instead of whatever
-branch the `HEAD` at the remote repository actually points at.
+up to point at remote's `<master>` branch. See also the set-head command.
 +
 In mirror mode, enabled with `\--mirror`, the refs will not be stored
 in the 'refs/remotes/' namespace, but in 'refs/heads/'.  This option
@@ -76,6 +76,30 @@ the configuration file format.
 Remove the remote named <name>. All remote tracking branches and
 configuration settings for the remote are removed.
 
+'set-head'::
+
+Sets or deletes the default branch (`$GIT_DIR/remotes/<name>/HEAD`) for
+the named remote. Having a default branch for a remote is not required,
+but allows the name of the remote to be specified in lieu of a specific
+branch. For example, if the default branch for `origin` is set to
+`master`, then `origin` may be specified wherever you would normally
+specify `origin/master`.
++
+With `-d`, `$GIT_DIR/remotes/<name>/HEAD` is deleted.
++
+With `-a`, the remote is queried to determine its `HEAD`, then
+`$GIT_DIR/remotes/<name>/HEAD` is set to the same branch. e.g., if the remote
+`HEAD` is pointed at `next`, "`git remote set-head origin -a`" will set
+`$GIT_DIR/refs/remotes/origin/HEAD` to `refs/remotes/origin/next`. This will
+only work if `refs/remotes/origin/next` already exists; if not it must be
+fetched first.
++
+Use `<branch>` to set `$GIT_DIR/remotes/<name>/HEAD` explicitly. e.g., "git
+remote set-head origin master" will set `$GIT_DIR/refs/remotes/origin/HEAD` to
+`refs/remotes/origin/master`. This will only work if
+`refs/remotes/origin/master` already exists; if not it must be fetched first.
++
+
 'show'::
 
 Gives some information about the remote <name>.
diff --git a/builtin-remote.c b/builtin-remote.c
index 4543cf0..640e4da 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -12,6 +12,7 @@ static const char * const builtin_remote_usage[] = {
 	"git remote add [-t <branch>] [-m <master>] [-f] [--mirror] <name> <url>",
 	"git remote rename <old> <new>",
 	"git remote rm <name>",
+	"git remote set-head <name> [-a | -d | <branch>]",
 	"git remote show [-n] <name>",
 	"git remote prune [-n | --dry-run] <name>",
 	"git remote [-v | --verbose] update [group]",
@@ -792,6 +793,65 @@ static int show(int argc, const char **argv)
 	return result;
 }
 
+static int set_head(int argc, const char **argv)
+{
+	int i, opt_a = 0, opt_d = 0, result = 0;
+	struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;
+	char *head_name = NULL;
+
+	struct option options[] = {
+		OPT_GROUP("set-head specific options"),
+		OPT_BOOLEAN('a', "auto", &opt_a,
+			    "set refs/remotes/<name>/HEAD according to remote"),
+		OPT_BOOLEAN('d', "delete", &opt_d,
+			    "delete refs/remotes/<name>/HEAD"),
+		OPT_END()
+	};
+	argc = parse_options(argc, argv, options, builtin_remote_usage, 0);
+	if (argc)
+		strbuf_addf(&buf, "refs/remotes/%s/HEAD", argv[0]);
+
+	if (!opt_a && !opt_d && argc == 2) {
+		head_name = xstrdup(argv[1]);
+	} else if (opt_a && !opt_d && argc == 1) {
+		struct ref_states states;
+		memset(&states, 0, sizeof(states));
+		get_remote_ref_states(argv[0], &states, GET_HEAD_NAMES);
+		if (!states.heads.nr)
+			result |= error("Cannot determine remote HEAD");
+		else if (states.heads.nr > 1) {
+			result |= error("Multiple remote HEAD branches. "
+					"Please choose one explicitly with:");
+			for (i = 0; i < states.heads.nr; i++)
+				fprintf(stderr, "  git remote set-head %s %s\n",
+					argv[0], states.heads.items[i].string);
+		} else
+			head_name = xstrdup(states.heads.items[0].string);
+		free_remote_ref_states(&states);
+	} else if (opt_d && !opt_a && argc == 1) {
+		if (delete_ref(buf.buf, NULL, REF_NODEREF))
+			result |= error("Could not delete %s", buf.buf);
+	} else
+		usage_with_options(builtin_remote_usage, options);
+
+	if (head_name) {
+		unsigned char sha1[20];
+		strbuf_addf(&buf2, "refs/remotes/%s/%s", argv[0], head_name);
+		/* make sure it's valid */
+		if (!resolve_ref(buf2.buf, sha1, 1, NULL))
+			result |= error("Not a valid ref: %s", buf2.buf);
+		else if (create_symref(buf.buf, buf2.buf, "remote set-head"))
+			result |= error("Could not setup %s", buf.buf);
+		if (opt_a)
+			printf("%s/HEAD set to %s\n", argv[0], head_name);
+		free(head_name);
+	}
+
+	strbuf_release(&buf);
+	strbuf_release(&buf2);
+	return result;
+}
+
 static int prune(int argc, const char **argv)
 {
 	int dry_run = 0, result = 0;
@@ -962,6 +1022,8 @@ int cmd_remote(int argc, const char **argv, const char *prefix)
 		result = mv(argc, argv);
 	else if (!strcmp(argv[0], "rm"))
 		result = rm(argc, argv);
+	else if (!strcmp(argv[0], "set-head"))
+		result = set_head(argc, argv);
 	else if (!strcmp(argv[0], "show"))
 		result = show(argc, argv);
 	else if (!strcmp(argv[0], "prune"))
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0a3092f..15b938b 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1443,7 +1443,7 @@ _git_config ()
 
 _git_remote ()
 {
-	local subcommands="add rename rm show prune update"
+	local subcommands="add rename rm show prune update set-head"
 	local subcommand="$(__git_find_subcommand "$subcommands")"
 	if [ -z "$subcommand" ]; then
 		__gitcomp "$subcommands"
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 91525c3..de1d0fc 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -205,6 +205,46 @@ test_expect_success 'prune' '
 	 test_must_fail git rev-parse refs/remotes/origin/side)
 '
 
+test_expect_success 'set-head --delete' '
+	(cd test &&
+	 git symbolic-ref refs/remotes/origin/HEAD &&
+	 git remote set-head --delete origin &&
+	 test_must_fail git symbolic-ref refs/remotes/origin/HEAD)
+'
+
+test_expect_success 'set-head --auto' '
+	(cd test &&
+	 git remote set-head --auto origin &&
+	 echo refs/remotes/origin/master >expect &&
+	 git symbolic-ref refs/remotes/origin/HEAD >output &&
+	 test_cmp expect output
+	)
+'
+
+cat >test/expect <<EOF
+error: Multiple remote HEAD branches. Please choose one explicitly with:
+  git remote set-head two another
+  git remote set-head two master
+EOF
+
+test_expect_success 'set-head --auto fails w/multiple HEADs' '
+	(cd test &&
+	 test_must_fail git remote set-head --auto two >output 2>&1 &&
+	test_cmp expect output)
+'
+
+cat >test/expect <<EOF
+refs/remotes/origin/side2
+EOF
+
+test_expect_success 'set-head explicit' '
+	(cd test &&
+	 git remote set-head origin side2 &&
+	 git symbolic-ref refs/remotes/origin/HEAD >output &&
+	 git remote set-head origin master &&
+	 test_cmp expect output)
+'
+
 cat > test/expect << EOF
 Pruning origin
 URL: $(pwd)/one
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 19/21] remote: make guess_remote_head() use exact HEAD lookup if it is available
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (17 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 18/21] builtin-remote: add set-head subcommand Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 20/21] builtin-remote: new show output style Jay Soffian
                   ` (6 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

From: Jeff King <peff@peff.net>

Our usual method for determining the ref pointed to by HEAD
is to compare HEAD's sha1 to the sha1 of all refs, trying to
find a unique match.

However, some transports actually get to look at HEAD
directly; we should make use of that information when it is
available.  Currently, only http remotes support this
feature.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 remote.c              |   10 ++++++++++
 t/t5550-http-fetch.sh |   11 +++++++++++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/remote.c b/remote.c
index 926f842..1c09cf0 100644
--- a/remote.c
+++ b/remote.c
@@ -1475,6 +1475,16 @@ struct ref *guess_remote_head(const struct ref *head,
 	if (!head)
 		return NULL;
 
+	/*
+	 * Some transports support directly peeking at
+	 * where HEAD points; if that is the case, then
+	 * we don't have to guess.
+	 */
+	if (head->symref) {
+		r = find_ref_by_name(refs, head->symref);
+		return r ? copy_ref_with_peer(r) : NULL;
+	}
+
 	/* If refs/heads/master could be right, it is. */
 	if (!all) {
 		r = find_ref_by_name(refs, "refs/heads/master");
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index b6e6ec9..05b1b62 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -42,5 +42,16 @@ test_expect_success 'fetch changes via http' '
 	test_cmp file clone/file
 '
 
+test_expect_success 'http remote detects correct HEAD' '
+	git push public master:other &&
+	(cd clone &&
+	 git remote set-head origin -d &&
+	 git remote set-head origin -a &&
+	 git symbolic-ref refs/remotes/origin/HEAD > output &&
+	 echo refs/remotes/origin/master > expect &&
+	 test_cmp expect output
+	)
+'
+
 stop_httpd
 test_done
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 20/21] builtin-remote: new show output style
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (18 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 19/21] remote: make guess_remote_head() use exact HEAD lookup if it is available Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-25  8:32 ` [PATCH 21/21] builtin-remote: new show output style for push refspecs Jay Soffian
                   ` (5 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

The existing output of "git remote show <remote>" is too verbose for the
information it provides. This patch teaches it to provide more
information in less space.

The output for push refspecs is addressed in the next patch.

Before the patch:

$ git remote show origin
* remote origin
  URL: git://git.kernel.org/pub/scm/git/git.git
  HEAD branch: master
  Remote branch merged with 'git pull' while on branch master
    master
  Remote branch merged with 'git pull' while on branch next
    next
  Remote branches merged with 'git pull' while on branch octopus
    foo bar baz frotz
  New remote branch (next fetch will store in remotes/origin)
    html
  Stale tracking branch (use 'git remote prune')
    bogus
  Tracked remote branches
    maint
    man
    master
    next
    pu
    todo

After this patch:

$ git remote show origin
* remote origin
  URL: git://git.kernel.org/pub/scm/git/git.git
  HEAD branch: master
  Remote branches:
    bogus  stale (use 'git remote prune' to remove)
    html   new (next fetch will store in remotes/origin)
    maint  tracked
    man    tracked
    master tracked
    next   tracked
    pu     tracked
    todo   tracked
  Local branches configured for 'git pull':
    master  rebases onto remote master
    next    rebases onto remote next
    octopus  merges with remote foo
                and with remote bar
                and with remote baz
                and with remote frotz

$ git remote show origin -n
* remote origin
  URL: git://git.kernel.org/pub/scm/git/git.git
  HEAD branch: (not queried)
  Remote branches: (status not queried)
    bogus
    maint
    man
    master
    next
    pu
    todo
  Local branches configured for 'git pull':
    master  rebases onto remote master
    next    rebases onto remote next
    octopus  merges with remote foo
                and with remote bar
                and with remote baz
                and with remote frotz

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 builtin-remote.c  |  178 ++++++++++++++++++++++++++++++++++++++++-------------
 t/t5505-remote.sh |   38 ++++++-----
 2 files changed, 157 insertions(+), 59 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index 640e4da..379826e 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -149,6 +149,7 @@ static int add(int argc, const char **argv)
 struct branch_info {
 	char *remote_name;
 	struct string_list merge;
+	int rebase;
 };
 
 static struct string_list branch_list;
@@ -165,10 +166,11 @@ static const char *abbrev_ref(const char *name, const char *prefix)
 static int config_read_branches(const char *key, const char *value, void *cb)
 {
 	if (!prefixcmp(key, "branch.")) {
+		const char *orig_key = key;
 		char *name;
 		struct string_list_item *item;
 		struct branch_info *info;
-		enum { REMOTE, MERGE } type;
+		enum { REMOTE, MERGE, REBASE } type;
 
 		key += 7;
 		if (!postfixcmp(key, ".remote")) {
@@ -177,6 +179,9 @@ static int config_read_branches(const char *key, const char *value, void *cb)
 		} else if (!postfixcmp(key, ".merge")) {
 			name = xstrndup(key, strlen(key) - 6);
 			type = MERGE;
+		} else if (!postfixcmp(key, ".rebase")) {
+			name = xstrndup(key, strlen(key) - 7);
+			type = REBASE;
 		} else
 			return 0;
 
@@ -187,9 +192,9 @@ static int config_read_branches(const char *key, const char *value, void *cb)
 		info = item->util;
 		if (type == REMOTE) {
 			if (info->remote_name)
-				warning("more than one branch.%s", key);
+				warning("more than one %s", orig_key);
 			info->remote_name = xstrdup(value);
-		} else {
+		} else if (type == MERGE) {
 			char *space = strchr(value, ' ');
 			value = abbrev_branch(value);
 			while (space) {
@@ -200,7 +205,8 @@ static int config_read_branches(const char *key, const char *value, void *cb)
 				space = strchr(value, ' ');
 			}
 			string_list_append(xstrdup(value), &info->merge);
-		}
+		} else
+			info->rebase = git_config_bool(orig_key, value);
 	}
 	return 0;
 }
@@ -215,6 +221,7 @@ static void read_branches(void)
 struct ref_states {
 	struct remote *remote;
 	struct string_list new, stale, tracked, heads;
+	int queried;
 };
 
 static int handle_one_branch(const char *refname,
@@ -637,20 +644,6 @@ static int rm(int argc, const char **argv)
 	return result;
 }
 
-static void show_list(const char *title, struct string_list *list,
-		      const char *extra_arg)
-{
-	int i;
-
-	if (!list->nr)
-		return;
-
-	printf(title, list->nr > 1 ? "es" : "", extra_arg);
-	printf("\n");
-	for (i = 0; i < list->nr; i++)
-		printf("    %s\n", list->items[i].string);
-}
-
 static void free_remote_ref_states(struct ref_states *states)
 {
 	string_list_clear(&states->new, 0);
@@ -695,6 +688,7 @@ static int get_remote_ref_states(const char *name,
 		remote_refs = transport_get_remote_refs(transport);
 		transport_disconnect(transport);
 
+		states->queried = 1;
 		if (query & GET_REF_STATES)
 			get_ref_states(remote_refs, states);
 		if (query & GET_HEAD_NAMES)
@@ -707,6 +701,104 @@ static int get_remote_ref_states(const char *name,
 	return 0;
 }
 
+struct show_info {
+	struct string_list *list;
+	struct ref_states *states;
+	int width;
+	int any_rebase;
+};
+
+int add_remote_to_show_info(struct string_list_item *item, void *cb_data)
+{
+	struct show_info *info = cb_data;
+	int n = strlen(item->string);
+	if (n > info->width)
+		info->width = n;
+	string_list_insert(item->string, info->list);
+	return 0;
+}
+
+int show_remote_info_item(struct string_list_item *item, void *cb_data)
+{
+	struct show_info *info = cb_data;
+	struct ref_states *states = info->states;
+	const char *name = item->string;
+
+	if (states->queried) {
+		const char *fmt = "%s";
+		const char *arg = "";
+		if (string_list_has_string(&states->new, name)) {
+			fmt = " new (next fetch will store in remotes/%s)";
+			arg = states->remote->name;
+		} else if (string_list_has_string(&states->tracked, name))
+			arg = " tracked";
+		else if (string_list_has_string(&states->stale, name))
+			arg = " stale (use 'git remote prune' to remove)";
+		else
+			arg = " ???";
+		printf("    %-*s", info->width, name);
+		printf(fmt, arg);
+		printf("\n");
+	} else
+		printf("    %s\n", name);
+
+	return 0;
+}
+
+int add_local_to_show_info(struct string_list_item *branch_item, void *cb_data)
+{
+	struct show_info *show_info = cb_data;
+	struct ref_states *states = show_info->states;
+	struct branch_info *branch_info = branch_item->util;
+	struct string_list_item *item;
+	int n;
+
+	if (!branch_info->merge.nr || !branch_info->remote_name ||
+	    strcmp(states->remote->name, branch_info->remote_name))
+		return 0;
+	if ((n = strlen(branch_item->string)) > show_info->width)
+		show_info->width = n;
+	if (branch_info->rebase)
+		show_info->any_rebase = 1;
+
+	item = string_list_insert(branch_item->string, show_info->list);
+	item->util = branch_info;
+
+	return 0;
+}
+
+int show_local_info_item(struct string_list_item *item, void *cb_data)
+{
+	struct show_info *show_info = cb_data;
+	struct branch_info *branch_info = item->util;
+	struct string_list *merge = &branch_info->merge;
+	const char *also;
+	int i;
+
+	if (branch_info->rebase && branch_info->merge.nr > 1) {
+		error("invalid branch.%s.merge; cannot rebase onto > 1 branch",
+			item->string);
+		return 0;
+	}
+
+	printf("    %-*s ", show_info->width, item->string);
+	if (branch_info->rebase) {
+		printf("rebases onto remote %s\n", merge->items[0].string);
+		return 0;
+	} else if (show_info->any_rebase) {
+		printf(" merges with remote %s\n", merge->items[0].string);
+		also = "    and with remote";
+	} else {
+		printf("merges with remote %s\n", merge->items[0].string);
+		also = "   and with remote";
+	}
+	for (i = 1; i < merge->nr; i++)
+		printf("    %-*s %s %s\n", show_info->width, "", also,
+		       merge->items[i].string);
+
+	return 0;
+}
+
 static int show(int argc, const char **argv)
 {
 	int no_query = 0, result = 0, query_flag = 0;
@@ -716,6 +808,8 @@ static int show(int argc, const char **argv)
 		OPT_END()
 	};
 	struct ref_states states;
+	struct string_list info_list = { NULL, 0, 0, 0 };
+	struct show_info info;
 
 	argc = parse_options(argc, argv, options, builtin_remote_usage, 0);
 
@@ -726,6 +820,9 @@ static int show(int argc, const char **argv)
 		query_flag = (GET_REF_STATES | GET_HEAD_NAMES);
 
 	memset(&states, 0, sizeof(states));
+	memset(&info, 0, sizeof(info));
+	info.states = &states;
+	info.list = &info_list;
 	for (; argc; argc--, argv++) {
 		int i;
 
@@ -747,32 +844,29 @@ static int show(int argc, const char **argv)
 				printf("    %s\n", states.heads.items[i].string);
 		}
 
-		for (i = 0; i < branch_list.nr; i++) {
-			struct string_list_item *branch = branch_list.items + i;
-			struct branch_info *info = branch->util;
-			int j;
+		/* remote branch info */
+		info.width = 0;
+		for_each_string_list(add_remote_to_show_info, &states.new, &info);
+		for_each_string_list(add_remote_to_show_info, &states.tracked, &info);
+		for_each_string_list(add_remote_to_show_info, &states.stale, &info);
+		if (info.list->nr)
+			printf("  Remote branch%s:%s\n",
+			       info.list->nr > 1 ? "es" : "",
+				no_query ? " (status not queried)" : "");
+		for_each_string_list(show_remote_info_item, info.list, &info);
+		string_list_clear(info.list, 0);
 
-			if (!info->merge.nr || strcmp(*argv, info->remote_name))
-				continue;
-			printf("  Remote branch%s merged with 'git pull' "
-				"while on branch %s\n   ",
-				info->merge.nr > 1 ? "es" : "",
-				branch->string);
-			for (j = 0; j < info->merge.nr; j++)
-				printf(" %s", info->merge.items[j].string);
-			printf("\n");
-		}
-
-		if (!no_query) {
-			show_list("  New remote branch%s (next fetch "
-				"will store in remotes/%s)",
-				&states.new, states.remote->name);
-			show_list("  Stale tracking branch%s (use 'git remote "
-				"prune')", &states.stale, "");
-		}
-
-		show_list("  Tracked remote branch%s", &states.tracked, "");
+		/* git pull info */
+		info.width = 0;
+		info.any_rebase = 0;
+		for_each_string_list(add_local_to_show_info, &branch_list, &info);
+		if (info.list->nr)
+			printf("  Local branch%s configured for 'git pull':\n",
+			       info.list->nr > 1 ? "es" : "");
+		for_each_string_list(show_local_info_item, info.list, &info);
+		string_list_clear(info.list, 0);
 
+		/* git push info */
 		if (states.remote->push_refspec_nr) {
 			printf("  Local branch%s pushed with 'git push'\n",
 				states.remote->push_refspec_nr > 1 ?
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index de1d0fc..69e241a 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -28,7 +28,7 @@ tokens_match () {
 }
 
 check_remote_track () {
-	actual=$(git remote show "$1" | sed -e '1,/Tracked/d') &&
+	actual=$(git remote show "$1" | sed -ne 's|^    \(.*\) tracked$|\1|p')
 	shift &&
 	tokens_match "$*" "$actual"
 }
@@ -137,13 +137,15 @@ cat > test/expect << EOF
 * remote origin
   URL: $(pwd)/one
   HEAD branch: master
-  Remote branch merged with 'git pull' while on branch master
-    master
-  New remote branch (next fetch will store in remotes/origin)
-    master
-  Tracked remote branches
-    master
-    side
+  Remote branches:
+    master new (next fetch will store in remotes/origin)
+    side   tracked
+  Local branches configured for 'git pull':
+    master   merges with remote master
+    octopus  merges with remote topic-a
+                and with remote topic-b
+                and with remote topic-c
+    rebase  rebases onto remote master
   Local branches pushed with 'git push'
     master:upstream
     +refs/tags/lastbackup
@@ -156,20 +158,22 @@ EOF
 
 test_expect_success 'show' '
 	(cd test &&
-	 git config --add remote.origin.fetch \
-		refs/heads/master:refs/heads/upstream &&
+	 git config --add remote.origin.fetch refs/heads/master:refs/heads/upstream &&
 	 git fetch &&
+	 git branch --track octopus origin/master &&
+	 git branch --track rebase origin/master &&
 	 git branch -d -r origin/master &&
 	 git config --add remote.two.url ../two &&
+	 git config branch.rebase.rebase true &&
+	 git config branch.octopus.merge "topic-a topic-b topic-c" &&
 	 (cd ../one &&
 	  echo 1 > file &&
 	  test_tick &&
 	  git commit -m update file) &&
-	 git config remote.origin.push \
-		refs/heads/master:refs/heads/upstream &&
-	 git config --add remote.origin.push \
-		+refs/tags/lastbackup &&
+	 git config remote.origin.push refs/heads/master:refs/heads/upstream &&
+	 git config --add remote.origin.push +refs/tags/lastbackup &&
 	 git remote show origin two > output &&
+	 git branch -d rebase octopus &&
 	 test_cmp expect output)
 '
 
@@ -177,11 +181,11 @@ cat > test/expect << EOF
 * remote origin
   URL: $(pwd)/one
   HEAD branch: (not queried)
-  Remote branch merged with 'git pull' while on branch master
-    master
-  Tracked remote branches
+  Remote branches: (status not queried)
     master
     side
+  Local branch configured for 'git pull':
+    master merges with remote master
   Local branches pushed with 'git push'
     master:upstream
     +refs/tags/lastbackup
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 21/21] builtin-remote: new show output style for push refspecs
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (19 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 20/21] builtin-remote: new show output style Jay Soffian
@ 2009-02-25  8:32 ` Jay Soffian
  2009-02-26 14:53 ` [PATCH 00/21] git remote: set-head and new show output Jeff King
                   ` (4 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-25  8:32 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

The existing output of "git remote show <remote>" with respect to push
ref specs is basically just to show the raw refspec. This patch teaches
the command to interpret the refspecs and show how each branch will be
pushed to the destination. The output gives the user an idea of what
"git push" should do if it is run w/o any arguments.

Example new output:

1a. Typical output with no push refspec (i.e. matching branches only)

$ git remote show origin
* remote origin
  [...]
  Local refs configured for 'git push':
    master pushes to master (up to date)
    next   pushes to next   (local out of date)

1b. Same as above, w/o querying the remote:

$ git remote show origin -n
* remote origin
  [...]
  Local ref configured for 'git push' (status not queried):
    (matching) pushes to (matching)

2a. With a forcing refspec (+), and a new topic
    (something like push = refs/heads/*:refs/heads/*):

$ git remote show origin
* remote origin
  [...]
  Local refs configured for 'git push':
    master     pushes to master    (fast forwardable)
    new-topic  pushes to new-topic (create)
    next       pushes to next      (local out of date)
    pu         forces to pu        (up to date)

2b. Same as above, w/o querying the remote

$ git remote show origin -n
* remote origin
  [...]
  Local refs configured for 'git push' (status not queried):
    master     pushes to master
    new-topic  pushes to new-topic
    next       pushes to next
    pu         forces to pu

3. With a remote configured as a mirror:

* remote backup
  [...]
  Local refs will be mirrored by 'git push'

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 builtin-remote.c  |  201 ++++++++++++++++++++++++++++++++++++++++++++++++----
 t/t5505-remote.sh |   30 ++++++--
 2 files changed, 207 insertions(+), 24 deletions(-)

diff --git a/builtin-remote.c b/builtin-remote.c
index 379826e..7e82a52 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -21,6 +21,7 @@ static const char * const builtin_remote_usage[] = {
 
 #define GET_REF_STATES (1<<0)
 #define GET_HEAD_NAMES (1<<1)
+#define GET_PUSH_REF_STATES (1<<2)
 
 static int verbose;
 
@@ -220,7 +221,7 @@ static void read_branches(void)
 
 struct ref_states {
 	struct remote *remote;
-	struct string_list new, stale, tracked, heads;
+	struct string_list new, stale, tracked, heads, push;
 	int queried;
 };
 
@@ -275,6 +276,112 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
 	return 0;
 }
 
+struct push_info {
+	char *dest;
+	int forced;
+	enum {
+		PUSH_STATUS_CREATE = 0,
+		PUSH_STATUS_DELETE,
+		PUSH_STATUS_UPTODATE,
+		PUSH_STATUS_FASTFORWARD,
+		PUSH_STATUS_OUTOFDATE,
+		PUSH_STATUS_NOTQUERIED,
+	} status;
+};
+
+static int get_push_ref_states(const struct ref *remote_refs,
+	struct ref_states *states)
+{
+	struct remote *remote = states->remote;
+	struct ref *ref, *local_refs, *push_map, **push_tail;
+	if (remote->mirror)
+		return 0;
+
+	local_refs = get_local_heads();
+	ref = push_map = copy_ref_list(remote_refs);
+	while (ref->next)
+		ref = ref->next;
+	push_tail = &ref->next;
+
+	match_refs(local_refs, push_map, &push_tail, remote->push_refspec_nr,
+		   remote->push_refspec, MATCH_REFS_NONE);
+
+	states->push.strdup_strings = 1;
+	for (ref = push_map; ref; ref = ref->next) {
+		struct string_list_item *item;
+		struct push_info *info;
+
+		if (!ref->peer_ref)
+			continue;
+		hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
+
+		item = string_list_append(abbrev_branch(ref->peer_ref->name),
+					  &states->push);
+		item->util = xcalloc(sizeof(struct push_info), 1);
+		info = item->util;
+		info->forced = ref->force;
+		info->dest = xstrdup(abbrev_branch(ref->name));
+
+		if (is_null_sha1(ref->new_sha1)) {
+			info->status = PUSH_STATUS_DELETE;
+		} else if (!hashcmp(ref->old_sha1, ref->new_sha1))
+			info->status = PUSH_STATUS_UPTODATE;
+		else if (is_null_sha1(ref->old_sha1))
+			info->status = PUSH_STATUS_CREATE;
+		else if (has_sha1_file(ref->old_sha1) &&
+			 ref_newer(ref->new_sha1, ref->old_sha1))
+			info->status = PUSH_STATUS_FASTFORWARD;
+		else
+			info->status = PUSH_STATUS_OUTOFDATE;
+		// ref->peer_ref = NULL; /* local ref which is freed below */
+	}
+	free_refs(local_refs);
+	free_refs(push_map);
+	return 0;
+}
+
+static int get_push_ref_states_noquery(struct ref_states *states)
+{
+	int i;
+	struct remote *remote = states->remote;
+	struct string_list_item *item;
+	struct push_info *info;
+
+	if (remote->mirror)
+		return 0;
+
+	states->push.strdup_strings = 1;
+	if (!remote->push_refspec_nr) {
+		item = string_list_append("(matching)", &states->push);
+		info = item->util = xcalloc(sizeof(struct push_info), 1);
+		info->status = PUSH_STATUS_NOTQUERIED;
+		info->dest = xstrdup(item->string);
+	}
+	for (i = 0; i < remote->push_refspec_nr; i++) {
+		struct refspec *spec = remote->push + i;
+		char buf[PATH_MAX];
+		if (spec->matching)
+			item = string_list_append("(matching)", &states->push);
+		else if (spec->pattern) {
+			snprintf(buf, (sizeof(buf)), "%s*", spec->src);
+			item = string_list_append(buf, &states->push);
+			snprintf(buf, (sizeof(buf)), "%s*", spec->dst);
+		} else if (strlen(spec->src))
+			item = string_list_append(spec->src, &states->push);
+		else
+			item = string_list_append("(delete)", &states->push);
+
+		info = item->util = xcalloc(sizeof(struct push_info), 1);
+		info->forced = spec->force;
+		info->status = PUSH_STATUS_NOTQUERIED;
+		if (spec->pattern)
+			info->dest = xstrdup(buf);
+		else
+			info->dest = xstrdup(spec->dst ? spec->dst : item->string);
+	}
+	return 0;
+}
+
 static int get_head_names(const struct ref *remote_refs, struct ref_states *states)
 {
 	struct ref *ref, *matches;
@@ -644,12 +751,20 @@ static int rm(int argc, const char **argv)
 	return result;
 }
 
+void clear_push_info(void *util, const char *string)
+{
+	struct push_info *info = util;
+	free(info->dest);
+	free(info);
+}
+
 static void free_remote_ref_states(struct ref_states *states)
 {
 	string_list_clear(&states->new, 0);
 	string_list_clear(&states->stale, 0);
 	string_list_clear(&states->tracked, 0);
 	string_list_clear(&states->heads, 0);
+	string_list_clear_func(&states->push, clear_push_info);
 }
 
 static int append_ref_to_tracked_list(const char *refname,
@@ -693,9 +808,12 @@ static int get_remote_ref_states(const char *name,
 			get_ref_states(remote_refs, states);
 		if (query & GET_HEAD_NAMES)
 			get_head_names(remote_refs, states);
+		if (query & GET_PUSH_REF_STATES)
+			get_push_ref_states(remote_refs, states);
 	} else {
 		for_each_ref(append_ref_to_tracked_list, states);
 		sort_string_list(&states->tracked);
+		get_push_ref_states_noquery(states);
 	}
 
 	return 0;
@@ -704,7 +822,7 @@ static int get_remote_ref_states(const char *name,
 struct show_info {
 	struct string_list *list;
 	struct ref_states *states;
-	int width;
+	int width, width2;
 	int any_rebase;
 };
 
@@ -799,6 +917,58 @@ int show_local_info_item(struct string_list_item *item, void *cb_data)
 	return 0;
 }
 
+int add_push_to_show_info(struct string_list_item *push_item, void *cb_data)
+{
+	struct show_info *show_info = cb_data;
+	struct push_info *push_info = push_item->util;
+	struct string_list_item *item;
+	int n;
+	if ((n = strlen(push_item->string)) > show_info->width)
+		show_info->width = n;
+	if ((n = strlen(push_info->dest)) > show_info->width2)
+		show_info->width2 = n;
+	item = string_list_append(push_item->string, show_info->list);
+	item->util = push_item->util;
+	return 0;
+}
+
+int show_push_info_item(struct string_list_item *item, void *cb_data)
+{
+	struct show_info *show_info = cb_data;
+	struct push_info *push_info = item->util;
+	char *src = item->string, *status = NULL;
+
+	switch (push_info->status) {
+	case PUSH_STATUS_CREATE:
+		status = "create";
+		break;
+	case PUSH_STATUS_DELETE:
+		status = "delete";
+		src = "(none)";
+		break;
+	case PUSH_STATUS_UPTODATE:
+		status = "up to date";
+		break;
+	case PUSH_STATUS_FASTFORWARD:
+		status = "fast forwardable";
+		break;
+	case PUSH_STATUS_OUTOFDATE:
+		status = "local out of date";
+		break;
+	case PUSH_STATUS_NOTQUERIED:
+		break;
+	}
+	if (status)
+		printf("    %-*s %s to %-*s (%s)\n", show_info->width, src,
+			push_info->forced ? "forces" : "pushes",
+			show_info->width2, push_info->dest, status);
+	else
+		printf("    %-*s %s to %s\n", show_info->width, src,
+			push_info->forced ? "forces" : "pushes",
+			push_info->dest);
+	return 0;
+}
+
 static int show(int argc, const char **argv)
 {
 	int no_query = 0, result = 0, query_flag = 0;
@@ -817,7 +987,7 @@ static int show(int argc, const char **argv)
 		return show_all();
 
 	if (!no_query)
-		query_flag = (GET_REF_STATES | GET_HEAD_NAMES);
+		query_flag = (GET_REF_STATES | GET_HEAD_NAMES | GET_PUSH_REF_STATES);
 
 	memset(&states, 0, sizeof(states));
 	memset(&info, 0, sizeof(info));
@@ -867,19 +1037,18 @@ static int show(int argc, const char **argv)
 		string_list_clear(info.list, 0);
 
 		/* git push info */
-		if (states.remote->push_refspec_nr) {
-			printf("  Local branch%s pushed with 'git push'\n",
-				states.remote->push_refspec_nr > 1 ?
-					"es" : "");
-			for (i = 0; i < states.remote->push_refspec_nr; i++) {
-				struct refspec *spec = states.remote->push + i;
-				printf("    %s%s%s%s\n",
-				       spec->force ? "+" : "",
-				       abbrev_branch(spec->src),
-				       spec->dst ? ":" : "",
-				       spec->dst ? abbrev_branch(spec->dst) : "");
-			}
-		}
+		if (states.remote->mirror)
+			printf("  Local refs will be mirrored by 'git push'\n");
+
+		info.width = info.width2 = 0;
+		for_each_string_list(add_push_to_show_info, &states.push, &info);
+		sort_string_list(info.list);
+		if (info.list->nr)
+			printf("  Local ref%s configured for 'git push'%s:\n",
+				info.list->nr > 1 ? "s" : "",
+				no_query ? " (status not queried)" : "");
+		for_each_string_list(show_push_info_item, info.list, &info);
+		string_list_clear(info.list, 0);
 
 		free_remote_ref_states(&states);
 	}
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 69e241a..5ec668d 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -141,25 +141,34 @@ cat > test/expect << EOF
     master new (next fetch will store in remotes/origin)
     side   tracked
   Local branches configured for 'git pull':
+    ahead    merges with remote master
     master   merges with remote master
     octopus  merges with remote topic-a
                 and with remote topic-b
                 and with remote topic-c
     rebase  rebases onto remote master
-  Local branches pushed with 'git push'
-    master:upstream
-    +refs/tags/lastbackup
+  Local refs configured for 'git push':
+    master pushes to master   (local out of date)
+    master pushes to upstream (create)
 * remote two
   URL: ../two
   HEAD branch (remote HEAD is ambiguous, may be one of the following):
     another
     master
+  Local refs configured for 'git push':
+    ahead  forces to master  (fast forwardable)
+    master pushes to another (up to date)
 EOF
 
 test_expect_success 'show' '
 	(cd test &&
 	 git config --add remote.origin.fetch refs/heads/master:refs/heads/upstream &&
 	 git fetch &&
+	 git checkout -b ahead origin/master &&
+	 echo 1 >> file &&
+	 test_tick &&
+	 git commit -m update file &&
+	 git checkout master &&
 	 git branch --track octopus origin/master &&
 	 git branch --track rebase origin/master &&
 	 git branch -d -r origin/master &&
@@ -170,8 +179,11 @@ test_expect_success 'show' '
 	  echo 1 > file &&
 	  test_tick &&
 	  git commit -m update file) &&
-	 git config remote.origin.push refs/heads/master:refs/heads/upstream &&
+	 git config --add remote.origin.push : &&
+	 git config --add remote.origin.push refs/heads/master:refs/heads/upstream &&
 	 git config --add remote.origin.push +refs/tags/lastbackup &&
+	 git config --add remote.two.push +refs/heads/ahead:refs/heads/master &&
+	 git config --add remote.two.push refs/heads/master:refs/heads/another &&
 	 git remote show origin two > output &&
 	 git branch -d rebase octopus &&
 	 test_cmp expect output)
@@ -184,11 +196,13 @@ cat > test/expect << EOF
   Remote branches: (status not queried)
     master
     side
-  Local branch configured for 'git pull':
+  Local branches configured for 'git pull':
+    ahead  merges with remote master
     master merges with remote master
-  Local branches pushed with 'git push'
-    master:upstream
-    +refs/tags/lastbackup
+  Local refs configured for 'git push' (status not queried):
+    (matching)           pushes to (matching)
+    refs/heads/master    pushes to refs/heads/upstream
+    refs/tags/lastbackup forces to refs/tags/lastbackup
 EOF
 
 test_expect_success 'show -n' '
-- 
1.6.2.rc1.291.g83eb

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH 08/21] remote: let guess_remote_head() optionally return all matches
  2009-02-25  8:32 ` [PATCH 08/21] remote: let guess_remote_head() optionally return all matches Jay Soffian
@ 2009-02-26 14:37   ` Jeff King
  2009-02-26 14:40     ` Jeff King
  0 siblings, 1 reply; 33+ messages in thread
From: Jeff King @ 2009-02-26 14:37 UTC (permalink / raw
  To: Jay Soffian; +Cc: git, Junio C Hamano

On Wed, Feb 25, 2009 at 03:32:15AM -0500, Jay Soffian wrote:

> +struct ref *copy_ref_with_peer(const struct ref *src)
> +{
> +	struct ref *dst = copy_ref(src);
> +	dst->peer_ref = copy_ref(src->peer_ref);
> +	return dst;
> +}

Hmm. This should probably be:

  dst->peer_ref = src->peer_ref ? copy_ref(src->peer_ref) : NULL;

(or copy_ref should return NULL when given NULL). I also wonder if the
copied ref's peer_ref should be explicitly NULL'd.

I don't think it matters for the current code, since we always feed it
"matched refs" which have a peer, but I think it is good to be a little
more defensive in such a generically-named function.

And yes, this bug was in my original patch. :)

-Peff

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 08/21] remote: let guess_remote_head() optionally return all matches
  2009-02-26 14:37   ` Jeff King
@ 2009-02-26 14:40     ` Jeff King
  2009-02-26 18:47       ` Jay Soffian
  0 siblings, 1 reply; 33+ messages in thread
From: Jeff King @ 2009-02-26 14:40 UTC (permalink / raw
  To: Jay Soffian; +Cc: git, Junio C Hamano

On Thu, Feb 26, 2009 at 09:37:29AM -0500, Jeff King wrote:

> Hmm. This should probably be:
> 
>   dst->peer_ref = src->peer_ref ? copy_ref(src->peer_ref) : NULL;
> 
> (or copy_ref should return NULL when given NULL). I also wonder if the
> copied ref's peer_ref should be explicitly NULL'd.

BTW, all of my "probably" and "I wonder" here are because I think the
"peer ref" pointer is a little vague as a concept. E.g., I think in most
cases src->peer_ref->peer_ref != src.

Rather than having ref structs with "next" and "peer" pointers, I think
a more natural data structure would be a list (or array) of "ref pairs".

But you didn't create that with this series, and I don't think it is
worth the major surgery to change it now.

-Peff

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 00/21] git remote: set-head and new show output
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (20 preceding siblings ...)
  2009-02-25  8:32 ` [PATCH 21/21] builtin-remote: new show output style for push refspecs Jay Soffian
@ 2009-02-26 14:53 ` Jeff King
  2009-02-26 17:04 ` Junio C Hamano
                   ` (3 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Jeff King @ 2009-02-26 14:53 UTC (permalink / raw
  To: Jay Soffian; +Cc: git, Junio C Hamano

On Wed, Feb 25, 2009 at 03:32:07AM -0500, Jay Soffian wrote:

> This series replaces three related topics from pu:
> 
>   js/remote-set-head
>   jk/head-lookup
>   js/remote-display

With the exception of my comments on 08/21, this looks ready for 'next'
to me. Though I have to admit, given the number of patches and the
number of times I've looked at these changes before, my eyes started to
glaze over near the end. ;)

> I signed off on Jeff's patches; please remove my SoB from those if it is
> inappropriate for me to have done so.

I think that is fine. It is really about "I think this code is OK
license-wise to go into the project". So if I SoB and you have no reason
to doubt me, and you didn't change anything (or you added minor things
which you would SoB), then your SoB makes sense.

-Peff

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 00/21] git remote: set-head and new show output
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (21 preceding siblings ...)
  2009-02-26 14:53 ` [PATCH 00/21] git remote: set-head and new show output Jeff King
@ 2009-02-26 17:04 ` Junio C Hamano
  2009-02-27 19:10 ` [PATCH 07a/21] remote: make copy_ref() perform a deep copy Jay Soffian
                   ` (2 subsequent siblings)
  25 siblings, 0 replies; 33+ messages in thread
From: Junio C Hamano @ 2009-02-26 17:04 UTC (permalink / raw
  To: Jay Soffian; +Cc: git, Jeff King

Jay Soffian <jaysoffian@gmail.com> writes:

> This series replaces three related topics from pu:
>
>   js/remote-set-head
>   jk/head-lookup
>   js/remote-display
> ...
> So I think this series is clean, and doesn't need an extensive
> re-review, but a quick look-over would be appreciated.

I have looked at the diff between:

 (1) the above three topics merged to 'next'; and

 (2) this 21-patch series applied to 'master' then merged to 'next'.

There indeed does not seem major changes, but there are some:

 - get_head_names() lost "const char *remote_name" parameter that was
   unused (good);

 - guess_remote_head() uses a local variable "r" instead of using an
   additional variable "m" (does not matter);

 - "builtin-send-pack.c" lost otherwise unused inclusion of "tag.h"
   (good);

 - one_local_ref() and get_local_heads() are moved around (does not
   matter);

 - t5505-remote.sh has one more test (ok).

 - The old series had distracting removal of a few blank lines in
   cmd_clone() and get_remote_ref_states(), but with this round these are
   gone (good).

Overall, the new series certainly looks cleaner.

> I signed off on Jeff's patches; please remove my SoB from those if it is
> inappropriate for me to have done so.

That is perfectly fine.

Thanks.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 08/21] remote: let guess_remote_head() optionally return  all matches
  2009-02-26 14:40     ` Jeff King
@ 2009-02-26 18:47       ` Jay Soffian
  2009-02-27 11:43         ` Jeff King
  0 siblings, 1 reply; 33+ messages in thread
From: Jay Soffian @ 2009-02-26 18:47 UTC (permalink / raw
  To: Jeff King; +Cc: git, Junio C Hamano

On Thu, Feb 26, 2009 at 9:40 AM, Jeff King <peff@peff.net> wrote:
> On Thu, Feb 26, 2009 at 09:37:29AM -0500, Jeff King wrote:
>
>> Hmm. This should probably be:
>>
>>   dst->peer_ref = src->peer_ref ? copy_ref(src->peer_ref) : NULL;
>>
>> (or copy_ref should return NULL when given NULL). I also wonder if the
>> copied ref's peer_ref should be explicitly NULL'd.

Well, if you wanted to be consistent about things (and I apologize if gmail
mangles the lines), I'd probably do something like:

diff --git a/remote.c b/remote.c
index 1c09cf0..9f1bf5e 100644
--- a/remote.c
+++ b/remote.c
@@ -779,10 +779,18 @@ struct ref *alloc_ref(const char *name)

 static struct ref *copy_ref(const struct ref *ref)
 {
-	struct ref *ret = xmalloc(sizeof(struct ref) + strlen(ref->name) + 1);
-	memcpy(ret, ref, sizeof(struct ref) + strlen(ref->name) + 1);
-	ret->next = NULL;
-	return ret;
+	struct ref *cpy;
+	if (!ref)
+		return NULL;
+	cpy = xmalloc(sizeof(struct ref) + strlen(ref->name) + 1);
+	memcpy(cpy, ref, sizeof(struct ref) + strlen(ref->name) + 1);
+	cpy->next = cpy->peer_ref = NULL;
+	if (ref->peer_ref) {
+		ref = ref->peer_ref;
+		cpy->peer_ref = xmalloc(sizeof(struct ref) + strlen(ref->name) + 1);
+		memcpy(cpy->peer_ref, ref, sizeof(struct ref) + strlen(ref->name) + 1);
+	}
+	return cpy;
 }

 struct ref *copy_ref_list(const struct ref *ref)
@@ -803,6 +811,7 @@ static void free_ref(struct ref *ref)
 		return;
 	free(ref->remote_status);
 	free(ref->symref);
+	free(ref->peer_ref);
 	free(ref);
 }

@@ -811,7 +820,6 @@ void free_refs(struct ref *ref)
 	struct ref *next;
 	while (ref) {
 		next = ref->next;
-		free(ref->peer_ref);
 		free_ref(ref);
 		ref = next;
 	}
@@ -1457,13 +1465,6 @@ struct ref *get_local_heads(void)
 	return local_refs;
 }

-struct ref *copy_ref_with_peer(const struct ref *src)
-{
-	struct ref *dst = copy_ref(src);
-	dst->peer_ref = copy_ref(src->peer_ref);
-	return dst;
-}
-
 struct ref *guess_remote_head(const struct ref *head,
 			      const struct ref *refs,
 			      int all)
@@ -1480,22 +1481,20 @@ struct ref *guess_remote_head(const struct ref *head,
 	 * where HEAD points; if that is the case, then
 	 * we don't have to guess.
 	 */
-	if (head->symref) {
-		r = find_ref_by_name(refs, head->symref);
-		return r ? copy_ref_with_peer(r) : NULL;
-	}
+	if (head->symref)
+		return copy_ref(find_ref_by_name(refs, head->symref));

 	/* If refs/heads/master could be right, it is. */
 	if (!all) {
 		r = find_ref_by_name(refs, "refs/heads/master");
 		if (r && !hashcmp(r->old_sha1, head->old_sha1))
-			return copy_ref_with_peer(r);
+			return copy_ref(r);
 	}

 	/* Look for another ref that points there */
 	for (r = refs; r; r = r->next) {
 		if (r != head && !hashcmp(r->old_sha1, head->old_sha1)) {
-			*tail = copy_ref_with_peer(r);
+			*tail = copy_ref(r);
 			tail = &((*tail)->next);
 			if (!all)
 				break;


Then peer_ref is consistently a copy, so we can free it consistently, we don't
need two separate copy functions, and copy_ref returns NULL upon receiving
NULL like most of the other foo_ref functions.

> BTW, all of my "probably" and "I wonder" here are because I think the
> "peer ref" pointer is a little vague as a concept. E.g., I think in most
> cases src->peer_ref->peer_ref != src.
>
> Rather than having ref structs with "next" and "peer" pointers, I think
> a more natural data structure would be a list (or array) of "ref pairs".

Actually, we don't need most of the fields in the peer_ref, so we could
probably just embed the extra fields that we need in a peer_struct inside the
ref struct. I can add this to my git todo list.

j.

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH 08/21] remote: let guess_remote_head() optionally return all matches
  2009-02-26 18:47       ` Jay Soffian
@ 2009-02-27 11:43         ` Jeff King
  2009-02-27 19:10           ` [PATCH 0/3] git remote: set-head and new show output (UPDATED) Jay Soffian
  0 siblings, 1 reply; 33+ messages in thread
From: Jeff King @ 2009-02-27 11:43 UTC (permalink / raw
  To: Jay Soffian; +Cc: git, Junio C Hamano

On Thu, Feb 26, 2009 at 01:47:45PM -0500, Jay Soffian wrote:

> Well, if you wanted to be consistent about things (and I apologize if gmail
> mangles the lines), I'd probably do something like:
> [...]
> Then peer_ref is consistently a copy, so we can free it consistently, we don't
> need two separate copy functions, and copy_ref returns NULL upon receiving
> NULL like most of the other foo_ref functions.

Good point. That is much cleaner, IMHO, and probably worth doing as part
of this series. Though I hesitate to make you reroll _again_ for such a
small cleanup. Maybe it is worth just putting on top.

> > Rather than having ref structs with "next" and "peer" pointers, I think
> > a more natural data structure would be a list (or array) of "ref pairs".
> 
> Actually, we don't need most of the fields in the peer_ref, so we could
> probably just embed the extra fields that we need in a peer_struct inside the
> ref struct. I can add this to my git todo list.

Sure, that might turn out cleaner (though you may run into problems
where you want to pass a "struct ref" to a helper function but you have
only the fake "peer_struct").

But while that cleanup might be nice, I don't think it is probably worth
the pain, assuming you are done messing with remotes for a little while,
now.

-Peff

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 0/3] git remote: set-head and new show output (UPDATED)
  2009-02-27 11:43         ` Jeff King
@ 2009-02-27 19:10           ` Jay Soffian
  2009-02-28  6:33             ` Jeff King
  0 siblings, 1 reply; 33+ messages in thread
From: Jay Soffian @ 2009-02-27 19:10 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

Jeff, Junio,

I didn't want to resend the whole series (gmane 111394), so what I did was
create a new patch which comes after patch 7 in that series. Call it patch 7a.
The rest of that series is then identical (modulo line number offsets in
patches touching remote.c) except for patches 8 and 19. Patches 7a, and the
new 8 and 19 follow this email.

You should be able to do this:

$ git checkout js/remote-improvements
$ git format-patch master
$ git reset --hard $(git merge-base master HEAD)

Copy new "[PATCH 07a/21] remote: make copy_ref() perform a deep copy"
to 0007a-remote-make-copy_ref-perform-a-deep-copy.patch

Copy new "[PATCH 08/21] remote: let guess_remote_head() optionally return all matches"
to 0008-remote-let-guess_remote_head-optionally-return-al.patch

Copy new "[PATCH 19/21] remote: make guess_remote_head() use exact HEAD lookup if it is available"
to 0019-remote-make-guess_remote_head-use-exact-HEAD-look.patch

$ git am -3 *.patch

For those playing along at home, js/remote-improvements is currently
commits 661763a..5e4c90a.

Which should give you a new js/remote-improvements that looks like this:

  1  test scripts: refactor start_httpd helper
  2  add basic http clone/fetch tests
  3  refactor find_ref_by_name() to accept const list
  4  move duplicated get_local_heads() to remote.c
  5  move duplicated ref_newer() to remote.c
  6  move locate_head() to remote.c
  7  remote: simplify guess_remote_head()
  8  remote: make copy_ref() perform a deep copy
  9  remote: let guess_remote_head() optionally return all matches
 10  remote: make match_refs() copy src ref before assigning to peer_ref
 11  remote: make match_refs() not short-circuit
 12  string-list: new for_each_string_list() function
 13  builtin-remote: refactor duplicated cleanup code
 14  builtin-remote: remove unused code in get_ref_states
 15  builtin-remote: rename variables and eliminate redundant function call
 16  builtin-remote: make get_remote_ref_states() always populate states.tracked
 17  builtin-remote: fix two inconsistencies in the output of "show <remote>"
 18  builtin-remote: teach show to display remote HEAD
 19  builtin-remote: add set-head subcommand
 20  remote: make guess_remote_head() use exact HEAD lookup if it is available
 21  builtin-remote: new show output style
 22  builtin-remote: new show output style for push refspecs

Here's the new diff stat:

 Documentation/git-remote.txt           |   28 ++-
 Makefile                               |    1 +
 builtin-clone.c                        |   41 +---
 builtin-remote.c                       |  561 ++++++++++++++++++++++++++------
 builtin-send-pack.c                    |   79 +-----
 cache.h                                |    2 +-
 contrib/completion/git-completion.bash |    2 +-
 http-push.c                            |   72 +----
 refs.c                                 |    4 +-
 remote.c                               |  145 ++++++++-
 remote.h                               |   12 +
 string-list.c                          |   10 +
 string-list.h                          |    5 +
 t/lib-httpd.sh                         |    9 +-
 t/t5505-remote.sh                      |  114 +++++--
 t/t5540-http-push.sh                   |    9 +-
 t/t5550-http-fetch.sh                  |   57 ++++
 17 files changed, 821 insertions(+), 330 deletions(-)
 create mode 100755 t/t5550-http-fetch.sh

And here's the inter-diff:

diff --git a/remote.c b/remote.c
index 1c09cf0..9b8522d 100644
--- a/remote.c
+++ b/remote.c
@@ -779,10 +779,18 @@ struct ref *alloc_ref(const char *name)
 
 static struct ref *copy_ref(const struct ref *ref)
 {
-	struct ref *ret = xmalloc(sizeof(struct ref) + strlen(ref->name) + 1);
-	memcpy(ret, ref, sizeof(struct ref) + strlen(ref->name) + 1);
-	ret->next = NULL;
-	return ret;
+	struct ref *cpy;
+	size_t len;
+	if (!ref)
+		return NULL;
+	len = strlen(ref->name);
+	cpy = xmalloc(sizeof(struct ref) + len + 1);
+	memcpy(cpy, ref, sizeof(struct ref) + len + 1);
+	cpy->next = NULL;
+	cpy->symref = ref->symref ? xstrdup(ref->symref) : NULL;
+	cpy->remote_status = ref->remote_status ? xstrdup(ref->remote_status) : NULL;
+	cpy->peer_ref = copy_ref(ref->peer_ref);
+	return cpy;
 }
 
 struct ref *copy_ref_list(const struct ref *ref)
@@ -801,6 +809,7 @@ static void free_ref(struct ref *ref)
 {
 	if (!ref)
 		return;
+	free_ref(ref->peer_ref);
 	free(ref->remote_status);
 	free(ref->symref);
 	free(ref);
@@ -811,7 +820,6 @@ void free_refs(struct ref *ref)
 	struct ref *next;
 	while (ref) {
 		next = ref->next;
-		free(ref->peer_ref);
 		free_ref(ref);
 		ref = next;
 	}
@@ -1457,13 +1465,6 @@ struct ref *get_local_heads(void)
 	return local_refs;
 }
 
-struct ref *copy_ref_with_peer(const struct ref *src)
-{
-	struct ref *dst = copy_ref(src);
-	dst->peer_ref = copy_ref(src->peer_ref);
-	return dst;
-}
-
 struct ref *guess_remote_head(const struct ref *head,
 			      const struct ref *refs,
 			      int all)
@@ -1480,22 +1481,20 @@ struct ref *guess_remote_head(const struct ref *head,
 	 * where HEAD points; if that is the case, then
 	 * we don't have to guess.
 	 */
-	if (head->symref) {
-		r = find_ref_by_name(refs, head->symref);
-		return r ? copy_ref_with_peer(r) : NULL;
-	}
+	if (head->symref)
+		return copy_ref(find_ref_by_name(refs, head->symref));
 
 	/* If refs/heads/master could be right, it is. */
 	if (!all) {
 		r = find_ref_by_name(refs, "refs/heads/master");
 		if (r && !hashcmp(r->old_sha1, head->old_sha1))
-			return copy_ref_with_peer(r);
+			return copy_ref(r);
 	}
 
 	/* Look for another ref that points there */
 	for (r = refs; r; r = r->next) {
 		if (r != head && !hashcmp(r->old_sha1, head->old_sha1)) {
-			*tail = copy_ref_with_peer(r);
+			*tail = copy_ref(r);
 			tail = &((*tail)->next);
 			if (!all)
 				break;

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 07a/21] remote: make copy_ref() perform a deep copy
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (22 preceding siblings ...)
  2009-02-26 17:04 ` Junio C Hamano
@ 2009-02-27 19:10 ` Jay Soffian
  2009-02-27 19:10 ` [PATCH 08/21] remote: let guess_remote_head() optionally return all matches Jay Soffian
  2009-02-27 19:10 ` [PATCH 19/21] remote: make guess_remote_head() use exact HEAD lookup if it is available Jay Soffian
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-27 19:10 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

To ensure that copied refs can always be freed w/o causing a
double-free, make copy_ref() perform a deep copy.

Also have copy_ref() return NULL if asked to copy NULL to simplify
things for the caller.

Background: currently copy_ref() performs a shallow copy. This is fine
for current callers who never free the result and/or only copy refs
which contain NULL pointers. But copy_ref() is about to gain a new
caller (guess_remote_head()) which copies refs where peer_ref is not
NULL and the caller of guess_remote_head() will want to free the result.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
This is a new patch, which should come after 07/21 in the original
series - http://article.gmane.org/gmane.comp.version-control.git/111394

 remote.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/remote.c b/remote.c
index aed760e..22203ea 100644
--- a/remote.c
+++ b/remote.c
@@ -779,10 +779,18 @@ struct ref *alloc_ref(const char *name)
 
 static struct ref *copy_ref(const struct ref *ref)
 {
-	struct ref *ret = xmalloc(sizeof(struct ref) + strlen(ref->name) + 1);
-	memcpy(ret, ref, sizeof(struct ref) + strlen(ref->name) + 1);
-	ret->next = NULL;
-	return ret;
+	struct ref *cpy;
+	size_t len;
+	if (!ref)
+		return NULL;
+	len = strlen(ref->name);
+	cpy = xmalloc(sizeof(struct ref) + len + 1);
+	memcpy(cpy, ref, sizeof(struct ref) + len + 1);
+	cpy->next = NULL;
+	cpy->symref = ref->symref ? xstrdup(ref->symref) : NULL;
+	cpy->remote_status = ref->remote_status ? xstrdup(ref->remote_status) : NULL;
+	cpy->peer_ref = copy_ref(ref->peer_ref);
+	return cpy;
 }
 
 struct ref *copy_ref_list(const struct ref *ref)
@@ -801,6 +809,7 @@ static void free_ref(struct ref *ref)
 {
 	if (!ref)
 		return;
+	free_ref(ref->peer_ref);
 	free(ref->remote_status);
 	free(ref->symref);
 	free(ref);
@@ -811,7 +820,6 @@ void free_refs(struct ref *ref)
 	struct ref *next;
 	while (ref) {
 		next = ref->next;
-		free(ref->peer_ref);
 		free_ref(ref);
 		ref = next;
 	}
-- 
1.6.2.rc1.309.g5f417

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 08/21] remote: let guess_remote_head() optionally return all matches
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (23 preceding siblings ...)
  2009-02-27 19:10 ` [PATCH 07a/21] remote: make copy_ref() perform a deep copy Jay Soffian
@ 2009-02-27 19:10 ` Jay Soffian
  2009-02-27 19:10 ` [PATCH 19/21] remote: make guess_remote_head() use exact HEAD lookup if it is available Jay Soffian
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-27 19:10 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

Determining HEAD is ambiguous since it is done by comparing SHA1s.

In the case of multiple matches we return refs/heads/master if it
matches, else we return the first match we encounter. builtin-remote
needs all matches returned to it, so add a flag for it to request such.

To be simple and consistent, the return value is now a copy (including
peer_ref) of the matching refs.

Originally contributed by Jeff King along with the prior commit as a
single patch.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
This patch replaces 08/21 in the original series
series - http://article.gmane.org/gmane.comp.version-control.git/111394

 builtin-clone.c |    2 +-
 remote.c        |   29 +++++++++++++++++++----------
 remote.h        |   14 ++++++++------
 3 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/builtin-clone.c b/builtin-clone.c
index f9ce4fb..3146ca8 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -510,7 +510,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		mapped_refs = write_remote_refs(refs, &refspec, reflog_msg.buf);
 
 		remote_head = find_ref_by_name(refs, "HEAD");
-		head_points_at = guess_remote_head(remote_head, mapped_refs);
+		head_points_at = guess_remote_head(remote_head, mapped_refs, 0);
 	}
 	else {
 		warning("You appear to have cloned an empty repository.");
diff --git a/remote.c b/remote.c
index 22203ea..304e967 100644
--- a/remote.c
+++ b/remote.c
@@ -1460,24 +1460,33 @@ struct ref *get_local_heads(void)
 	return local_refs;
 }
 
-const struct ref *guess_remote_head(const struct ref *head,
-				    const struct ref *refs)
+struct ref *guess_remote_head(const struct ref *head,
+			      const struct ref *refs,
+			      int all)
 {
 	const struct ref *r;
+	struct ref *list = NULL;
+	struct ref **tail = &list;
 
 	if (!head)
 		return NULL;
 
 	/* If refs/heads/master could be right, it is. */
-	r = find_ref_by_name(refs, "refs/heads/master");
-	if (r && !hashcmp(r->old_sha1, head->old_sha1))
-		return r;
+	if (!all) {
+		r = find_ref_by_name(refs, "refs/heads/master");
+		if (r && !hashcmp(r->old_sha1, head->old_sha1))
+			return copy_ref(r);
+	}
 
 	/* Look for another ref that points there */
-	for (r = refs; r; r = r->next)
-		if (r != head && !hashcmp(r->old_sha1, head->old_sha1))
-			return r;
+	for (r = refs; r; r = r->next) {
+		if (r != head && !hashcmp(r->old_sha1, head->old_sha1)) {
+			*tail = copy_ref(r);
+			tail = &((*tail)->next);
+			if (!all)
+				break;
+		}
+	}
 
-	/* Nothing is the same */
-	return NULL;
+	return list;
 }
diff --git a/remote.h b/remote.h
index db49ce0..de3d21b 100644
--- a/remote.h
+++ b/remote.h
@@ -139,12 +139,14 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs);
 int format_tracking_info(struct branch *branch, struct strbuf *sb);
 
 struct ref *get_local_heads(void);
-
 /*
- * Look for a ref in refs whose SHA1 matches head, first checking if
- * refs/heads/master matches. Return NULL if nothing matches or if head
- * is NULL.
+ * Find refs from a list which are likely to be pointed to by the given HEAD
+ * ref. If 'all' is false, returns the most likely ref; otherwise, returns a
+ * list of all candidate refs. If no match is found (or 'head' is NULL),
+ * returns NULL. All returns are newly allocated and should be freed.
  */
-const struct ref *guess_remote_head(const struct ref *head,
-				    const struct ref *refs);
+struct ref *guess_remote_head(const struct ref *head,
+			      const struct ref *refs,
+			      int all);
+
 #endif
-- 
1.6.2.rc1.309.g5f417

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 19/21] remote: make guess_remote_head() use exact HEAD lookup if it is available
  2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
                   ` (24 preceding siblings ...)
  2009-02-27 19:10 ` [PATCH 08/21] remote: let guess_remote_head() optionally return all matches Jay Soffian
@ 2009-02-27 19:10 ` Jay Soffian
  25 siblings, 0 replies; 33+ messages in thread
From: Jay Soffian @ 2009-02-27 19:10 UTC (permalink / raw
  To: git; +Cc: Jay Soffian, Jeff King, Junio C Hamano

From: Jeff King <peff@peff.net>

Our usual method for determining the ref pointed to by HEAD
is to compare HEAD's sha1 to the sha1 of all refs, trying to
find a unique match.

However, some transports actually get to look at HEAD
directly; we should make use of that information when it is
available.  Currently, only http remotes support this
feature.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
This patch replaces 19/21 in the original series
series - http://article.gmane.org/gmane.comp.version-control.git/111394

 remote.c              |    8 ++++++++
 t/t5550-http-fetch.sh |   11 +++++++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/remote.c b/remote.c
index 2123005..9b8522d 100644
--- a/remote.c
+++ b/remote.c
@@ -1476,6 +1476,14 @@ struct ref *guess_remote_head(const struct ref *head,
 	if (!head)
 		return NULL;
 
+	/*
+	 * Some transports support directly peeking at
+	 * where HEAD points; if that is the case, then
+	 * we don't have to guess.
+	 */
+	if (head->symref)
+		return copy_ref(find_ref_by_name(refs, head->symref));
+
 	/* If refs/heads/master could be right, it is. */
 	if (!all) {
 		r = find_ref_by_name(refs, "refs/heads/master");
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index b6e6ec9..05b1b62 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -42,5 +42,16 @@ test_expect_success 'fetch changes via http' '
 	test_cmp file clone/file
 '
 
+test_expect_success 'http remote detects correct HEAD' '
+	git push public master:other &&
+	(cd clone &&
+	 git remote set-head origin -d &&
+	 git remote set-head origin -a &&
+	 git symbolic-ref refs/remotes/origin/HEAD > output &&
+	 echo refs/remotes/origin/master > expect &&
+	 test_cmp expect output
+	)
+'
+
 stop_httpd
 test_done
-- 
1.6.2.rc1.309.g5f417

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH 0/3] git remote: set-head and new show output (UPDATED)
  2009-02-27 19:10           ` [PATCH 0/3] git remote: set-head and new show output (UPDATED) Jay Soffian
@ 2009-02-28  6:33             ` Jeff King
  0 siblings, 0 replies; 33+ messages in thread
From: Jeff King @ 2009-02-28  6:33 UTC (permalink / raw
  To: Jay Soffian; +Cc: git, Junio C Hamano

On Fri, Feb 27, 2009 at 02:10:03PM -0500, Jay Soffian wrote:

> I didn't want to resend the whole series (gmane 111394), so what I did was
> create a new patch which comes after patch 7 in that series. Call it patch 7a.
> The rest of that series is then identical (modulo line number offsets in
> patches touching remote.c) except for patches 8 and 19. Patches 7a, and the
> new 8 and 19 follow this email.

These look good to me.

-Peff

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2009-02-28  6:34 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-25  8:32 [PATCH 00/21] git remote: set-head and new show output Jay Soffian
2009-02-25  8:32 ` [PATCH 01/21] test scripts: refactor start_httpd helper Jay Soffian
2009-02-25  8:32 ` [PATCH 02/21] add basic http clone/fetch tests Jay Soffian
2009-02-25  8:32 ` [PATCH 03/21] refactor find_ref_by_name() to accept const list Jay Soffian
2009-02-25  8:32 ` [PATCH 04/21] move duplicated get_local_heads() to remote.c Jay Soffian
2009-02-25  8:32 ` [PATCH 05/21] move duplicated ref_newer() " Jay Soffian
2009-02-25  8:32 ` [PATCH 06/21] move locate_head() " Jay Soffian
2009-02-25  8:32 ` [PATCH 07/21] remote: simplify guess_remote_head() Jay Soffian
2009-02-25  8:32 ` [PATCH 08/21] remote: let guess_remote_head() optionally return all matches Jay Soffian
2009-02-26 14:37   ` Jeff King
2009-02-26 14:40     ` Jeff King
2009-02-26 18:47       ` Jay Soffian
2009-02-27 11:43         ` Jeff King
2009-02-27 19:10           ` [PATCH 0/3] git remote: set-head and new show output (UPDATED) Jay Soffian
2009-02-28  6:33             ` Jeff King
2009-02-25  8:32 ` [PATCH 09/21] remote: make match_refs() copy src ref before assigning to peer_ref Jay Soffian
2009-02-25  8:32 ` [PATCH 10/21] remote: make match_refs() not short-circuit Jay Soffian
2009-02-25  8:32 ` [PATCH 11/21] string-list: new for_each_string_list() function Jay Soffian
2009-02-25  8:32 ` [PATCH 12/21] builtin-remote: refactor duplicated cleanup code Jay Soffian
2009-02-25  8:32 ` [PATCH 13/21] builtin-remote: remove unused code in get_ref_states Jay Soffian
2009-02-25  8:32 ` [PATCH 14/21] builtin-remote: rename variables and eliminate redundant function call Jay Soffian
2009-02-25  8:32 ` [PATCH 15/21] builtin-remote: make get_remote_ref_states() always populate states.tracked Jay Soffian
2009-02-25  8:32 ` [PATCH 16/21] builtin-remote: fix two inconsistencies in the output of "show <remote>" Jay Soffian
2009-02-25  8:32 ` [PATCH 17/21] builtin-remote: teach show to display remote HEAD Jay Soffian
2009-02-25  8:32 ` [PATCH 18/21] builtin-remote: add set-head subcommand Jay Soffian
2009-02-25  8:32 ` [PATCH 19/21] remote: make guess_remote_head() use exact HEAD lookup if it is available Jay Soffian
2009-02-25  8:32 ` [PATCH 20/21] builtin-remote: new show output style Jay Soffian
2009-02-25  8:32 ` [PATCH 21/21] builtin-remote: new show output style for push refspecs Jay Soffian
2009-02-26 14:53 ` [PATCH 00/21] git remote: set-head and new show output Jeff King
2009-02-26 17:04 ` Junio C Hamano
2009-02-27 19:10 ` [PATCH 07a/21] remote: make copy_ref() perform a deep copy Jay Soffian
2009-02-27 19:10 ` [PATCH 08/21] remote: let guess_remote_head() optionally return all matches Jay Soffian
2009-02-27 19:10 ` [PATCH 19/21] remote: make guess_remote_head() use exact HEAD lookup if it is available Jay Soffian

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).