Really retire git-legacy-stash.sh

After proving out git stash for a very long time as a shell script, we eventually converted it to a proper built-in, leaving an escape hatch in place to call the scripted version.

After another very long time, we removed that escape hatch, leaving behind a warning.

Now, almost two years later, let's remove even that warning (and a couple of tidbits that we inadvertently left in the code base).

Johannes Schindelin (4):
git-sh-setup: remove remnant bits referring to git-legacy-stash
add: remove support for git-legacy-stash
Remove documentation for stash.useBuiltin
stash: stop warning about the obsolete stash.useBuiltin config
setting

 Documentation/config/stash.txt |  7 -------
 builtin/add.c                  | 14 --------------
 builtin/stash.c                | 10 ----------
 git-sh-setup.sh                |  1 -
 t/t3903-stash.sh               | 15 ---------------
 5 files changed, 47 deletions(-)

base-commit: 89bece5c8c96f0b962cfc89e63f82d603fd60bed
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1133%2Fdscho%2Freally-retire-legacy-stash-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1133/dscho/really-retire-legacy-stash-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1133

gitgitgadget

From: Johannes Schindelin <[email protected]>

In 8a2cd3f5123 (stash: remove the stash.useBuiltin setting, 2020-03-03), we removed git-legacy-stash.sh. But git-sh-setup.sh somehow still thinks about it. Let's just not.

---
 git-sh-setup.sh | 1 -
 1 file changed, 1 deletion(-)

index b93f39288ce..d92df37e992 100644
@@ -101,7 +101,6 @@ $LONG_USAGE")"
 	case "$1" in
 		-h)
 		echo "$LONG_USAGE"
-		case "$0" in *git-legacy-stash) exit 129;; esac
 		exit
 	esac
 fi

gitgitgadget

From: Johannes Schindelin <[email protected]>

In 90a6bb98d11 (legacy stash -p: respect the add.interactive.usebuiltin setting, 2019-12-21), we added support to use the built-in add -p from the scripted stash -p.

In 8a2cd3f5123 (stash: remove the stash.useBuiltin setting, 2020-03-03), we retired the scripted stash (including the scripted stash -p).

Therefore this support is no longer necessary.

---
 builtin/add.c | 14 --------------
 1 file changed, 14 deletions(-)

index 84dff3e7969..3ffb86a4338 100644
@@ -32,7 +32,6 @@ static int add_renormalize;
 static int pathspec_file_nul;
 static int include_sparse;
 static const char *pathspec_from_file;
-static int legacy_stash_p; /* support for the scripted `git stash` */
 
 struct update_callback_data {
 	int flags;

@@ -388,8 +387,6 @@ static struct option builtin_add_options[] = {
 		   N_("override the executable bit of the listed files")),
 	OPT_HIDDEN_BOOL(0, "warn-embedded-repo", &warn_on_embedded_repo,
 			N_("warn when adding an embedded repository")),
-	OPT_HIDDEN_BOOL(0, "legacy-stash-p", &legacy_stash_p,
-			N_("backend for `git stash -p`")),
 	OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
 	OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
 	OPT_END(),

@@ -512,17 +509,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 			die(_("options '%s' and '%s' cannot be used together"), "--pathspec-from-file", "--interactive/--patch");
 		exit(interactive_add(argv + 1, prefix, patch_interactive));
 	}
