[PATCH 2/3] checkout: advise when orphaned commit checks are slow

["Wyatt Holden via GitGitGadget" <gitgitgadget@gmail.com>]

From: Wyatt Holden <wyattholden@microsoft.com>

The orphaned-commit check can take much longer than updating the
working tree. Users who encounter this cost need a way to discover
the checkout.orphanedCommitWarning setting.

Measure the complete check and suggest disabling it when it takes
more than two seconds. Explain that opting out also removes the
warning about commits left behind, and allow the advice itself to be
disabled with advice.checkoutOrphanedCommitWarning.

Follow the slow-operation advice introduced by:

0a53561a622 (status: warn when a/b calculation takes too long, 2019-06-18)

Unlike that status advice, recommend a configuration setting rather
than a dedicated command-line option.

Keep a test hook to exercise advice and suppression without relying
on wall-clock delays. Force the advice condition without replacing
the measured duration, and check the formatted duration rather than
a fabricated fixed value.

Signed-off-by: Wyatt Holden <wyattholden@microsoft.com>
---
 Documentation/config/advice.adoc |  6 +++++
 advice.c                         |  1 +
 advice.h                         |  1 +
 builtin/checkout.c               | 13 +++++++++++
 t/README                         |  5 ++++
 t/t2020-checkout-detach.sh       | 40 ++++++++++++++++++++++++++++++++
 6 files changed, 66 insertions(+)

diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
index 3d91c90eda..eecf72d8d1 100644
--- a/Documentation/config/advice.adoc
+++ b/Documentation/config/advice.adoc
@@ -38,6 +38,12 @@ all advice messages.
 		configuration variable for how to set a given remote
 		to be used by default in some situations where this
 		advice would be printed.
+	checkoutOrphanedCommitWarning::
+		Shown when linkgit:git-checkout[1] or linkgit:git-switch[1]
+		takes more than 2 seconds to check for commits left behind
+		when leaving a detached `HEAD`. Suggests setting
+		`checkout.orphanedCommitWarning` to `false` to skip the check.
+		Disabling this advice does not disable the check itself.
 	commitBeforeMerge::
 		Shown when linkgit:git-merge[1] refuses to
 		merge to avoid overwriting local changes.
