Git Mailing List Archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees
@ 2023-04-17  9:33 Jacob Abel
  2023-04-17  9:33 ` [PATCH v9 1/8] worktree add: include -B in usage docs Jacob Abel
                   ` (9 more replies)
  0 siblings, 10 replies; 34+ messages in thread
From: Jacob Abel @ 2023-04-17  9:33 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

This patchset introduces the ability to create new worktrees from orphan/unborn
branches and introduces DWIM behavior to create worktrees from an orphan branch
when no valid refs exists locally in the repository (as is typical in newly
initialized repositories) or on a remote (when `--guess-remote` is used).

This addresses the issue of `git worktree add` failing when attempting to create
a worktree from a newly initialized repository (which can be seen in this SO
question [1]).

Note: The last 2 patches in this patchset were initially intended to be part of
a "part 2" patchset but given that they are so closely tied to this patchset I
have been developing the two together. I'm fine with either splitting the
patchset and resending it or leaving them rolled together, whichever is
easier/more convenient for everyone.

This patchset has eight parts:
  * adding `-B` to the usage docs (noticed during dev and it seemed too small
    to justify a separate submission)
  * updating test cases to still show the output of git commands when the test
    script is run with `-x` to aid in debugging.
  * adding a helper fn to simplify testing for mutual exclusion of options
    in `t/t2400-worktree-add.sh`
  * adding additional test cases to verify both that behavior doesn't change
    when using `--quiet` and that the extraneous output is properly suppressed.
  * adding the ability to create a worktree from an unborn/orphan branch
    to `git-worktree-add`
  * adding an advise for using --orphan when `git worktree add` fails due to
    a bad ref.
  * adding functionality to DWIM when there are no existing branches and the
    user likely intends to create an orphan branch.
  * updating worktree add to emit a warning (containing debug information
    about the current HEAD) when trying to use a HEAD that points to a
    non-existant (or unborn) reference and there exist other valid branches.

Changes from v8 (patches 1/8 - 6/8):
  * Rebase to a newer commit on main (from c03801e19c to 9857273be0) to bypass
    build failures caused by curl deprecation compile warnings & to eliminate
    merge conflicts. The below range diff is made against a v8 rebased against
    the same point
  * Touched up commit messages.
  * Shortened title for patch 6/8 to fit in 50 character limit.
  * Updated tests to print stderr on test exit for tests which capture stderr
    to improve debugging of individual test failures.
  * Added tests to verify `--quiet` is actually quiet and doesn't otherwise
    change command behavior.
  * Changed `--orphan` from an option to a flag that can be used either on its
    own or with `-b`/`-B` as requested in [2].
  * Pulled a conditional and `die()` out of `print_preparing_worktree_line()`
    so that it'd always be checked regardless of `--quiet`. This change was
    made because a bug was introduced in an early revision of v9 that caused
    behavior to differ depending on whether `--quiet` was supplied to the
    command. To limit the changes made, the original `die()` was left as a
    `BUG()`.
  * Moved `!lookup_commit_reference_by_name(branch)` check and `--orphan` hint
    to the line before the call to `print_preparing_worktree_line()` to combine
    with the conditional from the above/previous change.
  * Wrapped `--orphan` advice/hint in conditional to suppress display when
    using `--quiet`.
  * Updated `--orphan` advice/hint to match the `add -b branch dir/` vs
    `add dir/` syntax initially supplied by the user.
  * Updated `--orphan` hint tests to check presence on bad HEAD instead of
    empty repo.

Changes from v8 (patches 7/8 & 8/8):
  * Extended DWIM to infer `--orphan` when no other branches exist in the repo
    (or remotely when using `--guess-remote` while not using `-b`) [3][4].
  * Added checks to warn/die when inferring `--orphan` causes the set of
    supplied options & flags to produce an illegal combination.
  * Added extensive tests to verify new DWIM behavior.
  * Added a failure/warning when the user likely forgot to fetch from an
    upstream repo (i.e. when there is a remote, guess_remote is enabled, and
    there aren't any local or remote branches in the repo). Can be bypassed
    with `--force`/`-f` [3].
  * Updated documentation for worktree-add to mention `--orphan` when
    discussing situations where DWIM behavior does or does not occur.
  * Added a warning when the current namespace's HEAD points to an invalid
    or non-existant reference and the user is trying to create a new worktree
    from that HEAD.

1. https://stackoverflow.com/a/68717229/15064705/
2. https://lore.kernel.org/git/e5aadd5d-9b85-4dc9-e9f7-117892b4b283@dunelm.org.uk/
3. https://lore.kernel.org/git/20230119222003.qcdrhcsvjlyab6af@phi/
4. https://lore.kernel.org/git/20230118224020.vrytmeyt3vbanoh2@phi/

Jacob Abel (8):
  worktree add: include -B in usage docs
  t2400: print captured git output when finished
  t2400: refactor "worktree add" opt exclusion tests
  t2400: add tests to verify --quiet
  worktree add: add --orphan flag
  worktree add: introduce "try --orphan" hint
  worktree add: extend DWIM to infer --orphan
  worktree add: emit warn when there is a bad HEAD

 Documentation/config/advice.txt |   4 +
 Documentation/git-worktree.txt  |  16 +-
 advice.c                        |   1 +
 advice.h                        |   1 +
 builtin/worktree.c              | 226 +++++++++++++-
 t/t2400-worktree-add.sh         | 520 +++++++++++++++++++++++++++++++-
 6 files changed, 747 insertions(+), 21 deletions(-)

Range-diff against v8:
1:  cbda416378 = 1:  91153fdb4c worktree add: include -B in usage docs
-:  ---------- > 2:  8cfbc89dd5 t2400: print captured git output when finished
2:  5f83015779 ! 3:  ab03d92c3a worktree add: refactor opt exclusion tests
    @@ Metadata
     Author: Jacob Abel <jacobabel@nullpo.dev>

      ## Commit message ##
    -    worktree add: refactor opt exclusion tests
    +    t2400: refactor "worktree add" opt exclusion tests

         Pull duplicate test code into a function so that additional opt
         combinations can be tested succinctly.
    @@ t/t2400-worktree-add.sh: test_expect_success '"add" no auto-vivify with --detach
     +test_wt_add_excl () {
     +	local opts="$*" &&
     +	test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
    ++		test_when_finished cat actual >&2 &&
     +		test_must_fail git worktree add $opts 2>actual &&
     +		grep -E "fatal:( options)? .* cannot be used together" actual
     +	'
-:  ---------- > 4:  d9a3468c93 t2400: add tests to verify --quiet
3:  6ac19eeeae ! 5:  8ef9587deb worktree add: add --orphan flag
    @@ Metadata
      ## Commit message ##
         worktree add: add --orphan flag

    -    Adds support for creating an orphan branch when adding a new worktree.
    -    This functionality is equivalent to git switch's --orphan flag.
    -
    -    The original reason this feature was implemented was to allow a user
    -    to initialise a new repository using solely the worktree oriented
    -    workflow.
    +    Add support for creating an orphan branch when adding a new worktree.
    +    The functionality of this flag is equivalent to git switch's --orphan
    +    option.

         Current Behavior:
         % git -C foo.git --no-pager branch -l
    @@ Commit message
         % git -C bar.git worktree add main/
         Preparing worktree (new branch 'main')
         fatal: invalid reference: HEAD
    -    % git -C bar.git worktree add --orphan main main/
    +    % git -C bar.git worktree add --orphan -b main/
         Preparing worktree (new branch 'main')
    +    % git -C bar.git worktree add --orphan -b newbranch worktreedir/
    +    Preparing worktree (new branch 'newbranch')
         %

         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
    @@ Commit message

      ## Documentation/git-worktree.txt ##
     @@ Documentation/git-worktree.txt: SYNOPSIS
    + --------
      [verse]
      'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
    - 		   [(-b | -B) <new-branch>] <path> [<commit-ish>]
    -+'git worktree add' [-f] [--lock [--reason <string>]]
    -+		   --orphan <new-branch> <path>
    +-		   [(-b | -B) <new-branch>] <path> [<commit-ish>]
    ++		   [--orphan] [(-b | -B) <new-branch>] <path> [<commit-ish>]
      'git worktree list' [-v | --porcelain [-z]]
      'git worktree lock' [--reason <string>] <worktree>
      'git worktree move' <worktree> <new-path>
    -@@ Documentation/git-worktree.txt: exist, a new branch based on `HEAD` is automatically created as if
    - `-b <branch>` was given.  If `<branch>` does exist, it will be checked out
    - in the new worktree, if it's not checked out anywhere else, otherwise the
    - command will refuse to create the worktree (unless `--force` is used).
    -++
    -+------------
    -+$ git worktree add --orphan <branch> <path>
    -+------------
    -++
    -+Create a worktree containing no files, with an empty index, and associated
    -+with a new orphan branch named `<branch>`. The first commit made on this new
    -+branch will have no parents and will be the root of a new history disconnected
    -+from any other branches.
    -
    - list::
    -
     @@ Documentation/git-worktree.txt: This can also be set up as the default behaviour by using the
      	With `prune`, do not remove anything; just report what it would
      	remove.

    -+--orphan <new-branch>::
    ++--orphan::
     +	With `add`, make the new worktree and index empty, associating
    -+	the worktree with a new orphan branch named `<new-branch>`.
    ++	the worktree with a new orphan/unborn branch named `<new-branch>`.
     +
      --porcelain::
      	With `list`, output in an easy-to-parse format for scripts.
    @@ builtin/worktree.c
      #define BUILTIN_WORKTREE_ADD_USAGE \
      	N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
     -	   "                 [(-b | -B) <new-branch>] <path> [<commit-ish>]")
    -+	   "                 [(-b | -B) <new-branch>] <path> [<commit-ish>]"), \
    -+	N_("git worktree add [-f] [--lock [--reason <string>]]\n" \
    -+	   "                 --orphan <new-branch> <path>")
    ++	   "                 [--orphan] [(-b | -B) <new-branch>] <path> [<commit-ish>]")
     +
      #define BUILTIN_WORKTREE_LIST_USAGE \
      	N_("git worktree list [-v | --porcelain [-z]]")
    @@ builtin/worktree.c: static int add_worktree(const char *path, const char *refnam
      		struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;

      		strvec_pushl(&opt.env, "GIT_DIR", "GIT_WORK_TREE", NULL);
    -@@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix)
    - 	char *path;
    - 	const char *branch;
    - 	const char *new_branch = NULL;
    -+	const char *orphan_branch = NULL;
    - 	const char *opt_track = NULL;
    - 	const char *lock_reason = NULL;
    - 	int keep_locked = 0;
    +@@ builtin/worktree.c: static void print_preparing_worktree_line(int detach,
    + 		else {
    + 			struct commit *commit = lookup_commit_reference_by_name(branch);
    + 			if (!commit)
    +-				die(_("invalid reference: %s"), branch);
    ++				BUG(_("unreachable: invalid reference: %s"), branch);
    + 			fprintf_ln(stderr, _("Preparing worktree (detached HEAD %s)"),
    + 				  repo_find_unique_abbrev(the_repository, &commit->object.oid, DEFAULT_ABBREV));
    + 		}
     @@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix)
      			   N_("create a new branch")),
      		OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
      			   N_("create or reset a branch")),
    -+		OPT_STRING(0, "orphan", &orphan_branch, N_("branch"),
    -+			   N_("new unparented branch")),
    ++		OPT_BOOL(0, "orphan", &opts.orphan, N_("create unborn/orphaned branch")),
      		OPT_BOOL('d', "detach", &opts.detach, N_("detach HEAD at named commit")),
      		OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new working tree")),
      		OPT_BOOL(0, "lock", &keep_locked, N_("keep the new working tree locked")),
     @@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix)
    - 	memset(&opts, 0, sizeof(opts));
    - 	opts.checkout = 1;
      	ac = parse_options(ac, av, prefix, options, git_worktree_add_usage, 0);
    -+	opts.orphan = !!orphan_branch;
      	if (!!opts.detach + !!new_branch + !!new_branch_force > 1)
      		die(_("options '%s', '%s', and '%s' cannot be used together"), "-b", "-B", "--detach");
    -+	if (!!opts.detach + !!opts.orphan + !!new_branch + !!new_branch_force > 1)
    -+		die(_("options '%s', '%s', '%s', and '%s' cannot be used together"),
    -+		    "-b", "-B", "--orphan", "--detach");
    ++	if (opts.detach && opts.orphan)
    ++		die(_("options '%s', and '%s' cannot be used together"),
    ++		    "--orphan", "--detach");
     +	if (opts.orphan && opt_track)
     +		die(_("'%s' and '%s' cannot be used together"), "--orphan", "--track");
     +	if (opts.orphan && !opts.checkout)
    @@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix)
      	}

     -	if (ac < 2 && !new_branch && !opts.detach) {
    -+	if (opts.orphan) {
    -+		new_branch = orphan_branch;
    -+	} else if (ac < 2 && !new_branch && !opts.detach) {
    ++	if (opts.orphan && !new_branch) {
    ++		int n;
    ++		const char *s = worktree_basename(path, &n);
    ++		new_branch = xstrndup(s, n);
    ++	} else if (new_branch || opts.detach || opts.orphan) {
    ++		// No-op
    ++	} else if (ac < 2) {
      		const char *s = dwim_branch(path, &new_branch);
      		if (s)
      			branch = s;
    +-	}
    +-
    +-	if (ac == 2 && !new_branch && !opts.detach) {
    ++	} else if (ac == 2) {
    + 		struct object_id oid;
    + 		struct commit *commit;
    + 		const char *remote;
     @@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix)
    + 			}
    + 		}
    + 	}
    ++
    ++	if (!opts.orphan && !lookup_commit_reference_by_name(branch)) {
    ++		die(_("invalid reference: %s"), branch);
    ++	}
    ++
      	if (!opts.quiet)
      		print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);

     -	if (new_branch) {
     +	if (opts.orphan) {
     +		branch = new_branch;
    -+	} else if (!lookup_commit_reference_by_name(branch)) {
    -+		die(_("invalid reference: %s"), branch);
     +	} else if (new_branch) {
      		struct child_process cp = CHILD_PROCESS_INIT;
      		cp.git_cmd = 1;
    @@ t/t2400-worktree-add.sh: test_wt_add_excl () {
      test_wt_add_excl -b poodle -B poodle bamboo main
      test_wt_add_excl -b poodle --detach bamboo main
      test_wt_add_excl -B poodle --detach bamboo main
    -+test_wt_add_excl -B poodle --orphan poodle bamboo
    -+test_wt_add_excl -b poodle --orphan poodle bamboo
    -+test_wt_add_excl --orphan poodle --detach bamboo
    -+test_wt_add_excl --orphan poodle --no-checkout bamboo
    -+test_wt_add_excl --orphan poodle bamboo main
    ++test_wt_add_excl --orphan --detach bamboo
    ++test_wt_add_excl --orphan --no-checkout bamboo
    ++test_wt_add_excl --orphan bamboo main
    ++test_wt_add_excl --orphan -b bamboo wtdir/ main

      test_expect_success '"add -B" fails if the branch is checked out' '
      	git rev-parse newmain >before &&
    -@@ t/t2400-worktree-add.sh: test_expect_success 'add --quiet' '
    +@@ t/t2400-worktree-add.sh: test_expect_success 'add --quiet -b' '
      	test_must_be_empty actual
      '

     +test_expect_success '"add --orphan"' '
     +	test_when_finished "git worktree remove -f -f orphandir" &&
    -+	git worktree add --orphan neworphan orphandir &&
    ++	git worktree add --orphan -b neworphan orphandir &&
    ++	echo refs/heads/neworphan >expected &&
    ++	git -C orphandir symbolic-ref HEAD >actual &&
    ++	test_cmp expected actual
    ++'
    ++
    ++test_expect_success '"add --orphan (no -b)"' '
    ++	test_when_finished "git worktree remove -f -f neworphan" &&
    ++	git worktree add --orphan neworphan &&
    ++	echo refs/heads/neworphan >expected &&
    ++	git -C neworphan symbolic-ref HEAD >actual &&
    ++	test_cmp expected actual
    ++'
    ++
    ++test_expect_success '"add --orphan --quiet"' '
    ++	test_when_finished "git worktree remove -f -f orphandir" &&
    ++	test_when_finished cat log.actual >&2 &&
    ++	git worktree add --quiet --orphan -b neworphan orphandir 2>log.actual &&
    ++	test_must_be_empty log.actual &&
     +	echo refs/heads/neworphan >expected &&
     +	git -C orphandir symbolic-ref HEAD >actual &&
     +	test_cmp expected actual
    @@ t/t2400-worktree-add.sh: test_expect_success 'add --quiet' '
     +
     +test_expect_success '"add --orphan" fails if the branch already exists' '
     +	test_when_finished "git branch -D existingbranch" &&
    -+	test_when_finished "git worktree remove -f -f orphandir" &&
     +	git worktree add -b existingbranch orphandir main &&
    -+	test_must_fail git worktree add --orphan existingbranch orphandir2 &&
    -+	test_path_is_missing orphandir2
    ++	git worktree remove orphandir &&
    ++	test_must_fail git worktree add --orphan -b existingbranch orphandir
     +'
     +
     +test_expect_success '"add --orphan" with empty repository' '
     +	test_when_finished "rm -rf empty_repo" &&
     +	echo refs/heads/newbranch >expected &&
     +	GIT_DIR="empty_repo" git init --bare &&
    -+	git -C empty_repo  worktree add --orphan newbranch worktreedir &&
    ++	git -C empty_repo  worktree add --orphan -b newbranch worktreedir &&
     +	git -C empty_repo/worktreedir symbolic-ref HEAD >actual &&
     +	test_cmp expected actual
     +'
     +
     +test_expect_success '"add" worktree with orphan branch and lock' '
    -+	git worktree add --lock --orphan orphanbr orphan-with-lock &&
    ++	git worktree add --lock --orphan -b orphanbr orphan-with-lock &&
     +	test_when_finished "git worktree unlock orphan-with-lock || :" &&
     +	test -f .git/worktrees/orphan-with-lock/locked
     +'
4:  3d76a5b6b8 ! 6:  d2800266f9 worktree add: add hint to direct users towards --orphan
    @@ Metadata
     Author: Jacob Abel <jacobabel@nullpo.dev>

      ## Commit message ##
    -    worktree add: add hint to direct users towards --orphan
    +    worktree add: introduce "try --orphan" hint

    -    Adds a new advice/hint in `git worktree add` for when the user
    +    Add a new advice/hint in `git worktree add` for when the user
         tries to create a new worktree from a reference that doesn't exist.

         Current Behavior:

    -    % git init --bare foo.git
    -    Initialized empty Git repository in /path/to/foo.git/
    -    % git -C foo.git worktree add main/
    -    Preparing worktree (new branch 'main')
    +    % git init foo
    +    Initialized empty Git repository in /path/to/foo/
    +    % touch file
    +    % git -C foo commit -q -a -m "test commit"
    +    % git -C foo switch --orphan norefbranch
    +    % git -C foo worktree add newbranch/
    +    Preparing worktree (new branch 'newbranch')
         fatal: invalid reference: HEAD
         %

         New Behavior:

    -    % git init --bare foo.git
    -    Initialized empty Git repository in /path/to/foo.git/
    -    % git -C foo.git worktree add main/
    -    Preparing worktree (new branch 'main')
    +    % git init --bare foo
    +    Initialized empty Git repository in /path/to/foo/
    +    % touch file
    +    % git -C foo commit -q -a -m "test commit"
    +    % git -C foo switch --orphan norefbranch
    +    % git -C foo worktree add newbranch/
    +    Preparing worktree (new branch 'newbranch')
         hint: If you meant to create a worktree containing a new orphan branch
         hint: (branch with no commits) for this repository, you can do so
         hint: using the --orphan option:
         hint:
    -    hint:   git worktree add --orphan main ./main
    +    hint:   git worktree add --orphan newbranch/
    +    hint:
    +    hint: Disable this message with "git config advice.worktreeAddOrphan false"
    +    fatal: invalid reference: HEAD
    +    % git -C foo worktree add -b newbranch2 new_wt/
    +    Preparing worktree (new branch 'newbranch')
    +    hint: If you meant to create a worktree containing a new orphan branch
    +    hint: (branch with no commits) for this repository, you can do so
    +    hint: using the --orphan option:
    +    hint:
    +    hint:   git worktree add --orphan -b newbranch2 new_wt/
         hint:
         hint: Disable this message with "git config advice.worktreeAddOrphan false"
         fatal: invalid reference: HEAD
    @@ advice.h: struct string_list;
      int git_default_advice_config(const char *var, const char *value);

      ## builtin/worktree.c ##
    +@@
    + #define BUILTIN_WORKTREE_UNLOCK_USAGE \
    + 	N_("git worktree unlock <worktree>")
    +
    ++#define WORKTREE_ADD_ORPHAN_WITH_DASH_B_HINT_TEXT \
    ++	_("If you meant to create a worktree containing a new orphan branch\n" \
    ++	"(branch with no commits) for this repository, you can do so\n" \
    ++	"using the --orphan flag:\n" \
    ++	"\n" \
    ++	"	git worktree add --orphan -b %s %s\n")
    ++
    ++#define WORKTREE_ADD_ORPHAN_NO_DASH_B_HINT_TEXT \
    ++	_("If you meant to create a worktree containing a new orphan branch\n" \
    ++	"(branch with no commits) for this repository, you can do so\n" \
    ++	"using the --orphan flag:\n" \
    ++	"\n" \
    ++	"	git worktree add --orphan %s\n")
    ++
    + static const char * const git_worktree_usage[] = {
    + 	BUILTIN_WORKTREE_ADD_USAGE,
    + 	BUILTIN_WORKTREE_LIST_USAGE,
    +@@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix)
    + 	const char *opt_track = NULL;
    + 	const char *lock_reason = NULL;
    + 	int keep_locked = 0;
    ++	int used_new_branch_options;
    + 	struct option options[] = {
    + 		OPT__FORCE(&opts.force,
    + 			   N_("checkout <branch> even if already checked out in other worktree"),
    +@@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix)
    +
    + 	path = prefix_filename(prefix, av[0]);
    + 	branch = ac < 2 ? "HEAD" : av[1];
    ++	used_new_branch_options = new_branch || new_branch_force;
    +
    + 	if (!strcmp(branch, "-"))
    + 		branch = "@{-1}";
     @@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix)
    - 	if (opts.orphan) {
    - 		branch = new_branch;
    - 	} else if (!lookup_commit_reference_by_name(branch)) {
    -+		advise_if_enabled(ADVICE_WORKTREE_ADD_ORPHAN,
    -+			_("If you meant to create a worktree containing a new orphan branch\n"
    -+			"(branch with no commits) for this repository, you can do so\n"
    -+			"using the --orphan option:\n"
    -+			"\n"
    -+			"	git worktree add --orphan %s %s\n"), new_branch, path);
    + 	}
    +
    + 	if (!opts.orphan && !lookup_commit_reference_by_name(branch)) {
    ++		int attempt_hint = !opts.quiet && (ac < 2);
    ++		if (attempt_hint && used_new_branch_options) {
    ++			advise_if_enabled(ADVICE_WORKTREE_ADD_ORPHAN,
    ++				WORKTREE_ADD_ORPHAN_WITH_DASH_B_HINT_TEXT,
    ++				new_branch, path);
    ++		} else if (attempt_hint) {
    ++			advise_if_enabled(ADVICE_WORKTREE_ADD_ORPHAN,
    ++				WORKTREE_ADD_ORPHAN_NO_DASH_B_HINT_TEXT, path);
    ++		}
      		die(_("invalid reference: %s"), branch);
    - 	} else if (new_branch) {
    - 		struct child_process cp = CHILD_PROCESS_INIT;
    + 	}
    +

      ## t/t2400-worktree-add.sh ##
     @@ t/t2400-worktree-add.sh: test_expect_success '"add" worktree with orphan branch, lock, and reason' '
    @@ t/t2400-worktree-add.sh: test_expect_success '"add" worktree with orphan branch,
      '

     +# Note: Quoted arguments containing spaces are not supported.
    -+test_wt_add_empty_repo_orphan_hint () {
    ++test_wt_add_orphan_hint () {
     +	local context="$1" &&
    -+	shift &&
    ++	local use_branch=$2 &&
    ++	shift 2 &&
     +	local opts="$*" &&
    -+	test_expect_success "'worktree add' show orphan hint in empty repo w/ $context" '
    -+		test_when_finished "rm -rf empty_repo" &&
    -+		GIT_DIR="empty_repo" git init --bare &&
    -+		test_must_fail git -C empty_repo worktree add $opts foobar/ 2>actual &&
    ++	test_expect_success "'worktree add' show orphan hint in bad/orphan HEAD w/ $context" '
    ++		test_when_finished "rm -rf repo" &&
    ++		git init repo &&
    ++		(cd repo && test_commit commit) &&
    ++		git -C repo switch --orphan noref &&
    ++		test_when_finished cat actual >&2 &&
    ++		test_must_fail git -C repo worktree add $opts foobar/ 2>actual &&
     +		! grep "error: unknown switch" actual &&
    -+		grep "hint: If you meant to create a worktree containing a new orphan branch" actual
    ++		grep "hint: If you meant to create a worktree containing a new orphan branch" actual &&
    ++		if [ $use_branch -eq 1 ]
    ++		then
    ++			grep -E "^hint:\s+git worktree add --orphan -b \S+ \S+\s*$" actual
    ++		else
    ++			grep -E "^hint:\s+git worktree add --orphan \S+\s*$" actual
    ++		fi
    ++
     +	'
     +}
     +
    -+test_wt_add_empty_repo_orphan_hint 'DWIM'
    -+test_wt_add_empty_repo_orphan_hint '-b' -b foobar_branch
    -+test_wt_add_empty_repo_orphan_hint '-B' -B foobar_branch
    ++test_wt_add_orphan_hint 'no opts' 0
    ++test_wt_add_orphan_hint '-b' 1 -b foobar_branch
    ++test_wt_add_orphan_hint '-B' 1 -B foobar_branch
    ++
    ++test_expect_success "'worktree add' doesn't show orphan hint in bad/orphan HEAD w/ --quiet" '
    ++	test_when_finished "rm -rf repo" &&
    ++	git init repo &&
    ++	(cd repo && test_commit commit) &&
    ++	test_when_finished cat actual >&2 &&
    ++	test_must_fail git -C repo worktree add --quiet foobar_branch foobar/ 2>actual &&
    ++	! grep "error: unknown switch" actual &&
    ++	! grep "hint: If you meant to create a worktree containing a new orphan branch" actual
    ++'
     +
      test_expect_success 'local clone from linked checkout' '
      	git clone --local here here-clone &&
-:  ---------- > 7:  e5e139766c worktree add: extend DWIM to infer --orphan
-:  ---------- > 8:  296226ffd5 worktree add: emit warn when there is a bad HEAD
--
2.39.2



^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH v9 1/8] worktree add: include -B in usage docs
  2023-04-17  9:33 [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
@ 2023-04-17  9:33 ` Jacob Abel
  2023-04-17  9:33 ` [PATCH v9 2/8] t2400: print captured git output when finished Jacob Abel
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-04-17  9:33 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Document `-B` next to where `-b` is already documented to bring the
usage docs in line with other commands such as git checkout.

Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 Documentation/git-worktree.txt | 2 +-
 builtin/worktree.c             | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 063d6eeb99..b9c12779f1 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
-		   [-b <new-branch>] <path> [<commit-ish>]
+		   [(-b | -B) <new-branch>] <path> [<commit-ish>]
 'git worktree list' [-v | --porcelain [-z]]
 'git worktree lock' [--reason <string>] <worktree>
 'git worktree move' <worktree> <new-path>
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 39e9e5c9ce..d1b4b53f2c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -22,7 +22,7 @@

 #define BUILTIN_WORKTREE_ADD_USAGE \
 	N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
-	   "                 [-b <new-branch>] <path> [<commit-ish>]")
+	   "                 [(-b | -B) <new-branch>] <path> [<commit-ish>]")
 #define BUILTIN_WORKTREE_LIST_USAGE \
 	N_("git worktree list [-v | --porcelain [-z]]")
 #define BUILTIN_WORKTREE_LOCK_USAGE \
--
2.39.2



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v9 2/8] t2400: print captured git output when finished
  2023-04-17  9:33 [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
  2023-04-17  9:33 ` [PATCH v9 1/8] worktree add: include -B in usage docs Jacob Abel
@ 2023-04-17  9:33 ` Jacob Abel
  2023-04-17 21:09   ` Junio C Hamano
  2023-04-17  9:33 ` [PATCH v9 3/8] t2400: refactor "worktree add" opt exclusion tests Jacob Abel
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 34+ messages in thread
From: Jacob Abel @ 2023-04-17  9:33 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Update tests that capture stderr so that at the end of the test they
print the captured text back out to stderr. This simplifies debugging
when inspecting test logs after executing with `-x`.

Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 t/t2400-worktree-add.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index d587e0b20d..9bc3db20e4 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -326,6 +326,8 @@ test_expect_success 'add -B' '
 '

 test_expect_success 'add --quiet' '
+	test_when_finished "git worktree remove -f -f another-worktree" &&
+	test_when_finished cat actual >&2 &&
 	git worktree add --quiet another-worktree main 2>actual &&
 	test_must_be_empty actual
 '
--
2.39.2



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v9 3/8] t2400: refactor "worktree add" opt exclusion tests
  2023-04-17  9:33 [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
  2023-04-17  9:33 ` [PATCH v9 1/8] worktree add: include -B in usage docs Jacob Abel
  2023-04-17  9:33 ` [PATCH v9 2/8] t2400: print captured git output when finished Jacob Abel
@ 2023-04-17  9:33 ` Jacob Abel
  2023-04-17 21:30   ` Junio C Hamano
  2023-04-17  9:34 ` [PATCH v9 4/8] t2400: add tests to verify --quiet Jacob Abel
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 34+ messages in thread
From: Jacob Abel @ 2023-04-17  9:33 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Pull duplicate test code into a function so that additional opt
combinations can be tested succinctly.

Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 t/t2400-worktree-add.sh | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index 9bc3db20e4..6822be4666 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -298,17 +298,21 @@ test_expect_success '"add" no auto-vivify with --detach and <branch> omitted' '
 	test_must_fail git -C mish/mash symbolic-ref HEAD
 '

-test_expect_success '"add" -b/-B mutually exclusive' '
-	test_must_fail git worktree add -b poodle -B poodle bamboo main
-'
-
-test_expect_success '"add" -b/--detach mutually exclusive' '
-	test_must_fail git worktree add -b poodle --detach bamboo main
-'
+# Helper function to test mutually exclusive options.
+#
+# Note: Quoted arguments containing spaces are not supported.
+test_wt_add_excl () {
+	local opts="$*" &&
+	test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
+		test_when_finished cat actual >&2 &&
+		test_must_fail git worktree add $opts 2>actual &&
+		grep -E "fatal:( options)? .* cannot be used together" actual
+	'
+}

-test_expect_success '"add" -B/--detach mutually exclusive' '
-	test_must_fail git worktree add -B poodle --detach bamboo main
-'
+test_wt_add_excl -b poodle -B poodle bamboo main
+test_wt_add_excl -b poodle --detach bamboo main
+test_wt_add_excl -B poodle --detach bamboo main

 test_expect_success '"add -B" fails if the branch is checked out' '
 	git rev-parse newmain >before &&
--
2.39.2



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v9 4/8] t2400: add tests to verify --quiet
  2023-04-17  9:33 [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
                   ` (2 preceding siblings ...)
  2023-04-17  9:33 ` [PATCH v9 3/8] t2400: refactor "worktree add" opt exclusion tests Jacob Abel
@ 2023-04-17  9:34 ` Jacob Abel
  2023-04-17 21:33   ` Junio C Hamano
  2023-04-17  9:34 ` [PATCH v9 5/8] worktree add: add --orphan flag Jacob Abel
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 34+ messages in thread
From: Jacob Abel @ 2023-04-17  9:34 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Add tests to verify that the command performs operations the same with
`--quiet` as without it. Additionally verifies that all non-fatal output
is suppressed.

Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 t/t2400-worktree-add.sh | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index 6822be4666..18831c4d93 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -336,6 +336,14 @@ test_expect_success 'add --quiet' '
 	test_must_be_empty actual
 '

+test_expect_success 'add --quiet -b' '
+	test_when_finished "git branch -D quietnewbranch" &&
+	test_when_finished "git worktree remove -f -f another-worktree" &&
+	test_when_finished cat actual >&2 &&
+	git worktree add --quiet -b quietnewbranch another-worktree 2>actual &&
+	test_must_be_empty actual
+'
+
 test_expect_success 'local clone from linked checkout' '
 	git clone --local here here-clone &&
 	( cd here-clone && git fsck )
@@ -534,6 +542,36 @@ test_expect_success 'git worktree add --guess-remote sets up tracking' '
 		test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo
 	)
 '
+test_expect_success 'git worktree add --guess-remote sets up tracking (quiet)' '
+	test_when_finished rm -rf repo_a repo_b foo &&
+	test_when_finished cat repo_b/actual >&2 &&
+	setup_remote_repo repo_a repo_b &&
+	(
+		cd repo_b &&
+		git worktree add --quiet --guess-remote ../foo 2>actual &&
+		test_must_be_empty actual
+	) &&
+	(
+		cd foo &&
+		test_branch_upstream foo repo_a foo &&
+		test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo
+	)
+'
+
+test_expect_success 'git worktree --no-guess-remote (quiet)' '
+	test_when_finished rm -rf repo_a repo_b foo &&
+	setup_remote_repo repo_a repo_b &&
+	(
+		cd repo_b &&
+		git worktree add --quiet --no-guess-remote ../foo
+	) &&
+	(
+		cd foo &&
+		test_must_fail git config "branch.foo.remote" &&
+		test_must_fail git config "branch.foo.merge" &&
+		test_cmp_rev ! refs/remotes/repo_a/foo refs/heads/foo
+	)
+'

 test_expect_success 'git worktree add with worktree.guessRemote sets up tracking' '
 	test_when_finished rm -rf repo_a repo_b foo &&
--
2.39.2



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v9 5/8] worktree add: add --orphan flag
  2023-04-17  9:33 [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
                   ` (3 preceding siblings ...)
  2023-04-17  9:34 ` [PATCH v9 4/8] t2400: add tests to verify --quiet Jacob Abel
@ 2023-04-17  9:34 ` Jacob Abel
  2023-04-17  9:34 ` [PATCH v9 6/8] worktree add: introduce "try --orphan" hint Jacob Abel
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-04-17  9:34 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Add support for creating an orphan branch when adding a new worktree.
The functionality of this flag is equivalent to git switch's --orphan
option.

Current Behavior:
% git -C foo.git --no-pager branch -l
+ main
% git -C foo.git worktree add main/
Preparing worktree (new branch 'main')
HEAD is now at 6c93a75 a commit
%

% git init bar.git
Initialized empty Git repository in /path/to/bar.git/
% git -C bar.git --no-pager branch -l

% git -C bar.git worktree add main/
Preparing worktree (new branch 'main')
fatal: not a valid object name: 'HEAD'
%

New Behavior:

% git -C foo.git --no-pager branch -l
+ main
% git -C foo.git worktree add main/
Preparing worktree (new branch 'main')
HEAD is now at 6c93a75 a commit
%

% git init --bare bar.git
Initialized empty Git repository in /path/to/bar.git/
% git -C bar.git --no-pager branch -l

% git -C bar.git worktree add main/
Preparing worktree (new branch 'main')
fatal: invalid reference: HEAD
% git -C bar.git worktree add --orphan -b main/
Preparing worktree (new branch 'main')
% git -C bar.git worktree add --orphan -b newbranch worktreedir/
Preparing worktree (new branch 'newbranch')
%

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 Documentation/git-worktree.txt |  6 ++-
 builtin/worktree.c             | 67 +++++++++++++++++++++++++++------
 t/t2400-worktree-add.sh        | 69 ++++++++++++++++++++++++++++++++++
 3 files changed, 130 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index b9c12779f1..485d865eb2 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
-		   [(-b | -B) <new-branch>] <path> [<commit-ish>]
+		   [--orphan] [(-b | -B) <new-branch>] <path> [<commit-ish>]
 'git worktree list' [-v | --porcelain [-z]]
 'git worktree lock' [--reason <string>] <worktree>
 'git worktree move' <worktree> <new-path>
@@ -222,6 +222,10 @@ This can also be set up as the default behaviour by using the
 	With `prune`, do not remove anything; just report what it would
 	remove.

+--orphan::
+	With `add`, make the new worktree and index empty, associating
+	the worktree with a new orphan/unborn branch named `<new-branch>`.
+
 --porcelain::
 	With `list`, output in an easy-to-parse format for scripts.
 	This format will remain stable across Git versions and regardless of user
diff --git a/builtin/worktree.c b/builtin/worktree.c
index d1b4b53f2c..48de7fc3b0 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -22,7 +22,8 @@

 #define BUILTIN_WORKTREE_ADD_USAGE \
 	N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
-	   "                 [(-b | -B) <new-branch>] <path> [<commit-ish>]")
+	   "                 [--orphan] [(-b | -B) <new-branch>] <path> [<commit-ish>]")
+
 #define BUILTIN_WORKTREE_LIST_USAGE \
 	N_("git worktree list [-v | --porcelain [-z]]")
 #define BUILTIN_WORKTREE_LOCK_USAGE \
