{"thread":{"id":"54661","subject":"[RFC/PATCH] Add fetch.updateHead option","startedAt":"2020-11-18T09:12:45Z","lastAt":"2020-11-24T07:09:41Z","messageCount":13,"participants":["Felipe Contreras","Ævar Arnfjörð Bjarmason","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"410212","messageId":"20201118091219.3341585-1-felipe.contreras@gmail.com","threadId":"54661","inReplyTo":null,"subject":"[RFC/PATCH] Add fetch.updateHead option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-18T09:12:19Z","receivedAt":"2020-11-18T09:12:45Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Users might change the behavior when running \"git fetch\" so that the\nremote's HEAD symbolic ref is updated at certain point.\n\nFor example after running \"git remote add\" the remote HEAD is not\nset like it is with \"git clone\".\n\nSetting \"fetch.updatehead = missing\" would probably be a sensible\ndefault that everyone would want, but for now the default behavior is to\nnever update HEAD, so there shouldn't be any functional changes.\n\nFor the next major version of Git, we might want to change this default.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n\nThis is just a RFC, the tests are missing.\n\n Documentation/config/fetch.txt  |  4 +++\n Documentation/config/remote.txt |  3 ++\n builtin/fetch.c                 | 53 ++++++++++++++++++++++++++++++++-\n remote.c                        | 21 +++++++++++++\n remote.h                        | 11 +++++++\n 5 files changed, 91 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt\nindex 6af6f5edb2..93d6c59fac 100644\n--- a/Documentation/config/fetch.txt\n+++ b/Documentation/config/fetch.txt\n@@ -94,3 +94,7 @@ fetch.writeCommitGraph::\n \tmerge and the write may take longer. Having an updated commit-graph\n \tfile helps performance of many Git commands, including `git merge-base`,\n \t`git push -f`, and `git log --graph`. Defaults to false.\n+\n+fetch.updateHead::\n+  Defines when to update the remote HEAD symbolic ref. Values are 'never',\n+  'missing' (update only when HEAD is missing), and 'always'.\ndiff --git a/Documentation/config/remote.txt b/Documentation/config/remote.txt\nindex a8e6437a90..905661c7f7 100644\n--- a/Documentation/config/remote.txt\n+++ b/Documentation/config/remote.txt\n@@ -84,3 +84,6 @@ remote.<name>.promisor::\n remote.<name>.partialclonefilter::\n \tThe filter that will be applied when fetching from this\n \tpromisor remote.\n+\n+remote.<name>.updateHead::\n+  Defines when to update the remote HEAD symbolic ref. See `fetch.updateHead`.\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex f9c3c49f14..b47b06f001 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -55,6 +55,8 @@ static int fetch_prune_tags_config = -1; /* unspecified */\n static int prune_tags = -1; /* unspecified */\n #define PRUNE_TAGS_BY_DEFAULT 0 /* do we prune tags by default? */\n \n+static int fetch_update_head = FETCH_UPDATE_HEAD_DEFAULT;\n+\n static int all, append, dry_run, force, keep, multiple, update_head_ok;\n static int write_fetch_head = 1;\n static int verbosity, deepen_relative, set_upstream;\n@@ -120,6 +122,9 @@ static int git_fetch_config(const char *k, const char *v, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(k, \"fetch.updatehead\"))\n+\t\treturn parse_update_head(&fetch_update_head, k, v);\n+\n \treturn git_default_config(k, v, cb);\n }\n \n@@ -1342,6 +1347,38 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)\n \t}\n }\n \n+static void update_head(int config, const struct ref *head, const struct remote *remote)\n+{\n+\tstruct strbuf ref = STRBUF_INIT, target = STRBUF_INIT;\n+\tconst char *r, *head_name = NULL;\n+\n+\tif (!head || !head->symref || !remote)\n+\t\treturn;\n+\n+\tstrbuf_addf(&ref, \"refs/remotes/%s/HEAD\", remote->name);\n+\tskip_prefix(head->symref, \"refs/heads/\", &head_name);\n+\tstrbuf_addf(&target, \"refs/remotes/%s/%s\", remote->name, head_name);\n+\n+\tr = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n+\t\tref.buf, RESOLVE_REF_READING,\n+\t\tNULL, NULL);\n+\n+\tif (r) {\n+\t\tif (config == FETCH_UPDATE_HEAD_MISSING)\n+\t\t\t/* already present */\n+\t\t\treturn;\n+\t\telse if (config == FETCH_UPDATE_HEAD_ALWAYS && !strcmp(r, target.buf))\n+\t\t\t/* already up-to-date */\n+\t\t\treturn;\n+\t\telse\n+\t\t\t/* should never happen */\n+\t\t\treturn;\n+\t}\n+\n+\tif (create_symref(ref.buf, target.buf, \"remote update head\"))\n+\t\twarning(_(\"could not set remote head\"));\n+}\n+\n static int do_fetch(struct transport *transport,\n \t\t    struct refspec *rs)\n {\n@@ -1351,6 +1388,7 @@ static int do_fetch(struct transport *transport,\n \tconst struct ref *remote_refs;\n \tstruct strvec ref_prefixes = STRVEC_INIT;\n \tint must_list_refs = 1;\n+\tint need_update_head = 0, update_head_config = 0;\n \n \tif (tags == TAGS_DEFAULT) {\n \t\tif (transport->remote->fetch_tags == 2)\n@@ -1382,8 +1420,18 @@ static int do_fetch(struct transport *transport,\n \t\t\t\tbreak;\n \t\t\t}\n \t\t}\n-\t} else if (transport->remote && transport->remote->fetch.nr)\n+\t} else if (transport->remote && transport->remote->fetch.nr) {\n+\t\tif (transport->remote->update_head)\n+\t\t\tupdate_head_config = transport->remote->update_head;\n+\t\telse\n+\t\t\tupdate_head_config = fetch_update_head;\n+\n+\t\tneed_update_head = update_head_config && update_head_config != FETCH_UPDATE_HEAD_NEVER;\n+\n+\t\tif (need_update_head)\n+\t\t\tstrvec_push(&ref_prefixes, \"HEAD\");\n \t\trefspec_ref_prefixes(&transport->remote->fetch, &ref_prefixes);\n+\t}\n \n \tif (tags == TAGS_SET || tags == TAGS_DEFAULT) {\n \t\tmust_list_refs = 1;\n@@ -1427,6 +1475,9 @@ static int do_fetch(struct transport *transport,\n \t\tgoto cleanup;\n \t}\n \n+\tif (need_update_head)\n+\t\tupdate_head(update_head_config, find_ref_by_name(remote_refs, \"HEAD\"), transport->remote);\n+\n \tif (set_upstream) {\n \t\tstruct branch *branch = branch_get(\"HEAD\");\n \t\tstruct ref *rm;\ndiff --git a/remote.c b/remote.c\nindex 8a6dbbb903..dad8062561 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -298,6 +298,25 @@ static void read_branches_file(struct remote *remote)\n \tremote->fetch_tags = 1; /* always auto-follow */\n }\n \n+int parse_update_head(int *r, const char *var, const char *value)\n+{\n+\tif (!r)\n+\t\treturn -1;\n+\telse if (!value)\n+\t\treturn config_error_nonbool(var);\n+\telse if (!strcmp(value, \"never\"))\n+\t\t*r = FETCH_UPDATE_HEAD_NEVER;\n+\telse if (!strcmp(value, \"missing\"))\n+\t\t*r = FETCH_UPDATE_HEAD_MISSING;\n+\telse if (!strcmp(value, \"always\"))\n+\t\t*r = FETCH_UPDATE_HEAD_ALWAYS;\n+\telse {\n+\t\terror(_(\"malformed value for %s: %s\"), var, value);\n+\t\treturn error(_(\"must be one of never, missing, or always\"));\n+\t}\n+\treturn 0;\n+}\n+\n static int handle_config(const char *key, const char *value, void *cb)\n {\n \tconst char *name;\n@@ -418,6 +437,8 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\t\t\t\t key, value);\n \t} else if (!strcmp(subkey, \"vcs\")) {\n \t\treturn git_config_string(&remote->foreign_vcs, key, value);\n+\t} else if (!strcmp(subkey, \"updatehead\")) {\n+\t\treturn parse_update_head(&remote->update_head, key, value);\n \t}\n \treturn 0;\n }\ndiff --git a/remote.h b/remote.h\nindex 3211abdf05..c16a2d7b2e 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -21,6 +21,13 @@ enum {\n \tREMOTE_BRANCHES\n };\n \n+enum {\n+\tFETCH_UPDATE_HEAD_DEFAULT = 0,\n+\tFETCH_UPDATE_HEAD_NEVER,\n+\tFETCH_UPDATE_HEAD_MISSING,\n+\tFETCH_UPDATE_HEAD_ALWAYS,\n+};\n+\n struct remote {\n \tstruct hashmap_entry ent;\n \n@@ -62,6 +69,8 @@ struct remote {\n \tint prune;\n \tint prune_tags;\n \n+\tint update_head;\n+\n \t/**\n \t * The configured helper programs to run on the remote side, for\n \t * Git-native protocols.\n@@ -372,4 +381,6 @@ int parseopt_push_cas_option(const struct option *, const char *arg, int unset);\n int is_empty_cas(const struct push_cas_option *);\n void apply_push_cas(struct push_cas_option *, struct remote *, struct ref *);\n \n+int parse_update_head(int *r, const char *var, const char *value);\n+\n #endif\n-- \n2.29.2\n\n"},{"id":"410214","messageId":"87wnyjdnb3.fsf@evledraar.gmail.com","threadId":"54661","inReplyTo":"20201118091219.3341585-1-felipe.contreras@gmail.com","subject":"Re: [RFC/PATCH] Add fetch.updateHead option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2020-11-18T09:30:40Z","receivedAt":"2020-11-18T09:31:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Nov 18 2020, Felipe Contreras wrote:\n\n> Users might change the behavior when running \"git fetch\" so that the\n> remote's HEAD symbolic ref is updated at certain point.\n>\n> For example after running \"git remote add\" the remote HEAD is not\n> set like it is with \"git clone\".\n>\n> Setting \"fetch.updatehead = missing\" would probably be a sensible\n> default that everyone would want, but for now the default behavior is to\n> never update HEAD, so there shouldn't be any functional changes.\n>\n> For the next major version of Git, we might want to change this default.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>\n> This is just a RFC, the tests are missing.\n\nI haven't taken much time to re-think through the patch/implications of\nthis, but I remember running into this and going through some pre-patch\ninvestigation at some point.\n\nIt's really annoying in some cases that \"clone\" isn't creating the same\nstate as \"remote\". IIRC I was doing some heuristics to figure out the\nremote branch name etc.\n\nIsn't this something we can just change without an option? There were a\nbunch of cases in clone/fetch that were different for no different\nreasons, IIRC I patched one or two of those in the past. But I haven't\ngone through the history of the feature and checked if it was\nintentional.\n"},{"id":"410215","messageId":"CAMP44s1i+defrfkCprTAOqyJSCAUMsqWe3Fyv+Y2TeAyiAdg8g@mail.gmail.com","threadId":"54661","inReplyTo":"87wnyjdnb3.fsf@evledraar.gmail.com","subject":"Re: [RFC/PATCH] Add fetch.updateHead option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-18T09:43:19Z","receivedAt":"2020-11-18T09:43:53Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 18, 2020 at 3:30 AM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n\n> I haven't taken much time to re-think through the patch/implications of\n> this, but I remember running into this and going through some pre-patch\n> investigation at some point.\n>\n> It's really annoying in some cases that \"clone\" isn't creating the same\n> state as \"remote\". IIRC I was doing some heuristics to figure out the\n> remote branch name etc.\n>\n> Isn't this something we can just change without an option? There were a\n> bunch of cases in clone/fetch that were different for no different\n> reasons, IIRC I patched one or two of those in the past. But I haven't\n> gone through the history of the feature and checked if it was\n> intentional.\n\nApparently remote/HEAD is supposed to be set manually, which is why\nthere is \"git remote add -m master\", and \"git remote set-head origin\nmaster\".\n\nPersonally I don't see any point in that.\n\nI think if no remote/HEAD is set (manually), it should be set\nautomatically on the first \"git fetch\", and that should mirror the\nbehavior of \"git clone\". This would be the equivalent of\n\"fetch.updatehead = missing\", which in my opinion should be the\ndefault.\n\nThis configuration was suggested by Jeff King, see:\n\nhttps://lore.kernel.org/git/20201118020618.GE650959@coredump.intra.peff.net/\n\nPersonally I don't need a configuration, I would be happy if\n\"fetch.updatehead = missing\" was the default.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"410237","messageId":"xmqqwnyi3bml.fsf@gitster.c.googlers.com","threadId":"54661","inReplyTo":"87wnyjdnb3.fsf@evledraar.gmail.com","subject":"Re: [RFC/PATCH] Add fetch.updateHead option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-18T15:53:06Z","receivedAt":"2020-11-18T15:53:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> On Wed, Nov 18 2020, Felipe Contreras wrote:\n>\n>> Users might change the behavior when running \"git fetch\" so that the\n>> remote's HEAD symbolic ref is updated at certain point.\n>>\n>> For example after running \"git remote add\" the remote HEAD is not\n>> set like it is with \"git clone\".\n>>\n>> Setting \"fetch.updatehead = missing\" would probably be a sensible\n>> default that everyone would want, but for now the default behavior is to\n>> never update HEAD, so there shouldn't be any functional changes.\n>>\n>> For the next major version of Git, we might want to change this default.\n>>\n>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> ---\n>>\n>> This is just a RFC, the tests are missing.\n>\n> I haven't taken much time to re-think through the patch/implications of\n> this, but I remember running into this and going through some pre-patch\n> investigation at some point.\n>\n> It's really annoying in some cases that \"clone\" isn't creating the same\n> state as \"remote\". IIRC I was doing some heuristics to figure out the\n> remote branch name etc.\n>\n> Isn't this something we can just change without an option? There were a\n> bunch of cases in clone/fetch that were different for no different\n> reasons, IIRC I patched one or two of those in the past. But I haven't\n> gone through the history of the feature and checked if it was\n> intentional.\n\nI think what Peff outlined earlier was reasonable.  \"remote add -f\",\nsince it talks with the remote, should be able to learn where their\nHEAD points at and set it up.  \"remote add\" that does not talk to\nthe remote cannot do so and \"fetch\" could help but we should not\ntouch existing refs/remotes/$name/HEAD by default [*1*], as the\nsymref is meant to indicate the local choice of which one of their\nbranches is significant to _us_ and what \"clone\" does is merely to\ngive it the initial value.\n\nBut when interacting with a remote whose choice of HEAD is always\nwhat the local user wants to follow, letting \"git fetch\" update\nrefs/remotes/$name/HEAD to a newly observed value would be a welcome\noptional feature.\n\nI haven't thought through what Jonathan (Nieder) said about an\noption to fail a fetch when refs/remotes/$name/HEAD and the remote\nHEAD \"git fetch\" observed do not match.  It may make sense in some\ncases but not always, and I do not know what the right explanation\nfor the use case the mode is supposed to be a good match is.\n\n\n[Footnote]\n\n*1* This is important when you work in more than one repositories\nwith working tree and use fetch+rebase to keep them in sync.\n"},{"id":"410263","messageId":"CAMP44s0Wvb_czYs6jaG=iwSOuGWvjksNo_sjjg3EKx18Ex2Fig@mail.gmail.com","threadId":"54661","inReplyTo":"xmqqwnyi3bml.fsf@gitster.c.googlers.com","subject":"Re: [RFC/PATCH] Add fetch.updateHead option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-18T19:04:54Z","receivedAt":"2020-11-18T19:05:27Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Nov 18, 2020 at 9:53 AM Junio C Hamano <gitster@pobox.com> wrote:\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> > I haven't taken much time to re-think through the patch/implications of\n> > this, but I remember running into this and going through some pre-patch\n> > investigation at some point.\n> >\n> > It's really annoying in some cases that \"clone\" isn't creating the same\n> > state as \"remote\". IIRC I was doing some heuristics to figure out the\n> > remote branch name etc.\n> >\n> > Isn't this something we can just change without an option? There were a\n> > bunch of cases in clone/fetch that were different for no different\n> > reasons, IIRC I patched one or two of those in the past. But I haven't\n> > gone through the history of the feature and checked if it was\n> > intentional.\n>\n> I think what Peff outlined earlier was reasonable.  \"remote add -f\",\n> since it talks with the remote, should be able to learn where their\n> HEAD points at and set it up.  \"remote add\" that does not talk to\n> the remote cannot do so and \"fetch\" could help but we should not\n> touch existing refs/remotes/$name/HEAD by default [*1*], as the\n> symref is meant to indicate the local choice of which one of their\n> branches is significant to _us_ and what \"clone\" does is merely to\n> give it the initial value.\n\nThe new suggested behavior (fetch.updatehead = missing) is that \"git\nfetch\" touches $remote/HEAD *only* when it doesn't exist.\n\nSo if you set $remote/HEAD manually, you would not affected.\n\nHowever, that behavior is clearly not ideal in the long term, since it\nseems basically nobody uses that feature.\n\nEither way I see no argument against adding this option, and making\nthe default fetch.updatehead = never, which doesn't change the current\nbehavior at all.\n\n> But when interacting with a remote whose choice of HEAD is always\n> what the local user wants to follow, letting \"git fetch\" update\n> refs/remotes/$name/HEAD to a newly observed value would be a welcome\n> optional feature.\n\nThat would be fetch.updatehead = always.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"410446","messageId":"20201120235203.GA353076@coredump.intra.peff.net","threadId":"54661","inReplyTo":"20201118091219.3341585-1-felipe.contreras@gmail.com","subject":"Re: [RFC/PATCH] Add fetch.updateHead option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-20T23:52:03Z","receivedAt":"2020-11-20T23:52:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 18, 2020 at 03:12:19AM -0600, Felipe Contreras wrote:\n\n> Users might change the behavior when running \"git fetch\" so that the\n> remote's HEAD symbolic ref is updated at certain point.\n> \n> For example after running \"git remote add\" the remote HEAD is not\n> set like it is with \"git clone\".\n> \n> Setting \"fetch.updatehead = missing\" would probably be a sensible\n> default that everyone would want, but for now the default behavior is to\n> never update HEAD, so there shouldn't be any functional changes.\n> \n> For the next major version of Git, we might want to change this default.\n\nThanks for working on this. Regardless of whether we change the default\nbehavior, this seems like an obvious improvement (and I do think it\nmakes sense to eventually change the default; I'd even be OK switching\nit to \"missing\" in the near term).\n\nThe implementation looks pretty straight-forward, but I have a few\ncomments below:\n\n> --- a/Documentation/config/remote.txt\n> +++ b/Documentation/config/remote.txt\n> @@ -84,3 +84,6 @@ remote.<name>.promisor::\n>  remote.<name>.partialclonefilter::\n>  \tThe filter that will be applied when fetching from this\n>  \tpromisor remote.\n> +\n> +remote.<name>.updateHead::\n> +  Defines when to update the remote HEAD symbolic ref. See `fetch.updateHead`.\n\nNice. I think fetch.updateHead is likely to be the commonly used one,\nbut if this isn't hard to support, it's a nice bonus for flexibility.\n\n> +static void update_head(int config, const struct ref *head, const struct remote *remote)\n> +{\n> +\tstruct strbuf ref = STRBUF_INIT, target = STRBUF_INIT;\n> +\tconst char *r, *head_name = NULL;\n> +\n> +\tif (!head || !head->symref || !remote)\n> +\t\treturn;\n\nI'd expect us to return early here, as well, if the config is set to\n\"never\". I think your function will usually still do the right thing\n(because we return early before writing), but it makes a pointless\nlookup of the current origin/HEAD. But see below.\n\n> +\tstrbuf_addf(&ref, \"refs/remotes/%s/HEAD\", remote->name);\n> +\tskip_prefix(head->symref, \"refs/heads/\", &head_name);\n\nShould we bail or complain if this skip_prefix() doesn't match? I think\nit would be a sign of weirdness on the remote side, since HEAD is never\nsupposed to point to anything except refs/heads/. But if we ever did see\nit, it's probably better to bail than to point to\nrefs/remotes/origin/refs/foo/bar or whatever.\n\n> +\tstrbuf_addf(&target, \"refs/remotes/%s/%s\", remote->name, head_name);\n> +\n> +\tr = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n> +\t\tref.buf, RESOLVE_REF_READING,\n> +\t\tNULL, NULL);\n\nThis won't resolve a symref pointing to an unborn branch, so it would\ncount as \"missing\". I.e.:\n\n  git symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/nope\n  git -c fetch.updatehead=missing fetch\n\nwill update it based on the remote HEAD.  I guess I could see some\nargument for defining \"missing\" in that way, but I suspect it is not\nwhat somebody in this situation would expect.\n\nI think it's hard to get into this situation intentionally. If you clone\nan empty repo, we won't write the symref at all (since we have nothing\nto write into it), so both sides would be missing. But it's easy enough\nto do the right thing; see symbolic-ref.c's check_symref() function\n(basically drop the READING flag).\n\nLikewise, I'd not expect to see a non-symref at that name, but what\nshould we do if we see one? I think overwrite it for \"always\", but not\nfor \"missing\". You can tell the difference by checking REF_ISSYMREF in\nthe returned flags, though the distinction may not matter if we follow\nthe semantics I just said.\n\n> +\tif (r) {\n> +\t\tif (config == FETCH_UPDATE_HEAD_MISSING)\n> +\t\t\t/* already present */\n> +\t\t\treturn;\n> +\t\telse if (config == FETCH_UPDATE_HEAD_ALWAYS && !strcmp(r, target.buf))\n> +\t\t\t/* already up-to-date */\n> +\t\t\treturn;\n> +\t\telse\n> +\t\t\t/* should never happen */\n> +\t\t\treturn;\n> +\t}\n> +\n> +\tif (create_symref(ref.buf, target.buf, \"remote update head\"))\n> +\t\twarning(_(\"could not set remote head\"));\n> +}\n\nIf we do update the symref, we should probably tell the user. Better\nstill if we can print it as part of the usual fetch output table.\n\n> @@ -1382,8 +1420,18 @@ static int do_fetch(struct transport *transport,\n>  \t\t\t\tbreak;\n>  \t\t\t}\n>  \t\t}\n> -\t} else if (transport->remote && transport->remote->fetch.nr)\n> +\t} else if (transport->remote && transport->remote->fetch.nr) {\n> +\t\tif (transport->remote->update_head)\n> +\t\t\tupdate_head_config = transport->remote->update_head;\n> +\t\telse\n> +\t\t\tupdate_head_config = fetch_update_head;\n> +\n> +\t\tneed_update_head = update_head_config && update_head_config != FETCH_UPDATE_HEAD_NEVER;\n> +\n> +\t\tif (need_update_head)\n> +\t\t\tstrvec_push(&ref_prefixes, \"HEAD\");\n>  \t\trefspec_ref_prefixes(&transport->remote->fetch, &ref_prefixes);\n> +\t}\n\nGood catch. We need this for:\n\n  git fetch origin\n\nsince otherwise it doesn't ask about HEAD in a v2 exchange. What about:\n\n  git fetch origin master\n\nThat won't report on HEAD either, even with your patch, because it hits\nthe part of the conditional before your \"else if\". What should it do?  I\ncan see an argument for \"nothing, we only update head on full configured\nfetches\", but if so we should definitely make that clear in the\ndocumentation. I can also see an argument for \"always, if we happen to\nhave heard about it\" (just like we opportunistically update tracking\nrefs even if they are fetched by name into FETCH_HEAD).\n\n> diff --git a/remote.h b/remote.h\n> index 3211abdf05..c16a2d7b2e 100644\n> --- a/remote.h\n> +++ b/remote.h\n\nI wondered if we should be interacting with remote.[ch]'s\nguess_remote_head() here, which is what powers \"remote set-head -a\", as\nwell as the initial clone decision. But I don't think it make sense. If\nwe were not explicitly given information about a symref, it does not\nmake much sense to guess without seeing all of the refs (which we may\nwell not, due to the v2 ref-prefix capability).\n\nIn most cases the distinction would not matter anyway. The \"guess\" part\ncomes in only for ancient pre-symref-capability versions of Git (which\nwe are unlikely to see and it's OK if they just don't make use of the\nfeature), or for remotes with detached HEADs (in which case not setting\nour local origin/HEAD is probably the best thing, as a bad guess would\nfoul up a later use of the \"missing\" mode).\n\n-Peff\n"},{"id":"410448","messageId":"xmqq3613tuwy.fsf@gitster.c.googlers.com","threadId":"54661","inReplyTo":"20201120235203.GA353076@coredump.intra.peff.net","subject":"Re: [RFC/PATCH] Add fetch.updateHead option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-21T00:28:45Z","receivedAt":"2020-11-21T00:28:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Thanks for working on this. Regardless of whether we change the default\n> behavior, this seems like an obvious improvement (and I do think it\n> makes sense to eventually change the default; I'd even be OK switching\n> it to \"missing\" in the near term).\n\nI agree that \"missing\" would be an easy thing to take, and I do not\nmind seeing it made the default in the near term.  It won't break\nexisting expectations too much, and can even be seen as a bugfix for\nthe current behaviour by making \"init && fetch\" a step closer to\n\"clone\".  Beyond that to modify what the end user already has is a\nmuch harder sell.  For some it may be an improvement, but for others\nit would be a breaking change.\n\n\n> The implementation looks pretty straight-forward, but I have a few\n> comments below:\n> ...\n>> +\tstrbuf_addf(&ref, \"refs/remotes/%s/HEAD\", remote->name);\n>> +\tskip_prefix(head->symref, \"refs/heads/\", &head_name);\n>\n> Should we bail or complain if this skip_prefix() doesn't match? I think\n> it would be a sign of weirdness on the remote side, ...\n\nYes, we should notice the weirdness and stop doing any further harm\nto the local side.\n\n>> +\tstrbuf_addf(&target, \"refs/remotes/%s/%s\", remote->name, head_name);\n>> +\n>> +\tr = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n>> +\t\tref.buf, RESOLVE_REF_READING,\n>> +\t\tNULL, NULL);\n>\n> This won't resolve a symref pointing to an unborn branch, so it would\n> count as \"missing\". I.e.:\n>\n>   git symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/nope\n>   git -c fetch.updatehead=missing fetch\n>\n> will update it based on the remote HEAD.  I guess I could see some\n> argument for defining \"missing\" in that way, but I suspect it is not\n> what somebody in this situation would expect.\n\nWhat do we do in \"git clone\" of an empty repository with the current\nbranch not yet to be born?  Modern Git tells where the HEAD points at\neven for unborn branch, so using that would be a natural thing to do.\n\n> If we do update the symref, we should probably tell the user. Better\n> still if we can print it as part of the usual fetch output table.\n\nTrue.\n\n>> +\t\tif (need_update_head)\n>> +\t\t\tstrvec_push(&ref_prefixes, \"HEAD\");\n>>  \t\trefspec_ref_prefixes(&transport->remote->fetch, &ref_prefixes);\n>> +\t}\n>\n> Good catch. We need this for:\n>\n>   git fetch origin\n>\n> since otherwise it doesn't ask about HEAD in a v2 exchange. What about:\n>\n>   git fetch origin master\n>\n> That won't report on HEAD either, even with your patch, because it hits\n> the part of the conditional before your \"else if\". What should it do?  I\n> can see an argument for \"nothing, we only update head on full configured\n> fetches\", but if so we should definitely make that clear in the\n> documentation. I can also see an argument for \"always, if we happen to\n> have heard about it\" (just like we opportunistically update tracking\n> refs even if they are fetched by name into FETCH_HEAD).\n\nAfter seeing that all users got accustomed to seeing an explicit\nfetch into FETCH_HEAD still update the remote-tracking branches, it\nis more consistent and easier to understand by users if the \"if we\nhappen to have heard about it\" is used, I would think.  Documenting\nthe behaviour certainly is needed in a real version after this RFC.\n\n> In most cases the distinction would not matter anyway. The \"guess\" part\n> comes in only for ancient pre-symref-capability versions of Git (which\n> we are unlikely to see and it's OK if they just don't make use of the\n> feature),\n\nI agree with you that at this point the versions of Git that does\nnot know about advertising symref can be left to rot.\n\n> or for remotes with detached HEADs (in which case not setting\n> our local origin/HEAD is probably the best thing, as a bad guess would\n> foul up a later use of the \"missing\" mode).\n\nYes.  That would help the \"missing\" mode.  With a mode stronger than\n\"missing\" that modifies what the local side has, detaching origin/HEAD\nin a similar way as the remote has would be OK (although I do not\nsee me using that mode personally).\n\nThanks for a detailed review.\n"},{"id":"410451","messageId":"20201121004033.GD353076@coredump.intra.peff.net","threadId":"54661","inReplyTo":"xmqq3613tuwy.fsf@gitster.c.googlers.com","subject":"Re: [RFC/PATCH] Add fetch.updateHead option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-21T00:40:33Z","receivedAt":"2020-11-21T00:40:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 20, 2020 at 04:28:45PM -0800, Junio C Hamano wrote:\n\n> > This won't resolve a symref pointing to an unborn branch, so it would\n> > count as \"missing\". I.e.:\n> >\n> >   git symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/nope\n> >   git -c fetch.updatehead=missing fetch\n> >\n> > will update it based on the remote HEAD.  I guess I could see some\n> > argument for defining \"missing\" in that way, but I suspect it is not\n> > what somebody in this situation would expect.\n> \n> What do we do in \"git clone\" of an empty repository with the current\n> branch not yet to be born?  Modern Git tells where the HEAD points at\n> even for unborn branch, so using that would be a natural thing to do.\n\nWe don't seem to do anything:\n\n  $ git init\n  $ git clone . dst\n  Cloning into 'dst'...\n  warning: You appear to have cloned an empty repository.\n  done.\n  $ find dst/.git/refs\n  dst/.git/refs\n  dst/.git/refs/tags\n  dst/.git/refs/heads\n\nLikewise with --no-local.\n\nI don't think we do advertise the symref in such a case. In v2, the\nsymref information is attached to individual lines in the ref\nadvertisement. And we don't advertise the unborn line (we could, but I\nthink we might need a special syntax for the oid field).\n\nIn v0, it comes in the capability section attached to the first line of\nthe advertisement, but it doesn't have to be about that particular line.\nIf there are no refs to advertise, we don't seem to send anything (I\n_thought_ we sent a capabilities^{} line, but I think that is only\nreceive-pack; if we have no refs to fetch, then capabilities are not\ninteresting on the upload-pack side anyway).\n\nBut even if we do have a ref in v0, it looks like we don't advertise the\nsymref:\n\n  $ git init\n  $ git commit --allow-empty -m foo\n  $ git checkout --orphan another-branch\n  $ git-upload-pack .\n  0104d4cebe701d3d7b36e6c383193e92ef4bd49ab2b0 refs/heads/mastermulti_ack thin-pack side-band side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative no-progress include-tag multi_ack_detailed object-format=sha1 agent=git/2.29.2.730.g3e418f96ba\n\nWe could likewise support it there, but I don't think modifying the v0\nprotocol at this point is that interesting.\n\n-Peff\n"},{"id":"410452","messageId":"CAMP44s0E+e11jGFDa1zLK5Tv+2Zuv-_wXNF0NysoqEjtMgv0Pw@mail.gmail.com","threadId":"54661","inReplyTo":"20201121004033.GD353076@coredump.intra.peff.net","subject":"Re: [RFC/PATCH] Add fetch.updateHead option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-21T01:18:39Z","receivedAt":"2020-11-21T01:19:16Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Nov 20, 2020 at 6:40 PM Jeff King <peff@peff.net> wrote:\n\n> I don't think we do advertise the symref in such a case. In v2, the\n> symref information is attached to individual lines in the ref\n> advertisement. And we don't advertise the unborn line (we could, but I\n> think we might need a special syntax for the oid field).\n\nThis may be worth considering changing.\n\nIf a hosting provider (e.g. GitHub) decides to configure an initial\nbranch (e.g. main) it would be nice for \"git clone\" to have\ninformation about that initial branch so the user doesn't have to\nchange it manually to please the provider's aesthetics.\n\nSo the instructions could be:\n\n  git clone $url .\n  echo \"# myproject\" >> README.md\n  git add README.md\n  git commit -m \"first commit\"\n  # git branch -M main # this step would not be necessary\n  git push -u origin HEAD\n\nPersonally I don't care about this. But others might find it useful.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"410453","messageId":"CAMP44s1LwYCMa3a7QyUBX2GRAu3XkRYPF1yy0DTQbUh+9eHxcg@mail.gmail.com","threadId":"54661","inReplyTo":"20201120235203.GA353076@coredump.intra.peff.net","subject":"Re: [RFC/PATCH] Add fetch.updateHead option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-21T01:41:48Z","receivedAt":"2020-11-21T01:42:02Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Nov 20, 2020 at 5:52 PM Jeff King <peff@peff.net> wrote:\n> On Wed, Nov 18, 2020 at 03:12:19AM -0600, Felipe Contreras wrote:\n\n> > +static void update_head(int config, const struct ref *head, const struct remote *remote)\n> > +{\n> > +     struct strbuf ref = STRBUF_INIT, target = STRBUF_INIT;\n> > +     const char *r, *head_name = NULL;\n> > +\n> > +     if (!head || !head->symref || !remote)\n> > +             return;\n>\n> I'd expect us to return early here, as well, if the config is set to\n> \"never\". I think your function will usually still do the right thing\n> (because we return early before writing), but it makes a pointless\n> lookup of the current origin/HEAD. But see below.\n\nIf the config is set to \"never\", the function update_head is never\ncalled, since the boolean need_update_head is never set in the outer\nfunction.\n\nI don't like the convolutedness of this approach, but couldn't think\nof anything better.\n\n> > +     strbuf_addf(&ref, \"refs/remotes/%s/HEAD\", remote->name);\n> > +     skip_prefix(head->symref, \"refs/heads/\", &head_name);\n>\n> Should we bail or complain if this skip_prefix() doesn't match? I think\n> it would be a sign of weirdness on the remote side, since HEAD is never\n> supposed to point to anything except refs/heads/. But if we ever did see\n> it, it's probably better to bail than to point to\n> refs/remotes/origin/refs/foo/bar or whatever.\n\nOK. An extra check doesn't hurt.\n\n> > +     strbuf_addf(&target, \"refs/remotes/%s/%s\", remote->name, head_name);\n> > +\n> > +     r = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),\n> > +             ref.buf, RESOLVE_REF_READING,\n> > +             NULL, NULL);\n>\n> This won't resolve a symref pointing to an unborn branch, so it would\n> count as \"missing\". I.e.:\n>\n>   git symbolic-ref refs/remotes/origin/HEAD refs/remotes/origin/nope\n>   git -c fetch.updatehead=missing fetch\n>\n> will update it based on the remote HEAD.  I guess I could see some\n> argument for defining \"missing\" in that way, but I suspect it is not\n> what somebody in this situation would expect.\n>\n> I think it's hard to get into this situation intentionally. If you clone\n> an empty repo, we won't write the symref at all (since we have nothing\n> to write into it), so both sides would be missing. But it's easy enough\n> to do the right thing; see symbolic-ref.c's check_symref() function\n> (basically drop the READING flag).\n>\n> Likewise, I'd not expect to see a non-symref at that name, but what\n> should we do if we see one? I think overwrite it for \"always\", but not\n> for \"missing\". You can tell the difference by checking REF_ISSYMREF in\n> the returned flags, though the distinction may not matter if we follow\n> the semantics I just said.\n\nAll right. So, still checking if REF_ISSYMREF is set, just in case.\n\n> > +     if (r) {\n> > +             if (config == FETCH_UPDATE_HEAD_MISSING)\n> > +                     /* already present */\n> > +                     return;\n> > +             else if (config == FETCH_UPDATE_HEAD_ALWAYS && !strcmp(r, target.buf))\n> > +                     /* already up-to-date */\n> > +                     return;\n> > +             else\n> > +                     /* should never happen */\n> > +                     return;\n> > +     }\n> > +\n> > +     if (create_symref(ref.buf, target.buf, \"remote update head\"))\n> > +             warning(_(\"could not set remote head\"));\n> > +}\n>\n> If we do update the symref, we should probably tell the user. Better\n> still if we can print it as part of the usual fetch output table.\n\nAgreed.\n\n> > @@ -1382,8 +1420,18 @@ static int do_fetch(struct transport *transport,\n> >                               break;\n> >                       }\n> >               }\n> > -     } else if (transport->remote && transport->remote->fetch.nr)\n> > +     } else if (transport->remote && transport->remote->fetch.nr) {\n> > +             if (transport->remote->update_head)\n> > +                     update_head_config = transport->remote->update_head;\n> > +             else\n> > +                     update_head_config = fetch_update_head;\n> > +\n> > +             need_update_head = update_head_config && update_head_config != FETCH_UPDATE_HEAD_NEVER;\n> > +\n> > +             if (need_update_head)\n> > +                     strvec_push(&ref_prefixes, \"HEAD\");\n> >               refspec_ref_prefixes(&transport->remote->fetch, &ref_prefixes);\n> > +     }\n>\n> Good catch. We need this for:\n>\n>   git fetch origin\n>\n> since otherwise it doesn't ask about HEAD in a v2 exchange. What about:\n>\n>   git fetch origin master\n>\n> That won't report on HEAD either, even with your patch, because it hits\n> the part of the conditional before your \"else if\". What should it do?  I\n> can see an argument for \"nothing, we only update head on full configured\n> fetches\", but if so we should definitely make that clear in the\n> documentation. I can also see an argument for \"always, if we happen to\n> have heard about it\" (just like we opportunistically update tracking\n> refs even if they are fetched by name into FETCH_HEAD).\n\nI don't see the point in complicating the code for those corner-cases.\n\nAnd I also don't see how HEAD can be fetched unless we specifically\nask for it. What would that command look like?\n\n\nAlso, there's two optimizations that apparently went unnoticed:\n\n1. In the case of \"missing\". We could preemptively check if there's\nalready an \"origin/HEAD\" before adding \"HEAD\" to the prefixes (and\nsetting need_update_head). That would probably complicate the code.\n\n2. Also in the case of \"missing\". There's no point in building the\n\"target\" string buffer, since we are never going to compare it to\nanything. Again, this would complicate the code.\n\nI decided against these in v1 because as I already stated: it\ncomplicates the code.\n\nPlus, I forgot to release the string buffers (I'm glad nobody\nnoticed). That will be in v2.\n\nCheers.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"410454","messageId":"CAMP44s2yi2v8ghoQ7ZsuxnFU66Hps1DRn3a1ytqBOqS2+pzLsA@mail.gmail.com","threadId":"54661","inReplyTo":"xmqq3613tuwy.fsf@gitster.c.googlers.com","subject":"Re: [RFC/PATCH] Add fetch.updateHead option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2020-11-21T01:53:11Z","receivedAt":"2020-11-21T01:53:26Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Nov 20, 2020 at 6:28 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Jeff King <peff@peff.net> writes:\n>\n> > Thanks for working on this. Regardless of whether we change the default\n> > behavior, this seems like an obvious improvement (and I do think it\n> > makes sense to eventually change the default; I'd even be OK switching\n> > it to \"missing\" in the near term).\n>\n> I agree that \"missing\" would be an easy thing to take, and I do not\n> mind seeing it made the default in the near term.  It won't break\n> existing expectations too much, and can even be seen as a bugfix for\n> the current behaviour by making \"init && fetch\" a step closer to\n> \"clone\".  Beyond that to modify what the end user already has is a\n> much harder sell.  For some it may be an improvement, but for others\n> it would be a breaking change.\n\nChanging the default to \"missing\" breaks a lot of tests. I already\nhave the fixes for the tests, but this can be seen as an indication\nthat the expectations of users would change.\n\nI never knew of this $remote/HEAD feature, and searching forums some\npeople seem perplexed by its existence and ask how to remove it.\n\nSo, if the \"missing\" behavior is the one we are targeting (which I\nargue we should), we probably need a warning before doing the flip, so\nthat users become aware of the feature and are not surprised by a\nsudden $remote/HEAD popping (or repopping) seemingly out of nowhere.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"410638","messageId":"X7yvJkCHa0+mpwPr@coredump.intra.peff.net","threadId":"54661","inReplyTo":"CAMP44s0E+e11jGFDa1zLK5Tv+2Zuv-_wXNF0NysoqEjtMgv0Pw@mail.gmail.com","subject":"Re: [RFC/PATCH] Add fetch.updateHead option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-24T06:58:46Z","receivedAt":"2020-11-24T06:58:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 20, 2020 at 07:18:39PM -0600, Felipe Contreras wrote:\n\n> On Fri, Nov 20, 2020 at 6:40 PM Jeff King <peff@peff.net> wrote:\n> \n> > I don't think we do advertise the symref in such a case. In v2, the\n> > symref information is attached to individual lines in the ref\n> > advertisement. And we don't advertise the unborn line (we could, but I\n> > think we might need a special syntax for the oid field).\n> \n> This may be worth considering changing.\n\nI agree it might be nice, but may not be worth the effort (it will\nrequire a protocol extension).\n\nHowever...\n\n> If a hosting provider (e.g. GitHub) decides to configure an initial\n> branch (e.g. main) it would be nice for \"git clone\" to have\n> information about that initial branch so the user doesn't have to\n> change it manually to please the provider's aesthetics.\n> \n> So the instructions could be:\n> \n>   git clone $url .\n>   echo \"# myproject\" >> README.md\n>   git add README.md\n>   git commit -m \"first commit\"\n>   # git branch -M main # this step would not be necessary\n>   git push -u origin HEAD\n> \n> Personally I don't care about this. But others might find it useful.\n\nI don't think you have to do the rename here if you don't want to.\nIf there is already content on the \"main\" branch at GitHub, then clone\nshould pick that as the default branch for your local clone.\n\nIf there isn't, then you'll get whatever your local git-clone decides\non. But when you push, at least to GitHub, if the server-side HEAD\npoints to an unborn branch, it will re-point to the newly pushed branch.\nSo with the current version of Git, you would end up with \"master\" on\nthe remote side. And that should continue to work as it always has\n(AFAIK any changes on GitHub's side are purely about default branch\nnames, such as when you add template files as part of the repo\ninitialization).\n\nOf course if you _want_ \"main\" in both cases, you'd have to rename the\nbranch. But you would probably set init.defaultBranch in that case.\n\n-Peff\n"},{"id":"410639","messageId":"X7yxsl9/L4SLiuCW@coredump.intra.peff.net","threadId":"54661","inReplyTo":"CAMP44s1LwYCMa3a7QyUBX2GRAu3XkRYPF1yy0DTQbUh+9eHxcg@mail.gmail.com","subject":"Re: [RFC/PATCH] Add fetch.updateHead option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-24T07:09:38Z","receivedAt":"2020-11-24T07:09:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 20, 2020 at 07:41:48PM -0600, Felipe Contreras wrote:\n\n> > > +static void update_head(int config, const struct ref *head, const struct remote *remote)\n> > > +{\n> > > +     struct strbuf ref = STRBUF_INIT, target = STRBUF_INIT;\n> > > +     const char *r, *head_name = NULL;\n> > > +\n> > > +     if (!head || !head->symref || !remote)\n> > > +             return;\n> >\n> > I'd expect us to return early here, as well, if the config is set to\n> > \"never\". I think your function will usually still do the right thing\n> > (because we return early before writing), but it makes a pointless\n> > lookup of the current origin/HEAD. But see below.\n> \n> If the config is set to \"never\", the function update_head is never\n> called, since the boolean need_update_head is never set in the outer\n> function.\n\nTrue.\n\n> I don't like the convolutedness of this approach, but couldn't think\n> of anything better.\n\nIn general, I think keeping as much logic as possible in update_head()\nmakes sense, rather than the caller. We don't need to avoid running\nupdate_head() when the config is \"never\" if it is a true noop in that\ncase.\n\nUnfortunately, we do still need to make a decision on whether to send a\nrequest for \"HEAD\" to the other side in some cases, so some logic has to\nbe there.\n\nBut if we don't send such a request, but we _do_ find out about HEAD\nsomehow (e.g., because the user said \"git fetch origin HEAD\", which\nseems like an obvious thing for someone to try in this case), we\nprobably should trigger the function.\n\n> > since otherwise it doesn't ask about HEAD in a v2 exchange. What about:\n> >\n> >   git fetch origin master\n> >\n> > That won't report on HEAD either, even with your patch, because it hits\n> > the part of the conditional before your \"else if\". What should it do?  I\n> > can see an argument for \"nothing, we only update head on full configured\n> > fetches\", but if so we should definitely make that clear in the\n> > documentation. I can also see an argument for \"always, if we happen to\n> > have heard about it\" (just like we opportunistically update tracking\n> > refs even if they are fetched by name into FETCH_HEAD).\n> \n> I don't see the point in complicating the code for those corner-cases.\n\nI would think:\n\n  git pull origin master\n\nisn't that much of a corner case, and people would expect a HEAD update\nto trigger (especially if that HEAD is master). That will run \"git fetch\norigin master\" under the hood (and update both FETCH_HEAD and\nrefs/remotes/origin/master).\n\n> And I also don't see how HEAD can be fetched unless we specifically\n> ask for it. What would that command look like?\n\nI think it would be nice if any of:\n\n  - git fetch\n\n  - git fetch origin\n\n  - git fetch origin master\n\n  - git fetch origin HEAD\n\ntriggered the auto-update, along with their git-pull equivalents. In the\nfirst three, v2 protocol will require mentioning HEAD as a ref-prefix.\n\n> Also, there's two optimizations that apparently went unnoticed:\n> \n> 1. In the case of \"missing\". We could preemptively check if there's\n> already an \"origin/HEAD\" before adding \"HEAD\" to the prefixes (and\n> setting need_update_head). That would probably complicate the code.\n\nHmm. That might be worth doing, as it does involve extra network traffic\n(and the server-side lookup is more complicated, as it looks up all\npossible variants of HEAD). But it is only a few bytes. I guess it would\ninvolve saving the value of origin/HEAD between the two calls, but that\ndoesn't seem all that hard.\n\nBut I'd also be OK with just unconditionally asking for HEAD (but as\nabove, I think it should happen even with a refspec, and anytime we hear\nabout HEAD we should consider updating the symref).\n\n-Peff\n"}]}