diff --git a/advice.c b/advice.c
index 401d047391..8142d73b5e 100644
--- a/advice.c
+++ b/advice.c
@@ -50,6 +50,7 @@ static struct {
 	[ADVICE_AMBIGUOUS_FETCH_REFSPEC]		= { "ambiguousFetchRefspec" },
 	[ADVICE_AM_WORK_DIR] 				= { "amWorkDir" },
 	[ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME] 	= { "checkoutAmbiguousRemoteBranchName" },
+	[ADVICE_CHECKOUT_ORPHANED_COMMIT_WARNING]		= { "checkoutOrphanedCommitWarning" },
 	[ADVICE_COMMIT_BEFORE_MERGE]			= { "commitBeforeMerge" },
 	[ADVICE_DEFAULT_BRANCH_NAME]			= { "defaultBranchName" },
 	[ADVICE_DETACHED_HEAD]				= { "detachedHead" },
diff --git a/advice.h b/advice.h
index 3f0b4f0485..7dd1fe109c 100644
--- a/advice.h
+++ b/advice.h
@@ -17,6 +17,7 @@ enum advice_type {
 	ADVICE_AMBIGUOUS_FETCH_REFSPEC,
 	ADVICE_AM_WORK_DIR,
 	ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME,
+	ADVICE_CHECKOUT_ORPHANED_COMMIT_WARNING,
 	ADVICE_COMMIT_BEFORE_MERGE,
 	ADVICE_DEFAULT_BRANCH_NAME, /* To be retired sometime after Git 3.0 */
 	ADVICE_DETACHED_HEAD,
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7cc9e97612..1e94280d35 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -35,6 +35,7 @@
 #include "strvec.h"
 #include "submodule.h"
 #include "symlinks.h"
+#include "trace.h"
 #include "trace2.h"
 #include "tree.h"
 #include "tree-walk.h"
@@ -1133,6 +1134,8 @@ static void orphaned_commit_warning(struct commit *old_commit, struct commit *ne
 {
 	struct rev_info revs;
 	struct object *object = &old_commit->object;
+	uint64_t t_begin = getnanotime();
+	uint64_t t_delta_in_ms;
 
 	trace2_region_enter("checkout", "orphaned_commit_warning", the_repository);
 	repo_init_revisions(the_repository, &revs, NULL);
@@ -1159,6 +1162,16 @@ static void orphaned_commit_warning(struct commit *old_commit, struct commit *ne
 	repo_clear_commit_marks(the_repository, ALL_REV_FLAGS);
 	release_revisions(&revs);
 	trace2_region_leave("checkout", "orphaned_commit_warning", the_repository);
+
+	t_delta_in_ms = (getnanotime() - t_begin) / 1000000;
+	if (t_delta_in_ms > 2000 ||
+	    git_env_bool("GIT_TEST_CHECKOUT_DELAY_WARNING", 0))
+		advise_if_enabled(ADVICE_CHECKOUT_ORPHANED_COMMIT_WARNING,
+				  _("Checking for orphaned commits took %.2f seconds.\n"
+				    "You can use 'git config set checkout.orphanedCommitWarning false'\n"
+				    "to skip this check, but you will no longer be warned when leaving\n"
+				    "commits behind that are not connected to any of your branches."),
+				  t_delta_in_ms / 1000.0);
 }
 
 static int switch_branches(const struct checkout_opts *opts,
diff --git a/t/README b/t/README
index fd7e8504b5..577f02732e 100644
--- a/t/README
+++ b/t/README
@@ -371,6 +371,11 @@ test suite. Accept any boolean values that are accepted by git-config.
 GIT_TEST_PROTOCOL_VERSION=<n>, when set, makes 'protocol.version'
 default to n.
 
+GIT_TEST_CHECKOUT_DELAY_WARNING=<boolean>, when true, forces the
+slow orphaned-commit check advice to be considered regardless of the
+elapsed time. It does not change the measured duration or override
+advice settings.
+
 GIT_TEST_FULL_IN_PACK_ARRAY=<boolean> exercises the uncommon
 pack-objects code path where there are more than 1024 packs even if
 the actual number of packs in repository is below this limit. Accept
diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh
index d21dbd5b72..7dfaaab1b8 100755
--- a/t/t2020-checkout-detach.sh
+++ b/t/t2020-checkout-detach.sh
@@ -186,6 +186,46 @@ do
 		check_not_detached
 	'
 
+	test_expect_success "$command advises when the orphan check is slow" '
+		git checkout --detach "$orphan2" &&
+		GIT_TEST_CHECKOUT_DELAY_WARNING=true \
+			git $command main 2>stderr &&
+		check_orphan_warning stderr "2 commits" &&
+		test_grep -E "^hint: Checking for orphaned commits took [0-9]+[.][0-9]{2} seconds[.]$" stderr &&
+		test_grep "git config set checkout.orphanedCommitWarning false" stderr &&
+		test_grep "git config set advice.checkoutOrphanedCommitWarning false" stderr
+	'
+
+	test_expect_success "$command can suppress slow orphan check advice" '
+		git checkout --detach "$orphan2" &&
+		GIT_TEST_CHECKOUT_DELAY_WARNING=true \
+			git -c advice.checkoutOrphanedCommitWarning=false \
+			$command main 2>stderr &&
+		check_orphan_warning stderr "2 commits" &&
+		test_grep ! "^hint:" stderr &&
+
+		git checkout --detach "$orphan2" &&
+		GIT_TEST_CHECKOUT_DELAY_WARNING=true GIT_ADVICE=0 \
+			git $command main 2>stderr &&
+		check_orphan_warning stderr "2 commits" &&
+		test_grep ! "^hint:" stderr
+	'
+
+	test_expect_success "$command does not advise when the orphan check is skipped" '
+		git checkout --detach "$orphan2" &&
+		GIT_TEST_CHECKOUT_DELAY_WARNING=true \
+			git -c checkout.orphanedCommitWarning=false \
+			$command main 2>stderr &&
+		check_no_orphan_warning stderr &&
+		test_grep ! "^hint:" stderr &&
+
+		git checkout --detach "$orphan2" &&
+		GIT_TEST_CHECKOUT_DELAY_WARNING=true \
+			git $command --quiet main 2>stderr &&
+		test_must_be_empty stderr &&
+		check_not_detached
+	'
+
 	test_expect_success "$command rejects invalid checkout.orphanedCommitWarning" '
 		test_must_fail git -c checkout.orphanedCommitWarning=invalid \
 			$command main 2>stderr &&
-- 
gitgitgadget