@@ -95,6 +96,7 @@ struct add_opts {
 	int detach;
 	int quiet;
 	int checkout;
+	int orphan;
 	const char *keep_locked;
 };

@@ -368,6 +370,22 @@ static int checkout_worktree(const struct add_opts *opts,
 	return run_command(&cp);
 }

+static int make_worktree_orphan(const char * ref, const struct add_opts *opts,
+				struct strvec *child_env)
+{
+	struct strbuf symref = STRBUF_INIT;
+	struct child_process cp = CHILD_PROCESS_INIT;
+
+	validate_new_branchname(ref, &symref, 0);
+	strvec_pushl(&cp.args, "symbolic-ref", "HEAD", symref.buf, NULL);
+	if (opts->quiet)
+		strvec_push(&cp.args, "--quiet");
+	strvec_pushv(&cp.env, child_env->v);
+	strbuf_release(&symref);
+	cp.git_cmd = 1;
+	return run_command(&cp);
+}
+
 static int add_worktree(const char *path, const char *refname,
 			const struct add_opts *opts)
 {
@@ -397,7 +415,7 @@ static int add_worktree(const char *path, const char *refname,
 			die_if_checked_out(symref.buf, 0);
 	}
 	commit = lookup_commit_reference_by_name(refname);
-	if (!commit)
+	if (!commit && !opts->orphan)
 		die(_("invalid reference: %s"), refname);

 	name = worktree_basename(path, &len);
@@ -486,10 +504,10 @@ static int add_worktree(const char *path, const char *refname,
 	strvec_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
 	cp.git_cmd = 1;

-	if (!is_branch)
+	if (!is_branch && commit) {
 		strvec_pushl(&cp.args, "update-ref", "HEAD",
 			     oid_to_hex(&commit->object.oid), NULL);
-	else {
+	} else {
 		strvec_pushl(&cp.args, "symbolic-ref", "HEAD",
 			     symref.buf, NULL);
 		if (opts->quiet)
@@ -501,6 +519,10 @@ static int add_worktree(const char *path, const char *refname,
 	if (ret)
 		goto done;

+	if (opts->orphan &&
+	    (ret = make_worktree_orphan(refname, opts, &child_env)))
+		goto done;
+
 	if (opts->checkout &&
 	    (ret = checkout_worktree(opts, &child_env)))
 		goto done;
@@ -520,7 +542,7 @@ static int add_worktree(const char *path, const char *refname,
 	 * Hook failure does not warrant worktree deletion, so run hook after
 	 * is_junk is cleared, but do return appropriate code when hook fails.
 	 */
-	if (!ret && opts->checkout) {
+	if (!ret && opts->checkout && !opts->orphan) {
 		struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;

 		strvec_pushl(&opt.env, "GIT_DIR", "GIT_WORK_TREE", NULL);
@@ -568,7 +590,7 @@ static void print_preparing_worktree_line(int detach,
 		else {
 			struct commit *commit = lookup_commit_reference_by_name(branch);
 			if (!commit)
-				die(_("invalid reference: %s"), branch);
+				BUG(_("unreachable: invalid reference: %s"), branch);
 			fprintf_ln(stderr, _("Preparing worktree (detached HEAD %s)"),
 				  repo_find_unique_abbrev(the_repository, &commit->object.oid, DEFAULT_ABBREV));
 		}
@@ -620,6 +642,7 @@ static int add(int ac, const char **av, const char *prefix)
 			   N_("create a new branch")),
 		OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
 			   N_("create or reset a branch")),
+		OPT_BOOL(0, "orphan", &opts.orphan, N_("create unborn/orphaned branch")),
 		OPT_BOOL('d', "detach", &opts.detach, N_("detach HEAD at named commit")),
 		OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new working tree")),
 		OPT_BOOL(0, "lock", &keep_locked, N_("keep the new working tree locked")),
@@ -640,6 +663,17 @@ static int add(int ac, const char **av, const char *prefix)
 	ac = parse_options(ac, av, prefix, options, git_worktree_add_usage, 0);
 	if (!!opts.detach + !!new_branch + !!new_branch_force > 1)
 		die(_("options '%s', '%s', and '%s' cannot be used together"), "-b", "-B", "--detach");
+	if (opts.detach && opts.orphan)
+		die(_("options '%s', and '%s' cannot be used together"),
+		    "--orphan", "--detach");
+	if (opts.orphan && opt_track)
+		die(_("'%s' and '%s' cannot be used together"), "--orphan", "--track");
+	if (opts.orphan && !opts.checkout)
+		die(_("'%s' and '%s' cannot be used together"), "--orphan",
+		    "--no-checkout");
+	if (opts.orphan && ac == 2)
+		die(_("'%s' and '%s' cannot be used together"), "--orphan",
+		    _("<commit-ish>"));
 	if (lock_reason && !keep_locked)
 		die(_("the option '%s' requires '%s'"), "--reason", "--lock");
 	if (lock_reason)
@@ -668,13 +702,17 @@ static int add(int ac, const char **av, const char *prefix)
 		strbuf_release(&symref);
 	}

-	if (ac < 2 && !new_branch && !opts.detach) {
+	if (opts.orphan && !new_branch) {
+		int n;
+		const char *s = worktree_basename(path, &n);
+		new_branch = xstrndup(s, n);
+	} else if (new_branch || opts.detach || opts.orphan) {
+		// No-op
+	} else if (ac < 2) {
 		const char *s = dwim_branch(path, &new_branch);
 		if (s)
 			branch = s;
-	}
-
-	if (ac == 2 && !new_branch && !opts.detach) {
+	} else if (ac == 2) {
 		struct object_id oid;
 		struct commit *commit;
 		const char *remote;
@@ -688,10 +726,17 @@ static int add(int ac, const char **av, const char *prefix)
 			}
 		}
 	}
+
+	if (!opts.orphan && !lookup_commit_reference_by_name(branch)) {
+		die(_("invalid reference: %s"), branch);
+	}
+
 	if (!opts.quiet)
 		print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);

-	if (new_branch) {
+	if (opts.orphan) {
+		branch = new_branch;
+	} else if (new_branch) {
 		struct child_process cp = CHILD_PROCESS_INIT;
 		cp.git_cmd = 1;
 		strvec_push(&cp.args, "branch");
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index 18831c4d93..2ea4342867 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -313,6 +313,10 @@ test_wt_add_excl () {
 test_wt_add_excl -b poodle -B poodle bamboo main
 test_wt_add_excl -b poodle --detach bamboo main
 test_wt_add_excl -B poodle --detach bamboo main
+test_wt_add_excl --orphan --detach bamboo
+test_wt_add_excl --orphan --no-checkout bamboo
+test_wt_add_excl --orphan bamboo main
+test_wt_add_excl --orphan -b bamboo wtdir/ main

 test_expect_success '"add -B" fails if the branch is checked out' '
 	git rev-parse newmain >before &&
@@ -344,6 +348,63 @@ test_expect_success 'add --quiet -b' '
 	test_must_be_empty actual
 '

+test_expect_success '"add --orphan"' '
+	test_when_finished "git worktree remove -f -f orphandir" &&
+	git worktree add --orphan -b neworphan orphandir &&
+	echo refs/heads/neworphan >expected &&
+	git -C orphandir symbolic-ref HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '"add --orphan (no -b)"' '
+	test_when_finished "git worktree remove -f -f neworphan" &&
+	git worktree add --orphan neworphan &&
+	echo refs/heads/neworphan >expected &&
+	git -C neworphan symbolic-ref HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '"add --orphan --quiet"' '
+	test_when_finished "git worktree remove -f -f orphandir" &&
+	test_when_finished cat log.actual >&2 &&
+	git worktree add --quiet --orphan -b neworphan orphandir 2>log.actual &&
+	test_must_be_empty log.actual &&
+	echo refs/heads/neworphan >expected &&
+	git -C orphandir symbolic-ref HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '"add --orphan" fails if the branch already exists' '
+	test_when_finished "git branch -D existingbranch" &&
+	git worktree add -b existingbranch orphandir main &&
+	git worktree remove orphandir &&
+	test_must_fail git worktree add --orphan -b existingbranch orphandir
+'
+
+test_expect_success '"add --orphan" with empty repository' '
+	test_when_finished "rm -rf empty_repo" &&
+	echo refs/heads/newbranch >expected &&
+	GIT_DIR="empty_repo" git init --bare &&
+	git -C empty_repo  worktree add --orphan -b newbranch worktreedir &&
+	git -C empty_repo/worktreedir symbolic-ref HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '"add" worktree with orphan branch and lock' '
+	git worktree add --lock --orphan -b orphanbr orphan-with-lock &&
+	test_when_finished "git worktree unlock orphan-with-lock || :" &&
+	test -f .git/worktrees/orphan-with-lock/locked
+'
+
+test_expect_success '"add" worktree with orphan branch, lock, and reason' '
+	lock_reason="why not" &&
+	git worktree add --detach --lock --reason "$lock_reason" orphan-with-lock-reason main &&
+	test_when_finished "git worktree unlock orphan-with-lock-reason || :" &&
+	test -f .git/worktrees/orphan-with-lock-reason/locked &&
+	echo "$lock_reason" >expect &&
+	test_cmp expect .git/worktrees/orphan-with-lock-reason/locked
+'
+
 test_expect_success 'local clone from linked checkout' '
 	git clone --local here here-clone &&
 	( cd here-clone && git fsck )
@@ -460,6 +521,14 @@ setup_remote_repo () {
 	)
 }

+test_expect_success '"add" <path> <remote/branch> w/ no HEAD' '
+	test_when_finished rm -rf repo_upstream repo_local foo &&
+	setup_remote_repo repo_upstream repo_local &&
+	git -C repo_local config --bool core.bare true &&
+	git -C repo_local branch -D main &&
+	git -C repo_local worktree add ./foo repo_upstream/foo
+'
+
 test_expect_success '--no-track avoids setting up tracking' '
 	test_when_finished rm -rf repo_upstream repo_local foo &&
 	setup_remote_repo repo_upstream repo_local &&
--
2.39.2



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v9 6/8] worktree add: introduce "try --orphan" hint
  2023-04-17  9:33 [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
                   ` (4 preceding siblings ...)
  2023-04-17  9:34 ` [PATCH v9 5/8] worktree add: add --orphan flag Jacob Abel
@ 2023-04-17  9:34 ` Jacob Abel
  2023-04-17  9:34 ` [PATCH v9 7/8] worktree add: extend DWIM to infer --orphan Jacob Abel
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-04-17  9:34 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Add a new advice/hint in `git worktree add` for when the user
tries to create a new worktree from a reference that doesn't exist.

Current Behavior:

% git init foo
Initialized empty Git repository in /path/to/foo/
% touch file
% git -C foo commit -q -a -m "test commit"
% git -C foo switch --orphan norefbranch
% git -C foo worktree add newbranch/
Preparing worktree (new branch 'newbranch')
fatal: invalid reference: HEAD
%

New Behavior:

% git init --bare foo
Initialized empty Git repository in /path/to/foo/
% touch file
% git -C foo commit -q -a -m "test commit"
% git -C foo switch --orphan norefbranch
% git -C foo worktree add newbranch/
Preparing worktree (new branch 'newbranch')
hint: If you meant to create a worktree containing a new orphan branch
hint: (branch with no commits) for this repository, you can do so
hint: using the --orphan option:
hint:
hint:   git worktree add --orphan newbranch/
hint:
hint: Disable this message with "git config advice.worktreeAddOrphan false"
fatal: invalid reference: HEAD
% git -C foo worktree add -b newbranch2 new_wt/
Preparing worktree (new branch 'newbranch')
hint: If you meant to create a worktree containing a new orphan branch
hint: (branch with no commits) for this repository, you can do so
hint: using the --orphan option:
hint:
hint:   git worktree add --orphan -b newbranch2 new_wt/
hint:
hint: Disable this message with "git config advice.worktreeAddOrphan false"
fatal: invalid reference: HEAD
%

Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 Documentation/config/advice.txt |  4 ++++
 advice.c                        |  1 +
 advice.h                        |  1 +
 builtin/worktree.c              | 25 +++++++++++++++++++++
 t/t2400-worktree-add.sh         | 39 +++++++++++++++++++++++++++++++++
 5 files changed, 70 insertions(+)

diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index c96b5b2e5d..c548a91e67 100644
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -138,4 +138,8 @@ advice.*::
 		checkout.
 	diverging::
 		Advice shown when a fast-forward is not possible.
+	worktreeAddOrphan::
+		Advice shown when a user tries to create a worktree from an
+		invalid reference, to instruct how to create a new orphan
+		branch instead.
 --
diff --git a/advice.c b/advice.c
index d6232439c3..e5a9bb9b44 100644
--- a/advice.c
+++ b/advice.c
@@ -78,6 +78,7 @@ static struct {
 	[ADVICE_SUBMODULES_NOT_UPDATED] 		= { "submodulesNotUpdated", 1 },
 	[ADVICE_UPDATE_SPARSE_PATH]			= { "updateSparsePath", 1 },
 	[ADVICE_WAITING_FOR_EDITOR]			= { "waitingForEditor", 1 },
+	[ADVICE_WORKTREE_ADD_ORPHAN]			= { "worktreeAddOrphan", 1 },
 };

 static const char turn_off_instructions[] =
diff --git a/advice.h b/advice.h
index 0f584163f5..2affbe1426 100644
--- a/advice.h
+++ b/advice.h
@@ -49,6 +49,7 @@ struct string_list;
 	ADVICE_UPDATE_SPARSE_PATH,
 	ADVICE_WAITING_FOR_EDITOR,
 	ADVICE_SKIPPED_CHERRY_PICKS,
+	ADVICE_WORKTREE_ADD_ORPHAN,
 };

 int git_default_advice_config(const char *var, const char *value);
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 48de7fc3b0..12348d3d16 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -39,6 +39,20 @@
 #define BUILTIN_WORKTREE_UNLOCK_USAGE \
 	N_("git worktree unlock <worktree>")

+#define WORKTREE_ADD_ORPHAN_WITH_DASH_B_HINT_TEXT \
+	_("If you meant to create a worktree containing a new orphan branch\n" \
+	"(branch with no commits) for this repository, you can do so\n" \
+	"using the --orphan flag:\n" \
+	"\n" \
+	"	git worktree add --orphan -b %s %s\n")
+
+#define WORKTREE_ADD_ORPHAN_NO_DASH_B_HINT_TEXT \
+	_("If you meant to create a worktree containing a new orphan branch\n" \
+	"(branch with no commits) for this repository, you can do so\n" \
+	"using the --orphan flag:\n" \
+	"\n" \
+	"	git worktree add --orphan %s\n")
+
 static const char * const git_worktree_usage[] = {
 	BUILTIN_WORKTREE_ADD_USAGE,
 	BUILTIN_WORKTREE_LIST_USAGE,
@@ -634,6 +648,7 @@ static int add(int ac, const char **av, const char *prefix)
 	const char *opt_track = NULL;
 	const char *lock_reason = NULL;
 	int keep_locked = 0;
+	int used_new_branch_options;
 	struct option options[] = {
 		OPT__FORCE(&opts.force,
 			   N_("checkout <branch> even if already checked out in other worktree"),
@@ -686,6 +701,7 @@ static int add(int ac, const char **av, const char *prefix)

 	path = prefix_filename(prefix, av[0]);
 	branch = ac < 2 ? "HEAD" : av[1];
+	used_new_branch_options = new_branch || new_branch_force;

 	if (!strcmp(branch, "-"))
 		branch = "@{-1}";
@@ -728,6 +744,15 @@ static int add(int ac, const char **av, const char *prefix)
 	}

 	if (!opts.orphan && !lookup_commit_reference_by_name(branch)) {
+		int attempt_hint = !opts.quiet && (ac < 2);
+		if (attempt_hint && used_new_branch_options) {
+			advise_if_enabled(ADVICE_WORKTREE_ADD_ORPHAN,
+				WORKTREE_ADD_ORPHAN_WITH_DASH_B_HINT_TEXT,
+				new_branch, path);
+		} else if (attempt_hint) {
+			advise_if_enabled(ADVICE_WORKTREE_ADD_ORPHAN,
+				WORKTREE_ADD_ORPHAN_NO_DASH_B_HINT_TEXT, path);
+		}
 		die(_("invalid reference: %s"), branch);
 	}

diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index 2ea4342867..7ea56ef7c1 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -405,6 +405,45 @@ test_expect_success '"add" worktree with orphan branch, lock, and reason' '
 	test_cmp expect .git/worktrees/orphan-with-lock-reason/locked
 '

+# Note: Quoted arguments containing spaces are not supported.
+test_wt_add_orphan_hint () {
+	local context="$1" &&
+	local use_branch=$2 &&
+	shift 2 &&
+	local opts="$*" &&
+	test_expect_success "'worktree add' show orphan hint in bad/orphan HEAD w/ $context" '
+		test_when_finished "rm -rf repo" &&
+		git init repo &&
+		(cd repo && test_commit commit) &&
+		git -C repo switch --orphan noref &&
+		test_when_finished cat actual >&2 &&
+		test_must_fail git -C repo worktree add $opts foobar/ 2>actual &&
+		! grep "error: unknown switch" actual &&
+		grep "hint: If you meant to create a worktree containing a new orphan branch" actual &&
+		if [ $use_branch -eq 1 ]
+		then
+			grep -E "^hint:\s+git worktree add --orphan -b \S+ \S+\s*$" actual
+		else
+			grep -E "^hint:\s+git worktree add --orphan \S+\s*$" actual
+		fi
+
+	'
+}
+
+test_wt_add_orphan_hint 'no opts' 0
+test_wt_add_orphan_hint '-b' 1 -b foobar_branch
+test_wt_add_orphan_hint '-B' 1 -B foobar_branch
+
+test_expect_success "'worktree add' doesn't show orphan hint in bad/orphan HEAD w/ --quiet" '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(cd repo && test_commit commit) &&
+	test_when_finished cat actual >&2 &&
+	test_must_fail git -C repo worktree add --quiet foobar_branch foobar/ 2>actual &&
+	! grep "error: unknown switch" actual &&
+	! grep "hint: If you meant to create a worktree containing a new orphan branch" actual
+'
+
 test_expect_success 'local clone from linked checkout' '
 	git clone --local here here-clone &&
 	( cd here-clone && git fsck )
--
2.39.2



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v9 7/8] worktree add: extend DWIM to infer --orphan
  2023-04-17  9:33 [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
                   ` (5 preceding siblings ...)
  2023-04-17  9:34 ` [PATCH v9 6/8] worktree add: introduce "try --orphan" hint Jacob Abel
@ 2023-04-17  9:34 ` Jacob Abel
  2023-04-17  9:34 ` [PATCH v9 8/8] worktree add: emit warn when there is a bad HEAD Jacob Abel
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-04-17  9:34 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Extend DWIM to try to infer `--orphan` when in an empty repository. i.e.
a repository with an invalid/unborn HEAD, no local branches, and if
`--guess-remote` is used then no remote branches.

This behavior is equivalent to `git switch -c` or `git checkout -b` in
an empty repository.

Also warn the user (overriden with `-f`/`--force`) when they likely
intend to checkout a remote branch to the worktree but have not yet
fetched from the remote. i.e. when using `--guess-remote` and there is a
remote but no local or remote refs.

Current Behavior:
% git --no-pager branch --list --remotes
% git remote
origin
% git workree add ../main
hint: If you meant to create a worktree containing a new orphan branch
[...]
hint: Disable this message with "git config advice.worktreeAddOrphan false"
fatal: invalid reference: HEAD
% git workree add --guess-remote ../main
hint: If you meant to create a worktree containing a new orphan branch
[...]
hint: Disable this message with "git config advice.worktreeAddOrphan false"
fatal: invalid reference: HEAD
% git fetch --quiet
% git --no-pager branch --list --remotes
origin/HEAD -> origin/main
origin/main
% git workree add --guess-remote ../main
Preparing worktree (new branch 'main')
branch 'main' set up to track 'origin/main'.
HEAD is now at dadc8e6dac commit message
%

New Behavior:
% git --no-pager branch --list --remotes
% git remote
origin
% git workree add ../main
No possible source branch, inferring '--orphan'
Preparing worktree (new branch 'main')
% git worktree remove ../main
% git workree add --guess-remote ../main
fatal: No local or remote refs exist despite at least one remote
present, stopping; use 'add -f' to overide or fetch a remote first
% git workree add --guess-remote -f ../main
No possible source branch, inferring '--orphan'
Preparing worktree (new branch 'main')
% git worktree remove ../main
% git fetch --quiet
% git --no-pager branch --list --remotes
origin/HEAD -> origin/main
origin/main
% git workree add --guess-remote ../main
Preparing worktree (new branch 'main')
branch 'main' set up to track 'origin/main'.
HEAD is now at dadc8e6dac commit message
%

Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 Documentation/git-worktree.txt |  10 +
 builtin/worktree.c             | 114 ++++++++++-
 t/t2400-worktree-add.sh        | 332 +++++++++++++++++++++++++++++++++
 3 files changed, 455 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 485d865eb2..a4fbf5e838 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -95,6 +95,16 @@ exist, a new branch based on `HEAD` is automatically created as if
 `-b <branch>` was given.  If `<branch>` does exist, it will be checked out
 in the new worktree, if it's not checked out anywhere else, otherwise the
 command will refuse to create the worktree (unless `--force` is used).
++
+If `<commit-ish>` is omitted, neither `--detach`, or `--orphan` is
+used, and there are no valid local branches (or remote branches if
+`--guess-remote` is specified) then, as a convenience, the new worktree is
+associated with a new orphan branch named `<branch>` (after
+`$(basename <path>)` if neither `-b` or `-B` is used) as if `--orphan` was
+passed to the command. In the event the repository has a remote and
+`--guess-remote` is used, but no remote or local branches exist, then the
+command fails with a warning reminding the user to fetch from their remote
+first (or override by using `-f/--force`).

 list::

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 12348d3d16..95b5bbb1d2 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -11,6 +11,7 @@
 #include "strvec.h"
 #include "branch.h"
 #include "refs.h"
+#include "remote.h"
 #include "run-command.h"
 #include "hook.h"
 #include "sigchain.h"
@@ -39,6 +40,9 @@
 #define BUILTIN_WORKTREE_UNLOCK_USAGE \
 	N_("git worktree unlock <worktree>")

+#define WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT \
+	_("No possible source branch, inferring '--orphan'")
+
 #define WORKTREE_ADD_ORPHAN_WITH_DASH_B_HINT_TEXT \
 	_("If you meant to create a worktree containing a new orphan branch\n" \
 	"(branch with no commits) for this repository, you can do so\n" \
@@ -612,6 +616,107 @@ static void print_preparing_worktree_line(int detach,
 	}
 }

+/**
+ * Callback to short circuit iteration over refs on the first reference
+ * corresponding to a valid oid.
+ *
+ * Returns 0 on failure and non-zero on success.
+ */
+static int first_valid_ref(const char *refname,
+			   const struct object_id *oid,
+			   int flags,
+			   void *cb_data)
+{
+	return 1;
+}
+
+/**
+ * Verifies HEAD and determines whether there exist any valid local references.
+ *
+ * - Checks whether HEAD points to a valid reference.
+ *
+ * - Checks whether any valid local branches exist.
+ *
+ * Returns 1 if any of the previous checks are true, otherwise returns 0.
+ */
+static int can_use_local_refs(const struct add_opts *opts)
+{
+	if (head_ref(first_valid_ref, NULL)) {
+		return 1;
+	} else if (for_each_branch_ref(first_valid_ref, NULL)) {
+		return 1;
+	}
+	return 0;
+}
+
+/**
+ * Reports whether the necessary flags were set and whether the repository has
+ * remote references to attempt DWIM tracking of upstream branches.
+ *
+ * 1. Checks that `--guess-remote` was used or `worktree.guessRemote = true`.
+ *
+ * 2. Checks whether any valid remote branches exist.
+ *
+ * 3. Checks that there exists at least one remote and emits a warning/error
+ *    if both checks 1. and 2. are false (can be bypassed with `--force`).
+ *
+ * Returns 1 if checks 1. and 2. are true, otherwise 0.
+ */
+static int can_use_remote_refs(const struct add_opts *opts)
+{
+	if (!guess_remote) {
+		if (!opts->quiet)
+			fprintf_ln(stderr, WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT);
+		return 0;
+	} else if (for_each_remote_ref(first_valid_ref, NULL)) {
+		return 1;
+	} else if (!opts->force && remote_get(NULL)) {
+		die(_("No local or remote refs exist despite at least one remote\n"
+		      "present, stopping; use 'add -f' to overide or fetch a remote first"));
+	} else if (!opts->quiet) {
+		fprintf_ln(stderr, WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT);
+	}
+	return 0;
+}
+
+/**
+ * Determines whether `--orphan` should be inferred in the evaluation of
+ * `worktree add path/` or `worktree add -b branch path/` and emits an error
+ * if the supplied arguments would produce an illegal combination  when the
+ * `--orphan` flag is included.
+ *
+ * `opts` and `opt_track` contain the other options & flags supplied to the
+ * command.
+ *
+ * remote determines whether to check `can_use_remote_refs()` or not. This
+ * is primarily to differentiate between the basic `add` DWIM and `add -b`.
+ *
+ * Returns 1 when inferring `--orphan`, 0 otherwise, and emits an error when
+ * `--orphan` is inferred but doing so produces an illegal combination of
+ * options and flags. Additionally produces an error when remote refs are
+ * checked and the repo is in a state that looks like the user added a remote
+ * but forgot to fetch (and did not override the warning with -f).
+ */
+static int dwim_orphan(const struct add_opts *opts, int opt_track, int remote)
+{
+	if (can_use_local_refs(opts)) {
+		return 0;
+	} else if (remote && can_use_remote_refs(opts)) {
+		return 0;
+	} else if (!opts->quiet) {
+		fprintf_ln(stderr, WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT);
+	}
+
+	if (opt_track) {
+		die(_("'%s' and '%s' cannot be used together"), "--orphan",
+		    "--track");
+	} else if (!opts->checkout) {
+		die(_("'%s' and '%s' cannot be used together"), "--orphan",
+		    "--no-checkout");
+	}
+	return 1;
+}
+
 static const char *dwim_branch(const char *path, const char **new_branch)
 {
 	int n;
@@ -722,12 +827,19 @@ static int add(int ac, const char **av, const char *prefix)
 		int n;
 		const char *s = worktree_basename(path, &n);
 		new_branch = xstrndup(s, n);
-	} else if (new_branch || opts.detach || opts.orphan) {
+	} else if (opts.orphan || opts.detach) {
 		// No-op
+	} else if (ac < 2 && new_branch) {
+		// DWIM: Infer --orphan when repo has no refs.
+		opts.orphan = dwim_orphan(&opts, !!opt_track, 0);
 	} else if (ac < 2) {
+		// DWIM: Guess branch name from path.
 		const char *s = dwim_branch(path, &new_branch);
 		if (s)
 			branch = s;
+
+		// DWIM: Infer --orphan when repo has no refs.
+		opts.orphan = (!s) && dwim_orphan(&opts, !!opt_track, 1);
 	} else if (ac == 2) {
 		struct object_id oid;
 		struct commit *commit;
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index 7ea56ef7c1..e5cca1d11b 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -712,6 +712,338 @@ test_expect_success 'git worktree --no-guess-remote option overrides config' '
 	)
 '

+test_dwim_orphan () {
+	local info_text="No possible source branch, inferring '--orphan'" &&
+	local fetch_error_text="fatal: No local or remote refs exist despite at least one remote" &&
+	local orphan_hint="hint: If you meant to create a worktree containing a new orphan branch" &&
+	local invalid_ref_regex="^fatal: invalid reference:\s\+.*" &&
+	local bad_combo_regex="^fatal: '[a-z-]\+' and '[a-z-]\+' cannot be used together" &&
+
+	local git_ns="repo" &&
+	local dashc_args="-C $git_ns" &&
+	local use_cd=0 &&
+
+	local bad_head=0 &&
+	local empty_repo=1 &&
+	local local_ref=0 &&
+	local use_quiet=0 &&
+	local remote=0 &&
+	local remote_ref=0 &&
+	local use_new_branch=0 &&
+
+	local outcome="$1" &&
+	local outcome_text &&
+	local success &&
+	shift &&
+	local args="" &&
+	local context="" &&
+	case "$outcome" in
+	"infer")
+		success=1 &&
+		outcome_text='"add" DWIM infer --orphan'
+		;;
+	"no_infer")
+		success=1 &&
+		outcome_text='"add" DWIM doesnt infer --orphan'
+		;;
+	"fetch_error")
+		success=0 &&
+		outcome_text='"add" error need fetch'
+		;;
+	"fatal_orphan_bad_combo")
+		success=0 &&
+		outcome_text='"add" error inferred "--orphan" gives illegal opts combo'
+		;;
+	*)
+		echo "test_dwim_orphan(): invalid outcome: '$outcome'" >&2 &&
+		return 1
+		;;
+	esac &&
+	while [ $# -gt 0 ]
+	do
+		case "$1" in
+		# How and from where to create the worktree
+		"-C_repo")
+			use_cd=0 &&
+			git_ns="repo" &&
+			dashc_args="-C $git_ns" &&
+			context="$context, 'git -C repo'"
+			;;
+		"-C_wt")
+			use_cd=0 &&
+			git_ns="wt" &&
+			dashc_args="-C $git_ns" &&
+			context="$context, 'git -C wt'"
+			;;
+		"cd_repo")
+			use_cd=1 &&
+			git_ns="repo" &&
+			dashc_args="" &&
+			context="$context, 'cd repo && git'"
+			;;
+		"cd_wt")
+			use_cd=1 &&
+			git_ns="wt" &&
+			dashc_args="" &&
+			context="$context, 'cd wt && git'"
+			;;
+
+		# Bypass the "pull first" warning
+		"force")
+			args="$args --force" &&
+			context="$context, --force"
+			;;
+
+		# Try to use remote refs when DWIM
+		"guess_remote")
+			args="$args --guess-remote" &&
+			context="$context, --guess-remote"
+			;;
+		"no_guess_remote")
+			args="$args --no-guess-remote" &&
+			context="$context, --no-guess-remote"
+			;;
+
+		# Whether there is at least one local branch present
+		"local_ref")
+			empty_repo=0 &&
+			local_ref=1 &&
+			context="$context, >=1 local branches"
+			;;
+		"no_local_ref")
+			empty_repo=0 &&
+			context="$context, 0 local branches"
+			;;
+
+		# Whether the HEAD points at a valid ref (skip this opt when no refs)
+		"good_head")
+			# requires: local_ref
+			context="$context, valid HEAD"
+			;;
+		"bad_head")
+			bad_head=1 &&
+			context="$context, invalid (or orphan) HEAD"
+			;;
+
+		# Whether the code path is tested with the base add command or -b
+		"no_-b")
+			use_new_branch=0 &&
+			context="$context, no --branch"
+			;;
+		"-b")
+			use_new_branch=1 &&
+			context="$context, --branch"
+			;;
+
+		# Whether to check that all output is suppressed (except errors)
+		# or that the output is as expected
+		"quiet")
+			use_quiet=1 &&
+			args="$args --quiet" &&
+			context="$context, --quiet"
+			;;
+		"no_quiet")
+			use_quiet=0 &&
+			context="$context, no --quiet (expect output)"
+			;;
+
+		# Whether there is at least one remote attached to the repo
+		"remote")
+			empty_repo=0 &&
+			remote=1 &&
+			context="$context, >=1 remotes"
+			;;
+		"no_remote")
+			empty_repo=0 &&
+			remote=0 &&
+			context="$context, 0 remotes"
+			;;
+
+		# Whether there is at least one valid remote ref
+		"remote_ref")
+			# requires: remote
+			empty_repo=0 &&
+			remote_ref=1 &&
+			context="$context, >=1 fetched remote branches"
+			;;
+		"no_remote_ref")
+			empty_repo=0 &&
+			remote_ref=0 &&
+			context="$context, 0 fetched remote branches"
+			;;
+
+		# Options or flags that become illegal when --orphan is inferred
+		"no_checkout")
+			args="$args --no-checkout" &&
+			context="$context, --no-checkout"
+			;;
+		"track")
+			args="$args --track" &&
+			context="$context, --track"
+			;;
+
+		# All other options are illegal
+		*)
+			echo "test_dwim_orphan(): invalid arg: '$1'" >&2 &&
+			return 1
+			;;
+		esac &&
+		shift
+	done &&
+	context="${context#', '}" &&
+	if [ $use_new_branch -eq 1 ]
+	then
+		args="$args -b foo"
+	else
+		context="DWIM (no --branch), $context"
+	fi &&
+	if [ $empty_repo -eq 1 ]
+	then
+		context="empty repo, $context"
+	fi &&
+	args="$args ../foo" &&
+	context="${context%', '}" &&
+	test_expect_success "$outcome_text w/ $context" '
+		test_when_finished "rm -rf repo" &&
+		git init repo &&
+		if [ $local_ref -eq 1 ] && [ "$git_ns" = "repo" ]
+		then
+			(cd repo && test_commit commit) &&
+			if [ $bad_head -eq 1 ]
+			then
+				git -C repo symbolic-ref HEAD refs/heads/badbranch
+			fi
+		elif [ $local_ref -eq 1 ] && [ "$git_ns" = "wt" ]
+		then
+			test_when_finished "git -C repo worktree remove -f ../wt" &&
+			git -C repo worktree add --orphan -b main ../wt &&
+			(cd wt && test_commit commit) &&
+			if [ $bad_head -eq 1 ]
+			then
+				git -C wt symbolic-ref HEAD refs/heads/badbranch
+			fi
+		elif [ $local_ref -eq 0 ] && [ "$git_ns" = "wt" ]
+		then
+			test_when_finished "git -C repo worktree remove -f ../wt" &&
+			git -C repo worktree add --orphan -b orphanbranch ../wt
+		fi &&
+
+		if [ $remote -eq 1 ]
+		then
+			test_when_finished "rm -rf upstream" &&
+			git init upstream &&
+			(cd upstream && test_commit commit) &&
+			git -C upstream switch -c foo &&
+			git -C repo remote add upstream ../upstream
+		fi &&
+
+		if [ $remote_ref -eq 1 ]
+		then
+			git -C repo fetch
+		fi &&
+		if [ $success -eq 1 ]
+		then
+			test_when_finished git -C repo worktree remove ../foo
+		fi &&
+		if [ $use_cd -eq 1 ]
+		then
+			test_when_finished cat "$git_ns/actual" >&2
+		else
+			test_when_finished cat actual >&2
+		fi &&
+		(
+			if [ $use_cd -eq 1 ]
+			then
+				cd $git_ns
+			fi &&
+			if [ "$outcome" = "infer" ]
+			then
+				git $dashc_args worktree add $args 2>actual &&
+				if [ $use_quiet -eq 1 ]
+				then
+					test_must_be_empty actual
+				else
+					grep "$info_text" actual
+				fi
+			elif [ "$outcome" = "no_infer" ]
+			then
+				git $dashc_args worktree add $args 2>actual &&
+				if [ $use_quiet -eq 1 ]
+				then
+					test_must_be_empty actual
+				else
+					! grep "$info_text" actual
+				fi
+			elif [ "$outcome" = "fetch_error" ]
+			then
+				test_must_fail git $dashc_args worktree add $args 2>actual &&
+				grep "$fetch_error_text" actual
+			elif [ "$outcome" = "fatal_orphan_bad_combo" ]
+			then
+				test_must_fail git $dashc_args worktree add $args 2>actual &&
+				if [ $use_quiet -eq 1 ]
+				then
+					! grep "$info_text" actual
+				else
+					grep "$info_text" actual
+				fi &&
+				grep "$bad_combo_regex" actual
+			elif [ "$outcome" = "warn_bad_head" ]
+			then
+				test_must_fail git $dashc_args worktree add $args 2>actual &&
+				if [ $use_quiet -eq 1 ]
+				then
+					grep "$invalid_ref_regex" actual &&
+					! grep "$orphan_hint" actual
+				else
+					headpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&
+					headcontents=$(cat "$headpath") &&
+					grep "HEAD points to an invalid (or orphaned) reference" actual &&
+					grep "HEAD path:\s*.$headpath." actual &&
+					grep "HEAD contents:\s*.$headcontents." actual &&
+					grep "$orphan_hint" actual &&
+					! grep "$info_text" actual
+				fi &&
+				grep "$invalid_ref_regex" actual
+			else
+				# Unreachable
+				false
+			fi
+		) &&
+		if [ $success -ne 1 ]
+		then
+			test_path_is_missing foo
+		fi
+	'
+}
+
+for quiet_mode in "no_quiet" "quiet"
+do
+	for changedir_type in "cd_repo" "cd_wt" "-C_repo" "-C_wt"
+	do
+		dwim_test_args="$quiet_mode $changedir_type"
+		test_dwim_orphan 'infer' $dwim_test_args no_-b
+		test_dwim_orphan 'no_infer' $dwim_test_args no_-b local_ref good_head
+		test_dwim_orphan 'infer' $dwim_test_args no_-b no_local_ref no_remote no_remote_ref no_guess_remote
+		test_dwim_orphan 'infer' $dwim_test_args no_-b no_local_ref remote no_remote_ref no_guess_remote
+		test_dwim_orphan 'fetch_error' $dwim_test_args no_-b no_local_ref remote no_remote_ref guess_remote
+		test_dwim_orphan 'infer' $dwim_test_args no_-b no_local_ref remote no_remote_ref guess_remote force
+		test_dwim_orphan 'no_infer' $dwim_test_args no_-b no_local_ref remote remote_ref guess_remote
+
+		test_dwim_orphan 'infer' $dwim_test_args -b
+		test_dwim_orphan 'no_infer' $dwim_test_args -b local_ref good_head
+		test_dwim_orphan 'infer' $dwim_test_args -b no_local_ref no_remote no_remote_ref no_guess_remote
+		test_dwim_orphan 'infer' $dwim_test_args -b no_local_ref remote no_remote_ref no_guess_remote
+		test_dwim_orphan 'infer' $dwim_test_args -b no_local_ref remote no_remote_ref guess_remote
+		test_dwim_orphan 'infer' $dwim_test_args -b no_local_ref remote remote_ref guess_remote
+	done
+
+	test_dwim_orphan 'fatal_orphan_bad_combo' $quiet_mode no_-b no_checkout
+	test_dwim_orphan 'fatal_orphan_bad_combo' $quiet_mode no_-b track
+	test_dwim_orphan 'fatal_orphan_bad_combo' $quiet_mode -b no_checkout
+	test_dwim_orphan 'fatal_orphan_bad_combo' $quiet_mode -b track
+done
+
 post_checkout_hook () {
 	test_when_finished "rm -rf .git/hooks" &&
 	mkdir .git/hooks &&
--
2.39.2



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH v9 8/8] worktree add: emit warn when there is a bad HEAD
  2023-04-17  9:33 [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
                   ` (6 preceding siblings ...)
  2023-04-17  9:34 ` [PATCH v9 7/8] worktree add: extend DWIM to infer --orphan Jacob Abel
@ 2023-04-17  9:34 ` Jacob Abel
  2023-04-20  3:05 ` [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
  2023-05-17 21:47 ` [RESEND PATCH v10 " Jacob Abel
  9 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-04-17  9:34 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Add a warning to `worktree add` when the command tries to reference
HEAD, there exist valid local branches, and the HEAD points to a
non-existent reference.

Current Behavior:
% git -C foo worktree list
/path/to/repo/foo     dadc8e6dac [main]
/path/to/repo/foo_wt  0000000000 [badref]
% git -C foo worktree add ../wt1
Preparing worktree (new branch 'wt1')
HEAD is now at dadc8e6dac dummy commit
% git -C foo_wt worktree add ../wt2
hint: If you meant to create a worktree containing a new orphan branch
[...]
hint: Disable this message with "git config advice.worktreeAddOrphan false"
fatal: invalid reference: HEAD
%

New Behavior:
% git -C foo worktree list
/path/to/repo/foo     dadc8e6dac [main]
/path/to/repo/foo_wt  0000000000 [badref]
% git -C foo worktree add ../wt1
Preparing worktree (new branch 'wt1')
HEAD is now at dadc8e6dac dummy commit
% git -C foo_wt worktree add ../wt2
warning: HEAD points to an invalid (or orphaned) reference.
HEAD path: '/path/to/repo/foo/.git/worktrees/foo_wt/HEAD'
HEAD contents: 'ref: refs/heads/badref'
hint: If you meant to create a worktree containing a new orphan branch
[...]
hint: Disable this message with "git config advice.worktreeAddOrphan false"
fatal: invalid reference: HEAD
%

Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 builtin/worktree.c      | 34 +++++++++++++++++++++++++++++-----
 t/t2400-worktree-add.sh | 18 +++++++++++++++++-
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 95b5bbb1d2..0fba4cfdf8 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -637,6 +637,9 @@ static int first_valid_ref(const char *refname,
  *
  * - Checks whether any valid local branches exist.
  *
+ * - Emits a warning if there exist any valid branches but HEAD does not point
+ *   to a valid reference.
+ *
  * Returns 1 if any of the previous checks are true, otherwise returns 0.
  */
 static int can_use_local_refs(const struct add_opts *opts)
@@ -644,6 +647,23 @@ static int can_use_local_refs(const struct add_opts *opts)
 	if (head_ref(first_valid_ref, NULL)) {
 		return 1;
 	} else if (for_each_branch_ref(first_valid_ref, NULL)) {
+		if (!opts->quiet) {
+			struct strbuf path = STRBUF_INIT;
+			struct strbuf contents = STRBUF_INIT;
+
+			strbuf_add_real_path(&path, get_worktree_git_dir(NULL));
+			strbuf_addstr(&path, "/HEAD");
+			strbuf_read_file(&contents, path.buf, 64);
+			strbuf_stripspace(&contents, 0);
+			strbuf_strip_suffix(&contents, "\n");
+
+			warning(_("HEAD points to an invalid (or orphaned) reference.\n"
+				  "HEAD path: '%s'\n"
+				  "HEAD contents: '%s'"),
+				  path.buf, contents.buf);
+			strbuf_release(&path);
+			strbuf_release(&contents);
+		}
 		return 1;
 	}
 	return 0;
@@ -665,16 +685,12 @@ static int can_use_local_refs(const struct add_opts *opts)
 static int can_use_remote_refs(const struct add_opts *opts)
 {
 	if (!guess_remote) {
-		if (!opts->quiet)
-			fprintf_ln(stderr, WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT);
 		return 0;
 	} else if (for_each_remote_ref(first_valid_ref, NULL)) {
 		return 1;
 	} else if (!opts->force && remote_get(NULL)) {
 		die(_("No local or remote refs exist despite at least one remote\n"
 		      "present, stopping; use 'add -f' to overide or fetch a remote first"));
-	} else if (!opts->quiet) {
-		fprintf_ln(stderr, WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT);
 	}
 	return 0;
 }
@@ -827,8 +843,12 @@ static int add(int ac, const char **av, const char *prefix)
 		int n;
 		const char *s = worktree_basename(path, &n);
 		new_branch = xstrndup(s, n);
-	} else if (opts.orphan || opts.detach) {
+	} else if (opts.orphan) {
 		// No-op
+	} else if (opts.detach) {
+		// Check HEAD
+		if (!strcmp(branch, "HEAD"))
+			can_use_local_refs(&opts);
 	} else if (ac < 2 && new_branch) {
 		// DWIM: Infer --orphan when repo has no refs.
 		opts.orphan = dwim_orphan(&opts, !!opt_track, 0);
@@ -853,6 +873,10 @@ static int add(int ac, const char **av, const char *prefix)
 				branch = remote;
 			}
 		}
+
+		if (!strcmp(branch, "HEAD"))
+			can_use_local_refs(&opts);
+
 	}

 	if (!opts.orphan && !lookup_commit_reference_by_name(branch)) {
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index e5cca1d11b..09bf506155 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -729,6 +729,7 @@ test_dwim_orphan () {
 	local use_quiet=0 &&
 	local remote=0 &&
 	local remote_ref=0 &&
+	local use_detach=0 &&
 	local use_new_branch=0 &&

 	local outcome="$1" &&
@@ -754,6 +755,10 @@ test_dwim_orphan () {
 		success=0 &&
 		outcome_text='"add" error inferred "--orphan" gives illegal opts combo'
 		;;
+	"warn_bad_head")
+		success=0 &&
+		outcome_text='"add" error, warn on bad HEAD, hint use orphan'
+		;;
 	*)
 		echo "test_dwim_orphan(): invalid outcome: '$outcome'" >&2 &&
 		return 1
@@ -825,7 +830,7 @@ test_dwim_orphan () {
 			context="$context, invalid (or orphan) HEAD"
 			;;

-		# Whether the code path is tested with the base add command or -b
+		# Whether the code path is tested with the base add command, -b, or --detach
 		"no_-b")
 			use_new_branch=0 &&
 			context="$context, no --branch"
@@ -834,6 +839,10 @@ test_dwim_orphan () {
 			use_new_branch=1 &&
 			context="$context, --branch"
 			;;
+		"detach")
+			use_detach=1 &&
+			context="$context, --detach"
+			;;

 		# Whether to check that all output is suppressed (except errors)
 		# or that the output is as expected
@@ -894,6 +903,9 @@ test_dwim_orphan () {
 	if [ $use_new_branch -eq 1 ]
 	then
 		args="$args -b foo"
+	elif [ $use_detach -eq 1 ]
+	then
+		args="$args --detach"
 	else
 		context="DWIM (no --branch), $context"
 	fi &&
@@ -1036,6 +1048,10 @@ do
 		test_dwim_orphan 'infer' $dwim_test_args -b no_local_ref remote no_remote_ref no_guess_remote
 		test_dwim_orphan 'infer' $dwim_test_args -b no_local_ref remote no_remote_ref guess_remote
 		test_dwim_orphan 'infer' $dwim_test_args -b no_local_ref remote remote_ref guess_remote
+
+		test_dwim_orphan 'warn_bad_head' $dwim_test_args no_-b local_ref bad_head
+		test_dwim_orphan 'warn_bad_head' $dwim_test_args -b local_ref bad_head
+		test_dwim_orphan 'warn_bad_head' $dwim_test_args detach local_ref bad_head
 	done

 	test_dwim_orphan 'fatal_orphan_bad_combo' $quiet_mode no_-b no_checkout
--
2.39.2



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v9 2/8] t2400: print captured git output when finished
  2023-04-17  9:33 ` [PATCH v9 2/8] t2400: print captured git output when finished Jacob Abel
@ 2023-04-17 21:09   ` Junio C Hamano
  2023-04-18  3:53     ` Jacob Abel
  0 siblings, 1 reply; 34+ messages in thread
From: Junio C Hamano @ 2023-04-17 21:09 UTC (permalink / raw)
  To: Jacob Abel
  Cc: git, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Phillip Wood, Rubén Justo, Taylor Blau, rsbecker

Jacob Abel <jacobabel@nullpo.dev> writes:

>  test_expect_success 'add --quiet' '
> +	test_when_finished "git worktree remove -f -f another-worktree" &&
> +	test_when_finished cat actual >&2 &&

I doubt that this redirection does anything you expect it do.
Doesn't it redirect the standard output that is emitted by the
test_when_finished shell function when it registers another
test_cleanup scriptlet to the standard error, and when test_cleanup
is indeed run, wouldn't "cat actual" send its output to the standard
output?

No, I am not suggesting to write the line as:

	test_when_finished "cat >&2 actual" &&

>  	git worktree add --quiet another-worktree main 2>actual &&
>  	test_must_be_empty actual

The reason why I do not suggest "fixing" the above is because
test_must_be_empty, when fails, does this:

        test_must_be_empty () {
                test "$#" -ne 1 && BUG "1 param"
                test_path_is_file "$1" &&
                if test -s "$1"
                then
                        echo "'$1' is not empty, it contains:"
                        cat "$1"
                        return 1
                fi
        }

i.e. it sends the contents of "actual" to the standard output
already.  When it succeeds, of course "actual" is empty, and there
is no point in showing its contents.

So "sh t2400-*.sh -x -i" already shows "cat actual" output.  Try
the attached patch on top of this one and running it would show
the above message shown by test_must_be_empty and the contents of
the file 'actual'.  "git worktree remove" fails and your "cat" in
the test_cleanup does not even trigger, by the way.

There may be cases where having something like this might help, but
running the test with "-x" is not it---that case is already covered
by what test_must_be_empty gives us, I think.

 t/t2400-worktree-add.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git c/t/t2400-worktree-add.sh w/t/t2400-worktree-add.sh
index 9bc3db20e4..814642c8ae 100755
--- c/t/t2400-worktree-add.sh
+++ w/t/t2400-worktree-add.sh
@@ -329,9 +329,12 @@ test_expect_success 'add --quiet' '
 	test_when_finished "git worktree remove -f -f another-worktree" &&
 	test_when_finished cat actual >&2 &&
 	git worktree add --quiet another-worktree main 2>actual &&
+echo foo >>actual &&
 	test_must_be_empty actual
 '
 
+exit
+
 test_expect_success 'local clone from linked checkout' '
 	git clone --local here here-clone &&
 	( cd here-clone && git fsck )


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v9 3/8] t2400: refactor "worktree add" opt exclusion tests
  2023-04-17  9:33 ` [PATCH v9 3/8] t2400: refactor "worktree add" opt exclusion tests Jacob Abel
@ 2023-04-17 21:30   ` Junio C Hamano
  2023-04-20  2:46     ` Jacob Abel
  0 siblings, 1 reply; 34+ messages in thread
From: Junio C Hamano @ 2023-04-17 21:30 UTC (permalink / raw)
  To: Jacob Abel
  Cc: git, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Phillip Wood, Rubén Justo, Taylor Blau, rsbecker

Jacob Abel <jacobabel@nullpo.dev> writes:

> +# Helper function to test mutually exclusive options.
> +#
> +# Note: Quoted arguments containing spaces are not supported.

Good to have this note here.

> +test_wt_add_excl () {
> +	local opts="$*" &&
> +	test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
> +		test_when_finished cat actual >&2 &&

Again, I do not think this sends output to the standard output at
the end of this test piece.

> +		test_must_fail git worktree add $opts 2>actual &&
> +		grep -E "fatal:( options)? .* cannot be used together" actual
> +	'
> +}

I do not think this patch is needed (I'd rather see people learn the
trick of running with "-i" and rely on the fact that the trash
directory is left intact to be inspected), but if you must, it may
make more sense to add test_must_contain to make a failed 'grep
"$@"' easier to see, similar to the way that test_must_be_empty
helps a failing 'test !  -s "$1"', something along the lines of ...

	test_must_contain () {
		if ! grep "$@"
		then
			echo "'grep $*' fails; the file contains"
			while test $# != 1
			do
				shift
			done
                        cat "$1"
			return 1
		fi
	}

Thanks.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v9 4/8] t2400: add tests to verify --quiet
  2023-04-17  9:34 ` [PATCH v9 4/8] t2400: add tests to verify --quiet Jacob Abel
@ 2023-04-17 21:33   ` Junio C Hamano
  2023-04-20  2:48     ` Jacob Abel
  0 siblings, 1 reply; 34+ messages in thread
From: Junio C Hamano @ 2023-04-17 21:33 UTC (permalink / raw)
  To: Jacob Abel
  Cc: git, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Phillip Wood, Rubén Justo, Taylor Blau, rsbecker

Jacob Abel <jacobabel@nullpo.dev> writes:

> +test_expect_success 'add --quiet -b' '
> +	test_when_finished "git branch -D quietnewbranch" &&
> +	test_when_finished "git worktree remove -f -f another-worktree" &&
> +	test_when_finished cat actual >&2 &&
> +	git worktree add --quiet -b quietnewbranch another-worktree 2>actual &&
> +	test_must_be_empty actual
> +'

It is good to test the --quiet option.  It is not good to have the
ineffective "cat actual" when test_must_be_empty is already used.
Probably the same comment applies to the rest of the patch.

Thanks.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v9 2/8] t2400: print captured git output when finished
  2023-04-17 21:09   ` Junio C Hamano
@ 2023-04-18  3:53     ` Jacob Abel
  2023-04-18 16:34       ` Junio C Hamano
  0 siblings, 1 reply; 34+ messages in thread
From: Jacob Abel @ 2023-04-18  3:53 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Phillip Wood, Rubén Justo, Taylor Blau, rsbecker

On 23/04/17 02:09PM, Junio C Hamano wrote:
> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> >  test_expect_success 'add --quiet' '
> > +	test_when_finished "git worktree remove -f -f another-worktree" &&
> > +	test_when_finished cat actual >&2 &&
>
> I doubt that this redirection does anything you expect it do.
> Doesn't it redirect the standard output that is emitted by the
> test_when_finished shell function when it registers another
> test_cleanup scriptlet to the standard error, and when test_cleanup
> is indeed run, wouldn't "cat actual" send its output to the standard
> output?

Yes that's correct. I figured "grab from stderr, cat to stderr" but yes
this isn't necessarily what we want here. Dropping the `>&2` causes it to
work as expected.

>
> No, I am not suggesting to write the line as:
>
> 	test_when_finished "cat >&2 actual" &&
>
> >  	git worktree add --quiet another-worktree main 2>actual &&
> >  	test_must_be_empty actual
>
> The reason why I do not suggest "fixing" the above is because
> test_must_be_empty, when fails, does this:
>
>         test_must_be_empty () {
>                 test "$#" -ne 1 && BUG "1 param"
>                 test_path_is_file "$1" &&
>                 if test -s "$1"
>                 then
>                         echo "'$1' is not empty, it contains:"
>                         cat "$1"
>                         return 1
>                 fi
>         }
>
> i.e. it sends the contents of "actual" to the standard output
> already.  When it succeeds, of course "actual" is empty, and there
> is no point in showing its contents.
>
> So "sh t2400-*.sh -x -i" already shows "cat actual" output.  Try
> the attached patch on top of this one and running it would show
> the above message shown by test_must_be_empty and the contents of
> the file 'actual'.  "git worktree remove" fails and your "cat" in
> the test_cleanup does not even trigger, by the way.

That should not be the case. From what I've seen, the test cleanup is
executed in reverse order from the order they are declared with
`test_when_finished`. So as long as `cat` is the last command added to test
cleanup it should always execute immediately after the first command in the
script fails. And as long as the `cat` is added immediately before the
`git worktree add`, that means it should be the most recently added in the
event that command fails.

>
> There may be cases where having something like this might help, but
> running the test with "-x" is not it---that case is already covered
> by what test_must_be_empty gives us, I think.
>
> [...]

I attached an example below to try to illustrate the issue I was attempting
to solve. If `git worktree add ... 2>actual` fails, redirecting stderr to
actual eats the output that would normally show w/ `-x`. Then because a
command fails, it never reaches the `test_must_be_empty`.

Test results of running `sh t2400-*.sh -x` for this test when
`git worktree add` fails (caused in this case by adding `--bad-arg` to the
command):

    expecting success of 2400.37 'add --quiet':
            test_when_finished "git worktree remove -f -f another-worktree" &&
            test_when_finished cat actual >&2 &&
            git worktree add --quiet --bad-arg another-worktree main 2>actual &&
            test_must_be_empty actual

    ++ test_when_finished 'git worktree remove -f -f another-worktree'
    ++ test 0 = 0
    ++ test_cleanup='{ git worktree remove -f -f another-worktree
                    } && (exit "$eval_ret"); eval_ret=$?; :'
    ++ test_when_finished cat actual
    ++ test 0 = 0
    ++ test_cleanup='{ cat actual
                    } && (exit "$eval_ret"); eval_ret=$?; { git worktree remove -f -f another-worktree
                    } && (exit "$eval_ret"); eval_ret=$?; :'
    ++ git worktree add --quiet --bad-arg another-worktree main
    error: last command exited with $?=129
    ++ cat actual
    error: unknown option `bad-arg'
    usage: git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]
                            [(-b | -B) <new-branch>] <path> [<commit-ish>]

        -f, --force           checkout <branch> even if already checked out in other worktree
        -b <branch>           create a new branch
        -B <branch>           create or reset a branch
        -d, --detach          detach HEAD at named commit
        --checkout            populate the new working tree
        --lock                keep the new working tree locked
        --reason <string>     reason for locking
        -q, --quiet           suppress progress reporting
        --track               set up tracking mode (see git-branch(1))
        --guess-remote        try to match the new branch name with a remote-tracking branch

    ++ exit 129
    ++ eval_ret=129
    ++ git worktree remove -f -f another-worktree
    fatal: 'another-worktree' is not a working tree
    ++ eval_ret=128
    ++ :
    not ok 37 - add --quiet

