{"thread":{"id":"17770","subject":"[PATCH 1/4] builtin-clone: move locate_head() to remote.c so it can be re-used","startedAt":"2009-02-13T08:54:31Z","lastAt":"2009-02-16T02:58:48Z","messageCount":41,"participants":["Jay Soffian","Junio C Hamano","Jeff King","Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"104497","messageId":"1234515275-91263-1-git-send-email-jaysoffian@gmail.com","threadId":"17770","inReplyTo":null,"subject":"[PATCH 0/4] remote HEAD improvements take 2","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T08:54:31Z","receivedAt":"2009-02-13T08:54:31Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"There is currently no porcelain for dealing with remote HEADs (i.e.\n$GIT_DIR/remotes//HEAD). This series:\n\n1) Refactors locate_head() from builtin-clone.c to remote.c so it can be used\n by builtin-remote.c as well. I also renamed it to guess_remote_head().\n\n Daniel suggested having it specifically check that it returns a ref from\n refs/heads/, but I wasn't sure what impact that might have (good or\n bad...), so I punted on that change.\n\n2) Teaches git remote show to display the remote HEAD:\n\n $ git remote show origin\n\n* remote origin\n URL: git://git.kernel.org/pub/scm/git/git.git\n HEAD: master\n\n3) Teaches git remote a new \"set-head\" verb:\n\n To set a remote HEAD explicitly:\n $ git remote set-head \n\n To set a remote HEAD to match the upstream repo:\n $ git remote set-head -a\n\n To delete a remote HEAD:\n $ git remote set-head -d\n\n I changed it from \"sethead\" to \"set-head\" per Jeff.\n\n I also remembered to update git-completion.bash this time.\n\n4) Documents the new set-head verb. I also correct the git remote man page\n w/respect to the \"-m \" option. The man page implied that the remote\n HEAD was set automatically when adding a remote (a la git clone), but this\n is not true. And, since I couldn't find anywhere else that the point of\n having a remote HEAD is documented, I documented it here.\n\nJay Soffian (4):\n builtin-clone: move locate_head() to remote.c so it can be re-used\n builtin-remote: move duplicated cleanup code its own function\n builtin-remote: teach show to display remote HEAD\n builtin-remote: add set-head verb\n\n Documentation/git-remote.txt | 20 ++++++-\n builtin-clone.c | 41 +-------------\n builtin-remote.c | 96 +++++++++++++++++++++++++++++---\n contrib/completion/git-completion.bash | 2 +-\n remote.c | 37 ++++++++++++\n remote.h | 9 +++\n 6 files changed, 156 insertions(+), 49 deletions(-)\n"},{"id":"104496","messageId":"1234515275-91263-2-git-send-email-jaysoffian@gmail.com","threadId":"17770","inReplyTo":"1234515275-91263-1-git-send-email-jaysoffian@gmail.com","subject":"[PATCH 1/4] builtin-clone: move locate_head() to remote.c so it can be re-used","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T08:54:32Z","receivedAt":"2009-02-13T08:54:32Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Move locate_head() to remote.c and rename it to guess_remote_head() to\nmore accurately reflect what it does. This is in preparation for being\nable to call it from builtin-remote.c\n\nSigned-off-by: Jay Soffian \n---\n builtin-clone.c | 41 +++--------------------------------------\n remote.c | 37 +++++++++++++++++++++++++++++++++++++\n remote.h | 9 +++++++++\n 3 files changed, 49 insertions(+), 38 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex f73029e..280b866 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -20,6 +20,7 @@\n #include \"dir.h\"\n #include \"pack-refs.h\"\n #include \"sigchain.h\"\n+#include \"remote.h\"\n \n /*\n * Overall FIXMEs:\n@@ -293,43 +294,6 @@ static void remove_junk_on_signal(int signo)\n \traise(signo);\n }\n \n-static const struct ref *locate_head(const struct ref *refs,\n-\t\t\t\t const struct ref *mapped_refs,\n-\t\t\t\t const struct ref **remote_head_p)\n-{\n-\tconst struct ref *remote_head = NULL;\n-\tconst struct ref *remote_master = NULL;\n-\tconst struct ref *r;\n-\tfor (r = refs; r; r = r->next)\n-\t\tif (!strcmp(r->name, \"HEAD\"))\n-\t\t\tremote_head = r;\n-\n-\tfor (r = mapped_refs; r; r = r->next)\n-\t\tif (!strcmp(r->name, \"refs/heads/master\"))\n-\t\t\tremote_master = r;\n-\n-\tif (remote_head_p)\n-\t\t*remote_head_p = remote_head;\n-\n-\t/* If there's no HEAD value at all, never mind. */\n-\tif (!remote_head)\n-\t\treturn NULL;\n-\n-\t/* If refs/heads/master could be right, it is. */\n-\tif (remote_master && !hashcmp(remote_master->old_sha1,\n-\t\t\t\t remote_head->old_sha1))\n-\t\treturn remote_master;\n-\n-\t/* Look for another ref that points there */\n-\tfor (r = mapped_refs; r; r = r->next)\n-\t\tif (r != remote_head &&\n-\t\t !hashcmp(r->old_sha1, remote_head->old_sha1))\n-\t\t\treturn r;\n-\n-\t/* Nothing is the same */\n-\treturn NULL;\n-}\n-\n static struct ref *write_remote_refs(const struct ref *refs,\n \t\tstruct refspec *refspec, const char *reflog)\n {\n@@ -532,7 +496,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \t\tmapped_refs = write_remote_refs(refs, &refspec, reflog_msg.buf);\n \n-\t\thead_points_at = locate_head(refs, mapped_refs, &remote_head);\n+\t\thead_points_at = guess_remote_head(refs, mapped_refs,\n+\t\t\t\t\t\t &remote_head);\n \t}\n \telse {\n \t\twarning(\"You appear to have cloned an empty repository.\");\ndiff --git a/remote.c b/remote.c\nindex d7079c6..447f091 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1376,3 +1376,40 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)\n \t\t\t base, num_ours, num_theirs);\n \treturn 1;\n }\n+\n+const struct ref *guess_remote_head(const struct ref *refs,\n+\t\t\t\t const struct ref *mapped_refs,\n+\t\t\t\t const struct ref **remote_head_p)\n+{\n+\tconst struct ref *remote_head = NULL;\n+\tconst struct ref *remote_master = NULL;\n+\tconst struct ref *r;\n+\tfor (r = refs; r; r = r->next)\n+\t\tif (!strcmp(r->name, \"HEAD\"))\n+\t\t\tremote_head = r;\n+\n+\tfor (r = mapped_refs; r; r = r->next)\n+\t\tif (!strcmp(r->name, \"refs/heads/master\"))\n+\t\t\tremote_master = r;\n+\n+\tif (remote_head_p)\n+\t\t*remote_head_p = remote_head;\n+\n+\t/* If there's no HEAD value at all, never mind. */\n+\tif (!remote_head)\n+\t\treturn NULL;\n+\n+\t/* If refs/heads/master could be right, it is. */\n+\tif (remote_master && !hashcmp(remote_master->old_sha1,\n+\t\t\t\t remote_head->old_sha1))\n+\t\treturn remote_master;\n+\n+\t/* Look for another ref that points there */\n+\tfor (r = mapped_refs; r; r = r->next)\n+\t\tif (r != remote_head &&\n+\t\t !hashcmp(r->old_sha1, remote_head->old_sha1))\n+\t\t\treturn r;\n+\n+\t/* Nothing is the same */\n+\treturn NULL;\n+}\ndiff --git a/remote.h b/remote.h\nindex a46a5be..cabb14a 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -137,4 +137,13 @@ enum match_refs_flags {\n int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs);\n int format_tracking_info(struct branch *branch, struct strbuf *sb);\n \n+/*\n+ * Look in refs for HEAD. Then look for a matching SHA1 in mapped_refs,\n+ * first checking if refs/heads/master matches. Return NULL if nothing matches\n+ * or if there is no HEAD in refs. remote_head_p is assigned HEAD if not NULL.\n+ */\n+const struct ref *guess_remote_head(const struct ref *refs,\n+\t\t\t\t const struct ref *mapped_refs,\n+\t\t\t\t const struct ref **remote_head_p);\n+\n #endif\n-- \n1.6.2.rc0.209.g7c178\n"},{"id":"104498","messageId":"1234515275-91263-3-git-send-email-jaysoffian@gmail.com","threadId":"17770","inReplyTo":"1234515275-91263-2-git-send-email-jaysoffian@gmail.com","subject":"[PATCH 2/4] builtin-remote: move duplicated cleanup code its own function","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T08:54:33Z","receivedAt":"2009-02-13T08:54:33Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Moved some identical lines of code into their own function in\npreparation for adding additional functionality which will use this\nfunction as well.\n\nAlso removed a bogus NEEDSWORK comment per Daniel Barkalow:\n\n Actually, the comment is wrong; \"remote\" comes from remote_get(),\n which returns things from a cache in remote.c; there could be a\n remote_put() to let the code know that the caller is done with the\n object, but it wouldn't presently do anything.\n\nSigned-off-by: Jay Soffian \n---\n builtin-remote.c | 17 +++++++++--------\n 1 files changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex ac69d37..b89a353 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -632,6 +632,13 @@ static void show_list(const char *title, struct string_list *list,\n \t\tprintf(\" %s\\n\", list->items[i].string);\n }\n \n+static void free_remote_ref_states(struct ref_states *states)\n+{\n+\tstring_list_clear(&states->new, 0);\n+\tstring_list_clear(&states->stale, 0);\n+\tstring_list_clear(&states->tracked, 0);\n+}\n+\n static int get_remote_ref_states(const char *name,\n \t\t\t\t struct ref_states *states,\n \t\t\t\t int query)\n@@ -738,10 +745,7 @@ static int show(int argc, const char **argv)\n \t\t\t}\n \t\t}\n \n-\t\t/* NEEDSWORK: free remote */\n-\t\tstring_list_clear(&states.new, 0);\n-\t\tstring_list_clear(&states.stale, 0);\n-\t\tstring_list_clear(&states.tracked, 0);\n+\t\tfree_remote_ref_states(&states);\n \t}\n \n \treturn result;\n@@ -792,10 +796,7 @@ static int prune(int argc, const char **argv)\n \t\t\twarn_dangling_symref(dangling_msg, refname);\n \t\t}\n \n-\t\t/* NEEDSWORK: free remote */\n-\t\tstring_list_clear(&states.new, 0);\n-\t\tstring_list_clear(&states.stale, 0);\n-\t\tstring_list_clear(&states.tracked, 0);\n+\t\tfree_remote_ref_states(&states);\n \t}\n \n \treturn result;\n-- \n1.6.2.rc0.209.g7c178\n"},{"id":"104499","messageId":"1234515275-91263-4-git-send-email-jaysoffian@gmail.com","threadId":"17770","inReplyTo":"1234515275-91263-3-git-send-email-jaysoffian@gmail.com","subject":"[PATCH 3/4] builtin-remote: teach show to display remote HEAD","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T08:54:34Z","receivedAt":"2009-02-13T08:54:34Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"This is in preparation for teaching remote how to set\nrefs/remotes//HEAD to match what HEAD is set to at , but\nis useful in its own right.\n\nSigned-off-by: Jay Soffian \n---\n builtin-remote.c | 26 ++++++++++++++++++++++++++\n 1 files changed, 26 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex b89a353..465c87a 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -212,6 +212,7 @@ static void read_branches(void)\n struct ref_states {\n \tstruct remote *remote;\n \tstruct string_list new, stale, tracked;\n+\tchar *head_name;\n };\n \n static int handle_one_branch(const char *refname,\n@@ -271,6 +272,26 @@ static int get_ref_states(const struct ref *ref, struct ref_states *states)\n \treturn 0;\n }\n \n+static char *get_head_name(const struct ref *refs)\n+{\n+\tconst struct ref *head_points_at;\n+\tstruct ref *mapped_refs = NULL;\n+\tstruct ref **tail = &mapped_refs;\n+\tstruct refspec refspec;\n+\n+\trefspec.force = 0;\n+\trefspec.pattern = 1;\n+\trefspec.src = refspec.dst = \"refs/heads/\";\n+\n+\tget_fetch_map(refs, &refspec, &tail, 0);\n+\n+\thead_points_at = guess_remote_head(refs, mapped_refs, NULL);\n+\tif (head_points_at)\n+\t\treturn xstrdup(abbrev_branch(head_points_at->name));\n+\n+\treturn NULL;\n+}\n+\n struct known_remote {\n \tstruct known_remote *next;\n \tstruct remote *remote;\n@@ -637,6 +658,7 @@ static void free_remote_ref_states(struct ref_states *states)\n \tstring_list_clear(&states->new, 0);\n \tstring_list_clear(&states->stale, 0);\n \tstring_list_clear(&states->tracked, 0);\n+\tfree(states->head_name);\n }\n \n static int get_remote_ref_states(const char *name,\n@@ -658,6 +680,7 @@ static int get_remote_ref_states(const char *name,\n \t\tref = transport_get_remote_refs(transport);\n \t\ttransport_disconnect(transport);\n \n+\t\tstates->head_name = get_head_name(ref);\n \t\tget_ref_states(ref, states);\n \t}\n \n@@ -702,6 +725,9 @@ static int show(int argc, const char **argv)\n \t\tprintf(\"* remote %s\\n URL: %s\\n\", *argv,\n \t\t\tstates.remote->url_nr > 0 ?\n \t\t\t\tstates.remote->url[0] : \"(no URL)\");\n+\t\tif (!no_query)\n+\t\t\tprintf(\" HEAD: %s\\n\", states.head_name ?\n+\t\t\t\tstates.head_name : \"(unknown)\");\n \n \t\tfor (i = 0; i < branch_list.nr; i++) {\n \t\t\tstruct string_list_item *branch = branch_list.items + i;\n-- \n1.6.2.rc0.209.g7c178\n"},{"id":"104500","messageId":"1234515275-91263-5-git-send-email-jaysoffian@gmail.com","threadId":"17770","inReplyTo":"1234515275-91263-4-git-send-email-jaysoffian@gmail.com","subject":"[PATCH 4/4] builtin-remote: add set-head verb","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T08:54:35Z","receivedAt":"2009-02-13T08:54:35Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Provide a porcelain command for setting/deleting\n$GIT_DIR/remotes//HEAD.\n\nWhile we're at it, document what $GIT_DIR/remotes//HEAD is all\nabout.\n\nSigned-off-by: Jay Soffian \n---\n Documentation/git-remote.txt | 20 ++++++++++-\n builtin-remote.c | 54 +++++++++++++++++++++++++++++++-\n contrib/completion/git-completion.bash | 2 +-\n 3 files changed, 72 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\nindex fad983e..80f2cfe 100644\n--- a/Documentation/git-remote.txt\n+++ b/Documentation/git-remote.txt\n@@ -13,6 +13,7 @@ SYNOPSIS\n 'git remote add' [-t ] [-m ] [-f] [--mirror] \n 'git remote rename' \n 'git remote rm' \n+'git remote set-head' [-a | -d | ]\n 'git remote show' [-n] \n 'git remote prune' [-n | --dry-run] \n 'git remote update' [group]\n@@ -53,8 +54,7 @@ is created. You can give more than one `-t ` to track\n multiple branches without grabbing all branches.\n +\n With `-m ` option, `$GIT_DIR/remotes//HEAD` is set\n-up to point at remote's `` branch instead of whatever\n-branch the `HEAD` at the remote repository actually points at.\n+up to point at remote's `` branch. See also the set-head command.\n +\n In mirror mode, enabled with `\\--mirror`, the refs will not be stored\n in the 'refs/remotes/' namespace, but in 'refs/heads/'. This option\n@@ -76,6 +76,22 @@ the configuration file format.\n Remove the remote named . All remote tracking branches and\n configuration settings for the remote are removed.\n \n+'set-head'::\n+\n+Sets or deletes the default branch (`$GIT_DIR/remotes//HEAD`) for\n+the named remote. Having a default branch for a remote is not required,\n+but allows the name of the remote to be specified in lieu of a specific\n+branch. For example, if the default branch for `origin` is set to\n+`master`, then `origin` may be specified wherever you would normally\n+specify `origin/master`.\n++\n+With `-d`, `$GIT_DIR/remotes//HEAD` is deleted.\n++\n+With `-a`, the remote is queried to determine its `HEAD`, then\n+`$GIT_DIR/remotes//HEAD` is set to the same branch.\n++\n+Use `` to set `$GIT_DIR/remotes//HEAD` explicitly.\n+\n 'show'::\n \n Gives some information about the remote .\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex 465c87a..677e20e 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -12,6 +12,7 @@ static const char * const builtin_remote_usage[] = {\n \t\"git remote add [-t ] [-m ] [-f] [--mirror] \",\n \t\"git remote rename \",\n \t\"git remote rm \",\n+\t\"git remote set-head [-a | -d | ]\",\n \t\"git remote show [-n] \",\n \t\"git remote prune [-n | --dry-run] \",\n \t\"git remote [-v | --verbose] update [group]\",\n@@ -658,7 +659,8 @@ static void free_remote_ref_states(struct ref_states *states)\n \tstring_list_clear(&states->new, 0);\n \tstring_list_clear(&states->stale, 0);\n \tstring_list_clear(&states->tracked, 0);\n-\tfree(states->head_name);\n+\tif (states->head_name)\n+\t\tfree(states->head_name);\n }\n \n static int get_remote_ref_states(const char *name,\n@@ -777,6 +779,54 @@ static int show(int argc, const char **argv)\n \treturn result;\n }\n \n+static int sethead(int argc, const char **argv)\n+{\n+\tint opt_a = 0, opt_d = 0, result = 0;\n+\tstruct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;\n+\tchar *head_name = NULL;\n+\n+\tstruct option options[] = {\n+\t\tOPT_GROUP(\"set-head specific options\"),\n+\t\tOPT_BOOLEAN('a', 0, &opt_a,\n+\t\t \"set refs/remotes//HEAD according to remote\"),\n+\t\tOPT_BOOLEAN('d', 0, &opt_d, \"delete refs/remotes//HEAD\"),\n+\t\tOPT_END()\n+\t};\n+\targc = parse_options(argc, argv, options, builtin_remote_usage, 0);\n+\tif ((argc == 1 && !(opt_a || opt_d)) ||\n+\t ((argc == 2 && (opt_a || opt_d))) || argc < 1 || argc > 2)\n+\t\tusage_with_options(builtin_remote_usage, options);\n+\n+\tstrbuf_addf(&buf, \"refs/remotes/%s/HEAD\", argv[0]);\n+\n+\tif (opt_d) {\n+\t\tif (result |= delete_ref(buf.buf, NULL, REF_NODEREF))\n+\t\t\terror(\"Could not delete %s\", buf.buf);\n+\t} else if (opt_a) {\n+\t\tstruct ref_states states;\n+\t\tmemset(&states, 0, sizeof(states));\n+\t\tget_remote_ref_states(argv[0], &states, 1);\n+\t\thead_name = xstrdup(states.head_name);\n+\t\tfree_remote_ref_states(&states);\n+\t} else\n+\t\thead_name = xstrdup(argv[1]);\n+\n+\tif (head_name) {\n+\t\tunsigned char sha1[20];\n+\t\tstrbuf_addf(&buf2, \"refs/remotes/%s/%s\", argv[0], head_name);\n+\t\t/* make sure it's valid */\n+\t\tif (!resolve_ref(buf2.buf, sha1, 1, NULL))\n+\t\t\tresult |= error(\"Not a valid ref: %s\", buf2.buf);\n+\t\telse if (create_symref(buf.buf, buf2.buf, \"remote set-head\"))\n+\t\t\tresult |= error(\"Could not setup %s\", buf.buf);\n+\t\tfree(head_name);\n+\t}\n+\n+\tstrbuf_release(&buf);\n+\tstrbuf_release(&buf2);\n+\treturn result;\n+}\n+\n static int prune(int argc, const char **argv)\n {\n \tint dry_run = 0, result = 0;\n@@ -947,6 +997,8 @@ int cmd_remote(int argc, const char **argv, const char *prefix)\n \t\tresult = mv(argc, argv);\n \telse if (!strcmp(argv[0], \"rm\"))\n \t\tresult = rm(argc, argv);\n+\telse if (!strcmp(argv[0], \"set-head\"))\n+\t\tresult = sethead(argc, argv);\n \telse if (!strcmp(argv[0], \"show\"))\n \t\tresult = show(argc, argv);\n \telse if (!strcmp(argv[0], \"prune\"))\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex f44f63c..5de1922 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1414,7 +1414,7 @@ _git_config ()\n \n _git_remote ()\n {\n-\tlocal subcommands=\"add rename rm show prune update\"\n+\tlocal subcommands=\"add rename rm show prune update set-head\"\n \tlocal subcommand=\"$(__git_find_subcommand \"$subcommands\")\"\n \tif [ -z \"$subcommand\" ]; then\n \t\t__gitcomp \"$subcommands\"\n-- \n1.6.2.rc0.209.g7c178\n"},{"id":"104501","messageId":"76718490902130057g4b05f87cn716bdf9769fda7bc@mail.gmail.com","threadId":"17770","inReplyTo":"1234515275-91263-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH 0/4] remote HEAD improvements take 2","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T08:57:13Z","receivedAt":"2009-02-13T08:57:13Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Feb 13, 2009 at 3:54 AM, Jay Soffian wrote:\n> There is currently no porcelain for dealing with remote HEADs (i.e.\n> $GIT_DIR/remotes//HEAD). This series:\n\nCrud, sorry for the duplicates.\n\nAnd I just noticed a send-email bug -- it doesn't handle multiple Cc:\nrecipients the way they are formated by format-patch. e.g, format\npatch does this:\n\nCc: firstperson@example.com,\n secondperson@example.com\n\nBut send-email only picks up the first address, and further, appears\nto mangle it.\n\nj.\n"},{"id":"104504","messageId":"7vtz6yabsy.fsf@gitster.siamese.dyndns.org","threadId":"17770","inReplyTo":"1234515275-91263-5-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH 4/4] builtin-remote: add set-head verb","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-13T10:09:01Z","receivedAt":"2009-02-13T10:09:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> Provide a porcelain command for setting/deleting\n> $GIT_DIR/remotes//HEAD.\n\nThe entire series looks sane from a very cursory look; especially the\nearlier ones are obviously good.\n\nCalling the subcommand a \"verb\" is somewhat new, though. Existing\ndocumentation for git commands that take multiple actions seem to call\nthem subcommands, including \"git-remote.txt\" itself.\n\n> diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\n> index fad983e..80f2cfe 100644\n> --- a/Documentation/git-remote.txt\n> +++ b/Documentation/git-remote.txt\n> @@ -76,6 +76,22 @@ the configuration file format.\n> Remove the remote named . All remote tracking branches and\n> configuration settings for the remote are removed.\n> \n> +'set-head'::\n> +\n> +Sets or deletes the default branch (`$GIT_DIR/remotes//HEAD`) for\n> +the named remote. Having a default branch for a remote is not required,\n> +but allows the name of the remote to be specified in lieu of a specific\n> +branch. For example, if the default branch for `origin` is set to\n> +`master`, then `origin` may be specified wherever you would normally\n> +specify `origin/master`.\n> ++\n> +With `-d`, `$GIT_DIR/remotes//HEAD` is deleted.\n> ++\n> +With `-a`, the remote is queried to determine its `HEAD`, then\n> +`$GIT_DIR/remotes//HEAD` is set to the same branch.\n> ++\n> +Use `` to set `$GIT_DIR/remotes//HEAD` explicitly.\n> +\n\nHmph, what does \"-a\" stand for? I would have expected to see \"-u\" that\nstands for \"update\" here.\n\nAlso it may be better to be more explicit about both the syntax and the\nsemantics of ``. Do you expect \"refs/remotes//master\" or\njust \"master\" (I assume the latter)? Is it an error if the branch does\nnot exist in the specified hierarchy? Can you force to set to a branch\nthat does not exist in your tracking side (yet) but you know exists on the\nremote side already?\n\n> diff --git a/builtin-remote.c b/builtin-remote.c\n> index 465c87a..677e20e 100644\n> --- a/builtin-remote.c\n> +++ b/builtin-remote.c\n> @@ -658,7 +659,8 @@ static void free_remote_ref_states(struct ref_states *states)\n> \tstring_list_clear(&states->new, 0);\n> \tstring_list_clear(&states->stale, 0);\n> \tstring_list_clear(&states->tracked, 0);\n> -\tfree(states->head_name);\n> +\tif (states->head_name)\n> +\t\tfree(states->head_name);\n> }\n\nRegression?\n\n> @@ -777,6 +779,54 @@ static int show(int argc, const char **argv)\n> \treturn result;\n> }\n> \n> +static int sethead(int argc, const char **argv)\n\nset_head()?\n\n> +{\n> +\tint opt_a = 0, opt_d = 0, result = 0;\n> +\tstruct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;\n> +\tchar *head_name = NULL;\n> +\n> +\tstruct option options[] = {\n> +\t\tOPT_GROUP(\"set-head specific options\"),\n> +\t\tOPT_BOOLEAN('a', 0, &opt_a,\n> +\t\t \"set refs/remotes//HEAD according to remote\"),\n> +\t\tOPT_BOOLEAN('d', 0, &opt_d, \"delete refs/remotes//HEAD\"),\n> +\t\tOPT_END()\n> +\t};\n> +\targc = parse_options(argc, argv, options, builtin_remote_usage, 0);\n> +\tif ((argc == 1 && !(opt_a || opt_d)) ||\n> +\t ((argc == 2 && (opt_a || opt_d))) || argc < 1 || argc > 2)\n> +\t\tusage_with_options(builtin_remote_usage, options);\n\nThe code will scale better, especially for a young subcommand that may acquire\nnew options, if the check is done by each codepath that deals with a\nspecific option to do this kind of check. That is, e.g.\n\n\tif (opt_delete) {\n \terror if the arg is not remote (alone)\n do the \"delete\" thing\n\t} else if (opt_update) {\n \terror if the arg is not remote (alone)\n do the \"update\" thing\n\t} else {\n \terror if the args are not (remote, branch)\n\t\tdo the \"set\" thing\n\t}\n"},{"id":"104505","messageId":"76718490902130221q3114fbdbt89692e79158530ff@mail.gmail.com","threadId":"17770","inReplyTo":"7vtz6yabsy.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/4] builtin-remote: add set-head verb","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T10:21:20Z","receivedAt":"2009-02-13T10:21:20Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Feb 13, 2009 at 5:09 AM, Junio C Hamano wrote:\n> Calling the subcommand a \"verb\" is somewhat new, though. Existing\n> documentation for git commands that take multiple actions seem to call\n> them subcommands, including \"git-remote.txt\" itself.\n\nOkay.\n\n> Hmph, what does \"-a\" stand for? I would have expected to see \"-u\" that\n> stands for \"update\" here.\n\n--automatic -- as in figure out the name automatically from the other side.\n\n> Also it may be better to be more explicit about both the syntax and the\n> semantics of ``.\n\nOkay.\n\n> Do you expect \"refs/remotes//master\" or\n> just \"master\" (I assume the latter)?\n\nYes, the latter. If you did the wrong thing the error ought clue you in:\n\n$ ./git remote set-head origin refs/remotes/origin/master\nerror: Not a valid ref: refs/remotes/origin/refs/remotes/origin/master\n\n> Is it an error if the branch does\n> not exist in the specified hierarchy?\n\nYes it is an error per-above. Well, at least on-top of next it is.\n\n> Can you force to set to a branch\n> that does not exist in your tracking side (yet) but you know exists on the\n> remote side already?\n\nNo.\n\n>> diff --git a/builtin-remote.c b/builtin-remote.c\n>> index 465c87a..677e20e 100644\n>> --- a/builtin-remote.c\n>> +++ b/builtin-remote.c\n>> @@ -658,7 +659,8 @@ static void free_remote_ref_states(struct ref_states *states)\n>> string_list_clear(&states->new, 0);\n>> string_list_clear(&states->stale, 0);\n>> string_list_clear(&states->tracked, 0);\n>> - free(states->head_name);\n>> + if (states->head_name)\n>> + free(states->head_name);\n>> }\n>\n> Regression?\n\nIndeed.\n\n> set_head()?\n\nYep.\n\n> The code will scale better, especially for a young subcommand that may acquire\n> new options, if the check is done by each codepath that deals with a\n> specific option to do this kind of check. That is, e.g.\n>\n> if (opt_delete) {\n> error if the arg is not remote (alone)\n> do the \"delete\" thing\n> } else if (opt_update) {\n> error if the arg is not remote (alone)\n> do the \"update\" thing\n> } else {\n> error if the args are not (remote, branch)\n> do the \"set\" thing\n> }\n\nGot it. I really am trying to match existing code, but it seems the\nstandards have gotten higher, so I need to do better than existing\ncode.\n\nThanks,\n\nj.\n"},{"id":"104506","messageId":"7vhc2yaak8.fsf@gitster.siamese.dyndns.org","threadId":"17770","inReplyTo":"7vtz6yabsy.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/4] builtin-remote: add set-head verb","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-13T10:35:51Z","receivedAt":"2009-02-13T10:35:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Also it may be better to be more explicit about both the syntax and the\n> semantics of ``. Do you expect \"refs/remotes//master\" or\n> just \"master\" (I assume the latter)? Is it an error if the branch does\n> not exist in the specified hierarchy? Can you force to set to a branch\n> that does not exist in your tracking side (yet) but you know exists on the\n> remote side already?\n\nA few things I forgot (and before I go to bed).\n\nIf remotes//HEAD already points at a branch frotz, and you peek the\nremote (i.e. you do not actually run \"fetch\" to download objects, but just\n\"ls-remote\" it) and find out that \"HEAD\" does not point at the same commit\nas \"frotz\" but it now points at the same commit as another branch\n\"nitfol\", you probably would want to update it to point at \"nitfol\", but\nit was unclear from the description in the documentation if this option\nwas meant to perform this kind fo update, or only to set a missing HEAD.\n\nIf you meant to do an update, there is one thing to watch out for when you\nreuse the logic used by clone. It favors \"master\" if more than one\nbranches point at the same commit as HEAD. You need to customize it to\nfavor the one that the HEAD points at on the local side before you start\n(iow, try to keep the current value when you can).\n\nFor example, if it points at \"frotz\" locally when the command was started,\nand you found out that HEAD now points at the commit at the tip of \"frotz\"\nand \"master\" branches by peeking, you do not want to repoint HEAD from\n\"frotz\" to \"master\".\n"},{"id":"104507","messageId":"76718490902130252t18f4f9dav7aada6fe7bb6ab1b@mail.gmail.com","threadId":"17770","inReplyTo":"7vhc2yaak8.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/4] builtin-remote: add set-head verb","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T10:52:57Z","receivedAt":"2009-02-13T10:52:57Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Feb 13, 2009 at 5:35 AM, Junio C Hamano wrote:\n> A few things I forgot (and before I go to bed).\n>\n> If remotes//HEAD already points at a branch frotz, and you peek the\n> remote (i.e. you do not actually run \"fetch\" to download objects, but just\n> \"ls-remote\" it) and find out that \"HEAD\" does not point at the same commit\n> as \"frotz\" but it now points at the same commit as another branch\n> \"nitfol\", you probably would want to update it to point at \"nitfol\", but\n> it was unclear from the description in the documentation if this option\n> was meant to perform this kind fo update, or only to set a missing HEAD.\n\n- set-head -d deletes\n- set-head -a sets HEAD to whatever remote it\n- set-head sets HEAD to foo\n\nI don't know how to be more explicit than that I think that's what the\ndocumentation indicates.\n\n> If you meant to do an update, there is one thing to watch out for when you\n> reuse the logic used by clone. It favors \"master\" if more than one\n> branches point at the same commit as HEAD.\n\nYes, I know.\n\n> You need to customize it to\n> favor the one that the HEAD points at on the local side before you start\n> (iow, try to keep the current value when you can).\n>\n> For example, if it points at \"frotz\" locally when the command was started,\n> and you found out that HEAD now points at the commit at the tip of \"frotz\"\n> and \"master\" branches by peeking, you do not want to repoint HEAD from\n> \"frotz\" to \"master\".\n\nI think that is an unnecessary complication for what is really a\ncorner case. And anyway, it is just hacking around the fact that we\ndon't really know what the remote side is if multiple branch heads\nhave the same SHA1 as HEAD.\n\nYou proposed a series a while back so that git could unambiguously\ndetermine what a remote symref points to, and I think that's the\nbetter way to fix this problem.\n\nIf the user runs \"git remote set-head -a \" they will get\nwhatever \"git show \" indicates is the remote HEAD. If they\ndon't like that, they can set their /HEAD explicitly using the\nalternate syntax.\n\nj.\n"},{"id":"104511","messageId":"1234525343-8014-1-git-send-email-jaysoffian@gmail.com","threadId":"17770","inReplyTo":"76718490902130221q3114fbdbt89692e79158530ff@mail.gmail.com","subject":"[PATCH v2 4/4] builtin-remote: add set-head subcommand","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-13T11:42:23Z","receivedAt":"2009-02-13T11:42:23Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Provide a porcelain command for setting/deleting\n$GIT_DIR/remotes//HEAD.\n\nWhile we're at it, document what $GIT_DIR/remotes//HEAD is all\nabout.\n\nSigned-off-by: Jay Soffian \n---\nThis replaces \"[PATCH 4/4] builtin-remote: add set-head verb\". The rest of\nthe series is unchanged.\n\n * Changed commit title to call it a subcommand, not a verb.\n * Clarified documentation slightly\n * Added --auto and --delete long options to make what -a and -d stand for\n more obvious.\n * Fixed small regression where I was checking a pointer for NULL before\n free()'ing it.\n * sethead() -> set_head()\n * Reworked the argument processing in set_head() to be more maintainable if\n additional switches are added later.\n\nOn Fri, Feb 13, 2009 at 5:35 AM, Junio C Hamano wrote:\n> You need to customize it to\n> favor the one that the HEAD points at on the local side before you start\n> (iow, try to keep the current value when you can).\n>\n> For example, if it points at \"frotz\" locally when the command was started,\n> and you found out that HEAD now points at the commit at the tip of \"frotz\"\n> and \"master\" branches by peeking, you do not want to repoint HEAD from\n> \"frotz\" to \"master\".\n\nThis I did not do, and I still don't feel it is necessary. However, if you\nfeel strongly about it I can do it as an additional patch to this series. But\nI'd like to see this in pu at least first. I really hope not doing it isn't a\nroadblock for the whole series.\n\n Documentation/git-remote.txt | 28 ++++++++++++++++-\n builtin-remote.c | 51 ++++++++++++++++++++++++++++++++\n contrib/completion/git-completion.bash | 2 +-\n 3 files changed, 78 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\nindex fad983e..c9c0e6f 100644\n--- a/Documentation/git-remote.txt\n+++ b/Documentation/git-remote.txt\n@@ -13,6 +13,7 @@ SYNOPSIS\n 'git remote add' [-t ] [-m ] [-f] [--mirror] \n 'git remote rename' \n 'git remote rm' \n+'git remote set-head' [-a | -d | ]\n 'git remote show' [-n] \n 'git remote prune' [-n | --dry-run] \n 'git remote update' [group]\n@@ -53,8 +54,7 @@ is created. You can give more than one `-t ` to track\n multiple branches without grabbing all branches.\n +\n With `-m ` option, `$GIT_DIR/remotes//HEAD` is set\n-up to point at remote's `` branch instead of whatever\n-branch the `HEAD` at the remote repository actually points at.\n+up to point at remote's `` branch. See also the set-head command.\n +\n In mirror mode, enabled with `\\--mirror`, the refs will not be stored\n in the 'refs/remotes/' namespace, but in 'refs/heads/'. This option\n@@ -76,6 +76,30 @@ the configuration file format.\n Remove the remote named . All remote tracking branches and\n configuration settings for the remote are removed.\n \n+'set-head'::\n+\n+Sets or deletes the default branch (`$GIT_DIR/remotes//HEAD`) for\n+the named remote. Having a default branch for a remote is not required,\n+but allows the name of the remote to be specified in lieu of a specific\n+branch. For example, if the default branch for `origin` is set to\n+`master`, then `origin` may be specified wherever you would normally\n+specify `origin/master`.\n++\n+With `-d`, `$GIT_DIR/remotes//HEAD` is deleted.\n++\n+With `-a`, the remote is queried to determine its `HEAD`, then\n+`$GIT_DIR/remotes//HEAD` is set to the same branch. e.g., if the remote\n+`HEAD` is pointed at `next`, \"`git remote set-head origin -a`\" will set\n+`$GIT_DIR/refs/remotes/origin/HEAD` to `refs/remotes/origin/next`. This will\n+only work if `refs/remotes/origin/next` already exists; if not it must be\n+fetched first.\n++\n+Use `` to set `$GIT_DIR/remotes//HEAD` explicitly. e.g., \"git\n+remote set-head origin master\" will set `$GIT_DIR/refs/remotes/origin/HEAD` to\n+`refs/remotes/origin/master`. This will only work if\n+`refs/remotes/origin/master` already exists; if not it must be fetched first.\n++\n+\n 'show'::\n \n Gives some information about the remote .\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex 465c87a..fcb166b 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -12,6 +12,7 @@ static const char * const builtin_remote_usage[] = {\n \t\"git remote add [-t ] [-m ] [-f] [--mirror] \",\n \t\"git remote rename \",\n \t\"git remote rm \",\n+\t\"git remote set-head [-a | -d | ]\",\n \t\"git remote show [-n] \",\n \t\"git remote prune [-n | --dry-run] \",\n \t\"git remote [-v | --verbose] update [group]\",\n@@ -777,6 +778,54 @@ static int show(int argc, const char **argv)\n \treturn result;\n }\n \n+static int set_head(int argc, const char **argv)\n+{\n+\tint opt_a = 0, opt_d = 0, result = 0;\n+\tstruct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;\n+\tchar *head_name = NULL;\n+\n+\tstruct option options[] = {\n+\t\tOPT_GROUP(\"set-head specific options\"),\n+\t\tOPT_BOOLEAN('a', \"auto\", &opt_a,\n+\t\t\t \"set refs/remotes//HEAD according to remote\"),\n+\t\tOPT_BOOLEAN('d', \"delete\", &opt_d,\n+\t\t\t \"delete refs/remotes//HEAD\"),\n+\t\tOPT_END()\n+\t};\n+\targc = parse_options(argc, argv, options, builtin_remote_usage, 0);\n+\tif (argc)\n+\t\tstrbuf_addf(&buf, \"refs/remotes/%s/HEAD\", argv[0]);\n+\n+\tif (!opt_a && !opt_d && argc == 2) {\n+\t\thead_name = xstrdup(argv[1]);\n+\t} else if (opt_a && !opt_d && argc == 1) {\n+\t\tstruct ref_states states;\n+\t\tmemset(&states, 0, sizeof(states));\n+\t\tget_remote_ref_states(argv[0], &states, 1);\n+\t\thead_name = xstrdup(states.head_name);\n+\t\tfree_remote_ref_states(&states);\n+\t} else if (opt_d && !opt_a && argc == 1) {\n+\t\tif (delete_ref(buf.buf, NULL, REF_NODEREF))\n+\t\t\tresult |= error(\"Could not delete %s\", buf.buf);\n+\t} else\n+\t\tusage_with_options(builtin_remote_usage, options);\n+\n+\tif (head_name) {\n+\t\tunsigned char sha1[20];\n+\t\tstrbuf_addf(&buf2, \"refs/remotes/%s/%s\", argv[0], head_name);\n+\t\t/* make sure it's valid */\n+\t\tif (!resolve_ref(buf2.buf, sha1, 1, NULL))\n+\t\t\tresult |= error(\"Not a valid ref: %s\", buf2.buf);\n+\t\telse if (create_symref(buf.buf, buf2.buf, \"remote set-head\"))\n+\t\t\tresult |= error(\"Could not setup %s\", buf.buf);\n+\t\tfree(head_name);\n+\t}\n+\n+\tstrbuf_release(&buf);\n+\tstrbuf_release(&buf2);\n+\treturn result;\n+}\n+\n static int prune(int argc, const char **argv)\n {\n \tint dry_run = 0, result = 0;\n@@ -947,6 +996,8 @@ int cmd_remote(int argc, const char **argv, const char *prefix)\n \t\tresult = mv(argc, argv);\n \telse if (!strcmp(argv[0], \"rm\"))\n \t\tresult = rm(argc, argv);\n+\telse if (!strcmp(argv[0], \"set-head\"))\n+\t\tresult = set_head(argc, argv);\n \telse if (!strcmp(argv[0], \"show\"))\n \t\tresult = show(argc, argv);\n \telse if (!strcmp(argv[0], \"prune\"))\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex f44f63c..5de1922 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1414,7 +1414,7 @@ _git_config ()\n \n _git_remote ()\n {\n-\tlocal subcommands=\"add rename rm show prune update\"\n+\tlocal subcommands=\"add rename rm show prune update set-head\"\n \tlocal subcommand=\"$(__git_find_subcommand \"$subcommands\")\"\n \tif [ -z \"$subcommand\" ]; then\n \t\t__gitcomp \"$subcommands\"\n-- \n1.6.2.rc0.67.g77afc\n"},{"id":"104557","messageId":"20090214002253.GA7769@coredump.intra.peff.net","threadId":"17770","inReplyTo":"7vtz6yabsy.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/4] builtin-remote: add set-head verb","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-14T00:22:53Z","receivedAt":"2009-02-14T00:22:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 13, 2009 at 02:09:01AM -0800, Junio C Hamano wrote:\n\n> The entire series looks sane from a very cursory look; especially the\n> earlier ones are obviously good.\n\nI also think it looks good. You raised a few style points below which I\nthought were sensible and won't bother repeating.\n\n> Hmph, what does \"-a\" stand for? I would have expected to see \"-u\" that\n> stands for \"update\" here.\n\nIt was immediately obvious to me as \"auto\" (I think I even suggested\n\"-a\" in another thread, so maybe that is why it seems so sensible to\nme). However, I think as a rule it is nice to always provide a \"long\"\nalternative for every option. With parse-options, there really is no\ndownside; it is literally s/0/\"auto\"/ on the option line. And I think\nit:\n\n - lets people who want to illustrate a command in a more readable\n manner do so (e.g., if they are showing it to somebody else)\n\n - makes it more obvious in the documentation and usage message what\n the command does. That is, I remember commands much better as \"this\n is the --auto option, whose meaningful name reminds me that it does\n X, and -a is the obvious shorthand\" rather than \"-a does X\".\n\n - enables extra parse-options syntax like automatic \"--no-\" handling\n (though I doubt anyone is likely to use \"--no-auto\" in this case,\n the point is that it is easier to allow such constructs consistently\n than to try to guess when people might want it)\n\nWhich is maybe more argument than you cared to read about this\nparticular option, but I want to make clear that I think this should be\nthe case for just about every command-line option we add.\n\n> Also it may be better to be more explicit about both the syntax and the\n> semantics of ``. Do you expect \"refs/remotes//master\" or\n> just \"master\" (I assume the latter)? Is it an error if the branch does\n\nI thought it was obvious that you would do:\n\n git remote set-head master\n\nin the same way that you would do:\n\n git remote add -m master $remote $url\n\nBut I suppose clarifying it doesn't hurt.\n\n-Peff\n\nPS There is a thread elsewhere on the list discussing \"what can I do to\nmake life easy for reviewers?\". I think this series (and Jay's patches\nin general) model many good behaviors: clearly labeled versions,\ndiscussion of what changed in each version, proper threading, and cc'ing\npeople who have been involved.\n"},{"id":"104565","messageId":"7vab8p4w1u.fsf@gitster.siamese.dyndns.org","threadId":"17770","inReplyTo":"20090214002253.GA7769@coredump.intra.peff.net","subject":"Re: [PATCH 4/4] builtin-remote: add set-head verb","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-14T02:00:29Z","receivedAt":"2009-02-14T02:00:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Feb 13, 2009 at 02:09:01AM -0800, Junio C Hamano wrote:\n>\n>> The entire series looks sane from a very cursory look; especially the\n>> earlier ones are obviously good.\n>\n> I also think it looks good. You raised a few style points below which I\n> thought were sensible and won't bother repeating.\n>\n>> Hmph, what does \"-a\" stand for? I would have expected to see \"-u\" that\n>> stands for \"update\" here.\n>\n> It was immediately obvious to me as \"auto\" (I think I even suggested\n> \"-a\" in another thread, so maybe that is why it seems so sensible to\n> me).\n\nYeah, latest round has --auto in it. Thanks, Jay.\n\n> I thought it was obvious that you would do:\n>\n> git remote set-head master\n>\n> in the same way that you would do:\n>\n> git remote add -m master $remote $url\n>\n> But I suppose clarifying it doesn't hurt.\n\nI do not care too deeply if an explicit request to \"set-head --auto\"\nscrews up and sets a HEAD that was pointing at the right branch to another\nbranch because the command is not taught to give preference to the branch\nHEAD originally points at, so I do not think I have any more issues with\nthe series for now, even though I may notice things later.\n\nI have this series queued to private topic branch but it still does not\npass tests (breaks #8 and #18 of t5505 at least) by itself; the previous\nround was no better. I think it is just the matter of updating the\nexpected output in the tests, but I didn't look further.\n"},{"id":"104572","messageId":"20090214021831.GB9907@coredump.intra.peff.net","threadId":"17770","inReplyTo":"7vab8p4w1u.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/4] builtin-remote: add set-head verb","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-14T02:18:31Z","receivedAt":"2009-02-14T02:18:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 13, 2009 at 06:00:29PM -0800, Junio C Hamano wrote:\n\n> > It was immediately obvious to me as \"auto\" (I think I even suggested\n> > \"-a\" in another thread, so maybe that is why it seems so sensible to\n> > me).\n> \n> Yeah, latest round has --auto in it. Thanks, Jay.\n\nOops. I even went back and double-checked to make sure that it was not\nthere, but it would have helped if I actually checked the latest\nversion. :(\n\n> I do not care too deeply if an explicit request to \"set-head --auto\"\n> screws up and sets a HEAD that was pointing at the right branch to another\n> branch because the command is not taught to give preference to the branch\n> HEAD originally points at, so I do not think I have any more issues with\n> the series for now, even though I may notice things later.\n\nI think that is reasonable; that is a separate enhancement which can\ncome later and is no reason to block the existing patches.\n\n> I have this series queued to private topic branch but it still does not\n> pass tests (breaks #8 and #18 of t5505 at least) by itself; the previous\n> round was no better. I think it is just the matter of updating the\n> expected output in the tests, but I didn't look further.\n\nTest #8 is just a matter of updating output. But #18 is explicitly about\nchecking that \"remote show\" does not show symbolic refs. But Jay's patch\nis about explicitly showing symbolic refs (just doing so as a ref-name\ninstead of a sha1):\n\n * FAIL 18: \"remote show\" does not show symbolic refs\n git clone one three &&\n (cd three &&\n git remote show origin > output &&\n ! grep HEAD < output &&\n ! grep -i stale < output)\n\nI guess we could tighten the grep to\n\n ! egrep \"HEAD: [0-9a-f]{40}\" < output\n\nbut it may just make sense to get rid of the test; the exact output is\nalready covered by test #8. Squashable patch is below.\n\n---\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex eb63718..8808580 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -136,6 +136,7 @@ EOF\n cat > test/expect << EOF\n * remote origin\n URL: $(pwd)/one\n+ HEAD: master\n Remote branch merged with 'git pull' while on branch master\n master\n New remote branch (next fetch will store in remotes/origin)\n@@ -338,16 +339,6 @@ test_expect_success 'update default (overridden, with funny whitespace)' '\n \n '\n \n-test_expect_success '\"remote show\" does not show symbolic refs' '\n-\n-\tgit clone one three &&\n-\t(cd three &&\n-\t git remote show origin > output &&\n-\t ! grep HEAD < output &&\n-\t ! grep -i stale < output)\n-\n-'\n-\n test_expect_success 'reject adding remote with an invalid name' '\n \n \ttest_must_fail git remote add some:url desired-name\n"},{"id":"104578","messageId":"76718490902131848sb335b8bn2694c3e569aa1acd@mail.gmail.com","threadId":"17770","inReplyTo":"20090214021831.GB9907@coredump.intra.peff.net","subject":"Re: [PATCH 4/4] builtin-remote: add set-head verb","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-14T02:48:29Z","receivedAt":"2009-02-14T02:48:29Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Feb 13, 2009 at 9:18 PM, Jeff King wrote:\n> Test #8 is just a matter of updating output. But #18 is explicitly about\n> checking that \"remote show\" does not show symbolic refs. But Jay's patch\n> is about explicitly showing symbolic refs (just doing so as a ref-name\n> instead of a sha1):\n>\n> * FAIL 18: \"remote show\" does not show symbolic refs\n> git clone one three &&\n> (cd three &&\n> git remote show origin > output &&\n> ! grep HEAD < output &&\n> ! grep -i stale < output)\n>\n> I guess we could tighten the grep to\n>\n> ! egrep \"HEAD: [0-9a-f]{40}\" < output\n>\n> but it may just make sense to get rid of the test; the exact output is\n> already covered by test #8. Squashable patch is below.\n\nThank you Jeff. I swear I ran t5505-remote.sh successfully but\nobviously I am misremembering.\n\nj.\n"},{"id":"104579","messageId":"76718490902131859u4645dc62h79632cb20ee90d69@mail.gmail.com","threadId":"17770","inReplyTo":"7vab8p4w1u.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/4] builtin-remote: add set-head verb","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-14T02:59:06Z","receivedAt":"2009-02-14T02:59:06Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Feb 13, 2009 at 9:00 PM, Junio C Hamano wrote:\n> I do not care too deeply if an explicit request to \"set-head --auto\"\n> screws up and sets a HEAD that was pointing at the right branch to another\n> branch because the command is not taught to give preference to the branch\n> HEAD originally points at\n\nSo I don't mind fixing this, but here's the thing.\n\nSay user has refs/remotes/origin/HEAD set to frotz. They then run \"git\nshow remote origin\" and we see that HEAD on the remote end could be\neither master or frotz (both have the same SHA1).\n\nWhat should we show in the output of \"git remote show origin\" next to\nthe HEAD line? master, or frotz? If we show master, then user might\nwonder why \"git remote set-head origin --auto\" leaves\nrefs/remotes/origin/HEAD set to frotz. If we show frotz, then user\nmight wonder why when they cloned the repo in the first place they\nended up with HEAD set to master.\n\nI'm bothered by that inconsistency, which is why I didn't follow-up\nwith another patch immediately.\n\nBut I will propose an alternative. In the output of \"get remote show\norigin\", we show all matching branches. If the user does a set-head\n--auto and we cannot determine HEAD unambiguously, we do something\nlike:\n\n$ git remote set-head origin --auto\nerror: Multiple branches match HEAD. Please choose one explicitly with:\n git remote set-head origin master\n git remote set-head origin frotz\n\nHmm?\n\nj.\n"},{"id":"104582","messageId":"20090214034345.GB24545@coredump.intra.peff.net","threadId":"17770","inReplyTo":"76718490902131859u4645dc62h79632cb20ee90d69@mail.gmail.com","subject":"Re: [PATCH 4/4] builtin-remote: add set-head verb","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-14T03:43:45Z","receivedAt":"2009-02-14T03:43:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 13, 2009 at 09:59:06PM -0500, Jay Soffian wrote:\n\n> What should we show in the output of \"git remote show origin\" next to\n> the HEAD line? master, or frotz? If we show master, then user might\n> wonder why \"git remote set-head origin --auto\" leaves\n> refs/remotes/origin/HEAD set to frotz. If we show frotz, then user\n> might wonder why when they cloned the repo in the first place they\n> ended up with HEAD set to master.\n> \n> I'm bothered by that inconsistency, which is why I didn't follow-up\n> with another patch immediately.\n\nHrm. Yeah, I think to avoid surprising the user, \"--auto\" has to use\nwhatever we showed in \"git remote show\".\n\n> But I will propose an alternative. In the output of \"get remote show\n> origin\", we show all matching branches. If the user does a set-head\n> --auto and we cannot determine HEAD unambiguously, we do something\n> like:\n> \n> $ git remote set-head origin --auto\n> error: Multiple branches match HEAD. Please choose one explicitly with:\n> git remote set-head origin master\n> git remote set-head origin frotz\n\nI like that proposal. It doesn't hide from the user that we are doing a\nmatching guess, which means we are less likely to surprise them in the\nlong run.\n\n-Peff\n"},{"id":"104623","messageId":"1234607430-5403-1-git-send-email-jaysoffian@gmail.com","threadId":"17770","inReplyTo":"20090214034345.GB24545@coredump.intra.peff.net","subject":"[PATCH] builtin-remote: better handling of multiple remote HEADs","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-14T10:30:30Z","receivedAt":"2009-02-14T10:30:30Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"It is not currently possible to determine the remote HEAD unambiguously\nwhen multiple remote branches share the same SHA1 as the remote HEAD.\n\nIn this situation, git remote set-head --auto should not try to guess\nwhich HEAD the user wants. This patch causes set-head to provide a\nuseful error instead:\n\n$ git remote set-head origin --auto\nerror: Multiple remote HEAD branches. Please choose one explicitly with:\n git remote set-head origin another\n git remote set-head origin master\n\nAlso, the output of git remote show now shows the multiple HEADs:\n\n$ git remote show origin\n* remote origin\n URL: ...\n HEAD branches:\n another\n master\n\nSigned-off-by: Jay Soffian \n---\nOn Fri, Feb 13, 2009 at 10:43 PM, Jeff King wrote:\n> On Fri, Feb 13, 2009 at 09:59:06PM -0500, Jay Soffian wrote:\n>\n>> But I will propose an alternative. In the output of \"get remote show\n>> origin\", we show all matching branches. If the user does a set-head\n>> --auto and we cannot determine HEAD unambiguously, we do something\n>> like:\n>>\n>> $ git remote set-head origin --auto\n>> error: Multiple branches match HEAD. Please choose one explicitly with:\n>> git remote set-head origin master\n>> git remote set-head origin frotz\n>\n> I like that proposal. It doesn't hide from the user that we are doing a\n> matching guess, which means we are less likely to surprise them in the\n> long run.\n>\n> -Peff\n\nVoilà\n\nJunio - this obviously goes on-top of the rest of the builtin-remote\nseries I sent.\n\n builtin-clone.c | 2 +-\n builtin-remote.c | 56 +++++++++++++++++++++++++++++++++++------------------\n remote.c | 28 ++++++++++++++++++-------\n remote.h | 6 ++++-\n t/t5505-remote.sh | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-\n 5 files changed, 114 insertions(+), 31 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex d179d1c..d57818c 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -510,7 +510,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tmapped_refs = write_remote_refs(refs, &refspec, reflog_msg.buf);\n \n \t\thead_points_at = guess_remote_head(refs, mapped_refs,\n-\t\t\t\t\t\t &remote_head);\n+\t\t\t\t\t\t &remote_head, NULL);\n \t}\n \telse {\n \t\twarning(\"You appear to have cloned an empty repository.\");\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex fcb166b..608c0f3 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -212,8 +212,7 @@ static void read_branches(void)\n \n struct ref_states {\n \tstruct remote *remote;\n-\tstruct string_list new, stale, tracked;\n-\tchar *head_name;\n+\tstruct string_list new, stale, tracked, heads;\n };\n \n static int handle_one_branch(const char *refname,\n@@ -273,24 +272,26 @@ static int get_ref_states(const struct ref *ref, struct ref_states *states)\n \treturn 0;\n }\n \n-static char *get_head_name(const struct ref *refs)\n+static int get_head_names(const struct ref *refs,\n+\tconst char *remote_name, struct ref_states *states)\n {\n-\tconst struct ref *head_points_at;\n-\tstruct ref *mapped_refs = NULL;\n-\tstruct ref **tail = &mapped_refs;\n+\tstruct ref *ref, *matches = NULL;\n+\tstruct ref *fetch_map = NULL, **fetch_map_tail = &fetch_map;\n \tstruct refspec refspec;\n \n \trefspec.force = 0;\n \trefspec.pattern = 1;\n \trefspec.src = refspec.dst = \"refs/heads/\";\n+\tstates->heads.strdup_strings = 1;\n+\tget_fetch_map(refs, &refspec, &fetch_map_tail, 0);\n+\tguess_remote_head(refs, fetch_map, NULL, &matches);\n+\tfor(ref = matches; ref; ref = ref->next)\n+\t\tstring_list_append(abbrev_branch(ref->name), &states->heads);\n \n-\tget_fetch_map(refs, &refspec, &tail, 0);\n-\n-\thead_points_at = guess_remote_head(refs, mapped_refs, NULL);\n-\tif (head_points_at)\n-\t\treturn xstrdup(abbrev_branch(head_points_at->name));\n+\tfree_refs(fetch_map);\n+\tfree_refs(matches);\n \n-\treturn NULL;\n+\treturn 0;\n }\n \n struct known_remote {\n@@ -659,7 +660,7 @@ static void free_remote_ref_states(struct ref_states *states)\n \tstring_list_clear(&states->new, 0);\n \tstring_list_clear(&states->stale, 0);\n \tstring_list_clear(&states->tracked, 0);\n-\tfree(states->head_name);\n+\tstring_list_clear(&states->heads, 0);\n }\n \n static int get_remote_ref_states(const char *name,\n@@ -681,7 +682,7 @@ static int get_remote_ref_states(const char *name,\n \t\tref = transport_get_remote_refs(transport);\n \t\ttransport_disconnect(transport);\n \n-\t\tstates->head_name = get_head_name(ref);\n+\t\tget_head_names(ref, name, states);\n \t\tget_ref_states(ref, states);\n \t}\n \n@@ -726,9 +727,15 @@ static int show(int argc, const char **argv)\n \t\tprintf(\"* remote %s\\n URL: %s\\n\", *argv,\n \t\t\tstates.remote->url_nr > 0 ?\n \t\t\t\tstates.remote->url[0] : \"(no URL)\");\n-\t\tif (!no_query)\n-\t\t\tprintf(\" HEAD: %s\\n\", states.head_name ?\n-\t\t\t\tstates.head_name : \"(unknown)\");\n+\t\tif (no_query)\n+\t\t\tprintf(\" HEAD branch: (not queried)\\n\");\n+\t\telse if (!states.heads.nr)\n+\t\t\tprintf(\" HEAD branch: (unknown)\\n\");\n+\t\telse if (states.heads.nr == 1)\n+\t\t\tprintf(\" HEAD branch: %s\\n\",\n+\t\t\t\tstates.heads.items[0].string);\n+\t\telse\n+\t\t\tshow_list(\" HEAD branch%s:\", &states.heads, \"\");\n \n \t\tfor (i = 0; i < branch_list.nr; i++) {\n \t\t\tstruct string_list_item *branch = branch_list.items + i;\n@@ -780,7 +787,7 @@ static int show(int argc, const char **argv)\n \n static int set_head(int argc, const char **argv)\n {\n-\tint opt_a = 0, opt_d = 0, result = 0;\n+\tint i, opt_a = 0, opt_d = 0, result = 0;\n \tstruct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;\n \tchar *head_name = NULL;\n \n@@ -802,7 +809,16 @@ static int set_head(int argc, const char **argv)\n \t\tstruct ref_states states;\n \t\tmemset(&states, 0, sizeof(states));\n \t\tget_remote_ref_states(argv[0], &states, 1);\n-\t\thead_name = xstrdup(states.head_name);\n+\t\tif (!states.heads.nr)\n+\t\t\tresult |= error(\"Cannot determine remote HEAD\");\n+\t\telse if (states.heads.nr > 1) {\n+\t\t\tresult |= error(\"Multiple remote HEAD branches. \"\n+\t\t\t\t\t\"Please choose one explicitly with:\");\n+\t\t\tfor (i = 0; i < states.heads.nr; i++)\n+\t\t\t\tfprintf(stderr, \" git remote set-head %s %s\\n\",\n+\t\t\t\t\targv[0], states.heads.items[i].string);\n+\t\t} else\n+\t\t\thead_name = xstrdup(states.heads.items[0].string);\n \t\tfree_remote_ref_states(&states);\n \t} else if (opt_d && !opt_a && argc == 1) {\n \t\tif (delete_ref(buf.buf, NULL, REF_NODEREF))\n@@ -818,6 +834,8 @@ static int set_head(int argc, const char **argv)\n \t\t\tresult |= error(\"Not a valid ref: %s\", buf2.buf);\n \t\telse if (create_symref(buf.buf, buf2.buf, \"remote set-head\"))\n \t\t\tresult |= error(\"Could not setup %s\", buf.buf);\n+\t\tif (opt_a)\n+\t\t\tprintf(\"%s/HEAD set to %s\\n\", argv[0], head_name);\n \t\tfree(head_name);\n \t}\n \ndiff --git a/remote.c b/remote.c\nindex 447f091..6385a22 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1379,18 +1379,23 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)\n \n const struct ref *guess_remote_head(const struct ref *refs,\n \t\t\t\t const struct ref *mapped_refs,\n-\t\t\t\t const struct ref **remote_head_p)\n+\t\t\t\t const struct ref **remote_head_p,\n+\t\t\t\t struct ref **all_matches_p)\n {\n \tconst struct ref *remote_head = NULL;\n \tconst struct ref *remote_master = NULL;\n+\tconst struct ref *ret = NULL;\n \tconst struct ref *r;\n+\tstruct ref **tail = all_matches_p;\n+\n \tfor (r = refs; r; r = r->next)\n \t\tif (!strcmp(r->name, \"HEAD\"))\n \t\t\tremote_head = r;\n \n-\tfor (r = mapped_refs; r; r = r->next)\n-\t\tif (!strcmp(r->name, \"refs/heads/master\"))\n-\t\t\tremote_master = r;\n+\tif (!all_matches_p)\n+\t\tfor (r = mapped_refs; r; r = r->next)\n+\t\t\tif (!strcmp(r->name, \"refs/heads/master\"))\n+\t\t\t\tremote_master = r;\n \n \tif (remote_head_p)\n \t\t*remote_head_p = remote_head;\n@@ -1407,9 +1412,16 @@ const struct ref *guess_remote_head(const struct ref *refs,\n \t/* Look for another ref that points there */\n \tfor (r = mapped_refs; r; r = r->next)\n \t\tif (r != remote_head &&\n-\t\t !hashcmp(r->old_sha1, remote_head->old_sha1))\n-\t\t\treturn r;\n+\t\t !hashcmp(r->old_sha1, remote_head->old_sha1)) {\n+\t\t\tstruct ref *cpy;\n+\t\t\tif (!ret)\n+\t\t\t\tret = r;\n+\t\t\tif (!all_matches_p)\n+\t\t\t\tbreak;\n+\t\t\t*tail = cpy = copy_ref(r);\n+\t\t\tcpy->peer_ref = NULL;\n+\t\t\ttail = &cpy->next;\n+\t\t}\n \n-\t/* Nothing is the same */\n-\treturn NULL;\n+\treturn ret;\n }\ndiff --git a/remote.h b/remote.h\nindex cabb14a..8409d42 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -141,9 +141,13 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb);\n * Look in refs for HEAD. Then look for a matching SHA1 in mapped_refs,\n * first checking if refs/heads/master matches. Return NULL if nothing matches\n * or if there is no HEAD in refs. remote_head_p is assigned HEAD if not NULL.\n+ * If all_matches_p is NULL, return after the first possible match. Otherwise\n+ * all_matches_p is set to a ref list of each branch head with the same SHA1 as\n+ * HEAD.\n */\n const struct ref *guess_remote_head(const struct ref *refs,\n \t\t\t\t const struct ref *mapped_refs,\n-\t\t\t\t const struct ref **remote_head_p);\n+\t\t\t\t const struct ref **remote_head_p,\n+\t\t\t\t struct ref **all_matches_p);\n \n #endif\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 8808580..49f99e9 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -136,7 +136,7 @@ EOF\n cat > test/expect << EOF\n * remote origin\n URL: $(pwd)/one\n- HEAD: master\n+ HEAD branch: master\n Remote branch merged with 'git pull' while on branch master\n master\n New remote branch (next fetch will store in remotes/origin)\n@@ -147,6 +147,11 @@ cat > test/expect << EOF\n Local branches pushed with 'git push'\n master:upstream\n +refs/tags/lastbackup\n+* remote two\n+ URL: ../two\n+ HEAD branches:\n+ another\n+ master\n EOF\n \n test_expect_success 'show' '\n@@ -155,6 +160,7 @@ test_expect_success 'show' '\n \t\trefs/heads/master:refs/heads/upstream &&\n \t git fetch &&\n \t git branch -d -r origin/master &&\n+\t git config --add remote.two.url ../two &&\n \t (cd ../one &&\n \t echo 1 > file &&\n \t test_tick &&\n@@ -163,13 +169,14 @@ test_expect_success 'show' '\n \t\trefs/heads/master:refs/heads/upstream &&\n \t git config --add remote.origin.push \\\n \t\t+refs/tags/lastbackup &&\n-\t git remote show origin > output &&\n+\t git remote show origin two > output &&\n \t test_cmp expect output)\n '\n \n cat > test/expect << EOF\n * remote origin\n URL: $(pwd)/one\n+ HEAD branch: (not queried)\n Remote branch merged with 'git pull' while on branch master\n master\n Tracked remote branches\n@@ -198,6 +205,48 @@ test_expect_success 'prune' '\n \t test_must_fail git rev-parse refs/remotes/origin/side)\n '\n \n+test_expect_success 'set-head --delete' '\n+\t(cd test &&\n+\t git symbolic-ref refs/remotes/origin/HEAD &&\n+\t git remote set-head --delete origin &&\n+\t test_must_fail git symbolic-ref refs/remotes/origin/HEAD)\n+'\n+\n+cat > test/expect < output &&\n+\t git symbolic-ref refs/remotes/origin/HEAD &&\n+\ttest_cmp expect output)\n+'\n+\n+cat >test/expect <& output &&\n+\ttest_cmp expect output)\n+'\n+\n+cat >test/expect <output &&\n+\t git remote set-head origin master &&\n+\t test_cmp expect output)\n+'\n+\n cat > test/expect << EOF\n Pruning origin\n URL: $(pwd)/one\n-- \n1.6.2.rc0.238.g0c1fe\n"},{"id":"104654","messageId":"20090214175420.GA3457@coredump.intra.peff.net","threadId":"17770","inReplyTo":"1234607430-5403-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] builtin-remote: better handling of multiple remote HEADs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-14T17:54:20Z","receivedAt":"2009-02-14T17:54:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 14, 2009 at 05:30:30AM -0500, Jay Soffian wrote:\n\n> In this situation, git remote set-head --auto should not try to guess\n> which HEAD the user wants. This patch causes set-head to provide a\n> useful error instead:\n> \n> $ git remote set-head origin --auto\n> error: Multiple remote HEAD branches. Please choose one explicitly with:\n> git remote set-head origin another\n> git remote set-head origin master\n\nThanks. The patch looks good to me, with two comments and one style nit:\n\n> +\t\telse if (states.heads.nr == 1)\n> +\t\t\tprintf(\" HEAD branch: %s\\n\",\n> +\t\t\t\tstates.heads.items[0].string);\n> +\t\telse\n> +\t\t\tshow_list(\" HEAD branch%s:\", &states.heads, \"\");\n\nI was happy to see the common case of \"we unambiguously determined HEAD\"\nfalls back to nicer output (though I admit I did a double-take seeing\nboth show_list and the states.heads.nr check, I see it is because\nshow_list always insists on a newline).\n\nThat should help current users with simple setups, but also support\nunambiguous HEAD reporting in the future (and based on what Daniel said\nearlier, http should just need a client patch to pass the information\nup the callstack).\n\n> +\t\tif (opt_a)\n> +\t\t\tprintf(\"%s/HEAD set to %s\\n\", argv[0], head_name);\n\nThis was a surprise based on reading the commit message, but I think it\nis a sensible enhancement.\n\n> +cat > test/expect < +origin/HEAD set to master\n> +EOF\n> +\n> +test_expect_success 'set-head --auto' '\n> +\t(cd test &&\n> +\t git remote set-head --auto origin > output &&\n> +\t git symbolic-ref refs/remotes/origin/HEAD &&\n> +\ttest_cmp expect output)\n> +'\n\nI had to read this test a few times to convince myself it was right,\nsince you throw away the output of symbolic-ref. I think it makes more\nsense to just test the post-command state, which is what you actually\ncare about (and then you are also not dependent on the human-readable\noutput of \"remote set-head\"). I.e.:\n\ncat > test/expect < output &&\n\ttest_cmp expect output)\n'\n\n-Peff\n"},{"id":"104656","messageId":"76718490902141035o5430707ck47cd72d9efe87318@mail.gmail.com","threadId":"17770","inReplyTo":"20090214175420.GA3457@coredump.intra.peff.net","subject":"Re: [PATCH] builtin-remote: better handling of multiple remote HEADs","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-14T18:35:03Z","receivedAt":"2009-02-14T18:35:03Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sat, Feb 14, 2009 at 12:54 PM, Jeff King wrote:\n>> + if (opt_a)\n>> + printf(\"%s/HEAD set to %s\\n\", argv[0], head_name);\n>\n> This was a surprise based on reading the commit message, but I think it\n> is a sensible enhancement.\n\nIt seemed that when doing something \"--automatically\" it might be nice\nto tell the user what we just did, but I'm confused why this was a\nsurprise.\n\n>> +cat > test/expect <> +origin/HEAD set to master\n>> +EOF\n>> +\n>> +test_expect_success 'set-head --auto' '\n>> + (cd test &&\n>> + git remote set-head --auto origin > output &&\n>> + git symbolic-ref refs/remotes/origin/HEAD &&\n>> + test_cmp expect output)\n>> +'\n>\n> I had to read this test a few times to convince myself it was right,\n> since you throw away the output of symbolic-ref. I think it makes more\n> sense to just test the post-command state, which is what you actually\n> care about (and then you are also not dependent on the human-readable\n> output of \"remote set-head\"). I.e.:\n>\n> cat > test/expect < refs/remotes/origin/master\n> EOF\n>\n> test_expect_success 'set-head --auto' '\n> (cd test &&\n> git remote set-head --auto origin &&\n> git symbolic-ref refs/remotes/origin/HEAD > output &&\n> test_cmp expect output)\n> '\n\nRight.\n\nj.\n"},{"id":"104658","messageId":"20090214185411.GA13121@coredump.intra.peff.net","threadId":"17770","inReplyTo":"76718490902141035o5430707ck47cd72d9efe87318@mail.gmail.com","subject":"Re: [PATCH] builtin-remote: better handling of multiple remote HEADs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-14T18:54:11Z","receivedAt":"2009-02-14T18:54:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 14, 2009 at 01:35:03PM -0500, Jay Soffian wrote:\n\n> On Sat, Feb 14, 2009 at 12:54 PM, Jeff King wrote:\n> >> + if (opt_a)\n> >> + printf(\"%s/HEAD set to %s\\n\", argv[0], head_name);\n> >\n> > This was a surprise based on reading the commit message, but I think it\n> > is a sensible enhancement.\n> \n> It seemed that when doing something \"--automatically\" it might be nice\n> to tell the user what we just did, but I'm confused why this was a\n> surprise.\n\nI just meant that the commit message did not mention changes in this\narea, and it is largely orthogonal to the rest of the patch (you could\njust as easily apply this hunk without the rest of your patch, and it\nwould have the same value). Thus I was surprised.\n\nBut I do think it is a good change.\n\n> > cat > test/expect < > refs/remotes/origin/master\n> > EOF\n> >\n> > test_expect_success 'set-head --auto' '\n> > (cd test &&\n> > git remote set-head --auto origin &&\n> > git symbolic-ref refs/remotes/origin/HEAD > output &&\n> > test_cmp expect output)\n> > '\n> \n> Right.\n\nI suspect Junio can just fix this up during application if he agrees.\n\n\nWhich made me think of something else, with all of this talk about\nreviewers that has been going on. Junio is actually in a little bit of a\nspecial position with small changes (like style issues) to say \"I'll\napply this, but tweak these changes\". But the rest of us are stuck\nsaying \"I would change this one line\" to the list; then either:\n\n - the original submitter re-rolls the patch, which takes their time\n and everyone else's time to look at the new patch, see that it is\n trivially changed, etc\n\n or\n\n - Junio has to read the followup comments, then go back and find the\n spot in the original patch to mark it up.\n\nWhich means that there is a transaction cost to little comments due to\nthe extra communication. And that cost can dwarf the actual time for the\nchange.\n\nI don't know if there is a better method, or better tool support. I\nguess reviewers could act like the maintainer, tweaking patches and then\npublishing the result, which Junio would then pull. Or instead of\npublishing the result, publishing an interdiff along with comments. But\nbasically putting the comments into a form that can be communicated and\napplied more easily, which cuts down on the communication costs.\n\nI don't know. Just thinking out loud.\n\n-Peff\n"},{"id":"104675","messageId":"7vy6w8sstp.fsf@gitster.siamese.dyndns.org","threadId":"17770","inReplyTo":"20090214185411.GA13121@coredump.intra.peff.net","subject":"Re: [PATCH] builtin-remote: better handling of multiple remote HEADs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-14T19:48:34Z","receivedAt":"2009-02-14T19:48:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Which made me think of something else, with all of this talk about\n> reviewers that has been going on. Junio is actually in a little bit of a\n> special position with small changes (like style issues) to say \"I'll\n> apply this, but tweak these changes\".\n\nIt is not that I am special.\n\nWhat is special is an otherwise obviously good patch with a few trivial\nmistakes that I can fix locally without worrying the fix-up may be wrong.\nIt is not even per author, it is per patch, and it is a rare exception.\n\nOften, I notice these things *after* I applied and reviewed the results,\nso it already is in my work area. I then judge the tradeoff between an\nextra round (which as you stated needs another fresh review, patch\napplication and testing here) and the possibility that I may make a silly\nmistake myself while attempting a fix-up (such a mistake by me will not\nbe seen on the list and others do not have chance to catch them).\n\nFor this reason, I try to keep these \"will fix up no need for resend\" to\nthe minimum and only to the most trivial cases.\n\n> ... But the rest of us are stuck\n> saying \"I would change this one line\" to the list; then either:\n>\n> - the original submitter re-rolls the patch, which takes their time\n> and everyone else's time to look at the new patch, see that it is\n> trivially changed, etc\n>\n> or\n>\n> - Junio has to read the followup comments, then go back and find the\n> spot in the original patch to mark it up.\n\nA third option is:\n\n\t\"I would change this and that\" review comment message, followed by\n\ta separate message \"Here is how I would have done it\", addressed\n\tTo: the original submitter (with in-body From: line), Cc: to the\n\tlist and me.\n\nThe original submitter can verify the latter one, and either agree to or\ndisagree with it. If the reroll is good, then I can just pick it up. I\nthink you have done that in the past yourself, and the process made my\nlife a lot easier.\n"},{"id":"104683","messageId":"alpine.LNX.1.00.0902141503230.19665@iabervon.org","threadId":"17770","inReplyTo":"20090214175420.GA3457@coredump.intra.peff.net","subject":"Re: [PATCH] builtin-remote: better handling of multiple remote HEADs","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-02-14T20:21:30Z","receivedAt":"2009-02-14T20:21:30Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sat, 14 Feb 2009, Jeff King wrote:\n\n> On Sat, Feb 14, 2009 at 05:30:30AM -0500, Jay Soffian wrote:\n> \n> > +\t\telse if (states.heads.nr == 1)\n> > +\t\t\tprintf(\" HEAD branch: %s\\n\",\n> > +\t\t\t\tstates.heads.items[0].string);\n> > +\t\telse\n> > +\t\t\tshow_list(\" HEAD branch%s:\", &states.heads, \"\");\n> \n> I was happy to see the common case of \"we unambiguously determined HEAD\"\n> falls back to nicer output (though I admit I did a double-take seeing\n> both show_list and the states.heads.nr check, I see it is because\n> show_list always insists on a newline).\n> \n> That should help current users with simple setups, but also support\n> unambiguous HEAD reporting in the future (and based on what Daniel said\n> earlier, http should just need a client patch to pass the information\n> up the callstack).\n\nI haven't checked lately, but I think that what's actually needed is to \nhave the locate_head() function notice if the struct ref for HEAD actually \nhas the symref field non-NULL, and report that as the unambiguous answer. \nThis should also allow it to automatically pick up any other \ndisambiguation by future sources of lists of refs that include HEAD, \nwhether that's git protocol extensions, filesystem access to the repo, or \nforeign VCSes where some branches is inherently primary, or whatever.\n\n(The direct purpose of collecting the information for http was so that it \ncould figure out the sha1 at all for the remote HEAD when it's a symref; \nthe code just doesn't then explicitly throw the information away.)\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"104698","messageId":"20090214211548.GA14898@coredump.intra.peff.net","threadId":"17770","inReplyTo":"alpine.LNX.1.00.0902141503230.19665@iabervon.org","subject":"Re: [PATCH] builtin-remote: better handling of multiple remote HEADs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-14T21:15:48Z","receivedAt":"2009-02-14T21:15:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 14, 2009 at 03:21:30PM -0500, Daniel Barkalow wrote:\n\n> I haven't checked lately, but I think that what's actually needed is to \n> have the locate_head() function notice if the struct ref for HEAD actually \n> has the symref field non-NULL, and report that as the unambiguous answer. \n\nIndeed. Something like the patch below works (on top of Jay's patches).\nBut it has two shortcomings:\n\n 1. There is no test script, since we have no infrastructure for\n testing over http. I might be able to build something on top of\n what's in the http-push tests.\n\n I was hoping we could do the same trick for local file repos, which\n would be easy to test. But the transport code just treats them as\n regular pack uploaders; only some specialized code in clone cares\n about the difference.\n\n In theory we could add a new transport for local repos. I don't\n think it would make sense for its get_remote_refs function to get\n _all_ of the refs, but it could specially peek at HEAD and set the\n symref field appropriately.\n\n 2. The guess_remote_head function is getting a little long. I think it\n would help to refactor it into two functions; one for finding the\n remote HEAD in the refs list, and the other for guessing at a ref\n which matches the HEAD.\n\nI will try to make something a little neater later today.\n\n> This should also allow it to automatically pick up any other \n> disambiguation by future sources of lists of refs that include HEAD, \n> whether that's git protocol extensions, filesystem access to the repo, or \n> foreign VCSes where some branches is inherently primary, or whatever.\n\nYes, I think the symref field for the ref is a very sensible way of\ncommunicating the information for that reason.\n\nPatch is below.\n\n---\ndiff --git a/remote.c b/remote.c\nindex 6385a22..afbaccc 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1404,6 +1404,20 @@ const struct ref *guess_remote_head(const struct ref *refs,\n \tif (!remote_head)\n \t\treturn NULL;\n \n+\t/* if the underlying transport can represent symrefs,\n+\t * then we don't need to guess at all */\n+\tif (remote_head->symref) {\n+\t\tfor (r = mapped_refs; r; r = r->next) {\n+\t\t\tif (!strcmp(r->name, remote_head->symref)) {\n+\t\t\t\tif (all_matches_p) {\n+\t\t\t\t\t*all_matches_p = copy_ref(r);\n+\t\t\t\t\t(*all_matches_p)->peer_ref = NULL;\n+\t\t\t\t}\n+\t\t\t\treturn r;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n \t/* If refs/heads/master could be right, it is. */\n \tif (remote_master && !hashcmp(remote_master->old_sha1,\n \t\t\t\t remote_head->old_sha1))\n"},{"id":"104728","messageId":"20090215052740.GA11522@coredump.intra.peff.net","threadId":"17770","inReplyTo":"1234607430-5403-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] builtin-remote: better handling of multiple remote HEADs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-15T05:27:40Z","receivedAt":"2009-02-15T05:27:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 14, 2009 at 05:30:30AM -0500, Jay Soffian wrote:\n\n> +test_expect_success 'set-head --auto fails w/multiple HEADs' '\n> +\t(cd test &&\n> +\t test_must_fail git remote set-head --auto two >& output &&\n> +\ttest_cmp expect output)\n\nI missed it before, but there is a typo in this test (>&) that causes it\nto barf.\n\n-Peff\n"},{"id":"104730","messageId":"20090215053412.GA14230@coredump.intra.peff.net","threadId":"17770","inReplyTo":"20090215052740.GA11522@coredump.intra.peff.net","subject":"Re: [PATCH] builtin-remote: better handling of multiple remote HEADs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-15T05:34:13Z","receivedAt":"2009-02-15T05:34:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 15, 2009 at 12:27:40AM -0500, Jeff King wrote:\n\n> On Sat, Feb 14, 2009 at 05:30:30AM -0500, Jay Soffian wrote:\n> \n> > +test_expect_success 'set-head --auto fails w/multiple HEADs' '\n> > +\t(cd test &&\n> > +\t test_must_fail git remote set-head --auto two >& output &&\n> > +\ttest_cmp expect output)\n> \n> I missed it before, but there is a typo in this test (>&) that causes it\n> to barf.\n\nActually, it should be \"> output 2>&1\" to pick up the error message we\nare expecting. Squashable diff is below.\n\nThe test script actually passes for me now.\n\n---\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 49f99e9..f0be105 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -231,7 +231,7 @@ EOF\n \n test_expect_success 'set-head --auto fails w/multiple HEADs' '\n \t(cd test &&\n-\t test_must_fail git remote set-head --auto two >& output &&\n+\t test_must_fail git remote set-head --auto two > output 2>&1 &&\n \ttest_cmp expect output)\n '\n \n"},{"id":"104732","messageId":"20090215060815.GA7473@coredump.intra.peff.net","threadId":"17770","inReplyTo":"20090214211548.GA14898@coredump.intra.peff.net","subject":"Re: [PATCH] builtin-remote: better handling of multiple remote HEADs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-15T06:08:15Z","receivedAt":"2009-02-15T06:08:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 14, 2009 at 04:15:48PM -0500, Jeff King wrote:\n\n> On Sat, Feb 14, 2009 at 03:21:30PM -0500, Daniel Barkalow wrote:\n> \n> > I haven't checked lately, but I think that what's actually needed is to \n> > have the locate_head() function notice if the struct ref for HEAD actually \n> > has the symref field non-NULL, and report that as the unambiguous answer. \n> \n> Indeed. Something like the patch below works (on top of Jay's patches).\n> [...]\n> I will try to make something a little neater later today.\n\nHere is a cleaner series. It depends on all of Jay's remote patches,\nincluding the set-head one. I can resend once the dust is a little more\nsettled on those patches.\n\nThe first two are prep for adding a test in 5/5:\n\n 1/5: test scripts: refactor start_httpd helper\n 2/5: add basic http clone/fetch tests\n\nThese ones are code cleanup for 5/5:\n\n 3/5: refactor find_refs_by_name to accept const list\n 4/5: remote: refactor guess_remote_head\n\nAnd this is the useful one.\n\n 5/5: remote: use exact HEAD lookup if it is available\n\n-Peff\n"},{"id":"104733","messageId":"20090215061015.GA30414@coredump.intra.peff.net","threadId":"17770","inReplyTo":"20090215060815.GA7473@coredump.intra.peff.net","subject":"[PATCH 1/5] test scripts: refactor start_httpd helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-15T06:10:15Z","receivedAt":"2009-02-15T06:10:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There are some redirects and some error checking that need\nto be done by the caller; let's move both into the\nstart_httpd function so that all callers don't have to\nrepeat them (there is only one caller now, but another will\nfollow in this series).\n\nThis doesn't violate any assumptions that aren't already\nbeing made by lib-httpd, which is happy to say \"skipping\"\nand call test_done for a number of other cases.\n\nSigned-off-by: Jeff King \n---\nCleanup for the next patch.\n\n t/lib-httpd.sh | 9 +++++++--\n t/t5540-http-push.sh | 8 +-------\n 2 files changed, 8 insertions(+), 9 deletions(-)\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 3824020..88cfc51 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -82,13 +82,18 @@ prepare_httpd() {\n }\n \n start_httpd() {\n-\tprepare_httpd\n+\tprepare_httpd >&3 2>&4\n \n \ttrap 'stop_httpd; die' EXIT\n \n \t\"$LIB_HTTPD_PATH\" -d \"$HTTPD_ROOT_PATH\" \\\n \t\t-f \"$TEST_PATH/apache.conf\" $HTTPD_PARA \\\n-\t\t-c \"Listen 127.0.0.1:$LIB_HTTPD_PORT\" -k start\n+\t\t-c \"Listen 127.0.0.1:$LIB_HTTPD_PORT\" -k start \\\n+\t\t>&3 2>&4\n+\tif ! test $? = 0; then\n+\t\tsay \"skipping test, web server setup failed\"\n+\t\ttest_done\n+\tfi\n }\n \n stop_httpd() {\ndiff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh\nindex c236b5e..5b93e5a 100755\n--- a/t/t5540-http-push.sh\n+++ b/t/t5540-http-push.sh\n@@ -20,13 +20,7 @@ then\n fi\n \n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n-\n-if ! start_httpd >&3 2>&4\n-then\n-\tsay \"skipping test, web server setup failed\"\n-\ttest_done\n-\texit\n-fi\n+start_httpd\n \n test_expect_success 'setup remote repository' '\n \tcd \"$ROOT_PATH\" &&\n-- \n1.6.2.rc0.256.gf004c.dirty\n"},{"id":"104734","messageId":"20090215061217.GB30414@coredump.intra.peff.net","threadId":"17770","inReplyTo":"20090215060815.GA7473@coredump.intra.peff.net","subject":"[PATCH 2/5] add basic http clone/fetch tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-15T06:12:17Z","receivedAt":"2009-02-15T06:12:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This was mostly being tested implicitly by the \"http push\"\ntests. But making a separate test script means that:\n\n - we will run fetch tests even when http pushing support\n is not built\n\n - when there are failures on fetching, they are easier to\n see and isolate, as they are not in the middle of push\n tests\n\nThis script defaults to running the webserver on port 5550,\nand puts the original t5540 on port 5540, so that the two\ncan be run simultaneously without conflict (but both still\nrespect an externally set LIB_HTTPD_PORT).\n\nSigned-off-by: Jeff King \n---\nI started this as test infrastructure for the final patch in the series,\nbut I think it's nice to have a few http sanity checks in general. Of\ncourse, one must use GIT_TEST_HTTPD to enable them. Junio, is that part\nof your usual integration testing?\n\n Makefile | 1 +\n t/t5540-http-push.sh | 1 +\n t/t5550-http-fetch.sh | 46 ++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 48 insertions(+), 0 deletions(-)\n create mode 100755 t/t5550-http-fetch.sh\n\ndiff --git a/Makefile b/Makefile\nindex ffef867..d689654 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1368,6 +1368,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS\n GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS\n \t@echo SHELL_PATH=\\''$(subst ','\\'',$(SHELL_PATH_SQ))'\\' >$@\n \t@echo TAR=\\''$(subst ','\\'',$(subst ','\\'',$(TAR)))'\\' >>$@\n+\t@echo NO_CURL=\\''$(subst ','\\'',$(subst ','\\'',$(NO_CURL)))'\\' >>$@\n \n ### Detect Tck/Tk interpreter path changes\n ifndef NO_TCLTK\ndiff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh\nindex 5b93e5a..072ef1b 100755\n--- a/t/t5540-http-push.sh\n+++ b/t/t5540-http-push.sh\n@@ -11,6 +11,7 @@ This test runs various sanity checks on http-push.'\n \n ROOT_PATH=\"$PWD\"\n LIB_HTTPD_DAV=t\n+LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5540'}\n \n if git http-push > /dev/null 2>&1 || [ $? -eq 128 ]\n then\ndiff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh\nnew file mode 100755\nindex 0000000..b6e6ec9\n--- /dev/null\n+++ b/t/t5550-http-fetch.sh\n@@ -0,0 +1,46 @@\n+#!/bin/sh\n+\n+test_description='test fetching over http'\n+. ./test-lib.sh\n+\n+if test -n \"$NO_CURL\"; then\n+\tsay 'skipping test, git built without http support'\n+\ttest_done\n+fi\n+\n+. \"$TEST_DIRECTORY\"/lib-httpd.sh\n+LIB_HTTPD_PORT=${LIB_HTTPD_PORT-'5550'}\n+start_httpd\n+\n+test_expect_success 'setup repository' '\n+\techo content >file &&\n+\tgit add file &&\n+\tgit commit -m one\n+'\n+\n+test_expect_success 'create http-accessible bare repository' '\n+\tmkdir \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n+\t(cd \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n+\t git --bare init &&\n+\t echo \"exec git update-server-info\" >hooks/post-update &&\n+\t chmod +x hooks/post-update\n+\t) &&\n+\tgit remote add public \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n+\tgit push public master:master\n+'\n+\n+test_expect_success 'clone http repository' '\n+\tgit clone $HTTPD_URL/repo.git clone &&\n+\ttest_cmp file clone/file\n+'\n+\n+test_expect_success 'fetch changes via http' '\n+\techo content >>file &&\n+\tgit commit -a -m two &&\n+\tgit push public\n+\t(cd clone && git pull) &&\n+\ttest_cmp file clone/file\n+'\n+\n+stop_httpd\n+test_done\n-- \n1.6.2.rc0.256.gf004c.dirty\n"},{"id":"104735","messageId":"20090215061239.GC30414@coredump.intra.peff.net","threadId":"17770","inReplyTo":"20090215060815.GA7473@coredump.intra.peff.net","subject":"[PATCH 3/5] refactor find_refs_by_name to accept const list","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-15T06:12:39Z","receivedAt":"2009-02-15T06:12:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Since it doesn't actually touch its argument, this makes\nsense.\n\nHowever, we still want to return a non-const version (which\nrequires a cast) so that this:\n\n struct ref *a, *b;\n a = find_ref_by_name(b);\n\nworks. Unfortunately, you can also silently strip the const\nfrom a variable:\n\n struct ref *a;\n const struct ref *b;\n a = find_ref_by_name(b);\n\nThis is a classic C const problem because there is no way to\nsay \"return the type with the same constness that was passed\nto us\"; we provide the same semantics as standard library\nfunctions like strchr.\n\nSigned-off-by: Jeff King \n---\n cache.h | 2 +-\n refs.c | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 7f1a6e8..ba6c070 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -801,7 +801,7 @@ struct ref {\n #define REF_HEADS\t(1u << 1)\n #define REF_TAGS\t(1u << 2)\n \n-extern struct ref *find_ref_by_name(struct ref *list, const char *name);\n+extern struct ref *find_ref_by_name(const struct ref *list, const char *name);\n \n #define CONNECT_VERBOSE (1u << 0)\n extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);\ndiff --git a/refs.c b/refs.c\nindex 6eb5f53..b2a37e1 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1628,10 +1628,10 @@ int update_ref(const char *action, const char *refname,\n \treturn 0;\n }\n \n-struct ref *find_ref_by_name(struct ref *list, const char *name)\n+struct ref *find_ref_by_name(const struct ref *list, const char *name)\n {\n \tfor ( ; list; list = list->next)\n \t\tif (!strcmp(list->name, name))\n-\t\t\treturn list;\n+\t\t\treturn (struct ref *)list;\n \treturn NULL;\n }\n-- \n1.6.2.rc0.256.gf004c.dirty\n"},{"id":"104736","messageId":"20090215061651.GD30414@coredump.intra.peff.net","threadId":"17770","inReplyTo":"20090215060815.GA7473@coredump.intra.peff.net","subject":"[PATCH 4/5] remote: refactor guess_remote_head","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-15T06:16:51Z","receivedAt":"2009-02-15T06:16:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This function had a lot of complications which made it hard\nto read and extend, and confusing to call (because of\noptional unused parameters). This patch attempts to address\nthat:\n\n - we used to manually search through the ref lists; this\n should be a one-line call to find_ref_by_name\n\n - guess_remote_head used to do two things: find the HEAD\n ref, and then find a matching ref. And only one of the\n two callers actually cared about returning the HEAD ref.\n Since it's a one-liner, just have the caller do it\n themselves (and remember it or not as they wish).\n\n - there were two ways of getting results out of the\n function (a return value, and an \"all_matched\" out\n parameter), but no caller cared about both. One of them\n returned a pointer into the passed-in ref list and one\n of them returned a newly allocated list (with the\n peer_ref fields stripped). Let's be simple and\n consistent: the return value is always a ref copy with\n a valid peer_ref. If \"all\" is requested, all candidates\n are returned. Otherwise, the best candidate is returned.\n\nSigned-off-by: Jeff King \n---\nThe diff is horrible to read, and would look much better as a whole\nfunction replacement. It would make a great test case for a \"short lines\nare uninteresting\" diff feature.\n\n builtin-clone.c | 5 +--\n builtin-remote.c | 5 ++-\n remote.c | 62 ++++++++++++++++++++++-------------------------------\n remote.h | 17 ++++++--------\n 4 files changed, 38 insertions(+), 51 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex d57818c..7130cab 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -508,9 +508,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tclear_extra_refs();\n \n \t\tmapped_refs = write_remote_refs(refs, &refspec, reflog_msg.buf);\n-\n-\t\thead_points_at = guess_remote_head(refs, mapped_refs,\n-\t\t\t\t\t\t &remote_head, NULL);\n+\t\tremote_head = find_ref_by_name(refs, \"HEAD\");\n+\t\thead_points_at = guess_remote_head(remote_head, mapped_refs, 0);\n \t}\n \telse {\n \t\twarning(\"You appear to have cloned an empty repository.\");\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex 608c0f3..d6958d4 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -275,7 +275,7 @@ static int get_ref_states(const struct ref *ref, struct ref_states *states)\n static int get_head_names(const struct ref *refs,\n \tconst char *remote_name, struct ref_states *states)\n {\n-\tstruct ref *ref, *matches = NULL;\n+\tstruct ref *ref, *matches;\n \tstruct ref *fetch_map = NULL, **fetch_map_tail = &fetch_map;\n \tstruct refspec refspec;\n \n@@ -284,7 +284,8 @@ static int get_head_names(const struct ref *refs,\n \trefspec.src = refspec.dst = \"refs/heads/\";\n \tstates->heads.strdup_strings = 1;\n \tget_fetch_map(refs, &refspec, &fetch_map_tail, 0);\n-\tguess_remote_head(refs, fetch_map, NULL, &matches);\n+\tmatches = guess_remote_head(find_ref_by_name(refs, \"HEAD\"),\n+\t\t\t\t\tfetch_map, 1);\n \tfor(ref = matches; ref; ref = ref->next)\n \t\tstring_list_append(abbrev_branch(ref->name), &states->heads);\n \ndiff --git a/remote.c b/remote.c\nindex 6385a22..7ba1bff 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1377,51 +1377,41 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)\n \treturn 1;\n }\n \n-const struct ref *guess_remote_head(const struct ref *refs,\n-\t\t\t\t const struct ref *mapped_refs,\n-\t\t\t\t const struct ref **remote_head_p,\n-\t\t\t\t struct ref **all_matches_p)\n+struct ref *copy_ref_with_peer(const struct ref *src)\n {\n-\tconst struct ref *remote_head = NULL;\n-\tconst struct ref *remote_master = NULL;\n-\tconst struct ref *ret = NULL;\n-\tconst struct ref *r;\n-\tstruct ref **tail = all_matches_p;\n-\n-\tfor (r = refs; r; r = r->next)\n-\t\tif (!strcmp(r->name, \"HEAD\"))\n-\t\t\tremote_head = r;\n-\n-\tif (!all_matches_p)\n-\t\tfor (r = mapped_refs; r; r = r->next)\n-\t\t\tif (!strcmp(r->name, \"refs/heads/master\"))\n-\t\t\t\tremote_master = r;\n+\tstruct ref *dst = copy_ref(src);\n+\tdst->peer_ref = copy_ref(src->peer_ref);\n+\treturn dst;\n+}\n \n-\tif (remote_head_p)\n-\t\t*remote_head_p = remote_head;\n+struct ref *guess_remote_head(const struct ref *head,\n+\t\t\t const struct ref *refs,\n+\t\t\t int all)\n+{\n+\tconst struct ref *r;\n+\tstruct ref *list = NULL;\n+\tstruct ref **tail = &list;\n \n-\t/* If there's no HEAD value at all, never mind. */\n-\tif (!remote_head)\n+\tif (!head)\n \t\treturn NULL;\n \n \t/* If refs/heads/master could be right, it is. */\n-\tif (remote_master && !hashcmp(remote_master->old_sha1,\n-\t\t\t\t remote_head->old_sha1))\n-\t\treturn remote_master;\n+\tif (!all) {\n+\t\tconst struct ref *m;\n+\t\tm = find_ref_by_name(refs, \"refs/heads/master\");\n+\t\tif (m && !hashcmp(m->old_sha1, head->old_sha1))\n+\t\t\treturn copy_ref_with_peer(m);\n+\t}\n \n \t/* Look for another ref that points there */\n-\tfor (r = mapped_refs; r; r = r->next)\n-\t\tif (r != remote_head &&\n-\t\t !hashcmp(r->old_sha1, remote_head->old_sha1)) {\n-\t\t\tstruct ref *cpy;\n-\t\t\tif (!ret)\n-\t\t\t\tret = r;\n-\t\t\tif (!all_matches_p)\n+\tfor (r = refs; r; r = r->next) {\n+\t\tif (r != head && !hashcmp(r->old_sha1, head->old_sha1)) {\n+\t\t\t*tail = copy_ref_with_peer(r);\n+\t\t\ttail = &((*tail)->next);\n+\t\t\tif (!all)\n \t\t\t\tbreak;\n-\t\t\t*tail = cpy = copy_ref(r);\n-\t\t\tcpy->peer_ref = NULL;\n-\t\t\ttail = &cpy->next;\n \t\t}\n+\t}\n \n-\treturn ret;\n+\treturn list;\n }\ndiff --git a/remote.h b/remote.h\nindex 8409d42..111c4ba 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -138,16 +138,13 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs);\n int format_tracking_info(struct branch *branch, struct strbuf *sb);\n \n /*\n- * Look in refs for HEAD. Then look for a matching SHA1 in mapped_refs,\n- * first checking if refs/heads/master matches. Return NULL if nothing matches\n- * or if there is no HEAD in refs. remote_head_p is assigned HEAD if not NULL.\n- * If all_matches_p is NULL, return after the first possible match. Otherwise\n- * all_matches_p is set to a ref list of each branch head with the same SHA1 as\n- * HEAD.\n+ * Find refs from a list which are likely to be pointed to by the given HEAD\n+ * ref. If 'all' is false, returns the most likely ref; otherwise, returns a\n+ * list of all candidate refs. If no match is found (or 'head' is NULL),\n+ * returns NULL. All returns are newly allocated and should be freed.\n */\n-const struct ref *guess_remote_head(const struct ref *refs,\n-\t\t\t\t const struct ref *mapped_refs,\n-\t\t\t\t const struct ref **remote_head_p,\n-\t\t\t\t struct ref **all_matches_p);\n+struct ref *guess_remote_head(const struct ref *head,\n+\t\t\t const struct ref *refs,\n+\t\t\t int all);\n \n #endif\n-- \n1.6.2.rc0.256.gf004c.dirty\n"},{"id":"104737","messageId":"20090215061818.GE30414@coredump.intra.peff.net","threadId":"17770","inReplyTo":"20090215060815.GA7473@coredump.intra.peff.net","subject":"[PATCH 5/5] remote: use exact HEAD lookup if it is available","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-15T06:18:18Z","receivedAt":"2009-02-15T06:18:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Our usual method for determining the ref pointed to by HEAD\nis to compare HEAD's sha1 to the sha1 of all refs, trying to\nfind a unique match.\n\nHowever, some transports actually get to look at HEAD\ndirectly; we should make use of that information when it is\navailable. Currently, only http remotes support this\nfeature.\n\nSigned-off-by: Jeff King \n---\nA possible 6/5 would be to do something similar for local repos (or\nresurrecting the HEAD proposal).\n\n remote.c | 10 ++++++++++\n t/t5550-http-fetch.sh | 11 +++++++++++\n 2 files changed, 21 insertions(+), 0 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 7ba1bff..a96a910 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1395,6 +1395,16 @@ struct ref *guess_remote_head(const struct ref *head,\n \tif (!head)\n \t\treturn NULL;\n \n+\t/*\n+\t * Some transports support directly peeking at\n+\t * where HEAD points; if that is the case, then\n+\t * we don't have to guess.\n+\t */\n+\tif (head->symref) {\n+\t\tr = find_ref_by_name(refs, head->symref);\n+\t\treturn r ? copy_ref_with_peer(r) : NULL;\n+\t}\n+\n \t/* If refs/heads/master could be right, it is. */\n \tif (!all) {\n \t\tconst struct ref *m;\ndiff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh\nindex b6e6ec9..05b1b62 100755\n--- a/t/t5550-http-fetch.sh\n+++ b/t/t5550-http-fetch.sh\n@@ -42,5 +42,16 @@ test_expect_success 'fetch changes via http' '\n \ttest_cmp file clone/file\n '\n \n+test_expect_success 'http remote detects correct HEAD' '\n+\tgit push public master:other &&\n+\t(cd clone &&\n+\t git remote set-head origin -d &&\n+\t git remote set-head origin -a &&\n+\t git symbolic-ref refs/remotes/origin/HEAD > output &&\n+\t echo refs/remotes/origin/master > expect &&\n+\t test_cmp expect output\n+\t)\n+'\n+\n stop_httpd\n test_done\n-- \n1.6.2.rc0.256.gf004c.dirty\n"},{"id":"104740","messageId":"7vmyconn7g.fsf@gitster.siamese.dyndns.org","threadId":"17770","inReplyTo":"20090215061217.GB30414@coredump.intra.peff.net","subject":"Re: [PATCH 2/5] add basic http clone/fetch tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-15T08:01:07Z","receivedAt":"2009-02-15T08:01:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> ... Of\n> course, one must use GIT_TEST_HTTPD to enable them. Junio, is that part\n> of your usual integration testing?\n\nNo. I do not run server tests myself. I could enable it for tests run on\nmy private machine, but the final pre-pushout testing don at k.org is more\nor less unattended, and it is a shared machine where I do not want to run\nserver-ish tests.\n"},{"id":"104778","messageId":"76718490902150613i79517e6dwcec8e1e40c56bcc8@mail.gmail.com","threadId":"17770","inReplyTo":"20090215052740.GA11522@coredump.intra.peff.net","subject":"Re: [PATCH] builtin-remote: better handling of multiple remote HEADs","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-15T14:13:09Z","receivedAt":"2009-02-15T14:13:09Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sun, Feb 15, 2009 at 12:27 AM, Jeff King wrote:\n> On Sat, Feb 14, 2009 at 05:30:30AM -0500, Jay Soffian wrote:\n>\n>> +test_expect_success 'set-head --auto fails w/multiple HEADs' '\n>> + (cd test &&\n>> + test_must_fail git remote set-head --auto two >& output &&\n>> + test_cmp expect output)\n>\n> I missed it before, but there is a typo in this test (>&) that causes it\n> to barf.\n\nDidn't barf for me, but it turns out it's because it's a bash'ism[1],\nand that's the default /bin/sh on OS X. Out of curiosity, on what\nplatform did it fail for you?\n\n[1] Redirecting Standard Output and Standard Error\n Bash allows both the standard output (file descriptor 1) and\nthe standard error\n output (file descriptor 2) to be redirected to the file whose\nname is the expan-\n sion of word with this construct.\n\n There are two formats for redirecting standard output and standard error:\n\n &>word\n and\n >&word\n\n Of the two forms, the first is preferred. This is semantically\nequivalent to\n\n >word 2>&1\n\nj.\n"},{"id":"104780","messageId":"20090215151259.GA15342@sigill.intra.peff.net","threadId":"17770","inReplyTo":"76718490902150613i79517e6dwcec8e1e40c56bcc8@mail.gmail.com","subject":"Re: [PATCH] builtin-remote: better handling of multiple remote HEADs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-15T15:12:59Z","receivedAt":"2009-02-15T15:12:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 15, 2009 at 09:13:09AM -0500, Jay Soffian wrote:\n\n> >> + (cd test &&\n> >> + test_must_fail git remote set-head --auto two >& output &&\n> >> + test_cmp expect output)\n> >\n> > I missed it before, but there is a typo in this test (>&) that causes it\n> > to barf.\n> \n> Didn't barf for me, but it turns out it's because it's a bash'ism[1],\n> and that's the default /bin/sh on OS X. Out of curiosity, on what\n> platform did it fail for you?\n\nAh, I didn't realize that was valid under any shell. ;) My /bin/sh is\ndash, which explains it.\n\n-Peff\n"},{"id":"104783","messageId":"76718490902150722p3cd4f98ay192df5136300d2d3@mail.gmail.com","threadId":"17770","inReplyTo":"20090215061818.GE30414@coredump.intra.peff.net","subject":"Re: [PATCH 5/5] remote: use exact HEAD lookup if it is available","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-15T15:22:02Z","receivedAt":"2009-02-15T15:22:02Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sun, Feb 15, 2009 at 1:18 AM, Jeff King wrote:\n> A possible 6/5 would be to do something similar for local repos (or\n> resurrecting the HEAD proposal).\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/102039\n\nI read that thread, and it wasn't clear to me why it died. Was there a\nv3 patch round that I cannot find? The only unaddressed reply I see\nis from Jeff:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/102039\n\nBut that seems tangential to the proposal.\n\nj.\n"},{"id":"104815","messageId":"20090215195826.GA26740@coredump.intra.peff.net","threadId":"17770","inReplyTo":"20090215061818.GE30414@coredump.intra.peff.net","subject":"Re: [PATCH 5/5] remote: use exact HEAD lookup if it is available","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-15T19:58:26Z","receivedAt":"2009-02-15T19:58:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 15, 2009 at 01:18:18AM -0500, Jeff King wrote:\n\n> A possible 6/5 would be to do something similar for local repos (or\n> resurrecting the HEAD proposal).\n\nHere is a quick and dirty series to unambiguously determine the HEAD for\nlocal repos, but I am undecided on whether this is actually a good idea.\n\nNote that this fails Jay's tests in t5505 which expect the ambiguity; to\nbe considered for inclusion, it would need to test \"../two\" as a remote\nas well as \"file://$(pwd)/../two\", making sure each behaved correctly.\n\nBut I am posting it here to stimulate discussion on whether it is even\nsomething we want.\n\n 1/2: transport: cleanup duplicated ref fetching code\n 2/2: transport: unambiguously determine local HEAD\n\n-Peff\n"},{"id":"104816","messageId":"20090215200057.GA26779@coredump.intra.peff.net","threadId":"17770","inReplyTo":"20090215195826.GA26740@coredump.intra.peff.net","subject":"[PATCH 1/2] transport: cleanup duplicated ref fetching code","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-15T20:00:57Z","receivedAt":"2009-02-15T20:00:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When fetching refs through the git protocol, the\nfetch_refs_via_pack will establish the connection and get\nthe ref list if it has not already been done.\n\nSince the code is only two lines, it was done inline rather\nthan calling the transport's get_refs function. However,\ncalling that function better matches the intent, and is\nfuture-proof against enhancements in get_refs_via_connect.\n\nSigned-off-by: Jeff King \n---\nEnhancements like the one that is coming in the next patch.\n\nThough I think that fetch_pack doesn't currently care if the HEAD symref\nis set up, it makes sense to me to be consistent.\n\n transport.c | 6 ++----\n 1 files changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 9ad4a16..c9f31f6 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -646,10 +646,8 @@ static int fetch_refs_via_pack(struct transport *transport,\n \tfor (i = 0; i < nr_heads; i++)\n \t\torigh[i] = heads[i] = xstrdup(to_fetch[i]->name);\n \n-\tif (!data->conn) {\n-\t\tconnect_setup(transport);\n-\t\tget_remote_heads(data->fd[0], &refs_tmp, 0, NULL, 0, NULL);\n-\t}\n+\tif (!data->conn)\n+\t\trefs_tmp = transport->get_refs_list(transport);\n \n \trefs = fetch_pack(&args, data->fd, data->conn,\n \t\t\t refs_tmp ? refs_tmp : transport->remote_refs,\n-- \n1.6.2.rc0.258.gcef3.dirty\n"},{"id":"104817","messageId":"20090215200131.GB26779@coredump.intra.peff.net","threadId":"17770","inReplyTo":"20090215195826.GA26740@coredump.intra.peff.net","subject":"[PATCH 2/2] transport: unambiguously determine local HEAD","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-15T20:01:31Z","receivedAt":"2009-02-15T20:01:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we fetch refs using the git protocol, we have to guess\nat which ref is pointed to by the HEAD. In the case of a\nlocal filesystem repo, however, we can cheat by going to\nthat repo and peeking directly at the contents of HEAD.\n\nSigned-off-by: Jeff King \n---\nAgain, this fails tests in t5505, and is not meant for inclusion.\n\n transport.c | 39 +++++++++++++++++++++++++++++++++++++++\n 1 files changed, 39 insertions(+), 0 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex c9f31f6..e575203 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -12,6 +12,8 @@\n #include \"dir.h\"\n #include \"refs.h\"\n \n+static int is_local(const char *url);\n+\n /* rsync support */\n \n /*\n@@ -609,6 +611,40 @@ static int connect_setup(struct transport *transport)\n \treturn 0;\n }\n \n+static void mark_head_via_local(const char *url, struct ref *refs)\n+{\n+\tstatic const char *argv[] = { \"symbolic-ref\", \"HEAD\", NULL };\n+\tstatic const char *env[] = { GIT_DIR_ENVIRONMENT, NULL };\n+\tstruct child_process cmd;\n+\tstruct ref *head;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\thead = find_ref_by_name(refs, \"HEAD\");\n+\tif (!head)\n+\t\treturn;\n+\n+\tmemset(&cmd, 0, sizeof cmd);\n+\tcmd.argv = argv;\n+\tcmd.env = env;\n+\tcmd.dir = url;\n+\tcmd.git_cmd = 1;\n+\tcmd.no_stdin = 1;\n+\tcmd.no_stderr = 1;\n+\tcmd.out = -1;\n+\n+\tif (start_command(&cmd) < 0)\n+\t\treturn;\n+\tif (strbuf_read(&buf, cmd.out, 64) < 0)\n+\t\treturn;\n+\tif (finish_command(&cmd) != 0) {\n+\t\tstrbuf_release(&buf);\n+\t\treturn;\n+\t}\n+\n+\tstrbuf_trim(&buf);\n+\thead->symref = strbuf_detach(&buf, NULL);\n+}\n+\n static struct ref *get_refs_via_connect(struct transport *transport)\n {\n \tstruct git_transport_data *data = transport->data;\n@@ -617,6 +653,9 @@ static struct ref *get_refs_via_connect(struct transport *transport)\n \tconnect_setup(transport);\n \tget_remote_heads(data->fd[0], &refs, 0, NULL, 0, NULL);\n \n+\tif (is_local(transport->url))\n+\t\tmark_head_via_local(transport->url, refs);\n+\n \treturn refs;\n }\n \n-- \n1.6.2.rc0.258.gcef3.dirty\n"},{"id":"104877","messageId":"7v7i3rglza.fsf@gitster.siamese.dyndns.org","threadId":"17770","inReplyTo":"76718490902150613i79517e6dwcec8e1e40c56bcc8@mail.gmail.com","subject":"Re: [PATCH] builtin-remote: better handling of multiple remote HEADs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-16T02:21:45Z","receivedAt":"2009-02-16T02:21:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> Bash allows both the standard output (file descriptor 1) and\n> the standard error\n> output (file descriptor 2) to be redirected to the file whose\n> name is the expansion of word with this construct.\n>\n> There are two formats for redirecting standard output and\n> standard error:\n>\n> &>word\n> and\n> >&word\n>\n> Of the two forms, the first is preferred. This is semantically\n> equivalent to\n>\n> >word 2>&1\n\nJust to clarify, the above is not a recommendation for shell scripts in\ngit project. The last one is the only one we recommend in our scripts.\n\nBy the say, does anybody know why bash people recommend &>word form?\n\nNeither &>word nor >&word to send both stderr and stdout to the file is\nfrom true Bourne, but at least the use of >&word form for this purpose is\nmore familiar to people who are used to Csh.\n"},{"id":"104885","messageId":"76718490902151858j78c0d06bxa659ffd3bd2dad01@mail.gmail.com","threadId":"17770","inReplyTo":"7v7i3rglza.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-remote: better handling of multiple remote HEADs","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-16T02:58:48Z","receivedAt":"2009-02-16T02:58:48Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Sun, Feb 15, 2009 at 9:21 PM, Junio C Hamano wrote:\n>> &>word\n>> >&word\n> Just to clarify, the above is not a recommendation for shell scripts in\n> git project.\n\nIndeed not! My use of it wasn't even intentional, I was a tcsh\n(cringe...) user for a long time and I only finally switched to bash\nabout a year ago. It must've been muscle memory that made me type it,\nbut a pox on bash for not rejecting it outright -- the bash man page\nclaims posix'ish compliance when invoked as /bin/sh, so I don't know\nwhy it allows such syntax in its posix'ish mode.\n\n> By the say, does anybody know why bash people recommend &>word form?\n>\n> Neither &>word nor >&word to send both stderr and stdout to the file is\n> from true Bourne, but at least the use of >&word form for this purpose is\n> more familiar to people who are used to Csh.\n\nit is exactly because it is familiar to csh people\nthat they recommend the opposite\n\nj.\n"}]}