{"thread":{"id":"56885","subject":"[PATCH v6 1/8] fetch: lowercase error messages","startedAt":"2021-11-13T03:34:24Z","lastAt":"2021-11-22T13:21:26Z","messageCount":21,"participants":["Anders Kaseorg","Junio C Hamano","Jiang Xin","Johannes Schindelin"],"isPatch":true,"patchVersion":6,"patchTotal":8},"messages":[{"id":"441042","messageId":"20211113033358.2179376-2-andersk@mit.edu","threadId":"56885","inReplyTo":"20211113033358.2179376-1-andersk@mit.edu","subject":"[PATCH v6 1/8] fetch: lowercase error messages","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-11-13T03:33:51Z","receivedAt":"2021-11-13T03:34:24Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"Documentation/CodingGuidelines says “do not end error messages with a\nfull stop” and “do not capitalize the first word”.  Reviewers requested\nupdating the existing messages to comply with these guidelines prior to\nthe following patches.\n\nSigned-off-by: Anders Kaseorg <andersk@mit.edu>\n---\n builtin/fetch.c | 46 ++++++++++++++++++++++++----------------------\n 1 file changed, 24 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex e064687dbd..e5971fa6e5 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -552,7 +552,7 @@ static struct ref *get_ref_map(struct remote *remote,\n \t\tfor (i = 0; i < fetch_refspec->nr; i++)\n \t\t\tget_fetch_map(ref_map, &fetch_refspec->items[i], &oref_tail, 1);\n \t} else if (refmap.nr) {\n-\t\tdie(\"--refmap option is only meaningful with command-line refspec(s).\");\n+\t\tdie(\"--refmap option is only meaningful with command-line refspec(s)\");\n \t} else {\n \t\t/* Use the defaults */\n \t\tstruct branch *branch = branch_get(NULL);\n@@ -583,7 +583,7 @@ static struct ref *get_ref_map(struct remote *remote,\n \t\t} else if (!prefetch) {\n \t\t\tref_map = get_remote_ref(remote_refs, \"HEAD\");\n \t\t\tif (!ref_map)\n-\t\t\t\tdie(_(\"Couldn't find remote ref HEAD\"));\n+\t\t\t\tdie(_(\"couldn't find remote ref HEAD\"));\n \t\t\tref_map->fetch_head_status = FETCH_HEAD_MERGE;\n \t\t\ttail = &ref_map->next;\n \t\t}\n@@ -1062,13 +1062,13 @@ static void close_fetch_head(struct fetch_head *fetch_head)\n }\n \n static const char warn_show_forced_updates[] =\n-N_(\"Fetch normally indicates which branches had a forced update,\\n\"\n-   \"but that check has been disabled. To re-enable, use '--show-forced-updates'\\n\"\n-   \"flag or run 'git config fetch.showForcedUpdates true'.\");\n+N_(\"fetch normally indicates which branches had a forced update,\\n\"\n+   \"but that check has been disabled; to re-enable, use '--show-forced-updates'\\n\"\n+   \"flag or run 'git config fetch.showForcedUpdates true'\");\n static const char warn_time_show_forced_updates[] =\n-N_(\"It took %.2f seconds to check forced updates. You can use\\n\"\n+N_(\"it took %.2f seconds to check forced updates; you can use\\n\"\n    \"'--no-show-forced-updates' or run 'git config fetch.showForcedUpdates false'\\n\"\n-   \" to avoid this check.\\n\");\n+   \" to avoid this check\\n\");\n \n static int store_updated_refs(const char *raw_url, const char *remote_name,\n \t\t\t      int connectivity_checked, struct ref *ref_map)\n@@ -1381,8 +1381,9 @@ static void check_not_current_branch(struct ref *ref_map)\n \tfor (; ref_map; ref_map = ref_map->next)\n \t\tif (ref_map->peer_ref && !strcmp(current_branch->refname,\n \t\t\t\t\tref_map->peer_ref->name))\n-\t\t\tdie(_(\"Refusing to fetch into current branch %s \"\n-\t\t\t    \"of non-bare repository\"), current_branch->refname);\n+\t\t\tdie(_(\"refusing to fetch into current branch %s \"\n+\t\t\t      \"of non-bare repository\"),\n+\t\t\t    current_branch->refname);\n }\n \n static int truncate_fetch_head(void)\n@@ -1400,10 +1401,10 @@ static void set_option(struct transport *transport, const char *name, const char\n {\n \tint r = transport_set_option(transport, name, value);\n \tif (r < 0)\n-\t\tdie(_(\"Option \\\"%s\\\" value \\\"%s\\\" is not valid for %s\"),\n+\t\tdie(_(\"option \\\"%s\\\" value \\\"%s\\\" is not valid for %s\"),\n \t\t    name, value, transport->url);\n \tif (r > 0)\n-\t\twarning(_(\"Option \\\"%s\\\" is ignored for %s\\n\"),\n+\t\twarning(_(\"option \\\"%s\\\" is ignored for %s\\n\"),\n \t\t\tname, transport->url);\n }\n \n@@ -1437,7 +1438,7 @@ static void add_negotiation_tips(struct git_transport_options *smart_options)\n \t\told_nr = oids->nr;\n \t\tfor_each_glob_ref(add_oid, s, oids);\n \t\tif (old_nr == oids->nr)\n-\t\t\twarning(\"Ignoring --negotiation-tip=%s because it does not match any refs\",\n+\t\t\twarning(\"ignoring --negotiation-tip=%s because it does not match any refs\",\n \t\t\t\ts);\n \t}\n \tsmart_options->negotiation_tips = oids;\n@@ -1475,7 +1476,7 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)\n \t\tif (transport->smart_options)\n \t\t\tadd_negotiation_tips(transport->smart_options);\n \t\telse\n-\t\t\twarning(\"Ignoring --negotiation-tip because the protocol does not support it.\");\n+\t\t\twarning(\"ignoring --negotiation-tip because the protocol does not support it\");\n \t}\n \treturn transport;\n }\n@@ -1638,8 +1639,8 @@ static int do_fetch(struct transport *transport,\n \t\t\telse\n \t\t\t\twarning(_(\"unknown branch type\"));\n \t\t} else {\n-\t\t\twarning(_(\"no source branch found.\\n\"\n-\t\t\t\t\"you need to specify exactly one branch with the --set-upstream option.\"));\n+\t\t\twarning(_(\"no source branch found;\\n\"\n+\t\t\t\t  \"you need to specify exactly one branch with the --set-upstream option\"));\n \t\t}\n \t}\n  skip:\n@@ -1893,8 +1894,8 @@ static int fetch_one(struct remote *remote, int argc, const char **argv,\n \tint remote_via_config = remote_is_configured(remote, 0);\n \n \tif (!remote)\n-\t\tdie(_(\"No remote repository specified.  Please, specify either a URL or a\\n\"\n-\t\t    \"remote name from which new revisions should be fetched.\"));\n+\t\tdie(_(\"no remote repository specified; please specify either a URL or a\\n\"\n+\t\t      \"remote name from which new revisions should be fetched\"));\n \n \tgtransport = prepare_transport(remote, 1);\n \n@@ -1929,7 +1930,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv,\n \t\tif (!strcmp(argv[i], \"tag\")) {\n \t\t\ti++;\n \t\t\tif (i >= argc)\n-\t\t\t\tdie(_(\"You need to specify a tag name.\"));\n+\t\t\t\tdie(_(\"you need to specify a tag name\"));\n \n \t\t\trefspec_appendf(&rs, \"refs/tags/%s:refs/tags/%s\",\n \t\t\t\t\targv[i], argv[i]);\n@@ -1997,7 +1998,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \n \tif (deepen_relative) {\n \t\tif (deepen_relative < 0)\n-\t\t\tdie(_(\"Negative depth in --deepen is not supported\"));\n+\t\t\tdie(_(\"negative depth in --deepen is not supported\"));\n \t\tif (depth)\n \t\t\tdie(_(\"--deepen and --depth are mutually exclusive\"));\n \t\tdepth = xstrfmt(\"%d\", deepen_relative);\n@@ -2034,14 +2035,15 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t\t/* All arguments are assumed to be remotes or groups */\n \t\tfor (i = 0; i < argc; i++)\n \t\t\tif (!add_remote_or_group(argv[i], &list))\n-\t\t\t\tdie(_(\"No such remote or remote group: %s\"), argv[i]);\n+\t\t\t\tdie(_(\"no such remote or remote group: %s\"),\n+\t\t\t\t    argv[i]);\n \t} else {\n \t\t/* Single remote or group */\n \t\t(void) add_remote_or_group(argv[0], &list);\n \t\tif (list.nr > 1) {\n \t\t\t/* More than one remote */\n \t\t\tif (argc > 1)\n-\t\t\t\tdie(_(\"Fetching a group and specifying refspecs does not make sense\"));\n+\t\t\t\tdie(_(\"fetching a group and specifying refspecs does not make sense\"));\n \t\t} else {\n \t\t\t/* Zero or one remotes */\n \t\t\tremote = remote_get(argv[0]);\n@@ -2062,7 +2064,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t\tif (gtransport->smart_options) {\n \t\t\tgtransport->smart_options->acked_commits = &acked_commits;\n \t\t} else {\n-\t\t\twarning(_(\"Protocol does not support --negotiate-only, exiting.\"));\n+\t\t\twarning(_(\"protocol does not support --negotiate-only, exiting\"));\n \t\t\treturn 1;\n \t\t}\n \t\tif (server_options.nr)\n-- \n2.33.1\n\n"},{"id":"441043","messageId":"20211113033358.2179376-3-andersk@mit.edu","threadId":"56885","inReplyTo":"20211113033358.2179376-1-andersk@mit.edu","subject":"[PATCH v6 2/8] receive-pack: lowercase error messages","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-11-13T03:33:52Z","receivedAt":"2021-11-13T03:34:27Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"Documentation/CodingGuidelines says “do not end error messages with a\nfull stop” and “do not capitalize the first word”.  Reviewers requested\nupdating the existing messages to comply with these guidelines prior to\nthe following patches.\n\nSigned-off-by: Anders Kaseorg <andersk@mit.edu>\n---\n builtin/receive-pack.c          | 6 +++---\n t/t5504-fetch-receive-strict.sh | 2 +-\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 2d1f97e1ca..a82b60f387 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -170,7 +170,7 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n \t\t\tstrbuf_addf(&fsck_msg_types, \"%c%s=%s\",\n \t\t\t\tfsck_msg_types.len ? ',' : '=', var, value);\n \t\telse\n-\t\t\twarning(\"Skipping unknown msg id '%s'\", var);\n+\t\t\twarning(\"skipping unknown msg id '%s'\", var);\n \t\treturn 0;\n \t}\n \n@@ -1584,9 +1584,9 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\tif (!parse_object(the_repository, old_oid)) {\n \t\t\told_oid = NULL;\n \t\t\tif (ref_exists(name)) {\n-\t\t\t\trp_warning(\"Allowing deletion of corrupt ref.\");\n+\t\t\t\trp_warning(\"allowing deletion of corrupt ref\");\n \t\t\t} else {\n-\t\t\t\trp_warning(\"Deleting a non-existent ref.\");\n+\t\t\t\trp_warning(\"deleting a non-existent ref\");\n \t\t\t\tcmd->did_not_exist = 1;\n \t\t\t}\n \t\t}\ndiff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh\nindex 6e5a9c20e7..b0b795aca9 100755\n--- a/t/t5504-fetch-receive-strict.sh\n+++ b/t/t5504-fetch-receive-strict.sh\n@@ -292,7 +292,7 @@ test_expect_success 'push with receive.fsck.missingEmail=warn' '\n \t\treceive.fsck.missingEmail warn &&\n \tgit push --porcelain dst bogus >act 2>&1 &&\n \tgrep \"missingEmail\" act &&\n-\ttest_i18ngrep \"Skipping unknown msg id.*whatever\" act &&\n+\ttest_i18ngrep \"skipping unknown msg id.*whatever\" act &&\n \tgit --git-dir=dst/.git branch -D bogus &&\n \tgit --git-dir=dst/.git config --add \\\n \t\treceive.fsck.missingEmail ignore &&\n-- \n2.33.1\n\n"},{"id":"441044","messageId":"20211113033358.2179376-6-andersk@mit.edu","threadId":"56885","inReplyTo":"20211113033358.2179376-1-andersk@mit.edu","subject":"[PATCH v6 5/8] fetch: protect branches checked out in all worktrees","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-11-13T03:33:55Z","receivedAt":"2021-11-13T03:34:28Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"Refuse to fetch into the currently checked out branch of any working\ntree, not just the current one.\n\nFixes this previously reported bug:\n\nhttps://public-inbox.org/git/cb957174-5e9a-5603-ea9e-ac9b58a2eaad@mathema.de\n\nAs a side effect of using find_shared_symref, we’ll also refuse the\nfetch when we’re on a detached HEAD because we’re rebasing or bisecting\non the branch in question. This seems like a sensible change.\n\nSigned-off-by: Anders Kaseorg <andersk@mit.edu>\n---\n builtin/fetch.c       | 75 +++++++++++++++++++++++--------------------\n t/t5516-fetch-push.sh | 18 +++++++++++\n 2 files changed, 58 insertions(+), 35 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex e5971fa6e5..f373252490 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -28,6 +28,7 @@\n #include \"promisor-remote.h\"\n #include \"commit-graph.h\"\n #include \"shallow.h\"\n+#include \"worktree.h\"\n \n #define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000)\n \n@@ -840,14 +841,13 @@ static void format_display(struct strbuf *display, char code,\n \n static int update_local_ref(struct ref *ref,\n \t\t\t    struct ref_transaction *transaction,\n-\t\t\t    const char *remote,\n-\t\t\t    const struct ref *remote_ref,\n-\t\t\t    struct strbuf *display,\n-\t\t\t    int summary_width)\n+\t\t\t    const char *remote, const struct ref *remote_ref,\n+\t\t\t    struct strbuf *display, int summary_width,\n+\t\t\t    struct worktree **worktrees)\n {\n \tstruct commit *current = NULL, *updated;\n \tenum object_type type;\n-\tstruct branch *current_branch = branch_get(NULL);\n+\tconst struct worktree *wt;\n \tconst char *pretty_ref = prettify_refname(ref->name);\n \tint fast_forward = 0;\n \n@@ -862,16 +862,17 @@ static int update_local_ref(struct ref *ref,\n \t\treturn 0;\n \t}\n \n-\tif (current_branch &&\n-\t    !strcmp(ref->name, current_branch->name) &&\n-\t    !(update_head_ok || is_bare_repository()) &&\n-\t    !is_null_oid(&ref->old_oid)) {\n+\tif (!update_head_ok &&\n+\t    (wt = find_shared_symref(worktrees, \"HEAD\", ref->name)) &&\n+\t    !wt->is_bare && !is_null_oid(&ref->old_oid)) {\n \t\t/*\n \t\t * If this is the head, and it's not okay to update\n \t\t * the head, and the old value of the head isn't empty...\n \t\t */\n \t\tformat_display(display, '!', _(\"[rejected]\"),\n-\t\t\t       _(\"can't fetch in current branch\"),\n+\t\t\t       wt->is_current ?\n+\t\t\t\t       _(\"can't fetch in current branch\") :\n+\t\t\t\t       _(\"checked out in another worktree\"),\n \t\t\t       remote, pretty_ref, summary_width);\n \t\treturn 1;\n \t}\n@@ -1071,7 +1072,8 @@ N_(\"it took %.2f seconds to check forced updates; you can use\\n\"\n    \" to avoid this check\\n\");\n \n static int store_updated_refs(const char *raw_url, const char *remote_name,\n-\t\t\t      int connectivity_checked, struct ref *ref_map)\n+\t\t\t      int connectivity_checked, struct ref *ref_map,\n+\t\t\t      struct worktree **worktrees)\n {\n \tstruct fetch_head fetch_head;\n \tstruct commit *commit;\n@@ -1188,7 +1190,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n \t\t\tstrbuf_reset(&note);\n \t\t\tif (ref) {\n \t\t\t\trc |= update_local_ref(ref, transaction, what,\n-\t\t\t\t\t\t       rm, &note, summary_width);\n+\t\t\t\t\t\t       rm, &note, summary_width,\n+\t\t\t\t\t\t       worktrees);\n \t\t\t\tfree(ref);\n \t\t\t} else if (write_fetch_head || dry_run) {\n \t\t\t\t/*\n@@ -1301,16 +1304,15 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)\n }\n \n /* Update local refs based on the ref values fetched from a remote */\n-static int consume_refs(struct transport *transport, struct ref *ref_map)\n+static int consume_refs(struct transport *transport, struct ref *ref_map,\n+\t\t\tstruct worktree **worktrees)\n {\n \tint connectivity_checked = transport->smart_options\n \t\t? transport->smart_options->connectivity_checked : 0;\n \tint ret;\n \ttrace2_region_enter(\"fetch\", \"consume_refs\", the_repository);\n-\tret = store_updated_refs(transport->url,\n-\t\t\t\t transport->remote->name,\n-\t\t\t\t connectivity_checked,\n-\t\t\t\t ref_map);\n+\tret = store_updated_refs(transport->url, transport->remote->name,\n+\t\t\t\t connectivity_checked, ref_map, worktrees);\n \ttransport_unlock_pack(transport);\n \ttrace2_region_leave(\"fetch\", \"consume_refs\", the_repository);\n \treturn ret;\n@@ -1371,19 +1373,18 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map,\n \treturn result;\n }\n \n-static void check_not_current_branch(struct ref *ref_map)\n+static void check_not_current_branch(struct ref *ref_map,\n+\t\t\t\t     struct worktree **worktrees)\n {\n-\tstruct branch *current_branch = branch_get(NULL);\n-\n-\tif (is_bare_repository() || !current_branch)\n-\t\treturn;\n-\n+\tconst struct worktree *wt;\n \tfor (; ref_map; ref_map = ref_map->next)\n-\t\tif (ref_map->peer_ref && !strcmp(current_branch->refname,\n-\t\t\t\t\tref_map->peer_ref->name))\n-\t\t\tdie(_(\"refusing to fetch into current branch %s \"\n-\t\t\t      \"of non-bare repository\"),\n-\t\t\t    current_branch->refname);\n+\t\tif (ref_map->peer_ref &&\n+\t\t    (wt = find_shared_symref(worktrees, \"HEAD\",\n+\t\t\t\t\t     ref_map->peer_ref->name)) &&\n+\t\t    !wt->is_bare)\n+\t\t\tdie(_(\"refusing to fetch into branch '%s' \"\n+\t\t\t      \"checked out at '%s'\"),\n+\t\t\t    ref_map->peer_ref->name, wt->path);\n }\n \n static int truncate_fetch_head(void)\n@@ -1481,7 +1482,8 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)\n \treturn transport;\n }\n \n-static void backfill_tags(struct transport *transport, struct ref *ref_map)\n+static void backfill_tags(struct transport *transport, struct ref *ref_map,\n+\t\t\t  struct worktree **worktrees)\n {\n \tint cannot_reuse;\n \n@@ -1503,7 +1505,7 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)\n \ttransport_set_option(transport, TRANS_OPT_DEPTH, \"0\");\n \ttransport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);\n \tif (!fetch_refs(transport, ref_map))\n-\t\tconsume_refs(transport, ref_map);\n+\t\tconsume_refs(transport, ref_map, worktrees);\n \n \tif (gsecondary) {\n \t\ttransport_disconnect(gsecondary);\n@@ -1521,6 +1523,7 @@ static int do_fetch(struct transport *transport,\n \tstruct transport_ls_refs_options transport_ls_refs_options =\n \t\tTRANSPORT_LS_REFS_OPTIONS_INIT;\n \tint must_list_refs = 1;\n+\tstruct worktree **worktrees = get_worktrees();\n \n \tif (tags == TAGS_DEFAULT) {\n \t\tif (transport->remote->fetch_tags == 2)\n@@ -1576,7 +1579,7 @@ static int do_fetch(struct transport *transport,\n \tref_map = get_ref_map(transport->remote, remote_refs, rs,\n \t\t\t      tags, &autotags);\n \tif (!update_head_ok)\n-\t\tcheck_not_current_branch(ref_map);\n+\t\tcheck_not_current_branch(ref_map, worktrees);\n \n \tif (tags == TAGS_DEFAULT && autotags)\n \t\ttransport_set_option(transport, TRANS_OPT_FOLLOWTAGS, \"1\");\n@@ -1594,7 +1597,8 @@ static int do_fetch(struct transport *transport,\n \t\t\t\t   transport->url);\n \t\t}\n \t}\n-\tif (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) {\n+\tif (fetch_refs(transport, ref_map) ||\n+\t    consume_refs(transport, ref_map, worktrees)) {\n \t\tfree_refs(ref_map);\n \t\tretcode = 1;\n \t\tgoto cleanup;\n@@ -1643,7 +1647,7 @@ static int do_fetch(struct transport *transport,\n \t\t\t\t  \"you need to specify exactly one branch with the --set-upstream option\"));\n \t\t}\n \t}\n- skip:\n+skip:\n \tfree_refs(ref_map);\n \n \t/* if neither --no-tags nor --tags was specified, do automated tag\n@@ -1653,11 +1657,12 @@ static int do_fetch(struct transport *transport,\n \t\tref_map = NULL;\n \t\tfind_non_local_tags(remote_refs, &ref_map, &tail);\n \t\tif (ref_map)\n-\t\t\tbackfill_tags(transport, ref_map);\n+\t\t\tbackfill_tags(transport, ref_map, worktrees);\n \t\tfree_refs(ref_map);\n \t}\n \n- cleanup:\n+cleanup:\n+\tfree_worktrees(worktrees);\n \treturn retcode;\n }\n \ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 4db8edd9c8..36fb90f4b0 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1770,4 +1770,22 @@ test_expect_success 'denyCurrentBranch and worktrees' '\n \tgit -C cloned push origin HEAD:new-wt &&\n \ttest_must_fail git -C cloned push --delete origin new-wt\n '\n+\n+test_expect_success 'refuse fetch to current branch of worktree' '\n+\ttest_when_finished \"git worktree remove --force wt && git branch -D wt\" &&\n+\tgit worktree add wt &&\n+\ttest_commit apple &&\n+\ttest_must_fail git fetch . HEAD:wt &&\n+\tgit fetch -u . HEAD:wt\n+'\n+\n+test_expect_success 'refuse fetch to current branch of bare repository worktree' '\n+\ttest_when_finished \"rm -fr bare.git\" &&\n+\tgit clone --bare . bare.git &&\n+\tgit -C bare.git worktree add wt &&\n+\ttest_commit banana &&\n+\ttest_must_fail git -C bare.git fetch .. HEAD:wt &&\n+\tgit -C bare.git fetch -u .. HEAD:wt\n+'\n+\n test_done\n-- \n2.33.1\n\n"},{"id":"441045","messageId":"20211113033358.2179376-4-andersk@mit.edu","threadId":"56885","inReplyTo":"20211113033358.2179376-1-andersk@mit.edu","subject":"[PATCH v6 3/8] branch: lowercase error messages","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-11-13T03:33:53Z","receivedAt":"2021-11-13T03:34:30Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"Documentation/CodingGuidelines says “do not end error messages with a\nfull stop” and “do not capitalize the first word”.  Reviewers requested\nupdating the existing messages to comply with these guidelines prior to\nthe following patches.\n\nSigned-off-by: Anders Kaseorg <andersk@mit.edu>\n---\n branch.c                   | 16 ++++++++--------\n t/t2018-checkout-branch.sh |  2 +-\n t/t3200-branch.sh          |  2 +-\n 3 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 7a88a4861e..147827cf46 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -153,7 +153,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,\n \t\t}\n \n \tif (tracking.matches > 1)\n-\t\tdie(_(\"Not tracking: ambiguous information for ref %s\"),\n+\t\tdie(_(\"not tracking: ambiguous information for ref %s\"),\n \t\t    orig_ref);\n \n \tif (install_branch_config(config_flags, new_ref, tracking.remote,\n@@ -186,7 +186,7 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)\n int validate_branchname(const char *name, struct strbuf *ref)\n {\n \tif (strbuf_check_branch_ref(ref, name))\n-\t\tdie(_(\"'%s' is not a valid branch name.\"), name);\n+\t\tdie(_(\"'%s' is not a valid branch name\"), name);\n \n \treturn ref_exists(ref->buf);\n }\n@@ -205,12 +205,12 @@ int validate_new_branchname(const char *name, struct strbuf *ref, int force)\n \t\treturn 0;\n \n \tif (!force)\n-\t\tdie(_(\"A branch named '%s' already exists.\"),\n+\t\tdie(_(\"a branch named '%s' already exists\"),\n \t\t    ref->buf + strlen(\"refs/heads/\"));\n \n \thead = resolve_ref_unsafe(\"HEAD\", 0, NULL, NULL);\n \tif (!is_bare_repository() && head && !strcmp(head, ref->buf))\n-\t\tdie(_(\"Cannot force update the current branch.\"));\n+\t\tdie(_(\"cannot force update the current branch\"));\n \n \treturn 1;\n }\n@@ -230,7 +230,7 @@ static int validate_remote_tracking_branch(char *ref)\n }\n \n static const char upstream_not_branch[] =\n-N_(\"Cannot setup tracking information; starting point '%s' is not a branch.\");\n+N_(\"cannot set up tracking information; starting point '%s' is not a branch\");\n static const char upstream_missing[] =\n N_(\"the requested upstream branch '%s' does not exist\");\n static const char upstream_advice[] =\n@@ -278,7 +278,7 @@ void create_branch(struct repository *r,\n \t\t\t}\n \t\t\tdie(_(upstream_missing), start_name);\n \t\t}\n-\t\tdie(_(\"Not a valid object name: '%s'.\"), start_name);\n+\t\tdie(_(\"not a valid object name: '%s'\"), start_name);\n \t}\n \n \tswitch (dwim_ref(start_name, strlen(start_name), &oid, &real_ref, 0)) {\n@@ -298,12 +298,12 @@ void create_branch(struct repository *r,\n \t\t}\n \t\tbreak;\n \tdefault:\n-\t\tdie(_(\"Ambiguous object name: '%s'.\"), start_name);\n+\t\tdie(_(\"ambiguous object name: '%s'\"), start_name);\n \t\tbreak;\n \t}\n \n \tif ((commit = lookup_commit_reference(r, &oid)) == NULL)\n-\t\tdie(_(\"Not a valid branch point: '%s'.\"), start_name);\n+\t\tdie(_(\"not a valid branch point: '%s'\"), start_name);\n \toidcpy(&oid, &commit->object.oid);\n \n \tif (reflog)\ndiff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh\nindex 93be1c0eae..3e93506c04 100755\n--- a/t/t2018-checkout-branch.sh\n+++ b/t/t2018-checkout-branch.sh\n@@ -148,7 +148,7 @@ test_expect_success 'checkout -b to an existing branch fails' '\n test_expect_success 'checkout -b to @{-1} fails with the right branch name' '\n \tgit checkout branch1 &&\n \tgit checkout branch2 &&\n-\techo  >expect \"fatal: A branch named '\\''branch1'\\'' already exists.\" &&\n+\techo  >expect \"fatal: a branch named '\\''branch1'\\'' already exists\" &&\n \ttest_must_fail git checkout -b @{-1} 2>actual &&\n \ttest_cmp expect actual\n '\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex e575ffb4ff..6496e5dd38 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -866,7 +866,7 @@ test_expect_success '--set-upstream-to fails on a missing src branch' '\n '\n \n test_expect_success '--set-upstream-to fails on a non-ref' '\n-\techo \"fatal: Cannot setup tracking information; starting point '\"'\"'HEAD^{}'\"'\"' is not a branch.\" >expect &&\n+\techo \"fatal: cannot set up tracking information; starting point '\"'\"'HEAD^{}'\"'\"' is not a branch\" >expect &&\n \ttest_must_fail git branch --set-upstream-to HEAD^{} 2>err &&\n \ttest_cmp expect err\n '\n-- \n2.33.1\n\n"},{"id":"441046","messageId":"20211113033358.2179376-1-andersk@mit.edu","threadId":"56885","inReplyTo":null,"subject":"[PATCH v6 0/8] protect branches checked out in all worktrees","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-11-13T03:33:50Z","receivedAt":"2021-11-13T03:34:31Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"‘git fetch’ (without ‘--update-head-ok’), ‘git receive-pack’, and ‘git\nbranch -M’ protect the currently checked out branch from being\naccidentally updated.  However, the code for these checks predates\n‘git worktree’.  Improve it to protect branches checked out in all\nworktrees, not just the current one.\n\nAnders Kaseorg (8):\n  fetch: lowercase error messages\n  receive-pack: lowercase error messages\n  branch: lowercase error messages\n  worktree: simplify find_shared_symref() memory ownership model\n  fetch: protect branches checked out in all worktrees\n  receive-pack: clean dead code from update_worktree()\n  receive-pack: protect current branch for bare repository worktree\n  branch: protect branches checked out in all worktrees\n\n branch.c                        |  41 +++++++-----\n builtin/branch.c                |   7 +-\n builtin/fetch.c                 | 115 +++++++++++++++++---------------\n builtin/notes.c                 |   6 +-\n builtin/receive-pack.c          |  88 +++++++++++++-----------\n t/t2018-checkout-branch.sh      |   2 +-\n t/t3200-branch.sh               |   9 ++-\n t/t5504-fetch-receive-strict.sh |   2 +-\n t/t5516-fetch-push.sh           |  32 +++++++++\n worktree.c                      |   8 +--\n worktree.h                      |   5 +-\n 11 files changed, 191 insertions(+), 124 deletions(-)\n\n-- \n2.33.1\n\n"},{"id":"441047","messageId":"20211113033358.2179376-7-andersk@mit.edu","threadId":"56885","inReplyTo":"20211113033358.2179376-1-andersk@mit.edu","subject":"[PATCH v6 6/8] receive-pack: clean dead code from update_worktree()","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-11-13T03:33:56Z","receivedAt":"2021-11-13T03:34:32Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"update_worktree() can only be called with a non-NULL worktree parameter,\nbecause that’s the only case where we set do_update_worktree = 1.\nworktree->path is always initialized to non-NULL.\n\nSigned-off-by: Anders Kaseorg <andersk@mit.edu>\n---\n builtin/receive-pack.c | 23 +++++++----------------\n 1 file changed, 7 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 5d7c4832c1..b04d4ad268 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1444,29 +1444,22 @@ static const char *push_to_checkout(unsigned char *hash,\n \n static const char *update_worktree(unsigned char *sha1, const struct worktree *worktree)\n {\n-\tconst char *retval, *work_tree, *git_dir = NULL;\n+\tconst char *retval, *git_dir;\n \tstruct strvec env = STRVEC_INIT;\n \n-\tif (worktree && worktree->path)\n-\t\twork_tree = worktree->path;\n-\telse if (git_work_tree_cfg)\n-\t\twork_tree = git_work_tree_cfg;\n-\telse\n-\t\twork_tree = \"..\";\n+\tif (!worktree || !worktree->path)\n+\t\tBUG(\"worktree->path must be non-NULL\");\n \n \tif (is_bare_repository())\n \t\treturn \"denyCurrentBranch = updateInstead needs a worktree\";\n-\tif (worktree)\n-\t\tgit_dir = get_worktree_git_dir(worktree);\n-\tif (!git_dir)\n-\t\tgit_dir = get_git_dir();\n+\tgit_dir = get_worktree_git_dir(worktree);\n \n \tstrvec_pushf(&env, \"GIT_DIR=%s\", absolute_path(git_dir));\n \n \tif (!find_hook(push_to_checkout_hook))\n-\t\tretval = push_to_deploy(sha1, &env, work_tree);\n+\t\tretval = push_to_deploy(sha1, &env, worktree->path);\n \telse\n-\t\tretval = push_to_checkout(sha1, &env, work_tree);\n+\t\tretval = push_to_checkout(sha1, &env, worktree->path);\n \n \tstrvec_clear(&env);\n \treturn retval;\n@@ -1587,9 +1580,7 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t}\n \n \tif (do_update_worktree) {\n-\t\tret = update_worktree(new_oid->hash,\n-\t\t\t\t      find_shared_symref(worktrees, \"HEAD\",\n-\t\t\t\t\t\t\t name));\n+\t\tret = update_worktree(new_oid->hash, worktree);\n \t\tif (ret)\n \t\t\tgoto out;\n \t}\n-- \n2.33.1\n\n"},{"id":"441048","messageId":"20211113033358.2179376-8-andersk@mit.edu","threadId":"56885","inReplyTo":"20211113033358.2179376-1-andersk@mit.edu","subject":"[PATCH v6 7/8] receive-pack: protect current branch for bare repository worktree","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-11-13T03:33:57Z","receivedAt":"2021-11-13T03:34:38Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"A bare repository won’t have a working tree at \"..\", but it may still\nhave separate working trees created with git worktree. We should protect\nthe current branch of such working trees from being updated or deleted,\naccording to receive.denyCurrentBranch.\n\nSigned-off-by: Anders Kaseorg <andersk@mit.edu>\n---\n builtin/receive-pack.c |  8 +++-----\n t/t5516-fetch-push.sh  | 14 ++++++++++++++\n 2 files changed, 17 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex b04d4ad268..d72058543e 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1450,7 +1450,7 @@ static const char *update_worktree(unsigned char *sha1, const struct worktree *w\n \tif (!worktree || !worktree->path)\n \t\tBUG(\"worktree->path must be non-NULL\");\n \n-\tif (is_bare_repository())\n+\tif (worktree->is_bare)\n \t\treturn \"denyCurrentBranch = updateInstead needs a worktree\";\n \tgit_dir = get_worktree_git_dir(worktree);\n \n@@ -1476,9 +1476,7 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \tint do_update_worktree = 0;\n \tstruct worktree **worktrees = get_worktrees();\n \tconst struct worktree *worktree =\n-\t\tis_bare_repository() ?\n-\t\t\tNULL :\n-\t\t\tfind_shared_symref(worktrees, \"HEAD\", name);\n+\t\tfind_shared_symref(worktrees, \"HEAD\", name);\n \n \t/* only refs/... are allowed */\n \tif (!starts_with(name, \"refs/\") || check_refname_format(name + 5, 0)) {\n@@ -1491,7 +1489,7 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \tfree(namespaced_name);\n \tnamespaced_name = strbuf_detach(&namespaced_name_buf, NULL);\n \n-\tif (worktree) {\n+\tif (worktree && !worktree->is_bare) {\n \t\tswitch (deny_current_branch) {\n \t\tcase DENY_IGNORE:\n \t\t\tbreak;\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 36fb90f4b0..ecdda807d3 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1768,9 +1768,23 @@ test_expect_success 'denyCurrentBranch and worktrees' '\n \ttest_must_fail git -C cloned push origin HEAD:new-wt &&\n \ttest_config receive.denyCurrentBranch updateInstead &&\n \tgit -C cloned push origin HEAD:new-wt &&\n+\ttest_path_exists new-wt/first.t &&\n \ttest_must_fail git -C cloned push --delete origin new-wt\n '\n \n+test_expect_success 'denyCurrentBranch and bare repository worktrees' '\n+\ttest_when_finished \"rm -fr bare.git\" &&\n+\tgit clone --bare . bare.git &&\n+\tgit -C bare.git worktree add wt &&\n+\ttest_commit grape &&\n+\tgit -C bare.git config receive.denyCurrentBranch refuse &&\n+\ttest_must_fail git push bare.git HEAD:wt &&\n+\tgit -C bare.git config receive.denyCurrentBranch updateInstead &&\n+\tgit push bare.git HEAD:wt &&\n+\ttest_path_exists bare.git/wt/grape.t &&\n+\ttest_must_fail git push --delete bare.git wt\n+'\n+\n test_expect_success 'refuse fetch to current branch of worktree' '\n \ttest_when_finished \"git worktree remove --force wt && git branch -D wt\" &&\n \tgit worktree add wt &&\n-- \n2.33.1\n\n"},{"id":"441049","messageId":"20211113033358.2179376-5-andersk@mit.edu","threadId":"56885","inReplyTo":"20211113033358.2179376-1-andersk@mit.edu","subject":"[PATCH v6 4/8] worktree: simplify find_shared_symref() memory ownership model","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-11-13T03:33:54Z","receivedAt":"2021-11-13T03:34:39Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"Storing the worktrees list in a static variable meant that\nfind_shared_symref() had to rebuild the list on each call (which is\ninefficient when the call site is in a loop), and also that each call\ninvalidated the pointer returned by the previous call (which is\nconfusing).\n\nInstead, make it the caller’s responsibility to pass in the worktrees\nlist and manage its lifetime.\n\nSigned-off-by: Anders Kaseorg <andersk@mit.edu>\n---\n branch.c               | 14 ++++++----\n builtin/branch.c       |  7 ++++-\n builtin/notes.c        |  6 +++-\n builtin/receive-pack.c | 63 +++++++++++++++++++++++++++---------------\n worktree.c             |  8 ++----\n worktree.h             |  5 ++--\n 6 files changed, 65 insertions(+), 38 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 147827cf46..c7b9ba0e10 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -357,14 +357,16 @@ void remove_branch_state(struct repository *r, int verbose)\n \n void die_if_checked_out(const char *branch, int ignore_current_worktree)\n {\n+\tstruct worktree **worktrees = get_worktrees();\n \tconst struct worktree *wt;\n \n-\twt = find_shared_symref(\"HEAD\", branch);\n-\tif (!wt || (ignore_current_worktree && wt->is_current))\n-\t\treturn;\n-\tskip_prefix(branch, \"refs/heads/\", &branch);\n-\tdie(_(\"'%s' is already checked out at '%s'\"),\n-\t    branch, wt->path);\n+\twt = find_shared_symref(worktrees, \"HEAD\", branch);\n+\tif (wt && (!ignore_current_worktree || !wt->is_current)) {\n+\t\tskip_prefix(branch, \"refs/heads/\", &branch);\n+\t\tdie(_(\"'%s' is already checked out at '%s'\"), branch, wt->path);\n+\t}\n+\n+\tfree_worktrees(worktrees);\n }\n \n int replace_each_worktree_head_symref(const char *oldref, const char *newref,\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 03c7b7253a..25785dc939 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -193,6 +193,7 @@ static void delete_branch_config(const char *branchname)\n static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\t\t   int quiet)\n {\n+\tstruct worktree **worktrees;\n \tstruct commit *head_rev = NULL;\n \tstruct object_id oid;\n \tchar *name = NULL;\n@@ -229,6 +230,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\tif (!head_rev)\n \t\t\tdie(_(\"Couldn't look up commit object for HEAD\"));\n \t}\n+\n+\tworktrees = get_worktrees();\n+\n \tfor (i = 0; i < argc; i++, strbuf_reset(&bname)) {\n \t\tchar *target = NULL;\n \t\tint flags = 0;\n@@ -239,7 +243,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \n \t\tif (kinds == FILTER_REFS_BRANCHES) {\n \t\t\tconst struct worktree *wt =\n-\t\t\t\tfind_shared_symref(\"HEAD\", name);\n+\t\t\t\tfind_shared_symref(worktrees, \"HEAD\", name);\n \t\t\tif (wt) {\n \t\t\t\terror(_(\"Cannot delete branch '%s' \"\n \t\t\t\t\t\"checked out at '%s'\"),\n@@ -300,6 +304,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \n \tfree(name);\n \tstrbuf_release(&bname);\n+\tfree_worktrees(worktrees);\n \n \treturn ret;\n }\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 71c59583a1..7f60408dbb 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -861,15 +861,19 @@ static int merge(int argc, const char **argv, const char *prefix)\n \t\tupdate_ref(msg.buf, default_notes_ref(), &result_oid, NULL, 0,\n \t\t\t   UPDATE_REFS_DIE_ON_ERR);\n \telse { /* Merge has unresolved conflicts */\n+\t\tstruct worktree **worktrees;\n \t\tconst struct worktree *wt;\n \t\t/* Update .git/NOTES_MERGE_PARTIAL with partial merge result */\n \t\tupdate_ref(msg.buf, \"NOTES_MERGE_PARTIAL\", &result_oid, NULL,\n \t\t\t   0, UPDATE_REFS_DIE_ON_ERR);\n \t\t/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */\n-\t\twt = find_shared_symref(\"NOTES_MERGE_REF\", default_notes_ref());\n+\t\tworktrees = get_worktrees();\n+\t\twt = find_shared_symref(worktrees, \"NOTES_MERGE_REF\",\n+\t\t\t\t\tdefault_notes_ref());\n \t\tif (wt)\n \t\t\tdie(_(\"a notes merge into %s is already in-progress at %s\"),\n \t\t\t    default_notes_ref(), wt->path);\n+\t\tfree_worktrees(worktrees);\n \t\tif (create_symref(\"NOTES_MERGE_REF\", default_notes_ref(), NULL))\n \t\t\tdie(_(\"failed to store link to current notes ref (%s)\"),\n \t\t\t    default_notes_ref());\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex a82b60f387..5d7c4832c1 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1481,12 +1481,17 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \tstruct object_id *old_oid = &cmd->old_oid;\n \tstruct object_id *new_oid = &cmd->new_oid;\n \tint do_update_worktree = 0;\n-\tconst struct worktree *worktree = is_bare_repository() ? NULL : find_shared_symref(\"HEAD\", name);\n+\tstruct worktree **worktrees = get_worktrees();\n+\tconst struct worktree *worktree =\n+\t\tis_bare_repository() ?\n+\t\t\tNULL :\n+\t\t\tfind_shared_symref(worktrees, \"HEAD\", name);\n \n \t/* only refs/... are allowed */\n \tif (!starts_with(name, \"refs/\") || check_refname_format(name + 5, 0)) {\n \t\trp_error(\"refusing to create funny ref '%s' remotely\", name);\n-\t\treturn \"funny refname\";\n+\t\tret = \"funny refname\";\n+\t\tgoto out;\n \t}\n \n \tstrbuf_addf(&namespaced_name_buf, \"%s%s\", get_git_namespace(), name);\n@@ -1505,7 +1510,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\t\trp_error(\"refusing to update checked out branch: %s\", name);\n \t\t\tif (deny_current_branch == DENY_UNCONFIGURED)\n \t\t\t\trefuse_unconfigured_deny();\n-\t\t\treturn \"branch is currently checked out\";\n+\t\t\tret = \"branch is currently checked out\";\n+\t\t\tgoto out;\n \t\tcase DENY_UPDATE_INSTEAD:\n \t\t\t/* pass -- let other checks intervene first */\n \t\t\tdo_update_worktree = 1;\n@@ -1516,13 +1522,15 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \tif (!is_null_oid(new_oid) && !has_object_file(new_oid)) {\n \t\terror(\"unpack should have generated %s, \"\n \t\t      \"but I can't find it!\", oid_to_hex(new_oid));\n-\t\treturn \"bad pack\";\n+\t\tret = \"bad pack\";\n+\t\tgoto out;\n \t}\n \n \tif (!is_null_oid(old_oid) && is_null_oid(new_oid)) {\n \t\tif (deny_deletes && starts_with(name, \"refs/heads/\")) {\n \t\t\trp_error(\"denying ref deletion for %s\", name);\n-\t\t\treturn \"deletion prohibited\";\n+\t\t\tret = \"deletion prohibited\";\n+\t\t\tgoto out;\n \t\t}\n \n \t\tif (worktree || (head_name && !strcmp(namespaced_name, head_name))) {\n@@ -1538,9 +1546,11 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\t\t\tif (deny_delete_current == DENY_UNCONFIGURED)\n \t\t\t\t\trefuse_unconfigured_deny_delete_current();\n \t\t\t\trp_error(\"refusing to delete the current branch: %s\", name);\n-\t\t\t\treturn \"deletion of the current branch prohibited\";\n+\t\t\t\tret = \"deletion of the current branch prohibited\";\n+\t\t\t\tgoto out;\n \t\t\tdefault:\n-\t\t\t\treturn \"Invalid denyDeleteCurrent setting\";\n+\t\t\t\tret = \"Invalid denyDeleteCurrent setting\";\n+\t\t\t\tgoto out;\n \t\t\t}\n \t\t}\n \t}\n@@ -1558,25 +1568,30 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\t    old_object->type != OBJ_COMMIT ||\n \t\t    new_object->type != OBJ_COMMIT) {\n \t\t\terror(\"bad sha1 objects for %s\", name);\n-\t\t\treturn \"bad ref\";\n+\t\t\tret = \"bad ref\";\n+\t\t\tgoto out;\n \t\t}\n \t\told_commit = (struct commit *)old_object;\n \t\tnew_commit = (struct commit *)new_object;\n \t\tif (!in_merge_bases(old_commit, new_commit)) {\n \t\t\trp_error(\"denying non-fast-forward %s\"\n \t\t\t\t \" (you should pull first)\", name);\n-\t\t\treturn \"non-fast-forward\";\n+\t\t\tret = \"non-fast-forward\";\n+\t\t\tgoto out;\n \t\t}\n \t}\n \tif (run_update_hook(cmd)) {\n \t\trp_error(\"hook declined to update %s\", name);\n-\t\treturn \"hook declined\";\n+\t\tret = \"hook declined\";\n+\t\tgoto out;\n \t}\n \n \tif (do_update_worktree) {\n-\t\tret = update_worktree(new_oid->hash, find_shared_symref(\"HEAD\", name));\n+\t\tret = update_worktree(new_oid->hash,\n+\t\t\t\t      find_shared_symref(worktrees, \"HEAD\",\n+\t\t\t\t\t\t\t name));\n \t\tif (ret)\n-\t\t\treturn ret;\n+\t\t\tgoto out;\n \t}\n \n \tif (is_null_oid(new_oid)) {\n@@ -1595,17 +1610,19 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\t\t\t\t   old_oid,\n \t\t\t\t\t   0, \"push\", &err)) {\n \t\t\trp_error(\"%s\", err.buf);\n-\t\t\tstrbuf_release(&err);\n-\t\t\treturn \"failed to delete\";\n+\t\t\tret = \"failed to delete\";\n+\t\t} else {\n+\t\t\tret = NULL; /* good */\n \t\t}\n \t\tstrbuf_release(&err);\n-\t\treturn NULL; /* good */\n \t}\n \telse {\n \t\tstruct strbuf err = STRBUF_INIT;\n \t\tif (shallow_update && si->shallow_ref[cmd->index] &&\n-\t\t    update_shallow_ref(cmd, si))\n-\t\t\treturn \"shallow error\";\n+\t\t    update_shallow_ref(cmd, si)) {\n+\t\t\tret = \"shallow error\";\n+\t\t\tgoto out;\n+\t\t}\n \n \t\tif (ref_transaction_update(transaction,\n \t\t\t\t\t   namespaced_name,\n@@ -1613,14 +1630,16 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\t\t\t\t   0, \"push\",\n \t\t\t\t\t   &err)) {\n \t\t\trp_error(\"%s\", err.buf);\n-\t\t\tstrbuf_release(&err);\n-\n-\t\t\treturn \"failed to update ref\";\n+\t\t\tret = \"failed to update ref\";\n+\t\t} else {\n+\t\t\tret = NULL; /* good */\n \t\t}\n \t\tstrbuf_release(&err);\n-\n-\t\treturn NULL; /* good */\n \t}\n+\n+out:\n+\tfree_worktrees(worktrees);\n+\treturn ret;\n }\n \n static void run_update_post_hook(struct command *commands)\ndiff --git a/worktree.c b/worktree.c\nindex 092a4f92ad..cf13d63845 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -402,17 +402,13 @@ int is_worktree_being_bisected(const struct worktree *wt,\n  * bisect). New commands that do similar things should update this\n  * function as well.\n  */\n-const struct worktree *find_shared_symref(const char *symref,\n+const struct worktree *find_shared_symref(struct worktree **worktrees,\n+\t\t\t\t\t  const char *symref,\n \t\t\t\t\t  const char *target)\n {\n \tconst struct worktree *existing = NULL;\n-\tstatic struct worktree **worktrees;\n \tint i = 0;\n \n-\tif (worktrees)\n-\t\tfree_worktrees(worktrees);\n-\tworktrees = get_worktrees();\n-\n \tfor (i = 0; worktrees[i]; i++) {\n \t\tstruct worktree *wt = worktrees[i];\n \t\tconst char *symref_target;\ndiff --git a/worktree.h b/worktree.h\nindex 8b7c408132..9e06fcbdf3 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -143,9 +143,10 @@ void free_worktrees(struct worktree **);\n /*\n  * Check if a per-worktree symref points to a ref in the main worktree\n  * or any linked worktree, and return the worktree that holds the ref,\n- * or NULL otherwise. The result may be destroyed by the next call.\n+ * or NULL otherwise.\n  */\n-const struct worktree *find_shared_symref(const char *symref,\n+const struct worktree *find_shared_symref(struct worktree **worktrees,\n+\t\t\t\t\t  const char *symref,\n \t\t\t\t\t  const char *target);\n \n /*\n-- \n2.33.1\n\n"},{"id":"441050","messageId":"20211113033358.2179376-9-andersk@mit.edu","threadId":"56885","inReplyTo":"20211113033358.2179376-1-andersk@mit.edu","subject":"[PATCH v6 8/8] branch: protect branches checked out in all worktrees","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-11-13T03:33:58Z","receivedAt":"2021-11-13T03:34:40Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"Refuse to force-move a branch over the currently checked out branch of\nany working tree, not just the current one.\n\nSigned-off-by: Anders Kaseorg <andersk@mit.edu>\n---\n branch.c          | 13 +++++++++----\n t/t3200-branch.sh |  7 +++++++\n 2 files changed, 16 insertions(+), 4 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex c7b9ba0e10..3a7d205fa4 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -199,7 +199,8 @@ int validate_branchname(const char *name, struct strbuf *ref)\n  */\n int validate_new_branchname(const char *name, struct strbuf *ref, int force)\n {\n-\tconst char *head;\n+\tstruct worktree **worktrees;\n+\tconst struct worktree *wt;\n \n \tif (!validate_branchname(name, ref))\n \t\treturn 0;\n@@ -208,9 +209,13 @@ int validate_new_branchname(const char *name, struct strbuf *ref, int force)\n \t\tdie(_(\"a branch named '%s' already exists\"),\n \t\t    ref->buf + strlen(\"refs/heads/\"));\n \n-\thead = resolve_ref_unsafe(\"HEAD\", 0, NULL, NULL);\n-\tif (!is_bare_repository() && head && !strcmp(head, ref->buf))\n-\t\tdie(_(\"cannot force update the current branch\"));\n+\tworktrees = get_worktrees();\n+\twt = find_shared_symref(worktrees, \"HEAD\", ref->buf);\n+\tif (wt && !wt->is_bare)\n+\t\tdie(_(\"cannot force update the branch '%s'\"\n+\t\t      \"checked out at '%s'\"),\n+\t\t    ref->buf + strlen(\"refs/heads/\"), wt->path);\n+\tfree_worktrees(worktrees);\n \n \treturn 1;\n }\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 6496e5dd38..797a2c752d 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -168,6 +168,13 @@ test_expect_success 'git branch -M foo bar should fail when bar is checked out'\n \ttest_must_fail git branch -M bar foo\n '\n \n+test_expect_success 'git branch -M foo bar should fail when bar is checked out in worktree' '\n+\tgit branch -f bar &&\n+\ttest_when_finished \"git worktree remove wt && git branch -D wt\" &&\n+\tgit worktree add wt &&\n+\ttest_must_fail git branch -M bar wt\n+'\n+\n test_expect_success 'git branch -M baz bam should succeed when baz is checked out' '\n \tgit checkout -b baz &&\n \tgit branch bam &&\n-- \n2.33.1\n\n"},{"id":"441246","messageId":"xmqqczn0d6er.fsf@gitster.g","threadId":"56885","inReplyTo":"20211113033358.2179376-2-andersk@mit.edu","subject":"Re: [PATCH v6 1/8] fetch: lowercase error messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-16T05:19:08Z","receivedAt":"2021-11-16T05:19:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Anders Kaseorg <andersk@mit.edu> writes:\n\n> Documentation/CodingGuidelines says “do not end error messages with a\n> full stop” and “do not capitalize the first word”.  Reviewers requested\n> updating the existing messages to comply with these guidelines prior to\n> the following patches.\n\nThanks.  Whether reviewers requested or you thought of it on your\nown, separating such a preliminary clean-up into its own patch would\nbe a good idea, especially if the later patches need to update (some\nof) them.\n\n\n> @@ -1062,13 +1062,13 @@ static void close_fetch_head(struct fetch_head *fetch_head)\n>  }\n>  \n>  static const char warn_show_forced_updates[] =\n> -N_(\"Fetch normally indicates which branches had a forced update,\\n\"\n> -   \"but that check has been disabled. To re-enable, use '--show-forced-updates'\\n\"\n> -   \"flag or run 'git config fetch.showForcedUpdates true'.\");\n> +N_(\"fetch normally indicates which branches had a forced update,\\n\"\n> +   \"but that check has been disabled; to re-enable, use '--show-forced-updates'\\n\"\n> +   \"flag or run 'git config fetch.showForcedUpdates true'\");\n>  static const char warn_time_show_forced_updates[] =\n> -N_(\"It took %.2f seconds to check forced updates. You can use\\n\"\n> +N_(\"it took %.2f seconds to check forced updates; you can use\\n\"\n>     \"'--no-show-forced-updates' or run 'git config fetch.showForcedUpdates false'\\n\"\n> -   \" to avoid this check.\\n\");\n> +   \" to avoid this check\\n\");\n\nThe two guidelines cited in the proposed log message are primarily\nto prefer\n\n    fatal: unrecognized argument: --no-such-option\n\nover\n\n    fatal: Unrecognized argument: --no-such-option.\n\nand does not say much what to do to a multi-sentence message.  In\nthis part (and other parts) of the patch, I can see that you thought\nthis one through when preparing this patch.  I very much appreciate\nit.\n\nThe approach chosen (consistently) in this patch is to\n\n (1) turn them into a (semi) single sentence, concatenated with ';'\n\n (2) as a side effect of not being a free-standing sentence anymore,\n     the second and subsequent sentences in the original, that are\n     now just pieces in a single sentence separated with ';', do not\n     get capitalized, and\n\n (3) the sentence as a whole lacks the full-stop, just like a single\n     sentence message.\n\nI think we are fine with these rules, especially given that these\nmulti-sentence messages are not the main part of this topic touches\nand are not the primary focus of this topic anyway.  \n\nI am highlighting this part of the change, just in case others think\nof a better set of rules to follow.  Existing multi-sentence messages\nfollow different ad-hoc patterns, it seems (e.g. \"git show 00000000\").\n\nThanks.\n\n"},{"id":"441247","messageId":"xmqq7dd8d5hf.fsf@gitster.g","threadId":"56885","inReplyTo":"20211113033358.2179376-5-andersk@mit.edu","subject":"Re: [PATCH v6 4/8] worktree: simplify find_shared_symref() memory ownership model","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-16T05:39:08Z","receivedAt":"2021-11-16T05:40:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Anders Kaseorg <andersk@mit.edu> writes:\n\n> Storing the worktrees list in a static variable meant that\n> find_shared_symref() had to rebuild the list on each call (which is\n> inefficient when the call site is in a loop), and also that each call\n> invalidated the pointer returned by the previous call (which is\n> confusing).\n>\n> Instead, make it the caller’s responsibility to pass in the worktrees\n> list and manage its lifetime.\n\nVery nice.\n\n> +\tstruct worktree **worktrees = get_worktrees();\n> +\tconst struct worktree *worktree =\n> +\t\tis_bare_repository() ?\n> +\t\t\tNULL :\n> +\t\t\tfind_shared_symref(worktrees, \"HEAD\", name);\n>  \n>  \t/* only refs/... are allowed */\n>  \tif (!starts_with(name, \"refs/\") || check_refname_format(name + 5, 0)) {\n>  \t\trp_error(\"refusing to create funny ref '%s' remotely\", name);\n> -\t\treturn \"funny refname\";\n> +\t\tret = \"funny refname\";\n> +\t\tgoto out;\n>  \t}\n\nNice touch to make the code clean after itself in a single place.\nGood.\n\n> ...\n> +out:\n> +\tfree_worktrees(worktrees);\n> +\treturn ret;\n>  }\n"},{"id":"441249","messageId":"xmqq1r3gd50r.fsf@gitster.g","threadId":"56885","inReplyTo":"20211113033358.2179376-6-andersk@mit.edu","subject":"Re: [PATCH v6 5/8] fetch: protect branches checked out in all worktrees","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-16T05:49:08Z","receivedAt":"2021-11-16T05:49:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Anders Kaseorg <andersk@mit.edu> writes:\n\n> Refuse to fetch into the currently checked out branch of any working\n> tree, not just the current one.\n>\n> Fixes this previously reported bug:\n>\n> https://public-inbox.org/git/cb957174-5e9a-5603-ea9e-ac9b58a2eaad@mathema.de\n>\n> As a side effect of using find_shared_symref, we’ll also refuse the\n> fetch when we’re on a detached HEAD because we’re rebasing or bisecting\n> on the branch in question. This seems like a sensible change.\n\nIndeed.\n\n> Signed-off-by: Anders Kaseorg <andersk@mit.edu>\n> ---\n>  builtin/fetch.c       | 75 +++++++++++++++++++++++--------------------\n>  t/t5516-fetch-push.sh | 18 +++++++++++\n>  2 files changed, 58 insertions(+), 35 deletions(-)\n>\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index e5971fa6e5..f373252490 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -28,6 +28,7 @@\n>  #include \"promisor-remote.h\"\n>  #include \"commit-graph.h\"\n>  #include \"shallow.h\"\n> +#include \"worktree.h\"\n>  \n>  #define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000)\n>  \n> @@ -840,14 +841,13 @@ static void format_display(struct strbuf *display, char code,\n>  \n>  static int update_local_ref(struct ref *ref,\n>  \t\t\t    struct ref_transaction *transaction,\n> -\t\t\t    const char *remote,\n> -\t\t\t    const struct ref *remote_ref,\n> -\t\t\t    struct strbuf *display,\n> -\t\t\t    int summary_width)\n> +\t\t\t    const char *remote, const struct ref *remote_ref,\n> +\t\t\t    struct strbuf *display, int summary_width,\n> +\t\t\t    struct worktree **worktrees)\n>  {\n>  \tstruct commit *current = NULL, *updated;\n>  \tenum object_type type;\n> -\tstruct branch *current_branch = branch_get(NULL);\n> +\tconst struct worktree *wt;\n>  \tconst char *pretty_ref = prettify_refname(ref->name);\n>  \tint fast_forward = 0;\n\nHaving to pass the parameter down to here through the\n\n    ->do_fetch()\n      ->backfill_tags() (or do_fetch() itself)\n        ->consume_refs()\n          ->store_updated_refs()\n            ->update_local_ref()\n\ncallchain makes the \"damage to the code\" by the patch look larger\nthan it actually is.  The real change is ...\n\n> @@ -862,16 +862,17 @@ static int update_local_ref(struct ref *ref,\n>  \t\treturn 0;\n>  \t}\n>  \n> -\tif (current_branch &&\n> -\t    !strcmp(ref->name, current_branch->name) &&\n> -\t    !(update_head_ok || is_bare_repository()) &&\n> -\t    !is_null_oid(&ref->old_oid)) {\n> +\tif (!update_head_ok &&\n> +\t    (wt = find_shared_symref(worktrees, \"HEAD\", ref->name)) &&\n> +\t    !wt->is_bare && !is_null_oid(&ref->old_oid)) {\n\n... this part, which looks very sensible.\n\n>  \t\t * If this is the head, and it's not okay to update\n>  \t\t * the head, and the old value of the head isn't empty...\n>  \t\t */\n>  \t\tformat_display(display, '!', _(\"[rejected]\"),\n> -\t\t\t       _(\"can't fetch in current branch\"),\n> +\t\t\t       wt->is_current ?\n> +\t\t\t\t       _(\"can't fetch in current branch\") :\n> +\t\t\t\t       _(\"checked out in another worktree\"),\n>  \t\t\t       remote, pretty_ref, summary_width);\n>  \t\treturn 1;\n>  \t}\n\n> @@ -1643,7 +1647,7 @@ static int do_fetch(struct transport *transport,\n>  \t\t\t\t  \"you need to specify exactly one branch with the --set-upstream option\"));\n>  \t\t}\n>  \t}\n> - skip:\n> +skip:\n>  \tfree_refs(ref_map);\n\n;-)\n\nI count 30 hits of \"^ [a-z0-9]*:\" and 255 hits of \"^[a-z0-9]*:\" in\nour codebase.  It must be some developers used to subscribe to\n\"don't place the label abut the left edge\" school but no longer, or\nsomething like that.\n\nThe code changes all look good to me.\n\nThanks.\n\n> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\n> index 4db8edd9c8..36fb90f4b0 100755\n> --- a/t/t5516-fetch-push.sh\n> +++ b/t/t5516-fetch-push.sh\n> @@ -1770,4 +1770,22 @@ test_expect_success 'denyCurrentBranch and worktrees' '\n>  \tgit -C cloned push origin HEAD:new-wt &&\n>  \ttest_must_fail git -C cloned push --delete origin new-wt\n>  '\n> +\n> +test_expect_success 'refuse fetch to current branch of worktree' '\n> +\ttest_when_finished \"git worktree remove --force wt && git branch -D wt\" &&\n> +\tgit worktree add wt &&\n> +\ttest_commit apple &&\n> +\ttest_must_fail git fetch . HEAD:wt &&\n> +\tgit fetch -u . HEAD:wt\n> +'\n> +\n> +test_expect_success 'refuse fetch to current branch of bare repository worktree' '\n> +\ttest_when_finished \"rm -fr bare.git\" &&\n> +\tgit clone --bare . bare.git &&\n> +\tgit -C bare.git worktree add wt &&\n> +\ttest_commit banana &&\n> +\ttest_must_fail git -C bare.git fetch .. HEAD:wt &&\n> +\tgit -C bare.git fetch -u .. HEAD:wt\n> +'\n> +\n>  test_done\n"},{"id":"441250","messageId":"xmqqv90sbqg9.fsf@gitster.g","threadId":"56885","inReplyTo":"20211113033358.2179376-7-andersk@mit.edu","subject":"Re: [PATCH v6 6/8] receive-pack: clean dead code from update_worktree()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-16T05:49:10Z","receivedAt":"2021-11-16T05:49:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Anders Kaseorg <andersk@mit.edu> writes:\n\n> update_worktree() can only be called with a non-NULL worktree parameter,\n> because that’s the only case where we set do_update_worktree = 1.\n> worktree->path is always initialized to non-NULL.\n\nYup.  I remember seeing this analysed in earlier rounds' reviews.\nNice lossage of lines ;-)\n\n>\n> Signed-off-by: Anders Kaseorg <andersk@mit.edu>\n> ---\n>  builtin/receive-pack.c | 23 +++++++----------------\n>  1 file changed, 7 insertions(+), 16 deletions(-)\n>\n> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\n> index 5d7c4832c1..b04d4ad268 100644\n> --- a/builtin/receive-pack.c\n> +++ b/builtin/receive-pack.c\n> @@ -1444,29 +1444,22 @@ static const char *push_to_checkout(unsigned char *hash,\n>  \n>  static const char *update_worktree(unsigned char *sha1, const struct worktree *worktree)\n>  {\n> -\tconst char *retval, *work_tree, *git_dir = NULL;\n> +\tconst char *retval, *git_dir;\n>  \tstruct strvec env = STRVEC_INIT;\n>  \n> -\tif (worktree && worktree->path)\n> -\t\twork_tree = worktree->path;\n> -\telse if (git_work_tree_cfg)\n> -\t\twork_tree = git_work_tree_cfg;\n> -\telse\n> -\t\twork_tree = \"..\";\n> +\tif (!worktree || !worktree->path)\n> +\t\tBUG(\"worktree->path must be non-NULL\");\n>  \n>  \tif (is_bare_repository())\n>  \t\treturn \"denyCurrentBranch = updateInstead needs a worktree\";\n> -\tif (worktree)\n> -\t\tgit_dir = get_worktree_git_dir(worktree);\n> -\tif (!git_dir)\n> -\t\tgit_dir = get_git_dir();\n> +\tgit_dir = get_worktree_git_dir(worktree);\n>  \n>  \tstrvec_pushf(&env, \"GIT_DIR=%s\", absolute_path(git_dir));\n>  \n>  \tif (!find_hook(push_to_checkout_hook))\n> -\t\tretval = push_to_deploy(sha1, &env, work_tree);\n> +\t\tretval = push_to_deploy(sha1, &env, worktree->path);\n>  \telse\n> -\t\tretval = push_to_checkout(sha1, &env, work_tree);\n> +\t\tretval = push_to_checkout(sha1, &env, worktree->path);\n>  \n>  \tstrvec_clear(&env);\n>  \treturn retval;\n> @@ -1587,9 +1580,7 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n>  \t}\n>  \n>  \tif (do_update_worktree) {\n> -\t\tret = update_worktree(new_oid->hash,\n> -\t\t\t\t      find_shared_symref(worktrees, \"HEAD\",\n> -\t\t\t\t\t\t\t name));\n> +\t\tret = update_worktree(new_oid->hash, worktree);\n>  \t\tif (ret)\n>  \t\t\tgoto out;\n>  \t}\n"},{"id":"441258","messageId":"a2897d9d-284a-4f38-5f07-12e81258fae1@mit.edu","threadId":"56885","inReplyTo":"xmqq1r3gd50r.fsf@gitster.g","subject":"Re: [PATCH v6 5/8] fetch: protect branches checked out in all worktrees","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-11-16T06:44:43Z","receivedAt":"2021-11-16T06:45:38Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"On 11/15/21 21:49, Junio C Hamano wrote:\n>> - skip:\n>> +skip:\n> \n> ;-)\n\n(This was to satisfy ‘make style’.  Otherwise it wanted the lines \nfollowing the 1-space indent to have a 9-space indent.)\n\nAnders\n"},{"id":"441260","messageId":"alpine.DEB.2.21.999.2111160206440.105644@scrubbing-bubbles.mit.edu","threadId":"56885","inReplyTo":"xmqqczn0d6er.fsf@gitster.g","subject":"Re: [PATCH v6 1/8] fetch: lowercase error messages","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-11-16T07:10:57Z","receivedAt":"2021-11-16T07:13:14Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"On Tue, 16 Nov 2021, Junio C Hamano wrote:\n> > Documentation/CodingGuidelines says “do not end error messages with a\n> > full stop” and “do not capitalize the first word”.  Reviewers requested\n> > updating the existing messages to comply with these guidelines prior to\n> > the following patches.\n> \n> Thanks.  Whether reviewers requested or you thought of it on your\n> own, separating such a preliminary clean-up into its own patch would\n> be a good idea, especially if the later patches need to update (some\n> of) them.\n\nIt was your request; I just mentioned it in case other reviewers wonder \nwhy this belongs in this topic.\n\nhttps://public-inbox.org/git/xmqq8rxvwp4b.fsf@gitster.g/\n\n> The approach chosen (consistently) in this patch is to\n> \n>  (1) turn them into a (semi) single sentence, concatenated with ';'\n> \n>  (2) as a side effect of not being a free-standing sentence anymore,\n>      the second and subsequent sentences in the original, that are\n>      now just pieces in a single sentence separated with ';', do not\n>      get capitalized, and\n> \n>  (3) the sentence as a whole lacks the full-stop, just like a single\n>      sentence message.\n> \n> I think we are fine with these rules, especially given that these\n> multi-sentence messages are not the main part of this topic touches\n> and are not the primary focus of this topic anyway.  \n\nI guess I should add that there are a few messages I left alone: \nrefuse_unconfigured_deny_msg and \nrefuse_unconfigured_deny_delete_current_msg in builtin/receive-pack.c \n(patch 2/8).  Not sure if they count as “error messages”, but these \nmulti-paragraph messages are too long to comfortably apply this approach.\n\nNow I see that I missed a few others.  This could be squashed into the \nfirst three patches:\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex f373252490..f3c7c057d0 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1783,7 +1783,7 @@ static int fetch_failed_to_start(struct strbuf *out, void *cb, void *task_cb)\n \tstruct parallel_fetch_state *state = cb;\n \tconst char *remote = task_cb;\n \n-\tstate->result = error(_(\"Could not fetch %s\"), remote);\n+\tstate->result = error(_(\"could not fetch %s\"), remote);\n \n \treturn 0;\n }\n@@ -1838,7 +1838,7 @@ static int fetch_multiple(struct string_list *list, int max_children)\n \t\t\tif (verbosity >= 0)\n \t\t\t\tprintf(_(\"Fetching %s\\n\"), name);\n \t\t\tif (run_command_v_opt(argv.v, RUN_GIT_CMD)) {\n-\t\t\t\terror(_(\"Could not fetch %s\"), name);\n+\t\t\t\terror(_(\"could not fetch %s\"), name);\n \t\t\t\tresult = 1;\n \t\t\t}\n \t\t\tstrvec_pop(&argv);\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex d72058543e..70b4e23a26 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -2495,9 +2495,9 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, options, receive_pack_usage, 0);\n \n \tif (argc > 1)\n-\t\tusage_msg_opt(_(\"Too many arguments.\"), receive_pack_usage, options);\n+\t\tusage_msg_opt(_(\"too many arguments\"), receive_pack_usage, options);\n \tif (argc == 0)\n-\t\tusage_msg_opt(_(\"You must specify a directory.\"), receive_pack_usage, options);\n+\t\tusage_msg_opt(_(\"you must specify a directory\"), receive_pack_usage, options);\n \n \tservice_dir = argv[0];\n \ndiff --git a/branch.c b/branch.c\nindex 3a7d205fa4..0bea1335ae 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -64,7 +64,7 @@ int install_branch_config(int flag, const char *local, const char *origin, const\n \tif (skip_prefix(remote, \"refs/heads/\", &shortname)\n \t    && !strcmp(local, shortname)\n \t    && !origin) {\n-\t\twarning(_(\"Not setting branch %s as its own upstream.\"),\n+\t\twarning(_(\"not setting branch %s as its own upstream\"),\n \t\t\tlocal);\n \t\treturn 0;\n \t}\n@@ -116,7 +116,7 @@ int install_branch_config(int flag, const char *local, const char *origin, const\n \n out_err:\n \tstrbuf_release(&key);\n-\terror(_(\"Unable to write upstream branch configuration\"));\n+\terror(_(\"unable to write upstream branch configuration\"));\n \n \tadvise(_(tracking_advice),\n \t       origin ? origin : \"\",\n\nAnders"},{"id":"441411","messageId":"xmqqv90r4317.fsf@gitster.g","threadId":"56885","inReplyTo":"alpine.DEB.2.21.999.2111160206440.105644@scrubbing-bubbles.mit.edu","subject":"Re: [PATCH v6 1/8] fetch: lowercase error messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-17T08:09:08Z","receivedAt":"2021-11-17T08:09:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Anders Kaseorg <andersk@mit.edu> writes:\n\n> On Tue, 16 Nov 2021, Junio C Hamano wrote:\n>> > Documentation/CodingGuidelines says “do not end error messages with a\n>> > full stop” and “do not capitalize the first word”.  Reviewers requested\n>> > updating the existing messages to comply with these guidelines prior to\n>> > the following patches.\n>> \n>> Thanks.  Whether reviewers requested or you thought of it on your\n>> own, separating such a preliminary clean-up into its own patch would\n>> be a good idea, especially if the later patches need to update (some\n>> of) them.\n>\n> It was your request; I just mentioned it in case other reviewers wonder \n> why this belongs in this topic.\n\nSorry, let me try again, as I wasn't clear enough.\n\nReaders of \"git log\", for whom we write our log messages, are not\ninterested if reviewers suggested, or you came up on your own.  The\nmore relevant thing for them to learn from our log messages is the\nreason why that solution was chosen (and the fact that the author is\nnow committed to the chosen solution---not \"this does not make much\nsense but I am randomly updating as I was told\"). E.g.\n\n    ... first word\".  This file has many existing messages that\n    violate these guidelines.  Clean them up in preparation for\n    subsequent patches that touch some of these messages.\n\nor something like that.\n"},{"id":"441563","messageId":"xmqqwnl612ul.fsf@gitster.g","threadId":"56885","inReplyTo":"20211113033358.2179376-3-andersk@mit.edu","subject":"Re: [PATCH v6 2/8] receive-pack: lowercase error messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-18T04:53:38Z","receivedAt":"2021-11-18T04:53:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Anders Kaseorg <andersk@mit.edu> writes:\n\n> Documentation/CodingGuidelines says “do not end error messages with a\n> full stop” and “do not capitalize the first word”.  Reviewers requested\n> updating the existing messages to comply with these guidelines prior to\n> the following patches.\n\nThe same comment applies to this part.  Reviewers may make\nsuggestion to help polish the topic, but it ultimately is the\nauthor's achievement.  More importantly, we write our log messages\nto help future developers to learn from.  The fact somebody asked\nsome changes made is much less important than the reason why such\nchanges are desirable, so that they can craft their future topics\nfollowing the same pattern.\n\n    Documentation... says X and Y.  Clean up existing messages, some\n    of which we will be touching in later steps in the series, that\n    deviate from these rules in this file, as a preparation for the\n    main part of the topic.\n\nmay convey the intention better, I would think.\n\nThe patch text looks good.  Thanks.\n\n> Signed-off-by: Anders Kaseorg <andersk@mit.edu>\n> ---\n>  builtin/receive-pack.c          | 6 +++---\n>  t/t5504-fetch-receive-strict.sh | 2 +-\n>  2 files changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\n> index 2d1f97e1ca..a82b60f387 100644\n> --- a/builtin/receive-pack.c\n> +++ b/builtin/receive-pack.c\n> @@ -170,7 +170,7 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n>  \t\t\tstrbuf_addf(&fsck_msg_types, \"%c%s=%s\",\n>  \t\t\t\tfsck_msg_types.len ? ',' : '=', var, value);\n>  \t\telse\n> -\t\t\twarning(\"Skipping unknown msg id '%s'\", var);\n> +\t\t\twarning(\"skipping unknown msg id '%s'\", var);\n>  \t\treturn 0;\n>  \t}\n>  \n> @@ -1584,9 +1584,9 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n>  \t\tif (!parse_object(the_repository, old_oid)) {\n>  \t\t\told_oid = NULL;\n>  \t\t\tif (ref_exists(name)) {\n> -\t\t\t\trp_warning(\"Allowing deletion of corrupt ref.\");\n> +\t\t\t\trp_warning(\"allowing deletion of corrupt ref\");\n>  \t\t\t} else {\n> -\t\t\t\trp_warning(\"Deleting a non-existent ref.\");\n> +\t\t\t\trp_warning(\"deleting a non-existent ref\");\n>  \t\t\t\tcmd->did_not_exist = 1;\n>  \t\t\t}\n>  \t\t}\n> diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh\n> index 6e5a9c20e7..b0b795aca9 100755\n> --- a/t/t5504-fetch-receive-strict.sh\n> +++ b/t/t5504-fetch-receive-strict.sh\n> @@ -292,7 +292,7 @@ test_expect_success 'push with receive.fsck.missingEmail=warn' '\n>  \t\treceive.fsck.missingEmail warn &&\n>  \tgit push --porcelain dst bogus >act 2>&1 &&\n>  \tgrep \"missingEmail\" act &&\n> -\ttest_i18ngrep \"Skipping unknown msg id.*whatever\" act &&\n> +\ttest_i18ngrep \"skipping unknown msg id.*whatever\" act &&\n>  \tgit --git-dir=dst/.git branch -D bogus &&\n>  \tgit --git-dir=dst/.git config --add \\\n>  \t\treceive.fsck.missingEmail ignore &&\n"},{"id":"441893","messageId":"CANYiYbFJiTfrErw9etMHsHLBkj3jQ2jPCqJ7H1gBGZmT6QF9kA@mail.gmail.com","threadId":"56885","inReplyTo":"20211113033358.2179376-2-andersk@mit.edu","subject":"Re: [PATCH v6 1/8] fetch: lowercase error messages","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2021-11-22T01:14:46Z","receivedAt":"2021-11-22T01:15:00Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"On Sat, Nov 13, 2021 at 11:34 AM Anders Kaseorg <andersk@mit.edu> wrote:\n>  static const char warn_show_forced_updates[] =\n> -N_(\"Fetch normally indicates which branches had a forced update,\\n\"\n> -   \"but that check has been disabled. To re-enable, use '--show-forced-updates'\\n\"\n> -   \"flag or run 'git config fetch.showForcedUpdates true'.\");\n> +N_(\"fetch normally indicates which branches had a forced update,\\n\"\n> +   \"but that check has been disabled; to re-enable, use '--show-forced-updates'\\n\"\n> +   \"flag or run 'git config fetch.showForcedUpdates true'\");\n>  static const char warn_time_show_forced_updates[] =\n> -N_(\"It took %.2f seconds to check forced updates. You can use\\n\"\n> +N_(\"it took %.2f seconds to check forced updates; you can use\\n\"\n>     \"'--no-show-forced-updates' or run 'git config fetch.showForcedUpdates false'\\n\"\n> -   \" to avoid this check.\\n\");\n> +   \" to avoid this check\\n\");\n\nThe leading space character before \"to avoid ...\" is not necessary.\nThis will change \"po/git.pot\" like this:\n\n    https://github.com/git-l10n/git-po/blob/pot/seen/2021-11-19.diff#L374-L377\n\nIt was introduced in commit 182f59daf0 (l10n: reformat some localized\nstrings for v2.23.0, 2019-08-06).\n\n--\nJiang Xin\n"},{"id":"441937","messageId":"nycvar.QRO.7.76.6.2111221339320.63@tvgsbejvaqbjf.bet","threadId":"56885","inReplyTo":"20211113033358.2179376-5-andersk@mit.edu","subject":"Re: [PATCH v6 4/8] worktree: simplify find_shared_symref() memory ownership model","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-22T12:45:29Z","receivedAt":"2021-11-22T12:45:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Anders,\n\ntl;dr this looks great!\n\nOn Fri, 12 Nov 2021, Anders Kaseorg wrote:\n\n> Storing the worktrees list in a static variable meant that\n> find_shared_symref() had to rebuild the list on each call (which is\n> inefficient when the call site is in a loop), and also that each call\n> invalidated the pointer returned by the previous call (which is\n> confusing).\n>\n> Instead, make it the caller’s responsibility to pass in the worktrees\n> list and manage its lifetime.\n\nThank you for cleaning this up!\n\n>\n> Signed-off-by: Anders Kaseorg <andersk@mit.edu>\n> ---\n>  branch.c               | 14 ++++++----\n>  builtin/branch.c       |  7 ++++-\n>  builtin/notes.c        |  6 +++-\n>  builtin/receive-pack.c | 63 +++++++++++++++++++++++++++---------------\n>  worktree.c             |  8 ++----\n>  worktree.h             |  5 ++--\n>  6 files changed, 65 insertions(+), 38 deletions(-)\n>\n> diff --git a/branch.c b/branch.c\n> index 147827cf46..c7b9ba0e10 100644\n> --- a/branch.c\n> +++ b/branch.c\n> @@ -357,14 +357,16 @@ void remove_branch_state(struct repository *r, int verbose)\n>\n>  void die_if_checked_out(const char *branch, int ignore_current_worktree)\n>  {\n> +\tstruct worktree **worktrees = get_worktrees();\n>  \tconst struct worktree *wt;\n>\n> -\twt = find_shared_symref(\"HEAD\", branch);\n> -\tif (!wt || (ignore_current_worktree && wt->is_current))\n> -\t\treturn;\n> -\tskip_prefix(branch, \"refs/heads/\", &branch);\n> -\tdie(_(\"'%s' is already checked out at '%s'\"),\n> -\t    branch, wt->path);\n> +\twt = find_shared_symref(worktrees, \"HEAD\", branch);\n> +\tif (wt && (!ignore_current_worktree || !wt->is_current)) {\n> +\t\tskip_prefix(branch, \"refs/heads/\", &branch);\n> +\t\tdie(_(\"'%s' is already checked out at '%s'\"), branch, wt->path);\n> +\t}\n> +\n> +\tfree_worktrees(worktrees);\n\nThis is the only caller that is not in `builtin/`, i.e. it is not at once\nclear how many times we would potentially re-generate the list.\n\nI had a quick look:\n\n$ git grep die_if_checked_out\nbranch.c:void die_if_checked_out(const char *branch, int ignore_current_worktree)\nbranch.h:void die_if_checked_out(const char *branch, int ignore_current_worktree);\nbuiltin/checkout.c:                     die_if_checked_out(new_branch_info->path, 1);\nbuiltin/rebase.c:                       die_if_checked_out(buf.buf, 1);\nbuiltin/worktree.c:                     die_if_checked_out(symref.buf, 0);\nbuiltin/worktree.c:                     die_if_checked_out(symref.buf, 0);\n\nThis suggests that all the callers of the `die_if_checked_out()` are in\nthe `builtin/` part, and only in the commands that were not touched\ndirectly by your patch.\n\nWhich means that all is fine and dandy, we are unlikely to introduce a\ncode flow where the worktrees array is populated multiple times (when\nbefore, it would only have been generated only once).\n\nVery good.\n\nCiao,\nDscho\n"},{"id":"441943","messageId":"nycvar.QRO.7.76.6.2111221358160.63@tvgsbejvaqbjf.bet","threadId":"56885","inReplyTo":"20211113033358.2179376-6-andersk@mit.edu","subject":"Re: [PATCH v6 5/8] fetch: protect branches checked out in all worktrees","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-22T13:13:51Z","receivedAt":"2021-11-22T13:14:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Anders,\n\ntl;dr the patch looks nice! A few remarks below, to prove that I did not\nmerely glance over the patch.\n\nOn Fri, 12 Nov 2021, Anders Kaseorg wrote:\n\n> Refuse to fetch into the currently checked out branch of any working\n> tree, not just the current one.\n>\n> Fixes this previously reported bug:\n>\n> https://public-inbox.org/git/cb957174-5e9a-5603-ea9e-ac9b58a2eaad@mathema.de\n\nThese days, I think the lore.kernel.org/git/ links are slightly preferred.\n\n>\n> As a side effect of using find_shared_symref, we’ll also refuse the\n> fetch when we’re on a detached HEAD because we’re rebasing or bisecting\n> on the branch in question. This seems like a sensible change.\n>\n> Signed-off-by: Anders Kaseorg <andersk@mit.edu>\n> ---\n>  builtin/fetch.c       | 75 +++++++++++++++++++++++--------------------\n>  t/t5516-fetch-push.sh | 18 +++++++++++\n>  2 files changed, 58 insertions(+), 35 deletions(-)\n>\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index e5971fa6e5..f373252490 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -28,6 +28,7 @@\n>  #include \"promisor-remote.h\"\n>  #include \"commit-graph.h\"\n>  #include \"shallow.h\"\n> +#include \"worktree.h\"\n>\n>  #define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000)\n>\n> @@ -840,14 +841,13 @@ static void format_display(struct strbuf *display, char code,\n>\n>  static int update_local_ref(struct ref *ref,\n>  \t\t\t    struct ref_transaction *transaction,\n> -\t\t\t    const char *remote,\n> -\t\t\t    const struct ref *remote_ref,\n> -\t\t\t    struct strbuf *display,\n> -\t\t\t    int summary_width)\n> +\t\t\t    const char *remote, const struct ref *remote_ref,\n> +\t\t\t    struct strbuf *display, int summary_width,\n\nAs a reviewer, I prefer to spend my time reviewing the actual change. In\nthis case, the re-wrapping is a bit distracting. Not really important\nhere, but maybe for future contributions. The easier a contributor makes\nit on reviewers, the better (it definitely helps avoid the grumpy judge\nbias[*1*]).\n\n> +\t\t\t    struct worktree **worktrees)\n>  {\n>  \tstruct commit *current = NULL, *updated;\n>  \tenum object_type type;\n> -\tstruct branch *current_branch = branch_get(NULL);\n> +\tconst struct worktree *wt;\n>  \tconst char *pretty_ref = prettify_refname(ref->name);\n>  \tint fast_forward = 0;\n>\n> @@ -862,16 +862,17 @@ static int update_local_ref(struct ref *ref,\n>  \t\treturn 0;\n>  \t}\n>\n> -\tif (current_branch &&\n> -\t    !strcmp(ref->name, current_branch->name) &&\n> -\t    !(update_head_ok || is_bare_repository()) &&\n> -\t    !is_null_oid(&ref->old_oid)) {\n> +\tif (!update_head_ok &&\n> +\t    (wt = find_shared_symref(worktrees, \"HEAD\", ref->name)) &&\n> +\t    !wt->is_bare && !is_null_oid(&ref->old_oid)) {\n>  \t\t/*\n>  \t\t * If this is the head, and it's not okay to update\n>  \t\t * the head, and the old value of the head isn't empty...\n>  \t\t */\n>  \t\tformat_display(display, '!', _(\"[rejected]\"),\n> -\t\t\t       _(\"can't fetch in current branch\"),\n> +\t\t\t       wt->is_current ?\n> +\t\t\t\t       _(\"can't fetch in current branch\") :\n> +\t\t\t\t       _(\"checked out in another worktree\"),\n>  \t\t\t       remote, pretty_ref, summary_width);\n>  \t\treturn 1;\n>  \t}\n> @@ -1071,7 +1072,8 @@ N_(\"it took %.2f seconds to check forced updates; you can use\\n\"\n>     \" to avoid this check\\n\");\n>\n>  static int store_updated_refs(const char *raw_url, const char *remote_name,\n> -\t\t\t      int connectivity_checked, struct ref *ref_map)\n> +\t\t\t      int connectivity_checked, struct ref *ref_map,\n> +\t\t\t      struct worktree **worktrees)\n>  {\n>  \tstruct fetch_head fetch_head;\n>  \tstruct commit *commit;\n> @@ -1188,7 +1190,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n>  \t\t\tstrbuf_reset(&note);\n>  \t\t\tif (ref) {\n>  \t\t\t\trc |= update_local_ref(ref, transaction, what,\n> -\t\t\t\t\t\t       rm, &note, summary_width);\n> +\t\t\t\t\t\t       rm, &note, summary_width,\n> +\t\t\t\t\t\t       worktrees);\n>  \t\t\t\tfree(ref);\n>  \t\t\t} else if (write_fetch_head || dry_run) {\n>  \t\t\t\t/*\n> @@ -1301,16 +1304,15 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)\n>  }\n>\n>  /* Update local refs based on the ref values fetched from a remote */\n> -static int consume_refs(struct transport *transport, struct ref *ref_map)\n> +static int consume_refs(struct transport *transport, struct ref *ref_map,\n> +\t\t\tstruct worktree **worktrees)\n>  {\n>  \tint connectivity_checked = transport->smart_options\n>  \t\t? transport->smart_options->connectivity_checked : 0;\n>  \tint ret;\n>  \ttrace2_region_enter(\"fetch\", \"consume_refs\", the_repository);\n> -\tret = store_updated_refs(transport->url,\n> -\t\t\t\t transport->remote->name,\n> -\t\t\t\t connectivity_checked,\n> -\t\t\t\t ref_map);\n> +\tret = store_updated_refs(transport->url, transport->remote->name,\n> +\t\t\t\t connectivity_checked, ref_map, worktrees);\n\nAgain, this rewrapping is slightly distracting to my brain.\n\n>  \ttransport_unlock_pack(transport);\n>  \ttrace2_region_leave(\"fetch\", \"consume_refs\", the_repository);\n>  \treturn ret;\n> @@ -1643,7 +1647,7 @@ static int do_fetch(struct transport *transport,\n>  \t\t\t\t  \"you need to specify exactly one branch with the --set-upstream option\"));\n>  \t\t}\n>  \t}\n> - skip:\n> +skip:\n\nOkay, this one is distracting _but also_ pleasing. I am only pointing this\nout to prove that I did not go over your patch sloppily. :-)\n\n> @@ -1653,11 +1657,12 @@ static int do_fetch(struct transport *transport,\n>  \t\tref_map = NULL;\n>  \t\tfind_non_local_tags(remote_refs, &ref_map, &tail);\n>  \t\tif (ref_map)\n> -\t\t\tbackfill_tags(transport, ref_map);\n> +\t\t\tbackfill_tags(transport, ref_map, worktrees);\n>  \t\tfree_refs(ref_map);\n>  \t}\n>\n> - cleanup:\n> +cleanup:\n\nSame here.\n\n> +\tfree_worktrees(worktrees);\n>  \treturn retcode;\n>  }\n>\n> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\n> index 4db8edd9c8..36fb90f4b0 100755\n> --- a/t/t5516-fetch-push.sh\n> +++ b/t/t5516-fetch-push.sh\n> @@ -1770,4 +1770,22 @@ test_expect_success 'denyCurrentBranch and worktrees' '\n>  \tgit -C cloned push origin HEAD:new-wt &&\n>  \ttest_must_fail git -C cloned push --delete origin new-wt\n>  '\n> +\n> +test_expect_success 'refuse fetch to current branch of worktree' '\n> +\ttest_when_finished \"git worktree remove --force wt && git branch -D wt\" &&\n> +\tgit worktree add wt &&\n> +\ttest_commit apple &&\n> +\ttest_must_fail git fetch . HEAD:wt &&\n> +\tgit fetch -u . HEAD:wt\n> +'\n> +\n> +test_expect_success 'refuse fetch to current branch of bare repository worktree' '\n> +\ttest_when_finished \"rm -fr bare.git\" &&\n> +\tgit clone --bare . bare.git &&\n> +\tgit -C bare.git worktree add wt &&\n> +\ttest_commit banana &&\n> +\ttest_must_fail git -C bare.git fetch .. HEAD:wt &&\n> +\tgit -C bare.git fetch -u .. HEAD:wt\n> +'\n> +\n>  test_done\n\nReads nicely.\n\nThanks,\nDscho\n\nFootnote: https://en.wikipedia.org/wiki/Hungry_judge_effect\n"},{"id":"441944","messageId":"nycvar.QRO.7.76.6.2111221417100.63@tvgsbejvaqbjf.bet","threadId":"56885","inReplyTo":"20211113033358.2179376-1-andersk@mit.edu","subject":"Re: [PATCH v6 0/8] protect branches checked out in all worktrees","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-22T13:21:09Z","receivedAt":"2021-11-22T13:21:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Anders,\n\nOn Fri, 12 Nov 2021, Anders Kaseorg wrote:\n\n> ‘git fetch’ (without ‘--update-head-ok’), ‘git receive-pack’, and ‘git\n> branch -M’ protect the currently checked out branch from being\n> accidentally updated.  However, the code for these checks predates\n> ‘git worktree’.  Improve it to protect branches checked out in all\n> worktrees, not just the current one.\n\nI read through these patches, and in particular the last three were a real\ndelight to read, a highlight of my catching-up with the mailing list\ntoday.\n\nI am very much in favor of these patches to advance to `next` as-are.\n\nThank you,\nDscho\n"}]}