{"thread":{"id":"59711","subject":"[PATCH v10 8/8] worktree add: emit warn when there is a bad HEAD","startedAt":"2023-05-07T12:07:40Z","lastAt":"2023-05-07T12:07:40Z","messageCount":1,"participants":["Jacob Abel"],"isPatch":true,"patchVersion":10,"patchTotal":8},"messages":[{"id":"476725","messageId":"20230507120530.14669-9-jacobabel@nullpo.dev","threadId":"59711","inReplyTo":null,"subject":"[PATCH v10 8/8] worktree add: emit warn when there is a bad HEAD","fromName":"Jacob Abel","fromEmail":"jacobabel@nullpo.dev","sentAt":"2023-05-07T12:07:14Z","receivedAt":"2023-05-07T12:07:40Z","isPatch":true,"sender":{"key":"jacobabel@nullpo.dev","avatar":"https://avatars.githubusercontent.com/u/9424043?v=4"},"body":"Add a warning to `worktree add` when the command tries to reference\nHEAD, there exist valid local branches, and the HEAD points to a\nnon-existent reference.\n\nCurrent Behavior:\n% git -C foo worktree list\n/path/to/repo/foo     dadc8e6dac [main]\n/path/to/repo/foo_wt  0000000000 [badref]\n% git -C foo worktree add ../wt1\nPreparing worktree (new branch 'wt1')\nHEAD is now at dadc8e6dac dummy commit\n% git -C foo_wt worktree add ../wt2\nhint: If you meant to create a worktree containing a new orphan branch\n[...]\nhint: Disable this message with \"git config advice.worktreeAddOrphan false\"\nfatal: invalid reference: HEAD\n%\n\nNew Behavior:\n% git -C foo worktree list\n/path/to/repo/foo     dadc8e6dac [main]\n/path/to/repo/foo_wt  0000000000 [badref]\n% git -C foo worktree add ../wt1\nPreparing worktree (new branch 'wt1')\nHEAD is now at dadc8e6dac dummy commit\n% git -C foo_wt worktree add ../wt2\nwarning: HEAD points to an invalid (or orphaned) reference.\nHEAD path: '/path/to/repo/foo/.git/worktrees/foo_wt/HEAD'\nHEAD contents: 'ref: refs/heads/badref'\nhint: If you meant to create a worktree containing a new orphan branch\n[...]\nhint: Disable this message with \"git config advice.worktreeAddOrphan false\"\nfatal: invalid reference: HEAD\n%\n\nSigned-off-by: Jacob Abel <jacobabel@nullpo.dev>\n---\n builtin/worktree.c      | 34 +++++++++++++++++++++++++++++-----\n t/t2400-worktree-add.sh | 18 +++++++++++++++++-\n 2 files changed, 46 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 093b2cb032..5f62084334 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -638,6 +638,9 @@ static int first_valid_ref(const char *refname,\n  *\n  * - Checks whether any valid local branches exist.\n  *\n+ * - Emits a warning if there exist any valid branches but HEAD does not point\n+ *   to a valid reference.\n+ *\n  * Returns 1 if any of the previous checks are true, otherwise returns 0.\n  */\n static int can_use_local_refs(const struct add_opts *opts)\n@@ -645,6 +648,23 @@ static int can_use_local_refs(const struct add_opts *opts)\n \tif (head_ref(first_valid_ref, NULL)) {\n \t\treturn 1;\n \t} else if (for_each_branch_ref(first_valid_ref, NULL)) {\n+\t\tif (!opts->quiet) {\n+\t\t\tstruct strbuf path = STRBUF_INIT;\n+\t\t\tstruct strbuf contents = STRBUF_INIT;\n+\n+\t\t\tstrbuf_add_real_path(&path, get_worktree_git_dir(NULL));\n+\t\t\tstrbuf_addstr(&path, \"/HEAD\");\n+\t\t\tstrbuf_read_file(&contents, path.buf, 64);\n+\t\t\tstrbuf_stripspace(&contents, 0);\n+\t\t\tstrbuf_strip_suffix(&contents, \"\\n\");\n+\n+\t\t\twarning(_(\"HEAD points to an invalid (or orphaned) reference.\\n\"\n+\t\t\t\t  \"HEAD path: '%s'\\n\"\n+\t\t\t\t  \"HEAD contents: '%s'\"),\n+\t\t\t\t  path.buf, contents.buf);\n+\t\t\tstrbuf_release(&path);\n+\t\t\tstrbuf_release(&contents);\n+\t\t}\n \t\treturn 1;\n \t}\n \treturn 0;\n@@ -666,16 +686,12 @@ static int can_use_local_refs(const struct add_opts *opts)\n static int can_use_remote_refs(const struct add_opts *opts)\n {\n \tif (!guess_remote) {\n-\t\tif (!opts->quiet)\n-\t\t\tfprintf_ln(stderr, WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT);\n \t\treturn 0;\n \t} else if (for_each_remote_ref(first_valid_ref, NULL)) {\n \t\treturn 1;\n \t} else if (!opts->force && remote_get(NULL)) {\n \t\tdie(_(\"No local or remote refs exist despite at least one remote\\n\"\n \t\t      \"present, stopping; use 'add -f' to overide or fetch a remote first\"));\n-\t} else if (!opts->quiet) {\n-\t\tfprintf_ln(stderr, WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT);\n \t}\n \treturn 0;\n }\n@@ -828,8 +844,12 @@ static int add(int ac, const char **av, const char *prefix)\n \t\tint n;\n \t\tconst char *s = worktree_basename(path, &n);\n \t\tnew_branch = xstrndup(s, n);\n-\t} else if (opts.orphan || opts.detach) {\n+\t} else if (opts.orphan) {\n \t\t// No-op\n+\t} else if (opts.detach) {\n+\t\t// Check HEAD\n+\t\tif (!strcmp(branch, \"HEAD\"))\n+\t\t\tcan_use_local_refs(&opts);\n \t} else if (ac < 2 && new_branch) {\n \t\t// DWIM: Infer --orphan when repo has no refs.\n \t\topts.orphan = dwim_orphan(&opts, !!opt_track, 0);\n@@ -854,6 +874,10 @@ static int add(int ac, const char **av, const char *prefix)\n \t\t\t\tbranch = remote;\n \t\t\t}\n \t\t}\n+\n+\t\tif (!strcmp(branch, \"HEAD\"))\n+\t\t\tcan_use_local_refs(&opts);\n+\n \t}\n\n \tif (!opts.orphan && !lookup_commit_reference_by_name(branch)) {\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex c7ca8df586..0ac468e69e 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -722,6 +722,7 @@ test_dwim_orphan () {\n \tlocal use_quiet=0 &&\n \tlocal remote=0 &&\n \tlocal remote_ref=0 &&\n+\tlocal use_detach=0 &&\n \tlocal use_new_branch=0 &&\n\n \tlocal outcome=\"$1\" &&\n@@ -747,6 +748,10 @@ test_dwim_orphan () {\n \t\tsuccess=0 &&\n \t\toutcome_text='\"add\" error inferred \"--orphan\" gives illegal opts combo'\n \t\t;;\n+\t\"warn_bad_head\")\n+\t\tsuccess=0 &&\n+\t\toutcome_text='\"add\" error, warn on bad HEAD, hint use orphan'\n+\t\t;;\n \t*)\n \t\techo \"test_dwim_orphan(): invalid outcome: '$outcome'\" >&2 &&\n \t\treturn 1\n@@ -818,7 +823,7 @@ test_dwim_orphan () {\n \t\t\tcontext=\"$context, invalid (or orphan) HEAD\"\n \t\t\t;;\n\n-\t\t# Whether the code path is tested with the base add command or -b\n+\t\t# Whether the code path is tested with the base add command, -b, or --detach\n \t\t\"no_-b\")\n \t\t\tuse_new_branch=0 &&\n \t\t\tcontext=\"$context, no --branch\"\n@@ -827,6 +832,10 @@ test_dwim_orphan () {\n \t\t\tuse_new_branch=1 &&\n \t\t\tcontext=\"$context, --branch\"\n \t\t\t;;\n+\t\t\"detach\")\n+\t\t\tuse_detach=1 &&\n+\t\t\tcontext=\"$context, --detach\"\n+\t\t\t;;\n\n \t\t# Whether to check that all output is suppressed (except errors)\n \t\t# or that the output is as expected\n@@ -887,6 +896,9 @@ test_dwim_orphan () {\n \tif [ $use_new_branch -eq 1 ]\n \tthen\n \t\targs=\"$args -b foo\"\n+\telif [ $use_detach -eq 1 ]\n+\tthen\n+\t\targs=\"$args --detach\"\n \telse\n \t\tcontext=\"DWIM (no --branch), $context\"\n \tfi &&\n@@ -1023,6 +1035,10 @@ do\n \t\ttest_dwim_orphan 'infer' $dwim_test_args -b no_local_ref remote no_remote_ref no_guess_remote\n \t\ttest_dwim_orphan 'infer' $dwim_test_args -b no_local_ref remote no_remote_ref guess_remote\n \t\ttest_dwim_orphan 'infer' $dwim_test_args -b no_local_ref remote remote_ref guess_remote\n+\n+\t\ttest_dwim_orphan 'warn_bad_head' $dwim_test_args no_-b local_ref bad_head\n+\t\ttest_dwim_orphan 'warn_bad_head' $dwim_test_args -b local_ref bad_head\n+\t\ttest_dwim_orphan 'warn_bad_head' $dwim_test_args detach local_ref bad_head\n \tdone\n\n \ttest_dwim_orphan 'fatal_orphan_bad_combo' $quiet_mode no_-b no_checkout\n--\n2.39.3\n\n\n"}]}