{"thread":{"id":"57011","subject":"[PATCH v7 1/8] fetch: lowercase error messages","startedAt":"2021-12-01T22:16:21Z","lastAt":"2022-01-12T19:16:44Z","messageCount":20,"participants":["Anders Kaseorg","Eric Sunshine","Jiang Xin","Junio C Hamano"],"isPatch":true,"patchVersion":7,"patchTotal":8},"messages":[{"id":"442811","messageId":"20211201221547.1796213-2-andersk@mit.edu","threadId":"57011","inReplyTo":"20211201221547.1796213-1-andersk@mit.edu","subject":"[PATCH v7 1/8] fetch: lowercase error messages","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-12-01T22:15:40Z","receivedAt":"2021-12-01T22:16:21Z","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”.  Clean up existing\nmessages, some of which we will be touching in later steps in the\nseries, that deviate from these rules in this file, as a preparation for\nthe main part of the topic.\n\nSigned-off-by: Anders Kaseorg <andersk@mit.edu>\n---\n builtin/fetch.c | 50 +++++++++++++++++++++++++------------------------\n 1 file changed, 26 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex f7abbc31ff..16b98523ce 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@@ -1067,13 +1067,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@@ -1395,8 +1395,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@@ -1414,10 +1415,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@@ -1451,7 +1452,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@@ -1489,7 +1490,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@@ -1651,8 +1652,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@@ -1790,7 +1791,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@@ -1845,7 +1846,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);\n@@ -1906,8 +1907,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@@ -1942,7 +1943,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@@ -2010,7 +2011,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@@ -2047,14 +2048,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@@ -2075,7 +2077,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.34.1\n\n"},{"id":"442812","messageId":"20211201221547.1796213-4-andersk@mit.edu","threadId":"57011","inReplyTo":"20211201221547.1796213-1-andersk@mit.edu","subject":"[PATCH v7 3/8] branch: lowercase error messages","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-12-01T22:15:42Z","receivedAt":"2021-12-01T22:16:22Z","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”.  Clean up existing\nmessages, some of which we will be touching in later steps in the\nseries, that deviate from these rules in this file, as a preparation for\nthe main part of the topic.\n\nSigned-off-by: Anders Kaseorg <andersk@mit.edu>\n---\n branch.c                   | 20 ++++++++++----------\n t/t2018-checkout-branch.sh |  2 +-\n t/t3200-branch.sh          |  4 ++--\n 3 files changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 07a46430b3..85777b939b 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@@ -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..498cea31bd 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@@ -953,7 +953,7 @@ test_expect_success 'disabled option --set-upstream fails' '\n test_expect_success '--set-upstream-to notices an error to set branch as own upstream' '\n \tgit branch --set-upstream-to refs/heads/my13 my13 2>actual &&\n \tcat >expect <<-\\EOF &&\n-\twarning: Not setting branch my13 as its own upstream.\n+\twarning: not setting branch my13 as its own upstream\n \tEOF\n \ttest_expect_code 1 git config branch.my13.remote &&\n \ttest_expect_code 1 git config branch.my13.merge &&\n-- \n2.34.1\n\n"},{"id":"442813","messageId":"20211201221547.1796213-1-andersk@mit.edu","threadId":"57011","inReplyTo":null,"subject":"[PATCH v7 0/8] protect branches checked out in all worktrees","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-12-01T22:15:39Z","receivedAt":"2021-12-01T22:16:26Z","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                        |  45 +++++++-----\n builtin/branch.c                |   7 +-\n builtin/fetch.c                 | 119 +++++++++++++++++---------------\n builtin/notes.c                 |   6 +-\n builtin/receive-pack.c          |  92 +++++++++++++-----------\n t/t2018-checkout-branch.sh      |   2 +-\n t/t3200-branch.sh               |  11 ++-\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, 198 insertions(+), 131 deletions(-)\n\n-- \n2.34.1\n\n"},{"id":"442814","messageId":"20211201221547.1796213-5-andersk@mit.edu","threadId":"57011","inReplyTo":"20211201221547.1796213-1-andersk@mit.edu","subject":"[PATCH v7 4/8] worktree: simplify find_shared_symref() memory ownership model","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-12-01T22:15:43Z","receivedAt":"2021-12-01T22:16:28Z","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 85777b939b..c66b222abd 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 7a1d1eeb07..d8f2164cd7 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 ddda27c184..525b021ec0 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1486,12 +1486,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@@ -1510,7 +1515,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@@ -1521,13 +1527,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@@ -1543,9 +1551,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@@ -1563,25 +1573,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@@ -1600,17 +1615,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@@ -1618,14 +1635,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.34.1\n\n"},{"id":"442815","messageId":"20211201221547.1796213-3-andersk@mit.edu","threadId":"57011","inReplyTo":"20211201221547.1796213-1-andersk@mit.edu","subject":"[PATCH v7 2/8] receive-pack: lowercase error messages","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-12-01T22:15:41Z","receivedAt":"2021-12-01T22:16:32Z","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”.  Clean up existing\nmessages, some of which we will be touching in later steps in the\nseries, that deviate from these rules in this file, as a preparation for\nthe main part of the topic.\n\nSigned-off-by: Anders Kaseorg <andersk@mit.edu>\n---\n builtin/receive-pack.c          | 10 +++++-----\n t/t5504-fetch-receive-strict.sh |  2 +-\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 49b846d960..ddda27c184 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -175,7 +175,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@@ -1589,9 +1589,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@@ -2490,9 +2490,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/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.34.1\n\n"},{"id":"442816","messageId":"20211201221547.1796213-6-andersk@mit.edu","threadId":"57011","inReplyTo":"20211201221547.1796213-1-andersk@mit.edu","subject":"[PATCH v7 5/8] fetch: protect branches checked out in all worktrees","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-12-01T22:15:44Z","receivedAt":"2021-12-01T22:16:34Z","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://lore.kernel.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 16b98523ce..e45185cf9c 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@@ -848,13 +849,12 @@ 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-\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@@ -868,16 +868,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@@ -1076,7 +1077,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 \tint url_len, i, rc = 0;\n@@ -1205,7 +1207,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@@ -1298,7 +1301,9 @@ static int check_exist_and_connected(struct ref *ref_map)\n \treturn check_connected(iterate_ref_map, &rm, &opt);\n }\n \n-static int fetch_and_consume_refs(struct transport *transport, struct ref *ref_map)\n+static int fetch_and_consume_refs(struct transport *transport,\n+\t\t\t\t  struct ref *ref_map,\n+\t\t\t\t  struct worktree **worktrees)\n {\n \tint connectivity_checked = 1;\n \tint ret;\n@@ -1319,10 +1324,8 @@ static int fetch_and_consume_refs(struct transport *transport, struct ref *ref_m\n \t}\n \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 \ttrace2_region_leave(\"fetch\", \"consume_refs\", the_repository);\n \n out:\n@@ -1385,19 +1388,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@@ -1495,7 +1497,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@@ -1516,7 +1519,7 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)\n \ttransport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);\n \ttransport_set_option(transport, TRANS_OPT_DEPTH, \"0\");\n \ttransport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);\n-\tfetch_and_consume_refs(transport, ref_map);\n+\tfetch_and_consume_refs(transport, ref_map, worktrees);\n \n \tif (gsecondary) {\n \t\ttransport_disconnect(gsecondary);\n@@ -1534,6 +1537,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@@ -1589,7 +1593,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@@ -1607,7 +1611,7 @@ static int do_fetch(struct transport *transport,\n \t\t\t\t   transport->url);\n \t\t}\n \t}\n-\tif (fetch_and_consume_refs(transport, ref_map)) {\n+\tif (fetch_and_consume_refs(transport, ref_map, worktrees)) {\n \t\tfree_refs(ref_map);\n \t\tretcode = 1;\n \t\tgoto cleanup;\n@@ -1656,7 +1660,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@@ -1666,11 +1670,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 8212ca56dc..f07e32126f 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1771,4 +1771,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.34.1\n\n"},{"id":"442817","messageId":"20211201221547.1796213-7-andersk@mit.edu","threadId":"57011","inReplyTo":"20211201221547.1796213-1-andersk@mit.edu","subject":"[PATCH v7 6/8] receive-pack: clean dead code from update_worktree()","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-12-01T22:15:45Z","receivedAt":"2021-12-01T22:16:41Z","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 525b021ec0..29dc0e56f7 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1449,29 +1449,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 (!hook_exists(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@@ -1592,9 +1585,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.34.1\n\n"},{"id":"442818","messageId":"20211201221547.1796213-8-andersk@mit.edu","threadId":"57011","inReplyTo":"20211201221547.1796213-1-andersk@mit.edu","subject":"[PATCH v7 7/8] receive-pack: protect current branch for bare repository worktree","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-12-01T22:15:46Z","receivedAt":"2021-12-01T22:16:42Z","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 29dc0e56f7..26ecaa5cd5 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1455,7 +1455,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@@ -1481,9 +1481,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@@ -1496,7 +1494,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 f07e32126f..cd2e144c37 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1769,9 +1769,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.34.1\n\n"},{"id":"442819","messageId":"20211201221547.1796213-9-andersk@mit.edu","threadId":"57011","inReplyTo":"20211201221547.1796213-1-andersk@mit.edu","subject":"[PATCH v7 8/8] branch: protect branches checked out in all worktrees","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-12-01T22:15:47Z","receivedAt":"2021-12-01T22:16:44Z","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 c66b222abd..2cfe496d24 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 498cea31bd..9c45170661 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.34.1\n\n"},{"id":"442826","messageId":"CAPig+cSNP-RBmsWWfT690-shFUCZ3J0X+FBiNjCqg=AkoBMBqQ@mail.gmail.com","threadId":"57011","inReplyTo":"20211201221547.1796213-5-andersk@mit.edu","subject":"Re: [PATCH v7 4/8] worktree: simplify find_shared_symref() memory ownership model","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-01T23:10:21Z","receivedAt":"2021-12-01T23:10:35Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Dec 1, 2021 at 5:16 PM Anders Kaseorg <andersk@mit.edu> wrote:\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>\n> Signed-off-by: Anders Kaseorg <andersk@mit.edu>\n> ---\n> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\n> @@ -1486,12 +1486,17 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n> -       const struct worktree *worktree = is_bare_repository() ? NULL : find_shared_symref(\"HEAD\", name);\n> +       struct worktree **worktrees = get_worktrees();\n> +       const struct worktree *worktree =\n> +               is_bare_repository() ?\n> +                       NULL :\n> +                       find_shared_symref(worktrees, \"HEAD\", name);\n\nAs far as I can see, this code only cares whether find_shared_symref()\nreturned a result; it doesn't actually consult the returned worktree\nat all, thus it semantically considers `worktree` as a boolean, not as\na `struct worktree`. I see in [7/8] that you do add an access to the\n`worktree.is_bare` field, but that also is used purely in a boolean\nfashion. If my understanding is correct, then it seems as if it would\nbe cleaner and make for a much less noisy patch if you did this\ninstead:\n\n    struct worktree **worktrees = get_worktrees();\n    int has_worktree = 0;\n    if (!is_bare_repository()) {\n        struct worktree *w = find_shared_symref(worktrees, ...);\n        has_worktree = !!w;\n    }\n    free_worktrees(worktrees);\n    ...\n    if (has_worktree) {\n        ...\n    }\n\nand in patch [7/8] augment that to:\n\n    int is_worktree_bare = 0;\n    if (!is_bare_repository()) {\n        struct worktree *w = find_shared_symref(worktrees, ...);\n        if (w) {\n            has_worktree = 1;\n            is_worktree_bare = w->is_bare;\n        }\n    }\n    free_worktrees(worktrees);\n\nwhich would then allow you to...\n\n>         if (!starts_with(name, \"refs/\") || check_refname_format(name + 5, 0)) {\n>                 rp_error(\"refusing to create funny ref '%s' remotely\", name);\n> -               return \"funny refname\";\n> +               ret = \"funny refname\";\n> +               goto out;\n>         }\n\n... drop this change and all the other similar changes to this file\nsince `worktrees` gets cleaned up as soon as `has_worktree` and\n`is_worktree_bare` have been determined, so there's no need to\nintroduce an `out` label just to clean up `worktrees` at the end of\nthe function.\n"},{"id":"442830","messageId":"f8262c3b-2ca5-4920-1379-41ff29b37d9f@mit.edu","threadId":"57011","inReplyTo":"CAPig+cSNP-RBmsWWfT690-shFUCZ3J0X+FBiNjCqg=AkoBMBqQ@mail.gmail.com","subject":"Re: [PATCH v7 4/8] worktree: simplify find_shared_symref() memory ownership model","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-12-01T23:47:40Z","receivedAt":"2021-12-01T23:48:14Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"On 12/1/21 15:10, Eric Sunshine wrote:\n> As far as I can see, this code only cares whether find_shared_symref()\n> returned a result; it doesn't actually consult the returned worktree\n> at all, thus it semantically considers `worktree` as a boolean, not as\n> a `struct worktree`.\n\nNo, the update_worktree(new_oid->hash, worktree) call uses it in a \nnon-boolean way, so we do need to keep it around.\n\nAnders\n"},{"id":"442834","messageId":"CAPig+cRX94vMLkW=dAKncPTuuk0Opaj+8-Yqe-qFD1b7mrZ89A@mail.gmail.com","threadId":"57011","inReplyTo":"f8262c3b-2ca5-4920-1379-41ff29b37d9f@mit.edu","subject":"Re: [PATCH v7 4/8] worktree: simplify find_shared_symref() memory ownership model","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-02T00:13:09Z","receivedAt":"2021-12-02T00:14:28Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Dec 1, 2021 at 6:48 PM Anders Kaseorg <andersk@mit.edu> wrote:\n> On 12/1/21 15:10, Eric Sunshine wrote:\n> > As far as I can see, this code only cares whether find_shared_symref()\n> > returned a result; it doesn't actually consult the returned worktree\n> > at all, thus it semantically considers `worktree` as a boolean, not as\n> > a `struct worktree`.\n>\n> No, the update_worktree(new_oid->hash, worktree) call uses it in a\n> non-boolean way, so we do need to keep it around.\n\nOkay, I missed that call to update_worktree() in the \"noise\" of all\nthose other changes, but looking at the code a bit more carefully, it\nseems as if we might still be able to drop all the noise changes\nanyhow by hoisting ownership of `worktrees` up a level or two. In\nparticular, both execute_commands_atomic() and\nexecute_commands_non_atomic() are calling update() in a loop, and\nupdate() is calling get_worktrees() / free_worktrees() each time it is\ncalled within those loops, which seems unnecessarily expensive.\n\nIf we instead hoist ownership of `worktrees` up to execute_commands()\n-- which calls execute_commands_atomic() or\nexecute_commands_non_atomic() -- then we can get by with retrieving\nthe worktrees just once, and all those noise changes in update() can\nbe dropped since it will no longer be responsible for allocating or\nfreeing `worktrees`. For instance:\n\n    static void execute_commands(...)\n    {\n        struct worktree **worktrees;\n        ...\n        worktrees = get_worktrees();\n         if (use_atomic)\n            execute_commands_atomic(commands, si, worktrees);\n        else\n            execute_commands_non_atomic(commands, si, worktrees);\n        free_worktrees(worktrees);\n        ...\n    }\n\nand then execute_commands_atomic() and execute_commands_non_atomic()\ncan pass `worktrees` along to update().\n"},{"id":"442836","messageId":"CAPig+cT3psyPhsssvJsQw4wzTMLOxf0rhP4H7V4s5g9Wd_kK1g@mail.gmail.com","threadId":"57011","inReplyTo":"CAPig+cRX94vMLkW=dAKncPTuuk0Opaj+8-Yqe-qFD1b7mrZ89A@mail.gmail.com","subject":"Re: [PATCH v7 4/8] worktree: simplify find_shared_symref() memory ownership model","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-02T00:32:15Z","receivedAt":"2021-12-02T00:32:32Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Dec 1, 2021 at 7:13 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> If we instead hoist ownership of `worktrees` up to execute_commands()\n> -- which calls execute_commands_atomic() or\n> execute_commands_non_atomic() -- then we can get by with retrieving\n> the worktrees just once, and all those noise changes in update() can\n> be dropped since it will no longer be responsible for allocating or\n> freeing `worktrees`. For instance:\n>\n>     static void execute_commands(...)\n>     {\n>         struct worktree **worktrees;\n>         ...\n>         worktrees = get_worktrees();\n>          if (use_atomic)\n>             execute_commands_atomic(commands, si, worktrees);\n>         else\n>             execute_commands_non_atomic(commands, si, worktrees);\n>         free_worktrees(worktrees);\n>         ...\n>     }\n>\n> and then execute_commands_atomic() and execute_commands_non_atomic()\n> can pass `worktrees` along to update().\n\nBy the way, I wouldn't typically recommend this sort of change for\npublic API since it probably exposes too much implementation detail to\ncallers, but as these are all private (static) worker functions --\nwith very few callers, moreover -- I don't find it terribly worrying.\n"},{"id":"442843","messageId":"CAPig+cQudoq3QrF29W8p511s1rPtgZajtk1Rjm+N2SgoLY-Ltw@mail.gmail.com","threadId":"57011","inReplyTo":"20211201221547.1796213-6-andersk@mit.edu","subject":"Re: [PATCH v7 5/8] fetch: protect branches checked out in all worktrees","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-02T02:51:51Z","receivedAt":"2021-12-02T02:52:08Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Dec 1, 2021 at 5:16 PM Anders Kaseorg <andersk@mit.edu> wrote:\n> Refuse to fetch into the currently checked out branch of any working\n> tree, not just the current one.\n> [...]\n> Signed-off-by: Anders Kaseorg <andersk@mit.edu>\n> ---\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> @@ -868,16 +868,17 @@ static int update_local_ref(struct ref *ref,\n> +       if (!update_head_ok &&\n> +           (wt = find_shared_symref(worktrees, \"HEAD\", ref->name)) &&\n> +           !wt->is_bare && !is_null_oid(&ref->old_oid)) {\n>                 /*\n>                  * If this is the head, and it's not okay to update\n>                  * the head, and the old value of the head isn't empty...\n>                  */\n>                 format_display(display, '!', _(\"[rejected]\"),\n> -                              _(\"can't fetch in current branch\"),\n> +                              wt->is_current ?\n> +                                      _(\"can't fetch in current branch\") :\n> +                                      _(\"checked out in another worktree\"),\n>                                remote, pretty_ref, summary_width);\n\nMinor observation: The \"checked out in another worktree\" message would\nbe much more helpful if it mentioned the worktree itself...\n\n> @@ -1385,19 +1388,18 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map,\n> -               if (ref_map->peer_ref && !strcmp(current_branch->refname,\n> -                                       ref_map->peer_ref->name))\n> -                       die(_(\"refusing to fetch into current branch %s \"\n> -                             \"of non-bare repository\"),\n> -                           current_branch->refname);\n> +               if (ref_map->peer_ref &&\n> +                   (wt = find_shared_symref(worktrees, \"HEAD\",\n> +                                            ref_map->peer_ref->name)) &&\n> +                   !wt->is_bare)\n> +                       die(_(\"refusing to fetch into branch '%s' \"\n> +                             \"checked out at '%s'\"),\n> +                           ref_map->peer_ref->name, wt->path);\n\n... much like this message has been updated to mention the worktree.\n\nHowever, I didn't look to see how much work it would be to improve the\nearlier message; such an improvement could always be done atop this\nseries later on by someone.\n"},{"id":"442858","messageId":"736c137a-1d41-e5bb-1424-959992e54e8c@mit.edu","threadId":"57011","inReplyTo":"CAPig+cQudoq3QrF29W8p511s1rPtgZajtk1Rjm+N2SgoLY-Ltw@mail.gmail.com","subject":"Re: [PATCH v7 5/8] fetch: protect branches checked out in all worktrees","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-12-02T08:37:37Z","receivedAt":"2021-12-02T08:38:04Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"On 12/1/21 18:51, Eric Sunshine wrote:\n> Minor observation: The \"checked out in another worktree\" message would\n> be much more helpful if it mentioned the worktree itself...\n\nPrevious discussion of that: \nhttps://lore.kernel.org/git/YYTtdZblnHYgDgBq@coredump.intra.peff.net/ \nand replies.\n\nIn short, this message seems to be unreachable because the other message \nwill always be triggered first, so I didn’t spend much effort improving \nit.  As you say, we can leave the task of sorting out the apparently \nduplicate check to another series.\n\nAnders\n"},{"id":"442860","messageId":"alpine.DEB.2.21.999.2112020357510.14727@scrubbing-bubbles.mit.edu","threadId":"57011","inReplyTo":"CAPig+cRX94vMLkW=dAKncPTuuk0Opaj+8-Yqe-qFD1b7mrZ89A@mail.gmail.com","subject":"Re: [PATCH v7 4/8] worktree: simplify find_shared_symref() memory ownership model","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2021-12-02T09:06:37Z","receivedAt":"2021-12-02T09:07:04Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"On Wed, 1 Dec 2021, Eric Sunshine wrote:\n> If we instead hoist ownership of `worktrees` up to execute_commands()\n> -- which calls execute_commands_atomic() or\n> execute_commands_non_atomic() -- then we can get by with retrieving\n> the worktrees just once, and all those noise changes in update() can\n> be dropped since it will no longer be responsible for allocating or\n> freeing `worktrees`.\n\nSeems reasonable to me.  It’s a smaller textual change, potentially a \nlarger conceptual change, but the efficiency improvement is probably \nworthwhile.\n\nThat would modify patch 4 to the below.  Patches 5 through 8 cleanly \nrebase past this modification.\n\nAnders\n\n-- >8 --\nSubject: [PATCH v7½ 4/8] worktree: simplify find_shared_symref() memory ownership model\n\nStoring 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 | 29 ++++++++++++++++++++---------\n worktree.c             |  8 ++------\n worktree.h             |  5 +++--\n 6 files changed, 44 insertions(+), 25 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 85777b939b..c66b222abd 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 7a1d1eeb07..d8f2164cd7 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 ddda27c184..925ce7f32d 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1477,7 +1477,8 @@ static const char *update_worktree(unsigned char *sha1, const struct worktree *w\n \treturn retval;\n }\n \n-static const char *update(struct command *cmd, struct shallow_info *si)\n+static const char *update(struct command *cmd, struct shallow_info *si,\n+\t\t\t  struct worktree **worktrees)\n {\n \tconst char *name = cmd->ref_name;\n \tstruct strbuf namespaced_name_buf = STRBUF_INIT;\n@@ -1486,7 +1487,10 @@ 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+\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@@ -1579,7 +1583,9 @@ 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, 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}\n@@ -1842,7 +1848,8 @@ static void warn_if_skipped_connectivity_check(struct command *commands,\n }\n \n static void execute_commands_non_atomic(struct command *commands,\n-\t\t\t\t\tstruct shallow_info *si)\n+\t\t\t\t\tstruct shallow_info *si,\n+\t\t\t\t\tstruct worktree **worktrees)\n {\n \tstruct command *cmd;\n \tstruct strbuf err = STRBUF_INIT;\n@@ -1859,7 +1866,7 @@ static void execute_commands_non_atomic(struct command *commands,\n \t\t\tcontinue;\n \t\t}\n \n-\t\tcmd->error_string = update(cmd, si);\n+\t\tcmd->error_string = update(cmd, si, worktrees);\n \n \t\tif (!cmd->error_string\n \t\t    && ref_transaction_commit(transaction, &err)) {\n@@ -1873,7 +1880,8 @@ static void execute_commands_non_atomic(struct command *commands,\n }\n \n static void execute_commands_atomic(struct command *commands,\n-\t\t\t\t\tstruct shallow_info *si)\n+\t\t\t\t    struct shallow_info *si,\n+\t\t\t\t    struct worktree **worktrees)\n {\n \tstruct command *cmd;\n \tstruct strbuf err = STRBUF_INIT;\n@@ -1891,7 +1899,7 @@ static void execute_commands_atomic(struct command *commands,\n \t\tif (!should_process_cmd(cmd) || cmd->run_proc_receive)\n \t\t\tcontinue;\n \n-\t\tcmd->error_string = update(cmd, si);\n+\t\tcmd->error_string = update(cmd, si, worktrees);\n \n \t\tif (cmd->error_string)\n \t\t\tgoto failure;\n@@ -1925,6 +1933,7 @@ static void execute_commands(struct command *commands,\n \tstruct async muxer;\n \tint err_fd = 0;\n \tint run_proc_receive = 0;\n+\tstruct worktree **worktrees;\n \n \tif (unpacker_error) {\n \t\tfor (cmd = commands; cmd; cmd = cmd->next)\n@@ -2004,10 +2013,12 @@ static void execute_commands(struct command *commands,\n \t\t\t    (cmd->run_proc_receive || use_atomic))\n \t\t\t\tcmd->error_string = \"fail to run proc-receive hook\";\n \n+\tworktrees = get_worktrees();\n \tif (use_atomic)\n-\t\texecute_commands_atomic(commands, si);\n+\t\texecute_commands_atomic(commands, si, worktrees);\n \telse\n-\t\texecute_commands_non_atomic(commands, si);\n+\t\texecute_commands_non_atomic(commands, si, worktrees);\n+\tfree_worktrees(worktrees);\n \n \tif (shallow_update)\n \t\twarn_if_skipped_connectivity_check(commands, si);\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.34.1"},{"id":"442915","messageId":"CAPig+cS9n91nstp0zK-JywK3WHOVZ38+Ok850=aajx+hL4T2ZQ@mail.gmail.com","threadId":"57011","inReplyTo":"alpine.DEB.2.21.999.2112020357510.14727@scrubbing-bubbles.mit.edu","subject":"Re: [PATCH v7 4/8] worktree: simplify find_shared_symref() memory ownership model","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-12-02T21:00:23Z","receivedAt":"2021-12-02T21:00:38Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Dec 2, 2021 at 4:07 AM Anders Kaseorg <andersk@mit.edu> wrote:\n> On Wed, 1 Dec 2021, Eric Sunshine wrote:\n> > If we instead hoist ownership of `worktrees` up to execute_commands()\n> > -- which calls execute_commands_atomic() or\n> > execute_commands_non_atomic() -- then we can get by with retrieving\n> > the worktrees just once, and all those noise changes in update() can\n> > be dropped since it will no longer be responsible for allocating or\n> > freeing `worktrees`.\n>\n> Seems reasonable to me.  It’s a smaller textual change, potentially a\n> larger conceptual change, but the efficiency improvement is probably\n> worthwhile.\n>\n> That would modify patch 4 to the below.  Patches 5 through 8 cleanly\n> rebase past this modification.\n\nYes, this version looks good. Thanks for accommodating my suggestion.\nNot only does it make for a less noisy diff (which is pleasing), but\nperhaps more importantly, it reduces cognitive load a small amount (at\nleast for me[1]).\n\n> Subject: [PATCH v7½ 4/8] worktree: simplify find_shared_symref() memory ownership model\n\nI suspect that Junio will find it easier to take a full re-roll than\ndealing with a single patch replacement since he doesn't necessarily\nfollow these discussions closely. It's more likely that he'll pick up\nthis change if you post v8 in its entirety.\n\n[1]: Although we regularly use `goto` for this sort of resource\ncleanup, most such functions are much shorter than the one here;\nsomehow the length of this function made the new `goto`s a tiny bit\nless easy to grasp-at-a-glance. Purely subjective, I know.\n"},{"id":"444849","messageId":"CANYiYbHBjM-1oezX_1Z+te+wvQpdj=rwggHHCLPepos_Njwqvw@mail.gmail.com","threadId":"57011","inReplyTo":"20211201221547.1796213-9-andersk@mit.edu","subject":"Re: [PATCH v7 8/8] branch: protect branches checked out in all worktrees","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2021-12-23T00:58:17Z","receivedAt":"2021-12-23T00:58:32Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"On Thu, Dec 2, 2021 at 6:16 AM Anders Kaseorg <andersk@mit.edu> wrote:\n>\n> Refuse to force-move a branch over the currently checked out branch of\n> any working tree, not just the current one.\n>\n> Signed-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>\n> diff --git a/branch.c b/branch.c\n> index c66b222abd..2cfe496d24 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> -       const char *head;\n> +       struct worktree **worktrees;\n> +       const struct worktree *wt;\n>\n>         if (!validate_branchname(name, ref))\n>                 return 0;\n> @@ -208,9 +209,13 @@ int validate_new_branchname(const char *name, struct strbuf *ref, int force)\n>                 die(_(\"a branch named '%s' already exists\"),\n>                     ref->buf + strlen(\"refs/heads/\"));\n>\n> -       head = resolve_ref_unsafe(\"HEAD\", 0, NULL, NULL);\n> -       if (!is_bare_repository() && head && !strcmp(head, ref->buf))\n> -               die(_(\"cannot force update the current branch\"));\n> +       worktrees = get_worktrees();\n> +       wt = find_shared_symref(worktrees, \"HEAD\", ref->buf);\n> +       if (wt && !wt->is_bare)\n> +               die(_(\"cannot force update the branch '%s'\"\n> +                     \"checked out at '%s'\"),\n\nThere is no space between \"'%s'\" and \"checkout\". Found this from [1],\nsorry for reporting this is a bit late.\n\n[1] https://github.com/git-l10n/git-po/blob/pot/master/2021-12-22.diff#L315\n"},{"id":"445983","messageId":"CANYiYbGdUE_NQFTA31EnEr57E00umfJQoUPMc=OZ3snO+GWZ=w@mail.gmail.com","threadId":"57011","inReplyTo":"CANYiYbHBjM-1oezX_1Z+te+wvQpdj=rwggHHCLPepos_Njwqvw@mail.gmail.com","subject":"Re: [PATCH v7 8/8] branch: protect branches checked out in all worktrees","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2022-01-12T06:31:33Z","receivedAt":"2022-01-12T06:32:12Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"On Thu, Dec 23, 2021 at 8:58 AM Jiang Xin <worldhello.net@gmail.com> wrote:\n>\n> On Thu, Dec 2, 2021 at 6:16 AM Anders Kaseorg <andersk@mit.edu> wrote:\n> > +       if (wt && !wt->is_bare)\n> > +               die(_(\"cannot force update the branch '%s'\"\n> > +                     \"checked out at '%s'\"),\n>\n> There is no space between \"'%s'\" and \"checkout\". Found this from [1],\n> sorry for reporting this is a bit late.\n>\n> [1] https://github.com/git-l10n/git-po/blob/pot/master/2021-12-22.diff#L315\n\nThis issue was still there in git v2.35.0-rc0. Peter Krefting and Emir\nSARI also reported this issue in pull requests:\n\n * https://github.com/git-l10n/git-po/pull/599\n * https://github.com/git-l10n/git-po/pull/600\n\nI see Bagas has sent a patch for this:\n\n * https://lore.kernel.org/git/20220111123627.58625-1-bagasdotme@gmail.com/\n\n--\nJiang Xin\n"},{"id":"446052","messageId":"xmqqa6g0vkgl.fsf@gitster.g","threadId":"57011","inReplyTo":"CANYiYbGdUE_NQFTA31EnEr57E00umfJQoUPMc=OZ3snO+GWZ=w@mail.gmail.com","subject":"Re: [PATCH v7 8/8] branch: protect branches checked out in all worktrees","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-12T19:10:34Z","receivedAt":"2022-01-12T19:16:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jiang Xin <worldhello.net@gmail.com> writes:\n\n> I see Bagas has sent a patch for this:\n>\n>  * https://lore.kernel.org/git/20220111123627.58625-1-bagasdotme@gmail.com/\n\nYup, picked up and will merge down before -rc1.\n\nThanks, all.\n"}]}