The same test but with the `test_when_finished cat actual` removed:

    expecting success of 2400.37 'add --quiet':
            test_when_finished "git worktree remove -f -f another-worktree" &&
            git worktree add --quiet --bad-arg another-worktree main 2>actual &&
            test_must_be_empty actual

    ++ test_when_finished 'git worktree remove -f -f another-worktree'
    ++ test 0 = 0
    ++ test_cleanup='{ git worktree remove -f -f another-worktree
                    } && (exit "$eval_ret"); eval_ret=$?; :'
    ++ git worktree add --quiet --bad-arg another-worktree main
    error: last command exited with $?=129
    ++ git worktree remove -f -f another-worktree
    fatal: 'another-worktree' is not a working tree
    ++ eval_ret=128
    ++ :
    not ok 37 - add --quiet



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v9 2/8] t2400: print captured git output when finished
  2023-04-18  3:53     ` Jacob Abel
@ 2023-04-18 16:34       ` Junio C Hamano
  2023-04-19 13:23         ` Jacob Abel
  2023-04-19 13:36         ` Jacob Abel
  0 siblings, 2 replies; 34+ messages in thread
From: Junio C Hamano @ 2023-04-18 16:34 UTC (permalink / raw)
  To: Jacob Abel
  Cc: git, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Phillip Wood, Rubén Justo, Taylor Blau, rsbecker