-	if (legacy_stash_p) {
-		struct pathspec pathspec;
-
-		parse_pathspec(&pathspec, 0,
-			PATHSPEC_PREFER_FULL |
-			PATHSPEC_SYMLINK_LEADING_PATH |
-			PATHSPEC_PREFIX_ORIGIN,
-			prefix, argv);
-
-		return run_add_interactive(NULL, "--patch=stash", &pathspec);
-	}
 
 	if (edit_interactive) {
 		if (pathspec_from_file)

gitgitgadget

From: Johannes Schindelin <[email protected]>

In 8a2cd3f5123 (stash: remove the stash.useBuiltin setting, 2020-03-03), we removed the setting, and for a couple of major versions, we still documented the setting, telling users that it is gone.

We can now safely remove even the documentation.

---
 Documentation/config/stash.txt | 7 -------
 1 file changed, 7 deletions(-)

index 9ed775281fb..b9f609ed76b 100644
@@ -1,10 +1,3 @@
-stash.useBuiltin::
-	Unused configuration variable.  Used in Git versions 2.22 to
-	2.26 as an escape hatch to enable the legacy shellscript
-	implementation of stash.  Now the built-in rewrite of it in C
-	is always used. Setting this will emit a warning, to alert any
-	remaining users that setting this now does nothing.
-
 stash.showIncludeUntracked::
 	If this is set to true, the `git stash show` command will show
 	the untracked files of a stash entry.  Defaults to false. See

gitgitgadget

From: Johannes Schindelin <[email protected]>

In 8a2cd3f5123 (stash: remove the stash.useBuiltin setting, 2020-03-03), we removed support for stash.useBuiltin, but left a warning in its place.

After almost two years, and several major versions, it is time to remove even that warning.

---
 builtin/stash.c  | 10 ----------
 t/t3903-stash.sh | 15 ---------------
 2 files changed, 25 deletions(-)

index 1ef2017c595..25793b5a75a 100644
@@ -788,7 +788,6 @@ static int list_stash(int argc, const char **argv, const char *prefix)
 static int show_stat = 1;
 static int show_patch;
 static int show_include_untracked;
-static int use_legacy_stash;
 
 static int git_stash_config(const char *var, const char *value, void *cb)
 {

@@ -804,10 +803,6 @@ static int git_stash_config(const char *var, const char *value, void *cb)
 		show_include_untracked = git_config_bool(var, value);
 		return 0;
 	}
-	if (!strcmp(var, "stash.usebuiltin")) {
-		use_legacy_stash = !git_config_bool(var, value);
-		return 0;
-	}
 	return git_diff_basic_config(var, value, cb);
 }
 

@@ -1778,11 +1773,6 @@ int cmd_stash(int argc, const char **argv, const char *prefix)
 
 	git_config(git_stash_config, NULL);
 
-	if (use_legacy_stash ||
-	    !git_env_bool("GIT_TEST_STASH_USE_BUILTIN", -1))
-		warning(_("the stash.useBuiltin support has been removed!\n"
-			  "See its entry in 'git help config' for details."));
-
 	argc = parse_options(argc, argv, prefix, options, git_stash_usage,
 			     PARSE_OPT_KEEP_UNKNOWN | PARSE_OPT_KEEP_DASHDASH);
 

index 686747e55a3..b149e2af441 100755
@@ -1272,7 +1272,6 @@ test_expect_success 'stash works when user.name and user.email are not set' '
 	>2 &&
 	git add 2 &&
 	test_config user.useconfigonly true &&
-	test_config stash.usebuiltin true &&
 	(
 		sane_unset GIT_AUTHOR_NAME &&
 		sane_unset GIT_AUTHOR_EMAIL &&

@@ -1323,20 +1322,6 @@ test_expect_success 'stash handles skip-worktree entries nicely' '
 	git rev-parse --verify refs/stash:A.t
 '
 
-test_expect_success 'stash -c stash.useBuiltin=false warning ' '
-	expected="stash.useBuiltin support has been removed" &&
-
-	git -c stash.useBuiltin=false stash 2>err &&
-	test_i18ngrep "$expected" err &&
-	env GIT_TEST_STASH_USE_BUILTIN=false git stash 2>err &&
-	test_i18ngrep "$expected" err &&
-
-	git -c stash.useBuiltin=true stash 2>err &&
-	test_must_be_empty err &&
-	env GIT_TEST_STASH_USE_BUILTIN=true git stash 2>err &&
-	test_must_be_empty err
-'
-
 test_expect_success 'git stash succeeds despite directory/file change' '
 	test_create_repo directory_file_switch_v1 &&
 	(

gitgitgadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

Yay. We probably could have done this when we disabled the escape hatch, but better late than never ;-)

It is surprisingly small removal, but matches the explanation above, i.e. the actual old implementation was gone long time ago.

Looking good.

Thanks.