All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jacob Abel <jacobabel@nullpo.dev>
To: git@vger.kernel.org
Cc: "Jacob Abel" <jacobabel@nullpo.dev>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Rubén Justo" <rjusto@gmail.com>, "Taylor Blau" <me@ttaylorr.com>,
	rsbecker@nexbridge.com
Subject: [PATCH v10 7/8] worktree add: extend DWIM to infer --orphan
Date: Sun, 07 May 2023 12:07:03 +0000	[thread overview]
Message-ID: <20230507120530.14669-8-jacobabel@nullpo.dev> (raw)

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



                 reply	other threads:[~2023-05-07 12:07 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230507120530.14669-8-jacobabel@nullpo.dev \
    --to=jacobabel@nullpo.dev \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=phillip.wood123@gmail.com \
    --cc=rjusto@gmail.com \
    --cc=rsbecker@nexbridge.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.