Jacob Abel <jacobabel@nullpo.dev> writes:

>> So "sh t2400-*.sh -x -i" already shows "cat actual" output.  Try
>> the attached patch on top of this one and running it would show
>> the above message shown by test_must_be_empty and the contents of
>> the file 'actual'.
>
> That should not be the case.

Here is how it fails and test_must_be_empty complains that the
"actual" file is not empty, if you run the script with "-x -i" after
applying the patch in the message you are responding to on top of
this step.

    expecting success of 2400.37 'add --quiet':
            test_when_finished "git worktree remove -f -f another-worktree" &&
            test_when_finished cat actual >&2 &&
            git worktree add --quiet another-worktree main 2>actual &&
    echo foo >>actual &&
            test_must_be_empty actual

    ++ test_when_finished 'git worktree remove -f -f another-worktree'
    ++ test 0 = 0
    ++ test_cleanup='{ git worktree remove -f -f another-worktree
                    } && (exit "$eval_ret"); eval_ret=$?; :'
    ++ test_when_finished cat actual
    ++ test 0 = 0
    ++ test_cleanup='{ cat actual
                    } && (exit "$eval_ret"); eval_ret=$?; { git worktree remove -f -f another-worktree
                    } && (exit "$eval_ret"); eval_ret=$?; :'
    ++ git worktree add --quiet another-worktree main
    ++ echo foo
    ++ test_must_be_empty actual
    ++ test 1 -ne 1
    ++ test_path_is_file actual
    ++ test 1 -ne 1
    ++ test -f actual
    ++ test -s actual
    ++ echo ''\''actual'\'' is not empty, it contains:'
    'actual' is not empty, it contains:
    ++ cat actual
    foo
    ++ return 1
    error: last command exited with $?=1
    not ok 37 - add --quiet

Observe what test_must_be_empty does in the last part of the
transcript above.  If you run it without "-i", then the trace will
show "cat actual" twice (one from test_must_be_empty above, then a
redundant one from the test_when_finished).

Another reason why we shouldn't add this to test_when_finished is
because it would not help those who run the tests with "-i" option.
The test_cleanup handlers are meant to be "clean-up" routines, and
they are not run when the user uses "-i", intending to go into the
test directory after seeing the test fail and inspect what is left
there.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v9 2/8] t2400: print captured git output when finished
  2023-04-18 16:34       ` Junio C Hamano
@ 2023-04-19 13:23         ` Jacob Abel
  2023-04-19 13:36         ` Jacob Abel
  1 sibling, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-04-19 13:23 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Phillip Wood, Rubén Justo, Taylor Blau, rsbecker

On 23/04/18 09:34AM, Junio C Hamano wrote:
> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> > [...]
>
> Here is how it fails and test_must_be_empty complains that the
> "actual" file is not empty, if you run the script with "-x -i" after
> applying the patch in the message you are responding to on top of
> this step.
>
> [...]
>
> Observe what test_must_be_empty does in the last part of the
> transcript above.  If you run it without "-i", then the trace will
> show "cat actual" twice (one from test_must_be_empty above, then a
> redundant one from the test_when_finished).
>
> Another reason why we shouldn't add this to test_when_finished is
> because it would not help those who run the tests with "-i" option.
> The test_cleanup handlers are meant to be "clean-up" routines, and
> they are not run when the user uses "-i", intending to go into the
> test directory after seeing the test fail and inspect what is left
> there.

Ah ok I understand what you mean now.

Would the following work? Since all we care about in `git worktree add` is
`stderr`, can't we just duplicate `stderr` to `stdout` while redirecting
stderr to `actual` so that in the event the git command fails, it's still
displayed in the output of `sh t2400-*.sh -x`?

diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index 82091cbb1f..a8f734b1c3 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -327,8 +327,7 @@ test_expect_success 'add -B' '

 test_expect_success 'add --quiet' '
 	test_when_finished "git worktree remove -f -f another-worktree" &&
-	test_when_finished "cat actual" &&
-	git worktree add --quiet another-worktree main 2>actual &&
+	git worktree add --quiet another-worktree main 2>actual 2>&1 &&
 	test_must_be_empty actual
 '



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH v9 2/8] t2400: print captured git output when finished
  2023-04-18 16:34       ` Junio C Hamano
  2023-04-19 13:23         ` Jacob Abel
@ 2023-04-19 13:36         ` Jacob Abel
  2023-04-19 15:41           ` Junio C Hamano
  1 sibling, 1 reply; 34+ messages in thread
From: Jacob Abel @ 2023-04-19 13:36 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Phillip Wood, Rubén Justo, Taylor Blau, rsbecker

On 23/04/19 09:23AM, Jacob Abel wrote:
> On 23/04/18 09:34AM, Junio C Hamano wrote:
> > [...]
>
> Ah ok I understand what you mean now.
>
> Would the following work? Since all we care about in `git worktree add` is
> `stderr`, can't we just duplicate `stderr` to `stdout` while redirecting
> stderr to `actual` so that in the event the git command fails, it's still
> displayed in the output of `sh t2400-*.sh -x`?
>
> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
> index 82091cbb1f..a8f734b1c3 100755
> --- a/t/t2400-worktree-add.sh
> +++ b/t/t2400-worktree-add.sh
> @@ -327,8 +327,7 @@ test_expect_success 'add -B' '
>
>  test_expect_success 'add --quiet' '
>  	test_when_finished "git worktree remove -f -f another-worktree" &&
> -	test_when_finished "cat actual" &&
> -	git worktree add --quiet another-worktree main 2>actual &&
> +	git worktree add --quiet another-worktree main 2>actual 2>&1 &&
>  	test_must_be_empty actual
>  '
>

Ok scratch that. I tried checking this a bit more and it doesn't work quite
as expected. I'll remove the `cat actual`s from the tests and if I can
think of a better alternative, I'll report back.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v9 2/8] t2400: print captured git output when finished
  2023-04-19 13:36         ` Jacob Abel
@ 2023-04-19 15:41           ` Junio C Hamano
  2023-04-19 16:50             ` Jacob Abel
  0 siblings, 1 reply; 34+ messages in thread
From: Junio C Hamano @ 2023-04-19 15:41 UTC (permalink / raw)
  To: Jacob Abel
  Cc: git, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Phillip Wood, Rubén Justo, Taylor Blau, rsbecker

Jacob Abel <jacobabel@nullpo.dev> writes:

> On 23/04/19 09:23AM, Jacob Abel wrote:
>> On 23/04/18 09:34AM, Junio C Hamano wrote:
>> > [...]
>>
>> Ah ok I understand what you mean now.
>>
>> Would the following work? Since all we care about in `git worktree add` is
>> `stderr`, can't we just duplicate `stderr` to `stdout` while redirecting
>> stderr to `actual` so that in the event the git command fails, it's still
>> displayed in the output of `sh t2400-*.sh -x`?
>>
>> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
>> index 82091cbb1f..a8f734b1c3 100755
>> --- a/t/t2400-worktree-add.sh
>> +++ b/t/t2400-worktree-add.sh
>> @@ -327,8 +327,7 @@ test_expect_success 'add -B' '
>>
>>  test_expect_success 'add --quiet' '
>>  	test_when_finished "git worktree remove -f -f another-worktree" &&
>> -	test_when_finished "cat actual" &&
>> -	git worktree add --quiet another-worktree main 2>actual &&


>> +	git worktree add --quiet another-worktree main 2>actual 2>&1 &&
>>  	test_must_be_empty actual
>>  '
>>
>
> Ok scratch that. I tried checking this a bit more and it doesn't work quite
> as expected. I'll remove the `cat actual`s from the tests and if I can
> think of a better alternative, I'll report back.

While I do not understand what you are trying to achieve here, if
you expect both the standard output and error streams are quiet,
then

	cmd >output 2>&1

would store both to 'output' and test_must_be_empty can check what
is in there.

On the other hand,

	cmd 2>&1 >output

would store the standard output of 'cmd' in 'output' while sending
the standard error of 'cmd' to the standard output of the whole
thing.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v9 2/8] t2400: print captured git output when finished
  2023-04-19 15:41           ` Junio C Hamano
@ 2023-04-19 16:50             ` Jacob Abel
  0 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-04-19 16:50 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Phillip Wood, Rubén Justo, Taylor Blau, rsbecker

On 23/04/19 08:41AM, Junio C Hamano wrote:
> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> > [...]
>
> While I do not understand what you are trying to achieve here, if
> you expect both the standard output and error streams are quiet,
> then
>
> 	cmd >output 2>&1
>
> would store both to 'output' and test_must_be_empty can check what
> is in there.
>
> On the other hand,
>
> 	cmd 2>&1 >output
>
> would store the standard output of 'cmd' in 'output' while sending
> the standard error of 'cmd' to the standard output of the whole
> thing.

I thought it was redirecting to both actual and stdout (like a tee would)
and jumped the gun (replying to your message) before verifying that it
worked as I assumed.

The goal was to be able to capture stderr without suppressing it in the
output of `sh t2400-*.sh -x`. The original solution I had did it's job
while I was debugging an issue for the later patches in this set and I
suppose I could have always used `-i` and looked inside `actual` myself
after the git command failed.

Since the issues with the patchset I was having were resolved and there is
an alternate way to debug the tests in the event they fail, I'm just going
to drop these changes (and drop the `cat actual` changes).


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v9 3/8] t2400: refactor "worktree add" opt exclusion tests
  2023-04-17 21:30   ` Junio C Hamano
@ 2023-04-20  2:46     ` Jacob Abel
  0 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-04-20  2:46 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Phillip Wood, Rubén Justo, Taylor Blau, rsbecker

On 23/04/17 02:30PM, Junio C Hamano wrote:
> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> [...]
>
> > [....]
>
> Again, I do not think this sends output to the standard output at
> the end of this test piece.
>
> > +		test_must_fail git worktree add $opts 2>actual &&
> > +		grep -E "fatal:( options)? .* cannot be used together" actual
> > +	'
> > +}
>
> I do not think this patch is needed (I'd rather see people learn the
> trick of running with "-i" and rely on the fact that the trash
> directory is left intact to be inspected), but if you must, it may
> make more sense to add test_must_contain to make a failed 'grep
> "$@"' easier to see, similar to the way that test_must_be_empty
> helps a failing 'test !  -s "$1"', something along the lines of ...
>
> [...]
>
> Thanks.

Understood. As mentioned in the discussion for patch 2/8, these changes
(trying to `cat actual` on `test_cleanup`) will be reverted for the next
revision across all patches.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v9 4/8] t2400: add tests to verify --quiet
  2023-04-17 21:33   ` Junio C Hamano
@ 2023-04-20  2:48     ` Jacob Abel
  0 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-04-20  2:48 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Phillip Wood, Rubén Justo, Taylor Blau, rsbecker

On 23/04/17 02:33PM, Junio C Hamano wrote:
> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> > [...]
>
> It is good to test the --quiet option.  It is not good to have the
> ineffective "cat actual" when test_must_be_empty is already used.
> Probably the same comment applies to the rest of the patch.
>
> Thanks.

Understood. As mentioned in the patch 2/8 discussion, this change (the `cat
actual`) will be reverted in the next revision.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees
  2023-04-17  9:33 [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
                   ` (7 preceding siblings ...)
  2023-04-17  9:34 ` [PATCH v9 8/8] worktree add: emit warn when there is a bad HEAD Jacob Abel
@ 2023-04-20  3:05 ` Jacob Abel
  2023-05-01 21:51   ` Junio C Hamano
  2023-05-17 21:47 ` [RESEND PATCH v10 " Jacob Abel
  9 siblings, 1 reply; 34+ messages in thread
From: Jacob Abel @ 2023-04-20  3:05 UTC (permalink / raw)
  To: git, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

On 23/04/17 09:33AM, Jacob Abel wrote:
>
> [...]

I've noticed on the lore that this revision didn't continue the existing
thread[1] for the patchset. It seems this is because the `In-Reply-To`
header was stripped from the message due to a bug with the MTA/mail bridge
I use on my dev machine [2].

Apologies for any confusion this may have caused.

For continuity purposes I can either:

A. RESEND the revision to the main thread with a note in the cover letter
tagging the discussions from this thread so far.

B. Reply to [1] in the main thread to point to this thread and then
eventually publish v10 in reply to that reply message in the main thread.

C. Reply to [1] in the main thread to point to this thread and then
eventually publish v10 in reply to this cover letter.

Let me know which would be the least disruptive/most ideal way forward or
if there is another way I should approach this.

Thanks.

1. https://lore.kernel.org/git/20230109173227.29264-1-jacobabel@nullpo.dev/
2. https://github.com/ProtonMail/proton-bridge/issues/374


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees
  2023-04-20  3:05 ` [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
@ 2023-05-01 21:51   ` Junio C Hamano
  2023-05-02  5:48     ` Jacob Abel
  0 siblings, 1 reply; 34+ messages in thread
From: Junio C Hamano @ 2023-05-01 21:51 UTC (permalink / raw)
  To: Jacob Abel
  Cc: git, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Phillip Wood, Rubén Justo, Taylor Blau, rsbecker

Jacob Abel <jacobabel@nullpo.dev> writes:

> I've noticed on the lore that this revision didn't continue the existing
> thread[1] for the patchset. It seems this is because the `In-Reply-To`
> header was stripped from the message due to a bug with the MTA/mail bridge
> I use on my dev machine [2].
>
> Apologies for any confusion this may have caused.
>
> For continuity purposes I can either:
>
> A. RESEND the revision to the main thread with a note in the cover letter
> tagging the discussions from this thread so far.
>
> B. Reply to [1] in the main thread to point to this thread and then
> eventually publish v10 in reply to that reply message in the main thread.
>
> C. Reply to [1] in the main thread to point to this thread and then
> eventually publish v10 in reply to this cover letter.
>
> Let me know which would be the least disruptive/most ideal way forward or
> if there is another way I should approach this.
>
> Thanks.
>
> 1. https://lore.kernel.org/git/20230109173227.29264-1-jacobabel@nullpo.dev/
> 2. https://github.com/ProtonMail/proton-bridge/issues/374

Once the thread is broken, it is broken.  You gave a link in the
message I am responding to, to make it easier for people to go back
to earlier iterations and that is good enough, I think.

Your eventual v10 can be sent with its cover set as a reply to the
cover of v9 and we will be fine.

It seems that we may need another iteration, but if I reclal
correctly what remains are all minor issues?

Thanks.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees
  2023-05-01 21:51   ` Junio C Hamano
@ 2023-05-02  5:48     ` Jacob Abel
  0 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-05-02  5:48 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Phillip Wood, Rubén Justo, Taylor Blau, rsbecker

On 23/05/01 02:51PM, Junio C Hamano wrote:
> Jacob Abel <jacobabel@nullpo.dev> writes:
> > [...]
>
> Once the thread is broken, it is broken.  You gave a link in the
> message I am responding to, to make it easier for people to go back
> to earlier iterations and that is good enough, I think.

Understood.

>
> Your eventual v10 can be sent with its cover set as a reply to the
> cover of v9 and we will be fine.
>
> It seems that we may need another iteration, but if I reclal
> correctly what remains are all minor issues?
>
> Thanks.

Based on the feedback so far yes. However I have yet to get any reviews
on patches 5/8 through 8/8 for this revision so it's unclear to me if
they were fine/didn't require any changes or if they just weren't
reviewed yet?

Also I have yet to hear anything back about whether to keep patches
7/8 and 8/8 in this patchset or break them out into their own followup
patchset (original note about this was in the cover letter [1]).

1. https://lore.kernel.org/git/20230417093255.31079-1-jacobabel@nullpo.dev/


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [RESEND PATCH v10 0/8] worktree: Support `--orphan` when creating new worktrees
  2023-04-17  9:33 [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
                   ` (8 preceding siblings ...)
  2023-04-20  3:05 ` [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
@ 2023-05-17 21:47 ` Jacob Abel
  2023-05-17 21:48   ` [RESEND PATCH v10 1/8] worktree add: include -B in usage docs Jacob Abel
                     ` (7 more replies)
  9 siblings, 8 replies; 34+ messages in thread
From: Jacob Abel @ 2023-05-17 21:47 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

This patchset introduces the ability to create new worktrees from orphan/unborn
branches and introduces DWIM behavior to create worktrees from an orphan branch
when no valid refs exists locally in the repository (as is typical in newly
initialized repositories) or on a remote (when `--guess-remote` is used). 

This addresses the issue of `git worktree add` failing when attempting to create
a worktree from a newly initialized repository (which can be seen in this SO
question [1]).

NOTE: Resend of Patch v10 as my mail setup tampered with and broke the patchset. 
original v10 cover: <20230507120530.14669-1-jacobabel@nullpo.dev>

This patchset has eight parts:
  * adding `-B` to the usage docs (noticed during dev and it seemed too small
    to justify a separate submission)
  * cleaning up a left-behind worktree in t2400
  * adding a helper fn to simplify testing for mutual exclusion of options
    in `t/t2400-worktree-add.sh`
  * adding additional test cases to verify both that behavior doesn't change
    when using `--quiet` and that the extraneous output is properly suppressed.
  * adding the ability to create a worktree from an unborn/orphan branch
    to `git-worktree-add`
  * adding an advise for using --orphan when `git worktree add` fails due to 
    a bad ref.
  * adding functionality to DWIM when there are no existing branches and the
    user likely intends to create an orphan branch.
  * updating worktree add to emit a warning (containing debug information 
    about the current HEAD) when trying to use a HEAD that points to a
    non-existant (or unborn) reference and there exist other valid branches.

Changes from v9:
  * Revert `test_when_finished cat actual` changes in t2400 (2/8)[2].
  * Rename commit 2/8 to reflect changes.
  * Revert `test_when_finished cat actual` changes in t2400 (3/8)[3].
  * Revert `test_when_finished cat actual` changes in t2400 (4/8)[4].
  * Revert `test_when_finished cat actual` changes in t2400 (5/8).
  * Remove extraneous whitespace from command in t2400 (5/8).
  * Revert `test_when_finished cat actual` changes in t2400 (6/8).
  * Include `advice.h` in `worktree.c` to resolve missing include when 
    applying patch on top of main (6/8).
  * Revert `test_when_finished cat actual` changes in t2400 (7/8).
  * Remove extraneous whitespace from comment in `worktree.c` (7/8).

1. https://stackoverflow.com/a/68717229/15064705/
2. https://lore.kernel.org/git/xmqq8reqkyfz.fsf@gitster.g/
3. https://lore.kernel.org/git/xmqqmt36jixr.fsf@gitster.g/
4. https://lore.kernel.org/git/xmqqfs8yjisl.fsf@gitster.g/

Jacob Abel (8):
  worktree add: include -B in usage docs
  t2400: cleanup created worktree in test
  t2400: refactor "worktree add" opt exclusion tests
  t2400: add tests to verify --quiet
  worktree add: add --orphan flag
  worktree add: introduce "try --orphan" hint
  worktree add: extend DWIM to infer --orphan
  worktree add: emit warn when there is a bad HEAD

 Documentation/config/advice.txt |   4 +
 Documentation/git-worktree.txt  |  16 +-
 advice.c                        |   1 +
 advice.h                        |   1 +
 builtin/worktree.c              | 227 +++++++++++++-
 t/t2400-worktree-add.sh         | 507 +++++++++++++++++++++++++++++++-
 6 files changed, 735 insertions(+), 21 deletions(-)

Range-diff against v9:
1:  91153fdb4c = 1:  91153fdb4c worktree add: include -B in usage docs
2:  8cfbc89dd5 ! 2:  0f30e9a9e3 t2400: print captured git output when finished
    @@ Metadata
     Author: Jacob Abel <jacobabel@nullpo.dev>
     
      ## Commit message ##
    -    t2400: print captured git output when finished
    -
    -    Update tests that capture stderr so that at the end of the test they
    -    print the captured text back out to stderr. This simplifies debugging
    -    when inspecting test logs after executing with `-x`.
    +    t2400: cleanup created worktree in test
     
         Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
     
    @@ t/t2400-worktree-add.sh: test_expect_success 'add -B' '
      
      test_expect_success 'add --quiet' '
     +	test_when_finished "git worktree remove -f -f another-worktree" &&
    -+	test_when_finished cat actual >&2 &&
      	git worktree add --quiet another-worktree main 2>actual &&
      	test_must_be_empty actual
      '
3:  ab03d92c3a ! 3:  06e8c53bc6 t2400: refactor "worktree add" opt exclusion tests
    @@ t/t2400-worktree-add.sh: test_expect_success '"add" no auto-vivify with --detach
     +test_wt_add_excl () {
     +	local opts="$*" &&
     +	test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
    -+		test_when_finished cat actual >&2 &&
     +		test_must_fail git worktree add $opts 2>actual &&
     +		grep -E "fatal:( options)? .* cannot be used together" actual
     +	'
4:  d9a3468c93 ! 4:  d9330db91f t2400: add tests to verify --quiet
    @@ t/t2400-worktree-add.sh: test_expect_success 'add --quiet' '
     +test_expect_success 'add --quiet -b' '
     +	test_when_finished "git branch -D quietnewbranch" &&
     +	test_when_finished "git worktree remove -f -f another-worktree" &&
    -+	test_when_finished cat actual >&2 &&
     +	git worktree add --quiet -b quietnewbranch another-worktree 2>actual &&
     +	test_must_be_empty actual
     +'
    @@ t/t2400-worktree-add.sh: test_expect_success 'git worktree add --guess-remote se
      '
     +test_expect_success 'git worktree add --guess-remote sets up tracking (quiet)' '
     +	test_when_finished rm -rf repo_a repo_b foo &&
    -+	test_when_finished cat repo_b/actual >&2 &&
     +	setup_remote_repo repo_a repo_b &&
     +	(
     +		cd repo_b &&
5:  8ef9587deb ! 5:  a5a78e5f53 worktree add: add --orphan flag
    @@ t/t2400-worktree-add.sh: test_expect_success 'add --quiet -b' '
     +
     +test_expect_success '"add --orphan --quiet"' '
     +	test_when_finished "git worktree remove -f -f orphandir" &&
    -+	test_when_finished cat log.actual >&2 &&
     +	git worktree add --quiet --orphan -b neworphan orphandir 2>log.actual &&
     +	test_must_be_empty log.actual &&
     +	echo refs/heads/neworphan >expected &&
    @@ t/t2400-worktree-add.sh: test_expect_success 'add --quiet -b' '
     +	test_when_finished "rm -rf empty_repo" &&
     +	echo refs/heads/newbranch >expected &&
     +	GIT_DIR="empty_repo" git init --bare &&
    -+	git -C empty_repo  worktree add --orphan -b newbranch worktreedir &&
    ++	git -C empty_repo worktree add --orphan -b newbranch worktreedir &&
     +	git -C empty_repo/worktreedir symbolic-ref HEAD >actual &&
     +	test_cmp expected actual
     +'
6:  d2800266f9 ! 6:  96b1946e64 worktree add: introduce "try --orphan" hint
    @@ advice.h: struct string_list;
      int git_default_advice_config(const char *var, const char *value);
     
      ## builtin/worktree.c ##
    +@@
    + #include "cache.h"
    + #include "abspath.h"
    ++#include "advice.h"
    + #include "checkout.h"
    + #include "config.h"
    + #include "builtin.h"
     @@
      #define BUILTIN_WORKTREE_UNLOCK_USAGE \
      	N_("git worktree unlock <worktree>")
    @@ t/t2400-worktree-add.sh: test_expect_success '"add" worktree with orphan branch,
     +		git init repo &&
     +		(cd repo && test_commit commit) &&
     +		git -C repo switch --orphan noref &&
    -+		test_when_finished cat actual >&2 &&
     +		test_must_fail git -C repo worktree add $opts foobar/ 2>actual &&
     +		! grep "error: unknown switch" actual &&
     +		grep "hint: If you meant to create a worktree containing a new orphan branch" actual &&
    @@ t/t2400-worktree-add.sh: test_expect_success '"add" worktree with orphan branch,
     +	test_when_finished "rm -rf repo" &&
     +	git init repo &&
     +	(cd repo && test_commit commit) &&
    -+	test_when_finished cat actual >&2 &&
     +	test_must_fail git -C repo worktree add --quiet foobar_branch foobar/ 2>actual &&
     +	! grep "error: unknown switch" actual &&
     +	! grep "hint: If you meant to create a worktree containing a new orphan branch" actual
7:  e5e139766c ! 7:  52fef9672c worktree add: extend DWIM to infer --orphan
    @@ builtin/worktree.c: static void print_preparing_worktree_line(int detach,
     +/**
     + * Determines whether `--orphan` should be inferred in the evaluation of
     + * `worktree add path/` or `worktree add -b branch path/` and emits an error
    -+ * if the supplied arguments would produce an illegal combination  when the
    ++ * if the supplied arguments would produce an illegal combination when the
     + * `--orphan` flag is included.
     + *
     + * `opts` and `opt_track` contain the other options & flags supplied to the
    @@ t/t2400-worktree-add.sh: test_expect_success 'git worktree --no-guess-remote opt
     +		then
     +			test_when_finished git -C repo worktree remove ../foo
     +		fi &&
    -+		if [ $use_cd -eq 1 ]
    -+		then
    -+			test_when_finished cat "$git_ns/actual" >&2
    -+		else
    -+			test_when_finished cat actual >&2
    -+		fi &&
     +		(
     +			if [ $use_cd -eq 1 ]
     +			then
8:  296226ffd5 = 8:  8c3fded12b worktree add: emit warn when there is a bad HEAD
-- 
2.39.3



^ permalink raw reply	[flat|nested] 34+ messages in thread

* [RESEND PATCH v10 1/8] worktree add: include -B in usage docs
  2023-05-17 21:47 ` [RESEND PATCH v10 " Jacob Abel
@ 2023-05-17 21:48   ` Jacob Abel
  2023-05-17 21:48   ` [RESEND PATCH v10 2/8] t2400: cleanup created worktree in test Jacob Abel
                     ` (6 subsequent siblings)
  7 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-05-17 21:48 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Document `-B` next to where `-b` is already documented to bring the
usage docs in line with other commands such as git checkout.

Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 Documentation/git-worktree.txt | 2 +-
 builtin/worktree.c             | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 063d6eeb99..b9c12779f1 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
-		   [-b <new-branch>] <path> [<commit-ish>]
+		   [(-b | -B) <new-branch>] <path> [<commit-ish>]
 'git worktree list' [-v | --porcelain [-z]]
 'git worktree lock' [--reason <string>] <worktree>
 'git worktree move' <worktree> <new-path>
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 39e9e5c9ce..d1b4b53f2c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -22,7 +22,7 @@
 
 #define BUILTIN_WORKTREE_ADD_USAGE \
 	N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
-	   "                 [-b <new-branch>] <path> [<commit-ish>]")
+	   "                 [(-b | -B) <new-branch>] <path> [<commit-ish>]")
 #define BUILTIN_WORKTREE_LIST_USAGE \
 	N_("git worktree list [-v | --porcelain [-z]]")
 #define BUILTIN_WORKTREE_LOCK_USAGE \
-- 
2.39.3



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RESEND PATCH v10 2/8] t2400: cleanup created worktree in test
  2023-05-17 21:47 ` [RESEND PATCH v10 " Jacob Abel
  2023-05-17 21:48   ` [RESEND PATCH v10 1/8] worktree add: include -B in usage docs Jacob Abel
@ 2023-05-17 21:48   ` Jacob Abel
  2023-05-17 21:48   ` [RESEND PATCH v10 3/8] t2400: refactor "worktree add" opt exclusion tests Jacob Abel
                     ` (5 subsequent siblings)
  7 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-05-17 21:48 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 t/t2400-worktree-add.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index d587e0b20d..a3f108347a 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -326,6 +326,7 @@ test_expect_success 'add -B' '
 '
 
 test_expect_success 'add --quiet' '
+	test_when_finished "git worktree remove -f -f another-worktree" &&
 	git worktree add --quiet another-worktree main 2>actual &&
 	test_must_be_empty actual
 '
-- 
2.39.3



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RESEND PATCH v10 3/8] t2400: refactor "worktree add" opt exclusion tests
  2023-05-17 21:47 ` [RESEND PATCH v10 " Jacob Abel
  2023-05-17 21:48   ` [RESEND PATCH v10 1/8] worktree add: include -B in usage docs Jacob Abel
  2023-05-17 21:48   ` [RESEND PATCH v10 2/8] t2400: cleanup created worktree in test Jacob Abel
@ 2023-05-17 21:48   ` Jacob Abel
  2023-05-17 21:48   ` [RESEND PATCH v10 4/8] t2400: add tests to verify --quiet Jacob Abel
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-05-17 21:48 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Pull duplicate test code into a function so that additional opt
combinations can be tested succinctly.

Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 t/t2400-worktree-add.sh | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index a3f108347a..0ca3ec2022 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -298,17 +298,20 @@ test_expect_success '"add" no auto-vivify with --detach and <branch> omitted' '
 	test_must_fail git -C mish/mash symbolic-ref HEAD
 '
 
-test_expect_success '"add" -b/-B mutually exclusive' '
-	test_must_fail git worktree add -b poodle -B poodle bamboo main
-'
-
-test_expect_success '"add" -b/--detach mutually exclusive' '
-	test_must_fail git worktree add -b poodle --detach bamboo main
-'
+# Helper function to test mutually exclusive options.
+#
+# Note: Quoted arguments containing spaces are not supported.
+test_wt_add_excl () {
+	local opts="$*" &&
+	test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
+		test_must_fail git worktree add $opts 2>actual &&
+		grep -E "fatal:( options)? .* cannot be used together" actual
+	'
+}
 
-test_expect_success '"add" -B/--detach mutually exclusive' '
-	test_must_fail git worktree add -B poodle --detach bamboo main
-'
+test_wt_add_excl -b poodle -B poodle bamboo main
+test_wt_add_excl -b poodle --detach bamboo main
+test_wt_add_excl -B poodle --detach bamboo main
 
 test_expect_success '"add -B" fails if the branch is checked out' '
 	git rev-parse newmain >before &&
-- 
2.39.3



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RESEND PATCH v10 4/8] t2400: add tests to verify --quiet
  2023-05-17 21:47 ` [RESEND PATCH v10 " Jacob Abel
                     ` (2 preceding siblings ...)
  2023-05-17 21:48   ` [RESEND PATCH v10 3/8] t2400: refactor "worktree add" opt exclusion tests Jacob Abel
@ 2023-05-17 21:48   ` Jacob Abel
  2023-05-17 21:48   ` [RESEND PATCH v10 5/8] worktree add: add --orphan flag Jacob Abel
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-05-17 21:48 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Add tests to verify that the command performs operations the same with
`--quiet` as without it. Additionally verifies that all non-fatal output
is suppressed.

Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 t/t2400-worktree-add.sh | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index 0ca3ec2022..841f15f59e 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -334,6 +334,13 @@ test_expect_success 'add --quiet' '
 	test_must_be_empty actual
 '
 
+test_expect_success 'add --quiet -b' '
+	test_when_finished "git branch -D quietnewbranch" &&
+	test_when_finished "git worktree remove -f -f another-worktree" &&
+	git worktree add --quiet -b quietnewbranch another-worktree 2>actual &&
+	test_must_be_empty actual
+'
+
 test_expect_success 'local clone from linked checkout' '
 	git clone --local here here-clone &&
 	( cd here-clone && git fsck )
@@ -532,6 +539,35 @@ test_expect_success 'git worktree add --guess-remote sets up tracking' '
 		test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo
 	)
 '
+test_expect_success 'git worktree add --guess-remote sets up tracking (quiet)' '
+	test_when_finished rm -rf repo_a repo_b foo &&
+	setup_remote_repo repo_a repo_b &&
+	(
+		cd repo_b &&
+		git worktree add --quiet --guess-remote ../foo 2>actual &&
+		test_must_be_empty actual
+	) &&
+	(
+		cd foo &&
+		test_branch_upstream foo repo_a foo &&
+		test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo
+	)
+'
+
+test_expect_success 'git worktree --no-guess-remote (quiet)' '
+	test_when_finished rm -rf repo_a repo_b foo &&
+	setup_remote_repo repo_a repo_b &&
+	(
+		cd repo_b &&
+		git worktree add --quiet --no-guess-remote ../foo
+	) &&
+	(
+		cd foo &&
+		test_must_fail git config "branch.foo.remote" &&
+		test_must_fail git config "branch.foo.merge" &&
+		test_cmp_rev ! refs/remotes/repo_a/foo refs/heads/foo
+	)
+'
 
 test_expect_success 'git worktree add with worktree.guessRemote sets up tracking' '
 	test_when_finished rm -rf repo_a repo_b foo &&
-- 
2.39.3



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RESEND PATCH v10 5/8] worktree add: add --orphan flag
  2023-05-17 21:47 ` [RESEND PATCH v10 " Jacob Abel
                     ` (3 preceding siblings ...)
  2023-05-17 21:48   ` [RESEND PATCH v10 4/8] t2400: add tests to verify --quiet Jacob Abel
@ 2023-05-17 21:48   ` Jacob Abel
  2023-05-17 21:48   ` [RESEND PATCH v10 6/8] worktree add: introduce "try --orphan" hint Jacob Abel
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-05-17 21:48 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Add support for creating an orphan branch when adding a new worktree.
The functionality of this flag is equivalent to git switch's --orphan
option.

Current Behavior:
% git -C foo.git --no-pager branch -l
+ main
% git -C foo.git worktree add main/
Preparing worktree (new branch 'main')
HEAD is now at 6c93a75 a commit
%

% git init bar.git
Initialized empty Git repository in /path/to/bar.git/
% git -C bar.git --no-pager branch -l

% git -C bar.git worktree add main/
Preparing worktree (new branch 'main')
fatal: not a valid object name: 'HEAD'
%

New Behavior:

% git -C foo.git --no-pager branch -l
+ main
% git -C foo.git worktree add main/
Preparing worktree (new branch 'main')
HEAD is now at 6c93a75 a commit
%

% git init --bare bar.git
Initialized empty Git repository in /path/to/bar.git/
% git -C bar.git --no-pager branch -l

% git -C bar.git worktree add main/
Preparing worktree (new branch 'main')
fatal: invalid reference: HEAD
% git -C bar.git worktree add --orphan -b main/
Preparing worktree (new branch 'main')
% git -C bar.git worktree add --orphan -b newbranch worktreedir/
Preparing worktree (new branch 'newbranch')
%

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 Documentation/git-worktree.txt |  6 ++-
 builtin/worktree.c             | 67 +++++++++++++++++++++++++++------
 t/t2400-worktree-add.sh        | 68 ++++++++++++++++++++++++++++++++++
 3 files changed, 129 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index b9c12779f1..485d865eb2 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
-		   [(-b | -B) <new-branch>] <path> [<commit-ish>]
+		   [--orphan] [(-b | -B) <new-branch>] <path> [<commit-ish>]
 'git worktree list' [-v | --porcelain [-z]]
 'git worktree lock' [--reason <string>] <worktree>
 'git worktree move' <worktree> <new-path>
@@ -222,6 +222,10 @@ This can also be set up as the default behaviour by using the
 	With `prune`, do not remove anything; just report what it would
 	remove.
 
+--orphan::
+	With `add`, make the new worktree and index empty, associating
+	the worktree with a new orphan/unborn branch named `<new-branch>`.
+
 --porcelain::
 	With `list`, output in an easy-to-parse format for scripts.
 	This format will remain stable across Git versions and regardless of user
diff --git a/builtin/worktree.c b/builtin/worktree.c
index d1b4b53f2c..48de7fc3b0 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -22,7 +22,8 @@
 
 #define BUILTIN_WORKTREE_ADD_USAGE \
 	N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
-	   "                 [(-b | -B) <new-branch>] <path> [<commit-ish>]")
+	   "                 [--orphan] [(-b | -B) <new-branch>] <path> [<commit-ish>]")
+
 #define BUILTIN_WORKTREE_LIST_USAGE \
 	N_("git worktree list [-v | --porcelain [-z]]")
 #define BUILTIN_WORKTREE_LOCK_USAGE \
@@ -95,6 +96,7 @@ struct add_opts {
 	int detach;
 	int quiet;
 	int checkout;
+	int orphan;
 	const char *keep_locked;
 };
 
@@ -368,6 +370,22 @@ static int checkout_worktree(const struct add_opts *opts,
 	return run_command(&cp);
 }
 
+static int make_worktree_orphan(const char * ref, const struct add_opts *opts,
+				struct strvec *child_env)
+{
+	struct strbuf symref = STRBUF_INIT;
+	struct child_process cp = CHILD_PROCESS_INIT;
+
+	validate_new_branchname(ref, &symref, 0);
+	strvec_pushl(&cp.args, "symbolic-ref", "HEAD", symref.buf, NULL);
+	if (opts->quiet)
+		strvec_push(&cp.args, "--quiet");
+	strvec_pushv(&cp.env, child_env->v);
+	strbuf_release(&symref);
+	cp.git_cmd = 1;
+	return run_command(&cp);
+}
+
 static int add_worktree(const char *path, const char *refname,
 			const struct add_opts *opts)
 {
@@ -397,7 +415,7 @@ static int add_worktree(const char *path, const char *refname,
 			die_if_checked_out(symref.buf, 0);
 	}
 	commit = lookup_commit_reference_by_name(refname);
-	if (!commit)
+	if (!commit && !opts->orphan)
 		die(_("invalid reference: %s"), refname);
 
 	name = worktree_basename(path, &len);
@@ -486,10 +504,10 @@ static int add_worktree(const char *path, const char *refname,
 	strvec_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
 	cp.git_cmd = 1;
 
-	if (!is_branch)
+	if (!is_branch && commit) {
 		strvec_pushl(&cp.args, "update-ref", "HEAD",
 			     oid_to_hex(&commit->object.oid), NULL);
-	else {
+	} else {
 		strvec_pushl(&cp.args, "symbolic-ref", "HEAD",
 			     symref.buf, NULL);
 		if (opts->quiet)
@@ -501,6 +519,10 @@ static int add_worktree(const char *path, const char *refname,
 	if (ret)
 		goto done;
 
+	if (opts->orphan &&
+	    (ret = make_worktree_orphan(refname, opts, &child_env)))
+		goto done;
+
 	if (opts->checkout &&
 	    (ret = checkout_worktree(opts, &child_env)))
 		goto done;
@@ -520,7 +542,7 @@ static int add_worktree(const char *path, const char *refname,
 	 * Hook failure does not warrant worktree deletion, so run hook after
 	 * is_junk is cleared, but do return appropriate code when hook fails.
 	 */
-	if (!ret && opts->checkout) {
+	if (!ret && opts->checkout && !opts->orphan) {
 		struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
 
 		strvec_pushl(&opt.env, "GIT_DIR", "GIT_WORK_TREE", NULL);
@@ -568,7 +590,7 @@ static void print_preparing_worktree_line(int detach,
 		else {
 			struct commit *commit = lookup_commit_reference_by_name(branch);
 			if (!commit)
-				die(_("invalid reference: %s"), branch);
+				BUG(_("unreachable: invalid reference: %s"), branch);
 			fprintf_ln(stderr, _("Preparing worktree (detached HEAD %s)"),
 				  repo_find_unique_abbrev(the_repository, &commit->object.oid, DEFAULT_ABBREV));
 		}
@@ -620,6 +642,7 @@ static int add(int ac, const char **av, const char *prefix)
 			   N_("create a new branch")),
 		OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
 			   N_("create or reset a branch")),
+		OPT_BOOL(0, "orphan", &opts.orphan, N_("create unborn/orphaned branch")),
 		OPT_BOOL('d', "detach", &opts.detach, N_("detach HEAD at named commit")),
 		OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new working tree")),
 		OPT_BOOL(0, "lock", &keep_locked, N_("keep the new working tree locked")),
@@ -640,6 +663,17 @@ static int add(int ac, const char **av, const char *prefix)
 	ac = parse_options(ac, av, prefix, options, git_worktree_add_usage, 0);
 	if (!!opts.detach + !!new_branch + !!new_branch_force > 1)
 		die(_("options '%s', '%s', and '%s' cannot be used together"), "-b", "-B", "--detach");
+	if (opts.detach && opts.orphan)
+		die(_("options '%s', and '%s' cannot be used together"),
+		    "--orphan", "--detach");
+	if (opts.orphan && opt_track)
+		die(_("'%s' and '%s' cannot be used together"), "--orphan", "--track");
+	if (opts.orphan && !opts.checkout)
+		die(_("'%s' and '%s' cannot be used together"), "--orphan",
+		    "--no-checkout");
+	if (opts.orphan && ac == 2)
+		die(_("'%s' and '%s' cannot be used together"), "--orphan",
+		    _("<commit-ish>"));
 	if (lock_reason && !keep_locked)
 		die(_("the option '%s' requires '%s'"), "--reason", "--lock");
 	if (lock_reason)
@@ -668,13 +702,17 @@ static int add(int ac, const char **av, const char *prefix)
 		strbuf_release(&symref);
 	}
 
-	if (ac < 2 && !new_branch && !opts.detach) {
+	if (opts.orphan && !new_branch) {
+		int n;
+		const char *s = worktree_basename(path, &n);
+		new_branch = xstrndup(s, n);
+	} else if (new_branch || opts.detach || opts.orphan) {
+		// No-op
+	} else if (ac < 2) {
 		const char *s = dwim_branch(path, &new_branch);
 		if (s)
 			branch = s;
-	}
-
-	if (ac == 2 && !new_branch && !opts.detach) {
+	} else if (ac == 2) {
 		struct object_id oid;
 		struct commit *commit;
 		const char *remote;
@@ -688,10 +726,17 @@ static int add(int ac, const char **av, const char *prefix)
 			}
 		}
 	}
+
+	if (!opts.orphan && !lookup_commit_reference_by_name(branch)) {
+		die(_("invalid reference: %s"), branch);
+	}
+
 	if (!opts.quiet)
 		print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);
 
-	if (new_branch) {
+	if (opts.orphan) {
+		branch = new_branch;
+	} else if (new_branch) {
 		struct child_process cp = CHILD_PROCESS_INIT;
 		cp.git_cmd = 1;
 		strvec_push(&cp.args, "branch");
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index 841f15f59e..fba90582b6 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -312,6 +312,10 @@ test_wt_add_excl () {
 test_wt_add_excl -b poodle -B poodle bamboo main
 test_wt_add_excl -b poodle --detach bamboo main
 test_wt_add_excl -B poodle --detach bamboo main
+test_wt_add_excl --orphan --detach bamboo
+test_wt_add_excl --orphan --no-checkout bamboo
+test_wt_add_excl --orphan bamboo main
+test_wt_add_excl --orphan -b bamboo wtdir/ main
 
 test_expect_success '"add -B" fails if the branch is checked out' '
 	git rev-parse newmain >before &&
@@ -341,6 +345,62 @@ test_expect_success 'add --quiet -b' '
 	test_must_be_empty actual
 '
 
+test_expect_success '"add --orphan"' '
+	test_when_finished "git worktree remove -f -f orphandir" &&
+	git worktree add --orphan -b neworphan orphandir &&
+	echo refs/heads/neworphan >expected &&
+	git -C orphandir symbolic-ref HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '"add --orphan (no -b)"' '
+	test_when_finished "git worktree remove -f -f neworphan" &&
+	git worktree add --orphan neworphan &&
+	echo refs/heads/neworphan >expected &&
+	git -C neworphan symbolic-ref HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '"add --orphan --quiet"' '
+	test_when_finished "git worktree remove -f -f orphandir" &&
+	git worktree add --quiet --orphan -b neworphan orphandir 2>log.actual &&
+	test_must_be_empty log.actual &&
+	echo refs/heads/neworphan >expected &&
+	git -C orphandir symbolic-ref HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '"add --orphan" fails if the branch already exists' '
+	test_when_finished "git branch -D existingbranch" &&
+	git worktree add -b existingbranch orphandir main &&
+	git worktree remove orphandir &&
+	test_must_fail git worktree add --orphan -b existingbranch orphandir
+'
+
+test_expect_success '"add --orphan" with empty repository' '
+	test_when_finished "rm -rf empty_repo" &&
+	echo refs/heads/newbranch >expected &&
+	GIT_DIR="empty_repo" git init --bare &&
+	git -C empty_repo worktree add --orphan -b newbranch worktreedir &&
+	git -C empty_repo/worktreedir symbolic-ref HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '"add" worktree with orphan branch and lock' '
+	git worktree add --lock --orphan -b orphanbr orphan-with-lock &&
+	test_when_finished "git worktree unlock orphan-with-lock || :" &&
+	test -f .git/worktrees/orphan-with-lock/locked
+'
+
+test_expect_success '"add" worktree with orphan branch, lock, and reason' '
+	lock_reason="why not" &&
+	git worktree add --detach --lock --reason "$lock_reason" orphan-with-lock-reason main &&
+	test_when_finished "git worktree unlock orphan-with-lock-reason || :" &&
+	test -f .git/worktrees/orphan-with-lock-reason/locked &&
+	echo "$lock_reason" >expect &&
+	test_cmp expect .git/worktrees/orphan-with-lock-reason/locked
+'
+
 test_expect_success 'local clone from linked checkout' '
 	git clone --local here here-clone &&
 	( cd here-clone && git fsck )
@@ -457,6 +517,14 @@ setup_remote_repo () {
 	)
 }
 
+test_expect_success '"add" <path> <remote/branch> w/ no HEAD' '
+	test_when_finished rm -rf repo_upstream repo_local foo &&
+	setup_remote_repo repo_upstream repo_local &&
+	git -C repo_local config --bool core.bare true &&
+	git -C repo_local branch -D main &&
+	git -C repo_local worktree add ./foo repo_upstream/foo
+'
+
 test_expect_success '--no-track avoids setting up tracking' '
 	test_when_finished rm -rf repo_upstream repo_local foo &&
 	setup_remote_repo repo_upstream repo_local &&
-- 
2.39.3



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RESEND PATCH v10 6/8] worktree add: introduce "try --orphan" hint
  2023-05-17 21:47 ` [RESEND PATCH v10 " Jacob Abel
                     ` (4 preceding siblings ...)
  2023-05-17 21:48   ` [RESEND PATCH v10 5/8] worktree add: add --orphan flag Jacob Abel
@ 2023-05-17 21:48   ` Jacob Abel
  2023-05-17 21:48   ` [RESEND PATCH v10 7/8] worktree add: extend DWIM to infer --orphan Jacob Abel
  2023-05-17 21:49   ` [RESEND PATCH v10 8/8] worktree add: emit warn when there is a bad HEAD Jacob Abel
  7 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-05-17 21:48 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Add a new advice/hint in `git worktree add` for when the user
tries to create a new worktree from a reference that doesn't exist.

Current Behavior:

% git init foo
Initialized empty Git repository in /path/to/foo/
% touch file
% git -C foo commit -q -a -m "test commit"
% git -C foo switch --orphan norefbranch
% git -C foo worktree add newbranch/
Preparing worktree (new branch 'newbranch')
fatal: invalid reference: HEAD
%

New Behavior:

% git init --bare foo
Initialized empty Git repository in /path/to/foo/
% touch file
% git -C foo commit -q -a -m "test commit"
% git -C foo switch --orphan norefbranch
% git -C foo worktree add newbranch/
Preparing worktree (new branch 'newbranch')
hint: If you meant to create a worktree containing a new orphan branch
hint: (branch with no commits) for this repository, you can do so
hint: using the --orphan option:
hint:
hint:   git worktree add --orphan newbranch/
hint:
hint: Disable this message with "git config advice.worktreeAddOrphan false"
fatal: invalid reference: HEAD
% git -C foo worktree add -b newbranch2 new_wt/
Preparing worktree (new branch 'newbranch')
hint: If you meant to create a worktree containing a new orphan branch
hint: (branch with no commits) for this repository, you can do so
hint: using the --orphan option:
hint:
hint:   git worktree add --orphan -b newbranch2 new_wt/
hint:
hint: Disable this message with "git config advice.worktreeAddOrphan false"
fatal: invalid reference: HEAD
%

Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 Documentation/config/advice.txt |  4 ++++
 advice.c                        |  1 +
 advice.h                        |  1 +
 builtin/worktree.c              | 26 +++++++++++++++++++++++
 t/t2400-worktree-add.sh         | 37 +++++++++++++++++++++++++++++++++
 5 files changed, 69 insertions(+)

diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index c96b5b2e5d..c548a91e67 100644
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -138,4 +138,8 @@ advice.*::
 		checkout.
 	diverging::
 		Advice shown when a fast-forward is not possible.
+	worktreeAddOrphan::
+		Advice shown when a user tries to create a worktree from an
+		invalid reference, to instruct how to create a new orphan
+		branch instead.
 --
diff --git a/advice.c b/advice.c
index d6232439c3..e5a9bb9b44 100644
--- a/advice.c
+++ b/advice.c
@@ -78,6 +78,7 @@ static struct {
 	[ADVICE_SUBMODULES_NOT_UPDATED] 		= { "submodulesNotUpdated", 1 },
 	[ADVICE_UPDATE_SPARSE_PATH]			= { "updateSparsePath", 1 },
 	[ADVICE_WAITING_FOR_EDITOR]			= { "waitingForEditor", 1 },
+	[ADVICE_WORKTREE_ADD_ORPHAN]			= { "worktreeAddOrphan", 1 },
 };
 
 static const char turn_off_instructions[] =
diff --git a/advice.h b/advice.h
index 0f584163f5..2affbe1426 100644
--- a/advice.h
+++ b/advice.h
@@ -49,6 +49,7 @@ struct string_list;
 	ADVICE_UPDATE_SPARSE_PATH,
 	ADVICE_WAITING_FOR_EDITOR,
 	ADVICE_SKIPPED_CHERRY_PICKS,
+	ADVICE_WORKTREE_ADD_ORPHAN,
 };
 
 int git_default_advice_config(const char *var, const char *value);
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 48de7fc3b0..15bdb380c7 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "abspath.h"
+#include "advice.h"
 #include "checkout.h"
 #include "config.h"
 #include "builtin.h"
@@ -39,6 +40,20 @@
 #define BUILTIN_WORKTREE_UNLOCK_USAGE \
 	N_("git worktree unlock <worktree>")
 
+#define WORKTREE_ADD_ORPHAN_WITH_DASH_B_HINT_TEXT \
+	_("If you meant to create a worktree containing a new orphan branch\n" \
+	"(branch with no commits) for this repository, you can do so\n" \
+	"using the --orphan flag:\n" \
+	"\n" \
+	"	git worktree add --orphan -b %s %s\n")
+
+#define WORKTREE_ADD_ORPHAN_NO_DASH_B_HINT_TEXT \
+	_("If you meant to create a worktree containing a new orphan branch\n" \
+	"(branch with no commits) for this repository, you can do so\n" \
+	"using the --orphan flag:\n" \
+	"\n" \
+	"	git worktree add --orphan %s\n")
+
 static const char * const git_worktree_usage[] = {
 	BUILTIN_WORKTREE_ADD_USAGE,
 	BUILTIN_WORKTREE_LIST_USAGE,
@@ -634,6 +649,7 @@ static int add(int ac, const char **av, const char *prefix)
 	const char *opt_track = NULL;
 	const char *lock_reason = NULL;
 	int keep_locked = 0;
+	int used_new_branch_options;
 	struct option options[] = {
 		OPT__FORCE(&opts.force,
 			   N_("checkout <branch> even if already checked out in other worktree"),
@@ -686,6 +702,7 @@ static int add(int ac, const char **av, const char *prefix)
 
 	path = prefix_filename(prefix, av[0]);
 	branch = ac < 2 ? "HEAD" : av[1];
+	used_new_branch_options = new_branch || new_branch_force;
 
 	if (!strcmp(branch, "-"))
 		branch = "@{-1}";
@@ -728,6 +745,15 @@ static int add(int ac, const char **av, const char *prefix)
 	}
 
 	if (!opts.orphan && !lookup_commit_reference_by_name(branch)) {
+		int attempt_hint = !opts.quiet && (ac < 2);
+		if (attempt_hint && used_new_branch_options) {
+			advise_if_enabled(ADVICE_WORKTREE_ADD_ORPHAN,
+				WORKTREE_ADD_ORPHAN_WITH_DASH_B_HINT_TEXT,
+				new_branch, path);
+		} else if (attempt_hint) {
+			advise_if_enabled(ADVICE_WORKTREE_ADD_ORPHAN,
+				WORKTREE_ADD_ORPHAN_NO_DASH_B_HINT_TEXT, path);
+		}
 		die(_("invalid reference: %s"), branch);
 	}
 
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index fba90582b6..46eef26179 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -401,6 +401,43 @@ test_expect_success '"add" worktree with orphan branch, lock, and reason' '
 	test_cmp expect .git/worktrees/orphan-with-lock-reason/locked
 '
 
+# Note: Quoted arguments containing spaces are not supported.
+test_wt_add_orphan_hint () {
+	local context="$1" &&
+	local use_branch=$2 &&
+	shift 2 &&
+	local opts="$*" &&
+	test_expect_success "'worktree add' show orphan hint in bad/orphan HEAD w/ $context" '
+		test_when_finished "rm -rf repo" &&
+		git init repo &&
+		(cd repo && test_commit commit) &&
+		git -C repo switch --orphan noref &&
+		test_must_fail git -C repo worktree add $opts foobar/ 2>actual &&
+		! grep "error: unknown switch" actual &&
+		grep "hint: If you meant to create a worktree containing a new orphan branch" actual &&
+		if [ $use_branch -eq 1 ]
+		then
+			grep -E "^hint:\s+git worktree add --orphan -b \S+ \S+\s*$" actual
+		else
+			grep -E "^hint:\s+git worktree add --orphan \S+\s*$" actual
+		fi
+
+	'
+}
+
+test_wt_add_orphan_hint 'no opts' 0
+test_wt_add_orphan_hint '-b' 1 -b foobar_branch
+test_wt_add_orphan_hint '-B' 1 -B foobar_branch
+
+test_expect_success "'worktree add' doesn't show orphan hint in bad/orphan HEAD w/ --quiet" '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(cd repo && test_commit commit) &&
+	test_must_fail git -C repo worktree add --quiet foobar_branch foobar/ 2>actual &&
+	! grep "error: unknown switch" actual &&
+	! grep "hint: If you meant to create a worktree containing a new orphan branch" actual
+'
+
 test_expect_success 'local clone from linked checkout' '
 	git clone --local here here-clone &&
 	( cd here-clone && git fsck )
-- 
2.39.3



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RESEND PATCH v10 7/8] worktree add: extend DWIM to infer --orphan
  2023-05-17 21:47 ` [RESEND PATCH v10 " Jacob Abel
                     ` (5 preceding siblings ...)
  2023-05-17 21:48   ` [RESEND PATCH v10 6/8] worktree add: introduce "try --orphan" hint Jacob Abel
@ 2023-05-17 21:48   ` Jacob Abel
  2023-08-09  6:47     ` RESEND [PATCH " Teng Long
  2023-05-17 21:49   ` [RESEND PATCH v10 8/8] worktree add: emit warn when there is a bad HEAD Jacob Abel
  7 siblings, 1 reply; 34+ messages in thread
From: Jacob Abel @ 2023-05-17 21:48 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Extend DWIM to try to infer `--orphan` when in an empty repository. i.e.
a repository with an invalid/unborn HEAD, no local branches, and if
`--guess-remote` is used then no remote branches.

This behavior is equivalent to `git switch -c` or `git checkout -b` in
an empty repository.

Also warn the user (overriden with `-f`/`--force`) when they likely
intend to checkout a remote branch to the worktree but have not yet
fetched from the remote. i.e. when using `--guess-remote` and there is a
remote but no local or remote refs.

Current Behavior:
% git --no-pager branch --list --remotes
% git remote
origin
% git workree add ../main
hint: If you meant to create a worktree containing a new orphan branch
[...]
hint: Disable this message with "git config advice.worktreeAddOrphan false"
fatal: invalid reference: HEAD
% git workree add --guess-remote ../main
hint: If you meant to create a worktree containing a new orphan branch
[...]
hint: Disable this message with "git config advice.worktreeAddOrphan false"
fatal: invalid reference: HEAD
% git fetch --quiet
% git --no-pager branch --list --remotes
origin/HEAD -> origin/main
origin/main
% git workree add --guess-remote ../main
Preparing worktree (new branch 'main')
branch 'main' set up to track 'origin/main'.
HEAD is now at dadc8e6dac commit message
%

New Behavior:
% git --no-pager branch --list --remotes
% git remote
origin
% git workree add ../main
No possible source branch, inferring '--orphan'
Preparing worktree (new branch 'main')
% git worktree remove ../main
% git workree add --guess-remote ../main
fatal: No local or remote refs exist despite at least one remote
present, stopping; use 'add -f' to overide or fetch a remote first
% git workree add --guess-remote -f ../main
No possible source branch, inferring '--orphan'
Preparing worktree (new branch 'main')
% git worktree remove ../main
% git fetch --quiet
% git --no-pager branch --list --remotes
origin/HEAD -> origin/main
origin/main
% git workree add --guess-remote ../main
Preparing worktree (new branch 'main')
branch 'main' set up to track 'origin/main'.
HEAD is now at dadc8e6dac commit message
%

Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 Documentation/git-worktree.txt |  10 +
 builtin/worktree.c             | 114 +++++++++++-
 t/t2400-worktree-add.sh        | 326 +++++++++++++++++++++++++++++++++
 3 files changed, 449 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 485d865eb2..a4fbf5e838 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -95,6 +95,16 @@ exist, a new branch based on `HEAD` is automatically created as if
 `-b <branch>` was given.  If `<branch>` does exist, it will be checked out
 in the new worktree, if it's not checked out anywhere else, otherwise the
 command will refuse to create the worktree (unless `--force` is used).
++
+If `<commit-ish>` is omitted, neither `--detach`, or `--orphan` is
+used, and there are no valid local branches (or remote branches if
+`--guess-remote` is specified) then, as a convenience, the new worktree is
+associated with a new orphan branch named `<branch>` (after
+`$(basename <path>)` if neither `-b` or `-B` is used) as if `--orphan` was
+passed to the command. In the event the repository has a remote and
+`--guess-remote` is used, but no remote or local branches exist, then the
+command fails with a warning reminding the user to fetch from their remote
+first (or override by using `-f/--force`).
 
 list::
 
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 15bdb380c7..093b2cb032 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -12,6 +12,7 @@
 #include "strvec.h"
 #include "branch.h"
 #include "refs.h"
+#include "remote.h"
 #include "run-command.h"
 #include "hook.h"
 #include "sigchain.h"
@@ -40,6 +41,9 @@
 #define BUILTIN_WORKTREE_UNLOCK_USAGE \
 	N_("git worktree unlock <worktree>")
 
+#define WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT \
+	_("No possible source branch, inferring '--orphan'")
+
 #define WORKTREE_ADD_ORPHAN_WITH_DASH_B_HINT_TEXT \
 	_("If you meant to create a worktree containing a new orphan branch\n" \
 	"(branch with no commits) for this repository, you can do so\n" \
@@ -613,6 +617,107 @@ static void print_preparing_worktree_line(int detach,
 	}
 }
 
+/**
+ * Callback to short circuit iteration over refs on the first reference
+ * corresponding to a valid oid.
+ *
+ * Returns 0 on failure and non-zero on success.
+ */
+static int first_valid_ref(const char *refname,
+			   const struct object_id *oid,
+			   int flags,
+			   void *cb_data)
+{
+	return 1;
+}
+
+/**
+ * Verifies HEAD and determines whether there exist any valid local references.
+ *
+ * - Checks whether HEAD points to a valid reference.
+ *
+ * - Checks whether any valid local branches exist.
+ *
+ * Returns 1 if any of the previous checks are true, otherwise returns 0.
+ */
+static int can_use_local_refs(const struct add_opts *opts)
+{
+	if (head_ref(first_valid_ref, NULL)) {
+		return 1;
+	} else if (for_each_branch_ref(first_valid_ref, NULL)) {
+		return 1;
+	}
+	return 0;
+}
+
+/**
+ * Reports whether the necessary flags were set and whether the repository has
+ * remote references to attempt DWIM tracking of upstream branches.
+ *
+ * 1. Checks that `--guess-remote` was used or `worktree.guessRemote = true`.
+ *
+ * 2. Checks whether any valid remote branches exist.
+ *
+ * 3. Checks that there exists at least one remote and emits a warning/error
+ *    if both checks 1. and 2. are false (can be bypassed with `--force`).
+ *
+ * Returns 1 if checks 1. and 2. are true, otherwise 0.
+ */
+static int can_use_remote_refs(const struct add_opts *opts)
+{
+	if (!guess_remote) {
+		if (!opts->quiet)
+			fprintf_ln(stderr, WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT);
+		return 0;
+	} else if (for_each_remote_ref(first_valid_ref, NULL)) {
+		return 1;
+	} else if (!opts->force && remote_get(NULL)) {
+		die(_("No local or remote refs exist despite at least one remote\n"
+		      "present, stopping; use 'add -f' to overide or fetch a remote first"));
+	} else if (!opts->quiet) {
+		fprintf_ln(stderr, WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT);
+	}
+	return 0;
+}
+
+/**
+ * Determines whether `--orphan` should be inferred in the evaluation of
+ * `worktree add path/` or `worktree add -b branch path/` and emits an error
+ * if the supplied arguments would produce an illegal combination when the
+ * `--orphan` flag is included.
+ *
+ * `opts` and `opt_track` contain the other options & flags supplied to the
+ * command.
+ *
+ * remote determines whether to check `can_use_remote_refs()` or not. This
+ * is primarily to differentiate between the basic `add` DWIM and `add -b`.
+ *
+ * Returns 1 when inferring `--orphan`, 0 otherwise, and emits an error when
+ * `--orphan` is inferred but doing so produces an illegal combination of
+ * options and flags. Additionally produces an error when remote refs are
+ * checked and the repo is in a state that looks like the user added a remote
+ * but forgot to fetch (and did not override the warning with -f).
+ */
+static int dwim_orphan(const struct add_opts *opts, int opt_track, int remote)
+{
+	if (can_use_local_refs(opts)) {
+		return 0;
+	} else if (remote && can_use_remote_refs(opts)) {
+		return 0;
+	} else if (!opts->quiet) {
+		fprintf_ln(stderr, WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT);
+	}
+
+	if (opt_track) {
+		die(_("'%s' and '%s' cannot be used together"), "--orphan",
+		    "--track");
+	} else if (!opts->checkout) {
+		die(_("'%s' and '%s' cannot be used together"), "--orphan",
+		    "--no-checkout");
+	}
+	return 1;
+}
+
 static const char *dwim_branch(const char *path, const char **new_branch)
 {
 	int n;
@@ -723,12 +828,19 @@ static int add(int ac, const char **av, const char *prefix)
 		int n;
 		const char *s = worktree_basename(path, &n);
 		new_branch = xstrndup(s, n);
-	} else if (new_branch || opts.detach || opts.orphan) {
+	} else if (opts.orphan || opts.detach) {
 		// No-op
+	} else if (ac < 2 && new_branch) {
+		// DWIM: Infer --orphan when repo has no refs.
+		opts.orphan = dwim_orphan(&opts, !!opt_track, 0);
 	} else if (ac < 2) {
+		// DWIM: Guess branch name from path.
 		const char *s = dwim_branch(path, &new_branch);
 		if (s)
 			branch = s;
+
+		// DWIM: Infer --orphan when repo has no refs.
+		opts.orphan = (!s) && dwim_orphan(&opts, !!opt_track, 1);
 	} else if (ac == 2) {
 		struct object_id oid;
 		struct commit *commit;
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index 46eef26179..c7ca8df586 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -705,6 +705,332 @@ test_expect_success 'git worktree --no-guess-remote option overrides config' '
 	)
 '
 
+test_dwim_orphan () {
+	local info_text="No possible source branch, inferring '--orphan'" &&
+	local fetch_error_text="fatal: No local or remote refs exist despite at least one remote" &&
+	local orphan_hint="hint: If you meant to create a worktree containing a new orphan branch" &&
+	local invalid_ref_regex="^fatal: invalid reference:\s\+.*" &&
+	local bad_combo_regex="^fatal: '[a-z-]\+' and '[a-z-]\+' cannot be used together" &&
+
+	local git_ns="repo" &&
+	local dashc_args="-C $git_ns" &&
+	local use_cd=0 &&
+
+	local bad_head=0 &&
+	local empty_repo=1 &&
+	local local_ref=0 &&
+	local use_quiet=0 &&
+	local remote=0 &&
+	local remote_ref=0 &&
+	local use_new_branch=0 &&
+
+	local outcome="$1" &&
+	local outcome_text &&
+	local success &&
+	shift &&
+	local args="" &&
+	local context="" &&
+	case "$outcome" in
+	"infer")
+		success=1 &&
+		outcome_text='"add" DWIM infer --orphan'
+		;;
+	"no_infer")
+		success=1 &&
+		outcome_text='"add" DWIM doesnt infer --orphan'
+		;;
+	"fetch_error")
+		success=0 &&
+		outcome_text='"add" error need fetch'
+		;;
+	"fatal_orphan_bad_combo")
+		success=0 &&
+		outcome_text='"add" error inferred "--orphan" gives illegal opts combo'
+		;;
+	*)
+		echo "test_dwim_orphan(): invalid outcome: '$outcome'" >&2 &&
+		return 1
+		;;
+	esac &&
+	while [ $# -gt 0 ]
+	do
+		case "$1" in
+		# How and from where to create the worktree
+		"-C_repo")
+			use_cd=0 &&
+			git_ns="repo" &&
+			dashc_args="-C $git_ns" &&
+			context="$context, 'git -C repo'"
+			;;
+		"-C_wt")
+			use_cd=0 &&
+			git_ns="wt" &&
+			dashc_args="-C $git_ns" &&
+			context="$context, 'git -C wt'"
+			;;
+		"cd_repo")
+			use_cd=1 &&
+			git_ns="repo" &&
+			dashc_args="" &&
+			context="$context, 'cd repo && git'"
+			;;
+		"cd_wt")
+			use_cd=1 &&
+			git_ns="wt" &&
+			dashc_args="" &&
+			context="$context, 'cd wt && git'"
+			;;
+
+		# Bypass the "pull first" warning
+		"force")
+			args="$args --force" &&
+			context="$context, --force"
+			;;
+
+		# Try to use remote refs when DWIM
+		"guess_remote")
+			args="$args --guess-remote" &&
+			context="$context, --guess-remote"
+			;;
+		"no_guess_remote")
+			args="$args --no-guess-remote" &&
+			context="$context, --no-guess-remote"
+			;;
+
+		# Whether there is at least one local branch present
+		"local_ref")
+			empty_repo=0 &&
+			local_ref=1 &&
+			context="$context, >=1 local branches"
+			;;
+		"no_local_ref")
+			empty_repo=0 &&
+			context="$context, 0 local branches"
+			;;
+
+		# Whether the HEAD points at a valid ref (skip this opt when no refs)
+		"good_head")
+			# requires: local_ref
+			context="$context, valid HEAD"
+			;;
+		"bad_head")
+			bad_head=1 &&
+			context="$context, invalid (or orphan) HEAD"
+			;;
+
+		# Whether the code path is tested with the base add command or -b
+		"no_-b")
+			use_new_branch=0 &&
+			context="$context, no --branch"
+			;;
+		"-b")
+			use_new_branch=1 &&
+			context="$context, --branch"
+			;;
+
+		# Whether to check that all output is suppressed (except errors)
+		# or that the output is as expected
+		"quiet")
+			use_quiet=1 &&
+			args="$args --quiet" &&
+			context="$context, --quiet"
+			;;
+		"no_quiet")
+			use_quiet=0 &&
+			context="$context, no --quiet (expect output)"
+			;;
+
+		# Whether there is at least one remote attached to the repo
+		"remote")
+			empty_repo=0 &&
+			remote=1 &&
+			context="$context, >=1 remotes"
+			;;
+		"no_remote")
+			empty_repo=0 &&
+			remote=0 &&
+			context="$context, 0 remotes"
+			;;
+
+		# Whether there is at least one valid remote ref
+		"remote_ref")
+			# requires: remote
+			empty_repo=0 &&
+			remote_ref=1 &&
+			context="$context, >=1 fetched remote branches"
+			;;
+		"no_remote_ref")
+			empty_repo=0 &&
+			remote_ref=0 &&
+			context="$context, 0 fetched remote branches"
+			;;
+
+		# Options or flags that become illegal when --orphan is inferred
+		"no_checkout")
+			args="$args --no-checkout" &&
+			context="$context, --no-checkout"
+			;;
+		"track")
+			args="$args --track" &&
+			context="$context, --track"
+			;;
+
+		# All other options are illegal
+		*)
+			echo "test_dwim_orphan(): invalid arg: '$1'" >&2 &&
+			return 1
+			;;
+		esac &&
+		shift
+	done &&
+	context="${context#', '}" &&
+	if [ $use_new_branch -eq 1 ]
+	then
+		args="$args -b foo"
+	else
+		context="DWIM (no --branch), $context"
+	fi &&
+	if [ $empty_repo -eq 1 ]
+	then
+		context="empty repo, $context"
+	fi &&
+	args="$args ../foo" &&
+	context="${context%', '}" &&
+	test_expect_success "$outcome_text w/ $context" '
+		test_when_finished "rm -rf repo" &&
+		git init repo &&
+		if [ $local_ref -eq 1 ] && [ "$git_ns" = "repo" ]
+		then
+			(cd repo && test_commit commit) &&
+			if [ $bad_head -eq 1 ]
+			then
+				git -C repo symbolic-ref HEAD refs/heads/badbranch
+			fi
+		elif [ $local_ref -eq 1 ] && [ "$git_ns" = "wt" ]
+		then
+			test_when_finished "git -C repo worktree remove -f ../wt" &&
+			git -C repo worktree add --orphan -b main ../wt &&
+			(cd wt && test_commit commit) &&
+			if [ $bad_head -eq 1 ]
+			then
+				git -C wt symbolic-ref HEAD refs/heads/badbranch
+			fi
+		elif [ $local_ref -eq 0 ] && [ "$git_ns" = "wt" ]
+		then
+			test_when_finished "git -C repo worktree remove -f ../wt" &&
+			git -C repo worktree add --orphan -b orphanbranch ../wt
+		fi &&
+
+		if [ $remote -eq 1 ]
+		then
+			test_when_finished "rm -rf upstream" &&
+			git init upstream &&
+			(cd upstream && test_commit commit) &&
+			git -C upstream switch -c foo &&
+			git -C repo remote add upstream ../upstream
+		fi &&
+
+		if [ $remote_ref -eq 1 ]
+		then
+			git -C repo fetch
+		fi &&
+		if [ $success -eq 1 ]
+		then
+			test_when_finished git -C repo worktree remove ../foo
+		fi &&
+		(
+			if [ $use_cd -eq 1 ]
+			then
+				cd $git_ns
+			fi &&
+			if [ "$outcome" = "infer" ]
+			then
+				git $dashc_args worktree add $args 2>actual &&
+				if [ $use_quiet -eq 1 ]
+				then
+					test_must_be_empty actual
+				else
+					grep "$info_text" actual
+				fi
+			elif [ "$outcome" = "no_infer" ]
+			then
+				git $dashc_args worktree add $args 2>actual &&
+				if [ $use_quiet -eq 1 ]
+				then
+					test_must_be_empty actual
+				else
+					! grep "$info_text" actual
+				fi
+			elif [ "$outcome" = "fetch_error" ]
+			then
+				test_must_fail git $dashc_args worktree add $args 2>actual &&
+				grep "$fetch_error_text" actual
+			elif [ "$outcome" = "fatal_orphan_bad_combo" ]
+			then
+				test_must_fail git $dashc_args worktree add $args 2>actual &&
+				if [ $use_quiet -eq 1 ]
+				then
+					! grep "$info_text" actual
+				else
+					grep "$info_text" actual
+				fi &&
+				grep "$bad_combo_regex" actual
+			elif [ "$outcome" = "warn_bad_head" ]
+			then
+				test_must_fail git $dashc_args worktree add $args 2>actual &&
+				if [ $use_quiet -eq 1 ]
+				then
+					grep "$invalid_ref_regex" actual &&
+					! grep "$orphan_hint" actual
+				else
+					headpath=$(git $dashc_args rev-parse --sq --path-format=absolute --git-path HEAD) &&
+					headcontents=$(cat "$headpath") &&
+					grep "HEAD points to an invalid (or orphaned) reference" actual &&
+					grep "HEAD path:\s*.$headpath." actual &&
+					grep "HEAD contents:\s*.$headcontents." actual &&
+					grep "$orphan_hint" actual &&
+					! grep "$info_text" actual
+				fi &&
+				grep "$invalid_ref_regex" actual
+			else
+				# Unreachable
+				false
+			fi
+		) &&
+		if [ $success -ne 1 ]
+		then
+			test_path_is_missing foo
+		fi
+	'
+}
+
+for quiet_mode in "no_quiet" "quiet"
+do
+	for changedir_type in "cd_repo" "cd_wt" "-C_repo" "-C_wt"
+	do
+		dwim_test_args="$quiet_mode $changedir_type"
+		test_dwim_orphan 'infer' $dwim_test_args no_-b
+		test_dwim_orphan 'no_infer' $dwim_test_args no_-b local_ref good_head
+		test_dwim_orphan 'infer' $dwim_test_args no_-b no_local_ref no_remote no_remote_ref no_guess_remote
+		test_dwim_orphan 'infer' $dwim_test_args no_-b no_local_ref remote no_remote_ref no_guess_remote
+		test_dwim_orphan 'fetch_error' $dwim_test_args no_-b no_local_ref remote no_remote_ref guess_remote
+		test_dwim_orphan 'infer' $dwim_test_args no_-b no_local_ref remote no_remote_ref guess_remote force
+		test_dwim_orphan 'no_infer' $dwim_test_args no_-b no_local_ref remote remote_ref guess_remote
+
+		test_dwim_orphan 'infer' $dwim_test_args -b
+		test_dwim_orphan 'no_infer' $dwim_test_args -b local_ref good_head
+		test_dwim_orphan 'infer' $dwim_test_args -b no_local_ref no_remote no_remote_ref no_guess_remote
+		test_dwim_orphan 'infer' $dwim_test_args -b no_local_ref remote no_remote_ref no_guess_remote
+		test_dwim_orphan 'infer' $dwim_test_args -b no_local_ref remote no_remote_ref guess_remote
+		test_dwim_orphan 'infer' $dwim_test_args -b no_local_ref remote remote_ref guess_remote
+	done
+
+	test_dwim_orphan 'fatal_orphan_bad_combo' $quiet_mode no_-b no_checkout
+	test_dwim_orphan 'fatal_orphan_bad_combo' $quiet_mode no_-b track
+	test_dwim_orphan 'fatal_orphan_bad_combo' $quiet_mode -b no_checkout
+	test_dwim_orphan 'fatal_orphan_bad_combo' $quiet_mode -b track
+done
+
 post_checkout_hook () {
 	test_when_finished "rm -rf .git/hooks" &&
 	mkdir .git/hooks &&
-- 
2.39.3



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [RESEND PATCH v10 8/8] worktree add: emit warn when there is a bad HEAD
  2023-05-17 21:47 ` [RESEND PATCH v10 " Jacob Abel
                     ` (6 preceding siblings ...)
  2023-05-17 21:48   ` [RESEND PATCH v10 7/8] worktree add: extend DWIM to infer --orphan Jacob Abel
@ 2023-05-17 21:49   ` Jacob Abel
  7 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-05-17 21:49 UTC (permalink / raw)
  To: git
  Cc: Jacob Abel, Ævar Arnfjörð Bjarmason, Eric Sunshine,
	Junio C Hamano, Phillip Wood, Rubén Justo, Taylor Blau,
	rsbecker

Add a warning to `worktree add` when the command tries to reference
HEAD, there exist valid local branches, and the HEAD points to a
non-existent reference.

Current Behavior:
% git -C foo worktree list
/path/to/repo/foo     dadc8e6dac [main]
/path/to/repo/foo_wt  0000000000 [badref]
% git -C foo worktree add ../wt1
Preparing worktree (new branch 'wt1')
HEAD is now at dadc8e6dac dummy commit
% git -C foo_wt worktree add ../wt2
hint: If you meant to create a worktree containing a new orphan branch
[...]
hint: Disable this message with "git config advice.worktreeAddOrphan false"
fatal: invalid reference: HEAD
%

New Behavior:
% git -C foo worktree list
/path/to/repo/foo     dadc8e6dac [main]
/path/to/repo/foo_wt  0000000000 [badref]
% git -C foo worktree add ../wt1
Preparing worktree (new branch 'wt1')
HEAD is now at dadc8e6dac dummy commit
% git -C foo_wt worktree add ../wt2
warning: HEAD points to an invalid (or orphaned) reference.
HEAD path: '/path/to/repo/foo/.git/worktrees/foo_wt/HEAD'
HEAD contents: 'ref: refs/heads/badref'
hint: If you meant to create a worktree containing a new orphan branch
[...]
hint: Disable this message with "git config advice.worktreeAddOrphan false"
fatal: invalid reference: HEAD
%

Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
---
 builtin/worktree.c      | 34 +++++++++++++++++++++++++++++-----
 t/t2400-worktree-add.sh | 18 +++++++++++++++++-
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 093b2cb032..5f62084334 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -638,6 +638,9 @@ static int first_valid_ref(const char *refname,
  *
  * - Checks whether any valid local branches exist.
  *
+ * - Emits a warning if there exist any valid branches but HEAD does not point
+ *   to a valid reference.
+ *
  * Returns 1 if any of the previous checks are true, otherwise returns 0.
  */
 static int can_use_local_refs(const struct add_opts *opts)
@@ -645,6 +648,23 @@ static int can_use_local_refs(const struct add_opts *opts)
 	if (head_ref(first_valid_ref, NULL)) {
 		return 1;
 	} else if (for_each_branch_ref(first_valid_ref, NULL)) {
+		if (!opts->quiet) {
+			struct strbuf path = STRBUF_INIT;
+			struct strbuf contents = STRBUF_INIT;
+
+			strbuf_add_real_path(&path, get_worktree_git_dir(NULL));
+			strbuf_addstr(&path, "/HEAD");
+			strbuf_read_file(&contents, path.buf, 64);
+			strbuf_stripspace(&contents, 0);
+			strbuf_strip_suffix(&contents, "\n");
+
+			warning(_("HEAD points to an invalid (or orphaned) reference.\n"
+				  "HEAD path: '%s'\n"
+				  "HEAD contents: '%s'"),
+				  path.buf, contents.buf);
+			strbuf_release(&path);
+			strbuf_release(&contents);
+		}
 		return 1;
 	}
 	return 0;
@@ -666,16 +686,12 @@ static int can_use_local_refs(const struct add_opts *opts)
 static int can_use_remote_refs(const struct add_opts *opts)
 {
 	if (!guess_remote) {
-		if (!opts->quiet)
-			fprintf_ln(stderr, WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT);
 		return 0;
 	} else if (for_each_remote_ref(first_valid_ref, NULL)) {
 		return 1;
 	} else if (!opts->force && remote_get(NULL)) {
 		die(_("No local or remote refs exist despite at least one remote\n"
 		      "present, stopping; use 'add -f' to overide or fetch a remote first"));
-	} else if (!opts->quiet) {
-		fprintf_ln(stderr, WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT);
 	}
 	return 0;
 }
@@ -828,8 +844,12 @@ static int add(int ac, const char **av, const char *prefix)
 		int n;
 		const char *s = worktree_basename(path, &n);
 		new_branch = xstrndup(s, n);
-	} else if (opts.orphan || opts.detach) {
+	} else if (opts.orphan) {
 		// No-op
+	} else if (opts.detach) {
+		// Check HEAD
+		if (!strcmp(branch, "HEAD"))
+			can_use_local_refs(&opts);
 	} else if (ac < 2 && new_branch) {
 		// DWIM: Infer --orphan when repo has no refs.
 		opts.orphan = dwim_orphan(&opts, !!opt_track, 0);
@@ -854,6 +874,10 @@ static int add(int ac, const char **av, const char *prefix)
 				branch = remote;
 			}
 		}
+
+		if (!strcmp(branch, "HEAD"))
+			can_use_local_refs(&opts);
+
 	}
 
 	if (!opts.orphan && !lookup_commit_reference_by_name(branch)) {
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index c7ca8df586..0ac468e69e 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -722,6 +722,7 @@ test_dwim_orphan () {
 	local use_quiet=0 &&
 	local remote=0 &&
 	local remote_ref=0 &&
+	local use_detach=0 &&
 	local use_new_branch=0 &&
 
 	local outcome="$1" &&
@@ -747,6 +748,10 @@ test_dwim_orphan () {
 		success=0 &&
 		outcome_text='"add" error inferred "--orphan" gives illegal opts combo'
 		;;
+	"warn_bad_head")
+		success=0 &&
+		outcome_text='"add" error, warn on bad HEAD, hint use orphan'
+		;;
 	*)
 		echo "test_dwim_orphan(): invalid outcome: '$outcome'" >&2 &&
 		return 1
@@ -818,7 +823,7 @@ test_dwim_orphan () {
 			context="$context, invalid (or orphan) HEAD"
 			;;
 
-		# Whether the code path is tested with the base add command or -b
+		# Whether the code path is tested with the base add command, -b, or --detach
 		"no_-b")
 			use_new_branch=0 &&
 			context="$context, no --branch"
@@ -827,6 +832,10 @@ test_dwim_orphan () {
 			use_new_branch=1 &&
 			context="$context, --branch"
 			;;
+		"detach")
+			use_detach=1 &&
+			context="$context, --detach"
+			;;
 
 		# Whether to check that all output is suppressed (except errors)
 		# or that the output is as expected
@@ -887,6 +896,9 @@ test_dwim_orphan () {
 	if [ $use_new_branch -eq 1 ]
 	then
 		args="$args -b foo"
+	elif [ $use_detach -eq 1 ]
+	then
+		args="$args --detach"
 	else
 		context="DWIM (no --branch), $context"
 	fi &&
@@ -1023,6 +1035,10 @@ do
 		test_dwim_orphan 'infer' $dwim_test_args -b no_local_ref remote no_remote_ref no_guess_remote
 		test_dwim_orphan 'infer' $dwim_test_args -b no_local_ref remote no_remote_ref guess_remote
 		test_dwim_orphan 'infer' $dwim_test_args -b no_local_ref remote remote_ref guess_remote
+
+		test_dwim_orphan 'warn_bad_head' $dwim_test_args no_-b local_ref bad_head
+		test_dwim_orphan 'warn_bad_head' $dwim_test_args -b local_ref bad_head
+		test_dwim_orphan 'warn_bad_head' $dwim_test_args detach local_ref bad_head
 	done
 
 	test_dwim_orphan 'fatal_orphan_bad_combo' $quiet_mode no_-b no_checkout
-- 
2.39.3



^ permalink raw reply related	[flat|nested] 34+ messages in thread

* RESEND [PATCH v10 7/8] worktree add: extend DWIM to infer --orphan
  2023-05-17 21:48   ` [RESEND PATCH v10 7/8] worktree add: extend DWIM to infer --orphan Jacob Abel
@ 2023-08-09  6:47     ` Teng Long
  2023-08-11 17:43       ` Jacob Abel
  0 siblings, 1 reply; 34+ messages in thread
From: Teng Long @ 2023-08-09  6:47 UTC (permalink / raw)
  To: jacobabel
  Cc: avarab, git, gitster, me, phillip.wood123, rjusto, rsbecker,
	sunshine



> +static int can_use_remote_refs(const struct add_opts *opts)
> +{
> +	if (!guess_remote) {
> +		if (!opts->quiet)
> +			fprintf_ln(stderr, WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT);
> +		return 0;
> +	} else if (for_each_remote_ref(first_valid_ref, NULL)) {
> +		return 1;
> +	} else if (!opts->force && remote_get(NULL)) {
> +		die(_("No local or remote refs exist despite at least one remote\n"
> +		      "present, stopping; use 'add -f' to overide or fetch a remote first"));
> +	} else if (!opts->quiet) {
> +		fprintf_ln(stderr, WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT);
> +	}
> +	return 0;
> +}

s/overide/override?

 builtin/worktree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 4cd01842..10db70b7 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -696,7 +696,7 @@ static int can_use_remote_refs(const struct add_opts *opts)
                return 1;
        } else if (!opts->force && remote_get(NULL)) {
                die(_("No local or remote refs exist despite at least one remote\n"
-                     "present, stopping; use 'add -f' to overide or fetch a remote first"));
+                     "present, stopping; use 'add -f' to override or fetch a remote first"));
        }
        return 0;
 }

---

I found it when working at l10n round of 2.42.0, maybe it's not late to fix
this typo:

Thanks.

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: RESEND [PATCH v10 7/8] worktree add: extend DWIM to infer --orphan
  2023-08-09  6:47     ` RESEND [PATCH " Teng Long
@ 2023-08-11 17:43       ` Jacob Abel
  0 siblings, 0 replies; 34+ messages in thread
From: Jacob Abel @ 2023-08-11 17:43 UTC (permalink / raw)
  To: Teng Long
  Cc: avarab, git, gitster, me, phillip.wood123, rjusto, rsbecker,
	sunshine

On 23/08/09 02:47PM, Teng Long wrote:
> 
> > [...]
> 
> s/overide/override?
> 
>  builtin/worktree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 4cd01842..10db70b7 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -696,7 +696,7 @@ static int can_use_remote_refs(const struct add_opts *opts)
>                 return 1;
>         } else if (!opts->force && remote_get(NULL)) {
>                 die(_("No local or remote refs exist despite at least one remote\n"
> -                     "present, stopping; use 'add -f' to overide or fetch a remote first"));
> +                     "present, stopping; use 'add -f' to override or fetch a remote first"));
>         }
>         return 0;
>  }
> 
> ---
> 
> I found it when working at l10n round of 2.42.0, maybe it's not late to fix
> this typo:
> 
> Thanks.

Ah yep. That's on me. I'll put together a patch of the above and send it
out, cc-ing you before the end of the day today.


^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2023-08-11 17:43 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-17  9:33 [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
2023-04-17  9:33 ` [PATCH v9 1/8] worktree add: include -B in usage docs Jacob Abel
2023-04-17  9:33 ` [PATCH v9 2/8] t2400: print captured git output when finished Jacob Abel
2023-04-17 21:09   ` Junio C Hamano
2023-04-18  3:53     ` Jacob Abel
2023-04-18 16:34       ` Junio C Hamano
2023-04-19 13:23         ` Jacob Abel
2023-04-19 13:36         ` Jacob Abel
2023-04-19 15:41           ` Junio C Hamano
2023-04-19 16:50             ` Jacob Abel
2023-04-17  9:33 ` [PATCH v9 3/8] t2400: refactor "worktree add" opt exclusion tests Jacob Abel
2023-04-17 21:30   ` Junio C Hamano
2023-04-20  2:46     ` Jacob Abel
2023-04-17  9:34 ` [PATCH v9 4/8] t2400: add tests to verify --quiet Jacob Abel
2023-04-17 21:33   ` Junio C Hamano
2023-04-20  2:48     ` Jacob Abel
2023-04-17  9:34 ` [PATCH v9 5/8] worktree add: add --orphan flag Jacob Abel
2023-04-17  9:34 ` [PATCH v9 6/8] worktree add: introduce "try --orphan" hint Jacob Abel
2023-04-17  9:34 ` [PATCH v9 7/8] worktree add: extend DWIM to infer --orphan Jacob Abel
2023-04-17  9:34 ` [PATCH v9 8/8] worktree add: emit warn when there is a bad HEAD Jacob Abel
2023-04-20  3:05 ` [PATCH v9 0/8] worktree: Support `--orphan` when creating new worktrees Jacob Abel
2023-05-01 21:51   ` Junio C Hamano
2023-05-02  5:48     ` Jacob Abel
2023-05-17 21:47 ` [RESEND PATCH v10 " Jacob Abel
2023-05-17 21:48   ` [RESEND PATCH v10 1/8] worktree add: include -B in usage docs Jacob Abel
2023-05-17 21:48   ` [RESEND PATCH v10 2/8] t2400: cleanup created worktree in test Jacob Abel
2023-05-17 21:48   ` [RESEND PATCH v10 3/8] t2400: refactor "worktree add" opt exclusion tests Jacob Abel
2023-05-17 21:48   ` [RESEND PATCH v10 4/8] t2400: add tests to verify --quiet Jacob Abel
2023-05-17 21:48   ` [RESEND PATCH v10 5/8] worktree add: add --orphan flag Jacob Abel
2023-05-17 21:48   ` [RESEND PATCH v10 6/8] worktree add: introduce "try --orphan" hint Jacob Abel
2023-05-17 21:48   ` [RESEND PATCH v10 7/8] worktree add: extend DWIM to infer --orphan Jacob Abel
2023-08-09  6:47     ` RESEND [PATCH " Teng Long
2023-08-11 17:43       ` Jacob Abel
2023-05-17 21:49   ` [RESEND PATCH v10 8/8] worktree add: emit warn when there is a bad HEAD Jacob Abel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).