threads / discuss / 52879

Git Rebase: test failing with GIT_TEST_STASH_USE_BUILTIN=false

Subject: Git Rebase: test failing with GIT_TEST_STASH_USE_BUILTIN=false

## tl;dr

21 messages between Feb 24, 2020 and Mar 6, 2020.

replies: 20people: 6as markdown or json

Son Luong Ngoc· Feb 24, 2020, 08:33 UTC · lore

Hey git folks,

I have been trying to build git from source and noticing that some tests have been failing since 2.25 with the flag "GIT_TEST_STASH_USE_BUILTIN=false"

I think in 2.25 t3903.103 started to fail (rebase related) and current master t3904 may be failing also.

Is "GIT_TEST_STASH_USE_BUILTIN=false" is still being tested with or are we totally deprecating this flag?

Cheers, Son Luong.

Junio C Hamano· Feb 25, 2020, 16:57 UTC · re: Son Luong Ngoc · lore

Re: Git Rebase: test failing with GIT_TEST_STASH_USE_BUILTIN=false

Son Luong Ngoc <son.luong@booking.com> writes:

Show 9 quoted lines

> I have been trying to build git from source and noticing that some
> tests have been failing since 2.25 with the flag
> "GIT_TEST_STASH_USE_BUILTIN=false"
>
> I think in 2.25 t3903.103 started to fail (rebase related) and
> current master t3904 may be failing also.
>
> Is "GIT_TEST_STASH_USE_BUILTIN=false" is still being tested with
> or are we totally deprecating this flag?

In the longer term, when "git stash" gains new features that did not exist in the original scripted version, tests that observe how these features work would start failing when using the scripted version.

I picked some people from "git shortlog --no-merges builtin/stash.c" and placed them on the CC line---perhaps they may know more. It happens that Johannes is also familiar with "rebase", which you said is involved in the test failure, so I'd imagine he would be the best person to ask.

Thanks for a report.

Thomas Gummerer· Mar 2, 2020, 18:18 UTC · re: Junio C Hamano · lore

[PATCH 1/2] stash: get git_stash_config at the top level

On 02/25, Junio C Hamano wrote:

Show 21 quoted lines

> Son Luong Ngoc <son.luong@booking.com> writes:
> 
> > I have been trying to build git from source and noticing that some
> > tests have been failing since 2.25 with the flag
> > "GIT_TEST_STASH_USE_BUILTIN=false"
> >
> > I think in 2.25 t3903.103 started to fail (rebase related) and
> > current master t3904 may be failing also.
> >
> > Is "GIT_TEST_STASH_USE_BUILTIN=false" is still being tested with
> > or are we totally deprecating this flag?
> 
> In the longer term, when "git stash" gains new features that did not
> exist in the original scripted version, tests that observe how these
> features work would start failing when using the scripted version.
> 
> I picked some people from "git shortlog --no-merges builtin/stash.c"
> and placed them on the CC line---perhaps they may know more.  It
> happens that Johannes is also familiar with "rebase", which you
> said is involved in the test failure, so I'd imagine he would be the
> best person to ask.

Thanks for the report Son, and sorry for taking so long. I'm a little behing on reading my emails right now.

I think it is time to get rid of legacy stash. Nobody seems to have noticed this test failure until now, but according to your bisection it looks like this test has been failing since 2.23, which I confirmed locally. In addition, the last bugfix that was related to the rewrite was in 2.25, though that was really a fix for another fix in 2.24 that we failed to catch earlier.

I think 2.26 should still ship with the option, but after that we can probably get rid of it. So here's a couple of patches to do just that, for merging after 2.26 ships.

--- >8 --- In the next commit we're adding another config variable to be read from 'git_stash_config', that is valid for the top level command instead of just a subset. Move the 'git_config' invocation for 'git_stash_config' to the top-level to prepare for that.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 builtin/stash.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index 879fc5f368..f371db270c 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -712,7 +712,7 @@ static int git_stash_config(const char *var, const char *value, void *cb)
 		show_patch = git_config_bool(var, value);
 		return 0;
 	}
-	return git_default_config(var, value, cb);
+	return git_diff_basic_config(var, value, cb);
 }
 
 static int show_stash(int argc, const char **argv, const char *prefix)
@@ -749,7 +749,6 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 	 * any options.
 	 */
 	if (revision_args.argc == 1) {
-		git_config(git_stash_config, NULL);
 		if (show_stat)
 			rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT;
 
@@ -1573,7 +1572,7 @@ int cmd_stash(int argc, const char **argv, const char *prefix)
 	trace_repo_setup(prefix);
 	setup_work_tree();
 
-	git_config(git_diff_basic_config, NULL);
+	git_config(git_stash_config, NULL);
 
 	argc = parse_options(argc, argv, prefix, options, git_stash_usage,
 			     PARSE_OPT_KEEP_UNKNOWN | PARSE_OPT_KEEP_DASHDASH);

-- 
2.25.1.377.g2d2118b814

Thomas Gummerer· Mar 2, 2020, 18:19 UTC · re: Thomas Gummerer · lore

[PATCH 2/2] stash: remove the stash.useBuiltin setting

Remove the stash.useBuiltin setting which was added as an escape hatch to disable the builtin version of stash first released with Git 2.22.

Carrying the legacy version is a maintenance burden, and has in fact become out of date failing a test since the 2.23 release, without anyone noticing until now. So users would be getting a hint to fall back to a potentially buggy version of the tool.

We used to shell out to git config to get the useBuiltin configuration to avoid changing any global state before spawning legacy-stash. However that is no longer necessary, so just use the 'git_config' function to get the setting instead.

Similar to what we've done in d03ebd411c ("rebase: remove the rebase.useBuiltin setting", 2019-03-18), where we remove the corresponding setting for rebase, we leave the documentation in place, so people can refer back to it when searching for it online, and so we can refer to it in the commit message.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 .gitignore                     |   1 -
 Documentation/config/stash.txt |  18 +-
 Makefile                       |   1 -
 builtin/stash.c                |  44 +-
 git-legacy-stash.sh            | 798 ---------------------------------
 t/README                       |   4 -
 t/t3903-stash.sh               |  14 +
 7 files changed, 30 insertions(+), 850 deletions(-)
 delete mode 100755 git-legacy-stash.sh

diff --git a/.gitignore b/.gitignore
index aebe7c0908..188bd1c3de 100644
--- a/.gitignore
+++ b/.gitignore
@@ -83,7 +83,6 @@
 /git-init-db
 /git-interpret-trailers
 /git-instaweb
-/git-legacy-stash
 /git-log
 /git-ls-files
 /git-ls-remote
diff --git a/Documentation/config/stash.txt b/Documentation/config/stash.txt
index abc7ef4a3a..0f3db821f6 100644
--- a/Documentation/config/stash.txt
+++ b/Documentation/config/stash.txt
@@ -1,17 +1,9 @@
 stash.useBuiltin::
-       Set to `false` to use the legacy shell script implementation of
-       linkgit:git-stash[1]. Is `true` by default, which means use
-       the built-in rewrite of it in C.
-+
-The C rewrite is first included with Git version 2.22 (and Git for Windows
-version 2.19). This option serves as an escape hatch to re-enable the
-legacy version in case any bugs are found in the rewrite. This option and
-the shell script version of linkgit:git-stash[1] will be removed in some
-future release.
-+
-If you find some reason to set this option to `false`, other than
-one-off testing, you should report the behavior difference as a bug in
-Git (see https://git-scm.com/community for details).
+	Unused configuration Variable.  Used in Git versions 2.22 to
+	2.26 as an escape hatch to enable the legacy shellscript
+	implementation of rebase. 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.showPatch::
 	If this is set to true, the `git stash show` command without an
diff --git a/Makefile b/Makefile
index 9804a0758b..096c6d1fbb 100644
--- a/Makefile
+++ b/Makefile
@@ -609,7 +609,6 @@ SCRIPT_SH += git-merge-one-file.sh
 SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
 SCRIPT_SH += git-quiltimport.sh
-SCRIPT_SH += git-legacy-stash.sh
 SCRIPT_SH += git-request-pull.sh
 SCRIPT_SH += git-submodule.sh
 SCRIPT_SH += git-web--browse.sh
diff --git a/builtin/stash.c b/builtin/stash.c
index f371db270c..5f4bd693df 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -701,6 +701,7 @@ static int list_stash(int argc, const char **argv, const char *prefix)
 
 static int show_stat = 1;
 static int show_patch;
+static int use_legacy_stash;
 
 static int git_stash_config(const char *var, const char *value, void *cb)
 {
@@ -712,6 +713,10 @@ static int git_stash_config(const char *var, const char *value, void *cb)
 		show_patch = 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);
 }
 
@@ -1524,29 +1529,6 @@ static int save_stash(int argc, const char **argv, const char *prefix)
 	return ret;
 }
 
-static int use_builtin_stash(void)
-{
-	struct child_process cp = CHILD_PROCESS_INIT;
-	struct strbuf out = STRBUF_INIT;
-	int ret, env = git_env_bool("GIT_TEST_STASH_USE_BUILTIN", -1);
-
-	if (env != -1)
-		return env;
-
-	argv_array_pushl(&cp.args,
-			 "config", "--bool", "stash.usebuiltin", NULL);
-	cp.git_cmd = 1;
-	if (capture_command(&cp, &out, 6)) {
-		strbuf_release(&out);
-		return 1;
-	}
-
-	strbuf_trim(&out);
-	ret = !strcmp("true", out.buf);
-	strbuf_release(&out);
-	return ret;
-}
-
 int cmd_stash(int argc, const char **argv, const char *prefix)
 {
 	int i = -1;
@@ -1558,22 +1540,18 @@ int cmd_stash(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	if (!use_builtin_stash()) {
-		const char *path = mkpath("%s/git-legacy-stash",
-					  git_exec_path());
-
-		if (sane_execvp(path, (char **)argv) < 0)
-			die_errno(_("could not exec %s"), path);
-		else
-			BUG("sane_execvp() returned???");
-	}
-
 	prefix = setup_git_directory();
 	trace_repo_setup(prefix);
 	setup_work_tree();
 
 	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);
 
diff --git a/git-legacy-stash.sh b/git-legacy-stash.sh
deleted file mode 100755
index 4d4ebb4f2b..0000000000
--- a/git-legacy-stash.sh
+++ /dev/null
@@ -1,798 +0,0 @@
-#!/bin/sh
-# Copyright (c) 2007, Nanako Shiraishi
-
-dashless=$(basename "$0" | sed -e 's/-/ /')
-USAGE="list [<options>]
-   or: $dashless show [<stash>]
-   or: $dashless drop [-q|--quiet] [<stash>]
-   or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
-   or: $dashless branch <branchname> [<stash>]
-   or: $dashless save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
-		      [-u|--include-untracked] [-a|--all] [<message>]
-   or: $dashless [push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
-		       [-u|--include-untracked] [-a|--all] [-m <message>]
-		       [-- <pathspec>...]]
-   or: $dashless clear"
-
-SUBDIRECTORY_OK=Yes
-OPTIONS_SPEC=
-START_DIR=$(pwd)
-. git-sh-setup
-require_work_tree
-prefix=$(git rev-parse --show-prefix) || exit 1
-cd_to_toplevel
-
-TMP="$GIT_DIR/.git-stash.$$"
-TMPindex=${GIT_INDEX_FILE-"$(git rev-parse --git-path index)"}.stash.$$
-trap 'rm -f "$TMP-"* "$TMPindex"' 0
-
-ref_stash=refs/stash
-
-if git config --get-colorbool color.interactive; then
-       help_color="$(git config --get-color color.interactive.help 'red bold')"
-       reset_color="$(git config --get-color '' reset)"
-else
-       help_color=
-       reset_color=
-fi
-
-no_changes () {
-	git diff-index --quiet --cached HEAD --ignore-submodules -- "$@" &&
-	git diff-files --quiet --ignore-submodules -- "$@" &&
-	(test -z "$untracked" || test -z "$(untracked_files "$@")")
-}
-
-untracked_files () {
-	if test "$1" = "-z"
-	then
-		shift
-		z=-z
-	else
-		z=
-	fi
-	excl_opt=--exclude-standard
-	test "$untracked" = "all" && excl_opt=
-	git ls-files -o $z $excl_opt -- "$@"
-}
-
-prepare_fallback_ident () {
-	if ! git -c user.useconfigonly=yes var GIT_COMMITTER_IDENT >/dev/null 2>&1
-	then
-		GIT_AUTHOR_NAME="git stash"
-		GIT_AUTHOR_EMAIL=git@stash
-		GIT_COMMITTER_NAME="git stash"
-		GIT_COMMITTER_EMAIL=git@stash
-		export GIT_AUTHOR_NAME
-		export GIT_AUTHOR_EMAIL
-		export GIT_COMMITTER_NAME
-		export GIT_COMMITTER_EMAIL
-	fi
-}
-
-clear_stash () {
-	if test $# != 0
-	then
-		die "$(gettext "git stash clear with parameters is unimplemented")"
-	fi
-	if current=$(git rev-parse --verify --quiet $ref_stash)
-	then
-		git update-ref -d $ref_stash $current
-	fi
-}
-
-maybe_quiet () {
-	case "$1" in
-	--keep-stdout)
-		shift
-		if test -n "$GIT_QUIET"
-		then
-			"$@" 2>/dev/null
-		else
-			"$@"
-		fi
-		;;
-	*)
-		if test -n "$GIT_QUIET"
-		then
-			"$@" >/dev/null 2>&1
-		else
-			"$@"
-		fi
-		;;
-	esac
-}
-
-create_stash () {
-
-	prepare_fallback_ident
-
-	stash_msg=
-	untracked=
-	while test $# != 0
-	do
-		case "$1" in
-		-m|--message)
-			shift
-			stash_msg=${1?"BUG: create_stash () -m requires an argument"}
-			;;
-		-m*)
-			stash_msg=${1#-m}
-			;;
-		--message=*)
-			stash_msg=${1#--message=}
-			;;
-		-u|--include-untracked)
-			shift
-			untracked=${1?"BUG: create_stash () -u requires an argument"}
-			;;
-		--)
-			shift
-			break
-			;;
-		esac
-		shift
-	done
-
-	git update-index -q --refresh
-	if maybe_quiet no_changes "$@"
-	then
-		exit 0
-	fi
-
-	# state of the base commit
-	if b_commit=$(maybe_quiet --keep-stdout git rev-parse --verify HEAD)
-	then
-		head=$(git rev-list --oneline -n 1 HEAD --)
-	elif test -n "$GIT_QUIET"
-	then
-		exit 1
-	else
-		die "$(gettext "You do not have the initial commit yet")"
-	fi
-
-	if branch=$(git symbolic-ref -q HEAD)
-	then
-		branch=${branch#refs/heads/}
-	else
-		branch='(no branch)'
-	fi
-	msg=$(printf '%s: %s' "$branch" "$head")
-
-	# state of the index
-	i_tree=$(git write-tree) &&
-	i_commit=$(printf 'index on %s\n' "$msg" |
-		git commit-tree $i_tree -p $b_commit) ||
-		die "$(gettext "Cannot save the current index state")"
-
-	if test -n "$untracked"
-	then
-		# Untracked files are stored by themselves in a parentless commit, for
-		# ease of unpacking later.
-		u_commit=$(
-			untracked_files -z "$@" | (
-				GIT_INDEX_FILE="$TMPindex" &&
-				export GIT_INDEX_FILE &&
-				rm -f "$TMPindex" &&
-				git update-index -z --add --remove --stdin &&
-				u_tree=$(git write-tree) &&
-				printf 'untracked files on %s\n' "$msg" | git commit-tree $u_tree  &&
-				rm -f "$TMPindex"
-		) ) || die "$(gettext "Cannot save the untracked files")"
-
-		untracked_commit_option="-p $u_commit";
-	else
-		untracked_commit_option=
-	fi
-
-	if test -z "$patch_mode"
-	then
-
-		# state of the working tree
-		w_tree=$( (
-			git read-tree --index-output="$TMPindex" -m $i_tree &&
-			GIT_INDEX_FILE="$TMPindex" &&
-			export GIT_INDEX_FILE &&
-			git diff-index --name-only -z HEAD -- "$@" >"$TMP-stagenames" &&
-			git update-index --ignore-skip-worktree-entries \
-				-z --add --remove --stdin <"$TMP-stagenames" &&
-			git write-tree &&
-			rm -f "$TMPindex"
-		) ) ||
-			die "$(gettext "Cannot save the current worktree state")"
-
-	else
-
-		rm -f "$TMP-index" &&
-		GIT_INDEX_FILE="$TMP-index" git read-tree HEAD &&
-
-		# find out what the user wants
-		GIT_INDEX_FILE="$TMP-index" \
-			git add --legacy-stash-p -- "$@" &&
-
-		# state of the working tree
-		w_tree=$(GIT_INDEX_FILE="$TMP-index" git write-tree) ||
-		die "$(gettext "Cannot save the current worktree state")"
-
-		git diff-tree -p HEAD $w_tree -- >"$TMP-patch" &&
-		test -s "$TMP-patch" ||
-		die "$(gettext "No changes selected")"
-
-		rm -f "$TMP-index" ||
-		die "$(gettext "Cannot remove temporary index (can't happen)")"
-
-	fi
-
-	# create the stash
-	if test -z "$stash_msg"
-	then
-		stash_msg=$(printf 'WIP on %s' "$msg")
-	else
-		stash_msg=$(printf 'On %s: %s' "$branch" "$stash_msg")
-	fi
-	w_commit=$(printf '%s\n' "$stash_msg" |
-	git commit-tree $w_tree -p $b_commit -p $i_commit $untracked_commit_option) ||
-	die "$(gettext "Cannot record working tree state")"
-}
-
-store_stash () {
-	while test $# != 0
-	do
-		case "$1" in
-		-m|--message)
-			shift
-			stash_msg="$1"
-			;;
-		-m*)
-			stash_msg=${1#-m}
-			;;
-		--message=*)
-			stash_msg=${1#--message=}
-			;;
-		-q|--quiet)
-			quiet=t
-			;;
-		*)
-			break
-			;;
-		esac
-		shift
-	done
-	test $# = 1 ||
-	die "$(eval_gettext "\"$dashless store\" requires one <commit> argument")"
-
-	w_commit="$1"
-	if test -z "$stash_msg"
-	then
-		stash_msg="Created via \"git stash store\"."
-	fi
-
-	git update-ref --create-reflog -m "$stash_msg" $ref_stash $w_commit
-	ret=$?
-	test $ret != 0 && test -z "$quiet" &&
-	die "$(eval_gettext "Cannot update \$ref_stash with \$w_commit")"
-	return $ret
-}
-
-push_stash () {
-	keep_index=
-	patch_mode=
-	untracked=
-	stash_msg=
-	while test $# != 0
-	do
-		case "$1" in
-		-k|--keep-index)
-			keep_index=t
-			;;
-		--no-keep-index)
-			keep_index=n
-			;;
-		-p|--patch)
-			patch_mode=t
-			# only default to keep if we don't already have an override
-			test -z "$keep_index" && keep_index=t
-			;;
-		-q|--quiet)
-			GIT_QUIET=t
-			;;
-		-u|--include-untracked)
-			untracked=untracked
-			;;
-		-a|--all)
-			untracked=all
-			;;
-		-m|--message)
-			shift
-			test -z ${1+x} && usage
-			stash_msg=$1
-			;;
-		-m*)
-			stash_msg=${1#-m}
-			;;
-		--message=*)
-			stash_msg=${1#--message=}
-			;;
-		--help)
-			show_help
-			;;
-		--)
-			shift
-			break
-			;;
-		-*)
-			option="$1"
-			eval_gettextln "error: unknown option for 'stash push': \$option"
-			usage
-			;;
-		*)
-			break
-			;;
-		esac
-		shift
-	done
-
-	eval "set $(git rev-parse --sq --prefix "$prefix" -- "$@")"
-
-	if test -n "$patch_mode" && test -n "$untracked"
-	then
-		die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
-	fi
-
-	test -n "$untracked" || git ls-files --error-unmatch -- "$@" >/dev/null || exit 1
-
-	git update-index -q --refresh
-	if maybe_quiet no_changes "$@"
-	then
-		say "$(gettext "No local changes to save")"
-		exit 0
-	fi
-
-	git reflog exists $ref_stash ||
-		clear_stash || die "$(gettext "Cannot initialize stash")"
-
-	create_stash -m "$stash_msg" -u "$untracked" -- "$@"
-	store_stash -m "$stash_msg" -q $w_commit ||
-	die "$(gettext "Cannot save the current status")"
-	say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
-
-	if test -z "$patch_mode"
-	then
-		test "$untracked" = "all" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=
-		if test -n "$untracked" && test $# = 0
-		then
-			git clean --force --quiet -d $CLEAN_X_OPTION
-		fi
-
-		if test $# != 0
-		then
-			test -z "$untracked" && UPDATE_OPTION="-u" || UPDATE_OPTION=
-			test "$untracked" = "all" && FORCE_OPTION="--force" || FORCE_OPTION=
-			git add $UPDATE_OPTION $FORCE_OPTION -- "$@"
-			git diff-index -p --cached --binary HEAD -- "$@" |
-			git apply --index -R
-		else
-			git reset --hard -q --no-recurse-submodules
-		fi
-
-		if test "$keep_index" = "t" && test -n "$i_tree"
-		then
-			git read-tree --reset $i_tree
-			git ls-files -z --modified -- "$@" |
-			git checkout-index -z --force --stdin
-		fi
-	else
-		git apply -R < "$TMP-patch" ||
-		die "$(gettext "Cannot remove worktree changes")"
-
-		if test "$keep_index" != "t"
-		then
-			git reset -q -- "$@"
-		fi
-	fi
-}
-
-save_stash () {
-	push_options=
-	while test $# != 0
-	do
-		case "$1" in
-		-q|--quiet)
-			GIT_QUIET=t
-			;;
-		--)
-			shift
-			break
-			;;
-		-*)
-			# pass all options through to push_stash
-			push_options="$push_options $1"
-			;;
-		*)
-			break
-			;;
-		esac
-		shift
-	done
-
-	stash_msg="$*"
-
-	if test -z "$stash_msg"
-	then
-		push_stash $push_options
-	else
-		push_stash $push_options -m "$stash_msg"
-	fi
-}
-
-have_stash () {
-	git rev-parse --verify --quiet $ref_stash >/dev/null
-}
-
-list_stash () {
-	have_stash || return 0
-	git log --format="%gd: %gs" -g --first-parent -m "$@" $ref_stash --
-}
-
-show_stash () {
-	ALLOW_UNKNOWN_FLAGS=t
-	assert_stash_like "$@"
-
-	if test -z "$FLAGS"
-	then
-		if test "$(git config --bool stash.showStat || echo true)" = "true"
-		then
-			FLAGS=--stat
-		fi
-
-		if test "$(git config --bool stash.showPatch || echo false)" = "true"
-		then
-			FLAGS=${FLAGS}${FLAGS:+ }-p
-		fi
-
-		if test -z "$FLAGS"
-		then
-			return 0
-		fi
-	fi
-
-	git diff ${FLAGS} $b_commit $w_commit
-}
-
-show_help () {
-	exec git help stash
-	exit 1
-}
-
-#
-# Parses the remaining options looking for flags and
-# at most one revision defaulting to ${ref_stash}@{0}
-# if none found.
-#
-# Derives related tree and commit objects from the
-# revision, if one is found.
-#
-# stash records the work tree, and is a merge between the
-# base commit (first parent) and the index tree (second parent).
-#
-#   REV is set to the symbolic version of the specified stash-like commit
-#   IS_STASH_LIKE is non-blank if ${REV} looks like a stash
-#   IS_STASH_REF is non-blank if the ${REV} looks like a stash ref
-#   s is set to the SHA1 of the stash commit
-#   w_commit is set to the commit containing the working tree
-#   b_commit is set to the base commit
-#   i_commit is set to the commit containing the index tree
-#   u_commit is set to the commit containing the untracked files tree
-#   w_tree is set to the working tree
-#   b_tree is set to the base tree
-#   i_tree is set to the index tree
-#   u_tree is set to the untracked files tree
-#
-#   GIT_QUIET is set to t if -q is specified
-#   INDEX_OPTION is set to --index if --index is specified.
-#   FLAGS is set to the remaining flags (if allowed)
-#
-# dies if:
-#   * too many revisions specified
-#   * no revision is specified and there is no stash stack
-#   * a revision is specified which cannot be resolve to a SHA1
-#   * a non-existent stash reference is specified
-#   * unknown flags were set and ALLOW_UNKNOWN_FLAGS is not "t"
-#
-
-parse_flags_and_rev()
-{
-	test "$PARSE_CACHE" = "$*" && return 0 # optimisation
-	PARSE_CACHE="$*"
-
-	IS_STASH_LIKE=
-	IS_STASH_REF=
-	INDEX_OPTION=
-	s=
-	w_commit=
-	b_commit=
-	i_commit=
-	u_commit=
-	w_tree=
-	b_tree=
-	i_tree=
-	u_tree=
-
-	FLAGS=
-	REV=
-	for opt
-	do
-		case "$opt" in
-			-q|--quiet)
-				GIT_QUIET=-t
-			;;
-			--index)
-				INDEX_OPTION=--index
-			;;
-			--help)
-				show_help
-			;;
-			-*)
-				test "$ALLOW_UNKNOWN_FLAGS" = t ||
-					die "$(eval_gettext "unknown option: \$opt")"
-				FLAGS="${FLAGS}${FLAGS:+ }$opt"
-			;;
-			*)
-				REV="${REV}${REV:+ }'$opt'"
-			;;
-		esac
-	done
-
-	eval set -- $REV
-
-	case $# in
-		0)
-			have_stash || die "$(gettext "No stash entries found.")"
-			set -- ${ref_stash}@{0}
-		;;
-		1)
-			:
-		;;
-		*)
-			die "$(eval_gettext "Too many revisions specified: \$REV")"
-		;;
-	esac
-
-	case "$1" in
-		*[!0-9]*)
-			:
-		;;
-		*)
-			set -- "${ref_stash}@{$1}"
-		;;
-	esac
-
-	REV=$(git rev-parse --symbolic --verify --quiet "$1") || {
-		reference="$1"
-		die "$(eval_gettext "\$reference is not a valid reference")"
-	}
-
-	i_commit=$(git rev-parse --verify --quiet "$REV^2") &&
-	set -- $(git rev-parse "$REV" "$REV^1" "$REV:" "$REV^1:" "$REV^2:" 2>/dev/null) &&
-	s=$1 &&
-	w_commit=$1 &&
-	b_commit=$2 &&
-	w_tree=$3 &&
-	b_tree=$4 &&
-	i_tree=$5 &&
-	IS_STASH_LIKE=t &&
-	test "$ref_stash" = "$(git rev-parse --symbolic-full-name "${REV%@*}")" &&
-	IS_STASH_REF=t
-
-	u_commit=$(git rev-parse --verify --quiet "$REV^3") &&
-	u_tree=$(git rev-parse "$REV^3:" 2>/dev/null)
-}
-
-is_stash_like()
-{
-	parse_flags_and_rev "$@"
-	test -n "$IS_STASH_LIKE"
-}
-
-assert_stash_like() {
-	is_stash_like "$@" || {
-		args="$*"
-		die "$(eval_gettext "'\$args' is not a stash-like commit")"
-	}
-}
-
-is_stash_ref() {
-	is_stash_like "$@" && test -n "$IS_STASH_REF"
-}
-
-assert_stash_ref() {
-	is_stash_ref "$@" || {
-		args="$*"
-		die "$(eval_gettext "'\$args' is not a stash reference")"
-	}
-}
-
-apply_stash () {
-
-	assert_stash_like "$@"
-
-	git update-index -q --refresh || die "$(gettext "unable to refresh index")"
-
-	# current index state
-	c_tree=$(git write-tree) ||
-		die "$(gettext "Cannot apply a stash in the middle of a merge")"
-
-	unstashed_index_tree=
-	if test -n "$INDEX_OPTION" && test "$b_tree" != "$i_tree" &&
-			test "$c_tree" != "$i_tree"
-	then
-		git diff-tree --binary $s^2^..$s^2 | git apply --cached
-		test $? -ne 0 &&
-			die "$(gettext "Conflicts in index. Try without --index.")"
-		unstashed_index_tree=$(git write-tree) ||
-			die "$(gettext "Could not save index tree")"
-		git reset
-	fi
-
-	if test -n "$u_tree"
-	then
-		GIT_INDEX_FILE="$TMPindex" git read-tree "$u_tree" &&
-		GIT_INDEX_FILE="$TMPindex" git checkout-index --all &&
-		rm -f "$TMPindex" ||
-		die "$(gettext "Could not restore untracked files from stash entry")"
-	fi
-
-	eval "
-		GITHEAD_$w_tree='Stashed changes' &&
-		GITHEAD_$c_tree='Updated upstream' &&
-		GITHEAD_$b_tree='Version stash was based on' &&
-		export GITHEAD_$w_tree GITHEAD_$c_tree GITHEAD_$b_tree
-	"
-
-	if test -n "$GIT_QUIET"
-	then
-		GIT_MERGE_VERBOSITY=0 && export GIT_MERGE_VERBOSITY
-	fi
-	if git merge-recursive $b_tree -- $c_tree $w_tree
-	then
-		# No conflict
-		if test -n "$unstashed_index_tree"
-		then
-			git read-tree "$unstashed_index_tree"
-		else
-			a="$TMP-added" &&
-			git diff-index --cached --name-only --diff-filter=A $c_tree >"$a" &&
-			git read-tree --reset $c_tree &&
-			git update-index --add --stdin <"$a" ||
-				die "$(gettext "Cannot unstage modified files")"
-			rm -f "$a"
-		fi
-		squelch=
-		if test -n "$GIT_QUIET"
-		then
-			squelch='>/dev/null 2>&1'
-		fi
-		(cd "$START_DIR" && eval "git status $squelch") || :
-	else
-		# Merge conflict; keep the exit status from merge-recursive
-		status=$?
-		git rerere
-		if test -n "$INDEX_OPTION"
-		then
-			gettextln "Index was not unstashed." >&2
-		fi
-		exit $status
-	fi
-}
-
-pop_stash() {
-	assert_stash_ref "$@"
-
-	if apply_stash "$@"
-	then
-		drop_stash "$@"
-	else
-		status=$?
-		say "$(gettext "The stash entry is kept in case you need it again.")"
-		exit $status
-	fi
-}
-
-drop_stash () {
-	assert_stash_ref "$@"
-
-	git reflog delete --updateref --rewrite "${REV}" &&
-		say "$(eval_gettext "Dropped \${REV} (\$s)")" ||
-		die "$(eval_gettext "\${REV}: Could not drop stash entry")"
-
-	# clear_stash if we just dropped the last stash entry
-	git rev-parse --verify --quiet "$ref_stash@{0}" >/dev/null ||
-	clear_stash
-}
-
-apply_to_branch () {
-	test -n "$1" || die "$(gettext "No branch name specified")"
-	branch=$1
-	shift 1
-
-	set -- --index "$@"
-	assert_stash_like "$@"
-
-	git checkout -b $branch $REV^ &&
-	apply_stash "$@" && {
-		test -z "$IS_STASH_REF" || drop_stash "$@"
-	}
-}
-
-test "$1" = "-p" && set "push" "$@"
-
-PARSE_CACHE='--not-parsed'
-# The default command is "push" if nothing but options are given
-seen_non_option=
-for opt
-do
-	case "$opt" in
-	--) break ;;
-	-*) ;;
-	*) seen_non_option=t; break ;;
-	esac
-done
-
-test -n "$seen_non_option" || set "push" "$@"
-
-# Main command set
-case "$1" in
-list)
-	shift
-	list_stash "$@"
-	;;
-show)
-	shift
-	show_stash "$@"
-	;;
-save)
-	shift
-	save_stash "$@"
-	;;
-push)
-	shift
-	push_stash "$@"
-	;;
-apply)
-	shift
-	apply_stash "$@"
-	;;
-clear)
-	shift
-	clear_stash "$@"
-	;;
-create)
-	shift
-	create_stash -m "$*" && echo "$w_commit"
-	;;
-store)
-	shift
-	store_stash "$@"
-	;;
-drop)
-	shift
-	drop_stash "$@"
-	;;
-pop)
-	shift
-	pop_stash "$@"
-	;;
-branch)
-	shift
-	apply_to_branch "$@"
-	;;
-*)
-	case $# in
-	0)
-		push_stash &&
-		say "$(gettext "(To restore them type \"git stash apply\")")"
-		;;
-	*)
-		usage
-	esac
-	;;
-esac
diff --git a/t/README b/t/README
index 9afd61e3ca..369e3a9ded 100644
--- a/t/README
+++ b/t/README
@@ -393,10 +393,6 @@ the --no-sparse command-line argument.
 GIT_TEST_PRELOAD_INDEX=<boolean> exercises the preload-index code path
 by overriding the minimum number of cache entries required per thread.
 
-GIT_TEST_STASH_USE_BUILTIN=<boolean>, when false, disables the
-built-in version of git-stash. See 'stash.useBuiltin' in
-git-config(1).
-
 GIT_TEST_ADD_I_USE_BUILTIN=<boolean>, when true, enables the
 built-in version of git add -i. See 'add.interactive.useBuiltin' in
 git-config(1).
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index ea56e85e70..b84d55a832 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1285,4 +1285,18 @@ 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_done

-- 
2.25.1.377.g2d2118b814

Junio C Hamano· Mar 2, 2020, 21:30 UTC · re: Thomas Gummerer · lore

Re: [PATCH 2/2] stash: remove the stash.useBuiltin setting

Thomas Gummerer <t.gummerer@gmail.com> writes:

Show 20 quoted lines

> -	if (!use_builtin_stash()) {
> -		const char *path = mkpath("%s/git-legacy-stash",
> -					  git_exec_path());
> -
> -		if (sane_execvp(path, (char **)argv) < 0)
> -			die_errno(_("could not exec %s"), path);
> -		else
> -			BUG("sane_execvp() returned???");
> -	}
> -
>  	prefix = setup_git_directory();
>  	trace_repo_setup(prefix);
>  	setup_work_tree();
>  
>  	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."));

Makes sense.

Junio C Hamano· Mar 2, 2020, 22:21 UTC · re: Thomas Gummerer · lore

non-regression bug in 'git merge' (really, xdiff/xmerge, I think)

Thomas Gummerer <t.gummerer@gmail.com> writes:

Show 12 quoted lines

> diff --git a/Makefile b/Makefile
> index 9804a0758b..096c6d1fbb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -609,7 +609,6 @@ SCRIPT_SH += git-merge-one-file.sh
>  SCRIPT_SH += git-merge-resolve.sh
>  SCRIPT_SH += git-mergetool.sh
>  SCRIPT_SH += git-quiltimport.sh
> -SCRIPT_SH += git-legacy-stash.sh
>  SCRIPT_SH += git-request-pull.sh
>  SCRIPT_SH += git-submodule.sh
>  SCRIPT_SH += git-web--browse.sh

Merging this topic to 'pu', which has dl/merge-autostash topic already merged, breaks the build and it is not fault of Thomas's topic. dl/merge-autostash has the following bit:

commit 4b981afaa03c00524f1d4986271a3f4cc119f4cd
Merge: 06928448f7 de4db86ff2
Author: Junio C Hamano <gitster@pobox.com>
Date:   Mon Mar 2 13:23:26 2020 -0800

    Merge branch 'dl/merge-autostash' into pu
    
    "git merge" learns the "--autostash" option.
    
    * dl/merge-autostash:
      ...
      Makefile: alphabetically sort += lists

diff --git a/Makefile b/Makefile
index a5961113d8..c0793ac75b 100644
--- a/Makefile
+++ b/Makefile
@@ -604,12 +604,12 @@ unexport CDPATH
 SCRIPT_SH += git-bisect.sh
 SCRIPT_SH += git-difftool--helper.sh
 SCRIPT_SH += git-filter-branch.sh
+SCRIPT_SH += git-legacy-stash.sh
 SCRIPT_SH += git-merge-octopus.sh
 SCRIPT_SH += git-merge-one-file.sh
 SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
 SCRIPT_SH += git-quiltimport.sh
-SCRIPT_SH += git-legacy-stash.sh
 SCRIPT_SH += git-request-pull.sh
 SCRIPT_SH += git-submodule.sh
 SCRIPT_SH += git-web--browse.sh
@@ -617,8 +617,8 @@ SCRIPT_SH += git-web--browse.sh

which makes it appear to the xmerge code that legacy-status that
used to exist in between quiltimport and request-pull has already
been removed.

As Thomas's above patch exactly wants to do that (i.e. we want to
remove legacy-stash between quiltimport and request-pull), the
xmerge code seems to think incorrectly that the requested change has
already been applied.  And as a result of that, there remains
legacy-stash between fliter-branch and merge-octopus, breaking the
build by having one unbuildable build target X-<.

Elijah Newren· Mar 6, 2020, 15:48 UTC · re: Junio C Hamano · lore

Re: non-regression bug in 'git merge' (really, xdiff/xmerge, I think)

On Mon, Mar 2, 2020 at 2:21 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 63 quoted lines

>
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > diff --git a/Makefile b/Makefile
> > index 9804a0758b..096c6d1fbb 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -609,7 +609,6 @@ SCRIPT_SH += git-merge-one-file.sh
> >  SCRIPT_SH += git-merge-resolve.sh
> >  SCRIPT_SH += git-mergetool.sh
> >  SCRIPT_SH += git-quiltimport.sh
> > -SCRIPT_SH += git-legacy-stash.sh
> >  SCRIPT_SH += git-request-pull.sh
> >  SCRIPT_SH += git-submodule.sh
> >  SCRIPT_SH += git-web--browse.sh
>
> Merging this topic to 'pu', which has dl/merge-autostash topic
> already merged, breaks the build and it is not fault of Thomas's
> topic.  dl/merge-autostash has the following bit:
>
> commit 4b981afaa03c00524f1d4986271a3f4cc119f4cd
> Merge: 06928448f7 de4db86ff2
> Author: Junio C Hamano <gitster@pobox.com>
> Date:   Mon Mar 2 13:23:26 2020 -0800
>
>     Merge branch 'dl/merge-autostash' into pu
>
>     "git merge" learns the "--autostash" option.
>
>     * dl/merge-autostash:
>       ...
>       Makefile: alphabetically sort += lists
>
> diff --git a/Makefile b/Makefile
> index a5961113d8..c0793ac75b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -604,12 +604,12 @@ unexport CDPATH
>  SCRIPT_SH += git-bisect.sh
>  SCRIPT_SH += git-difftool--helper.sh
>  SCRIPT_SH += git-filter-branch.sh
> +SCRIPT_SH += git-legacy-stash.sh
>  SCRIPT_SH += git-merge-octopus.sh
>  SCRIPT_SH += git-merge-one-file.sh
>  SCRIPT_SH += git-merge-resolve.sh
>  SCRIPT_SH += git-mergetool.sh
>  SCRIPT_SH += git-quiltimport.sh
> -SCRIPT_SH += git-legacy-stash.sh
>  SCRIPT_SH += git-request-pull.sh
>  SCRIPT_SH += git-submodule.sh
>  SCRIPT_SH += git-web--browse.sh
> @@ -617,8 +617,8 @@ SCRIPT_SH += git-web--browse.sh
>
> which makes it appear to the xmerge code that legacy-status that
> used to exist in between quiltimport and request-pull has already
> been removed.
>
> As Thomas's above patch exactly wants to do that (i.e. we want to
> remove legacy-stash between quiltimport and request-pull), the
> xmerge code seems to think incorrectly that the requested change has
> already been applied.  And as a result of that, there remains
> legacy-stash between fliter-branch and merge-octopus, breaking the
> build by having one unbuildable build target X-<.

Thanks for the heads up; I agree that it looks like an xdiff thing, and in particular it reminds me of https://lore.kernel.org/git/20190816184051.GB13894@sigill.intra.peff.net/ and https://lore.kernel.org/git/CABPp-BHvJHpSJT7sdFwfNcPn_sOXwJi3=o14qjZS3M8Rzcxe2A@mail.gmail.com/

I'm collecting these cases because I'd like to look into it at some point, though for now I'm more focused on merge-ort (my merge-recursive replacement).

Johannes Schindelin· Mar 3, 2020, 13:33 UTC · re: Thomas Gummerer · lore

Re: [PATCH 2/2] stash: remove the stash.useBuiltin setting

Hi Thomas,

On Mon, 2 Mar 2020, Thomas Gummerer wrote:

> Remove the stash.useBuiltin setting which was added as an escape hatch
> to disable the builtin version of stash first released with Git 2.22.

Great!

Show 21 quoted lines

> diff --git a/builtin/stash.c b/builtin/stash.c
> index f371db270c..5f4bd693df 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -1558,22 +1540,18 @@ int cmd_stash(int argc, const char **argv, const char *prefix)
>  		OPT_END()
>  	};
>
> -	if (!use_builtin_stash()) {
> -		const char *path = mkpath("%s/git-legacy-stash",
> -					  git_exec_path());
> -
> -		if (sane_execvp(path, (char **)argv) < 0)
> -			die_errno(_("could not exec %s"), path);
> -		else
> -			BUG("sane_execvp() returned???");
> -	}
> -
>  	prefix = setup_git_directory();
>  	trace_repo_setup(prefix);
>  	setup_work_tree();

However, this is only needed because we did not change `git.c` at the same time (as I had done for `rebase` in 80dfc9242eb (git: mark cmd_rebase as requiring a worktree, 2019-07-24)). In other words, I think we will also want to address this part of `git.c`:

        /*
         * NEEDSWORK: Until the builtin stash is thoroughly robust and no
         * longer needs redirection to the stash shell script this is kept as
         * is, then should be changed to RUN_SETUP | NEED_WORK_TREE
         */
        { "stash", cmd_stash },

Other than that, I think your patch pair is good to go!

Ciao, Dscho

Junio C Hamano· Mar 3, 2020, 16:47 UTC · re: Johannes Schindelin · lore

Re: [PATCH 2/2] stash: remove the stash.useBuiltin setting

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 15 quoted lines

>>  	prefix = setup_git_directory();
>>  	trace_repo_setup(prefix);
>>  	setup_work_tree();
>
> However, this is only needed because we did not change `git.c` at the same
> time (as I had done for `rebase` in 80dfc9242eb (git: mark cmd_rebase as
> requiring a worktree, 2019-07-24)). In other words, I think we will also
> want to address this part of `git.c`:
>
>         /*
>          * NEEDSWORK: Until the builtin stash is thoroughly robust and no
>          * longer needs redirection to the stash shell script this is kept as
>          * is, then should be changed to RUN_SETUP | NEED_WORK_TREE
>          */
>         { "stash", cmd_stash },

Ah, I missed that part; thanks for chiming in.

Thomas Gummerer· Mar 3, 2020, 17:36 UTC · re: Johannes Schindelin · lore

Re: [PATCH 2/2] stash: remove the stash.useBuiltin setting

On 03/03, Johannes Schindelin wrote:

Show 44 quoted lines

> Hi Thomas,
> 
> On Mon, 2 Mar 2020, Thomas Gummerer wrote:
> 
> > Remove the stash.useBuiltin setting which was added as an escape hatch
> > to disable the builtin version of stash first released with Git 2.22.
> 
> Great!
> 
> > diff --git a/builtin/stash.c b/builtin/stash.c
> > index f371db270c..5f4bd693df 100644
> > --- a/builtin/stash.c
> > +++ b/builtin/stash.c
> > @@ -1558,22 +1540,18 @@ int cmd_stash(int argc, const char **argv, const char *prefix)
> >  		OPT_END()
> >  	};
> >
> > -	if (!use_builtin_stash()) {
> > -		const char *path = mkpath("%s/git-legacy-stash",
> > -					  git_exec_path());
> > -
> > -		if (sane_execvp(path, (char **)argv) < 0)
> > -			die_errno(_("could not exec %s"), path);
> > -		else
> > -			BUG("sane_execvp() returned???");
> > -	}
> > -
> >  	prefix = setup_git_directory();
> >  	trace_repo_setup(prefix);
> >  	setup_work_tree();
> 
> However, this is only needed because we did not change `git.c` at the same
> time (as I had done for `rebase` in 80dfc9242eb (git: mark cmd_rebase as
> requiring a worktree, 2019-07-24)). In other words, I think we will also
> want to address this part of `git.c`:
> 
>         /*
>          * NEEDSWORK: Until the builtin stash is thoroughly robust and no
>          * longer needs redirection to the stash shell script this is kept as
>          * is, then should be changed to RUN_SETUP | NEED_WORK_TREE
>          */
>         { "stash", cmd_stash },
> 
> Other than that, I think your patch pair is good to go!

Ah I had completely missed that, thanks for catching it! Will send the updated patches shortly.

Johannes Schindelin· Mar 4, 2020, 16:41 UTC · re: Thomas Gummerer · lore

Re: [PATCH 2/2] stash: remove the stash.useBuiltin setting

Hi Thomas,

On Tue, 3 Mar 2020, Thomas Gummerer wrote:

Show 47 quoted lines

> On 03/03, Johannes Schindelin wrote:
> >
> > On Mon, 2 Mar 2020, Thomas Gummerer wrote:
> >
> > > Remove the stash.useBuiltin setting which was added as an escape hatch
> > > to disable the builtin version of stash first released with Git 2.22.
> >
> > Great!
> >
> > > diff --git a/builtin/stash.c b/builtin/stash.c
> > > index f371db270c..5f4bd693df 100644
> > > --- a/builtin/stash.c
> > > +++ b/builtin/stash.c
> > > @@ -1558,22 +1540,18 @@ int cmd_stash(int argc, const char **argv, const char *prefix)
> > >  		OPT_END()
> > >  	};
> > >
> > > -	if (!use_builtin_stash()) {
> > > -		const char *path = mkpath("%s/git-legacy-stash",
> > > -					  git_exec_path());
> > > -
> > > -		if (sane_execvp(path, (char **)argv) < 0)
> > > -			die_errno(_("could not exec %s"), path);
> > > -		else
> > > -			BUG("sane_execvp() returned???");
> > > -	}
> > > -
> > >  	prefix = setup_git_directory();
> > >  	trace_repo_setup(prefix);
> > >  	setup_work_tree();
> >
> > However, this is only needed because we did not change `git.c` at the same
> > time (as I had done for `rebase` in 80dfc9242eb (git: mark cmd_rebase as
> > requiring a worktree, 2019-07-24)). In other words, I think we will also
> > want to address this part of `git.c`:
> >
> >         /*
> >          * NEEDSWORK: Until the builtin stash is thoroughly robust and no
> >          * longer needs redirection to the stash shell script this is kept as
> >          * is, then should be changed to RUN_SETUP | NEED_WORK_TREE
> >          */
> >         { "stash", cmd_stash },
> >
> > Other than that, I think your patch pair is good to go!
>
> Ah I had completely missed that, thanks for catching it!  Will send
> the updated patches shortly.

Thank you for fixing this so quickly!

Ciao, Dscho

Thomas Gummerer· Mar 3, 2020, 17:46 UTC · re: Thomas Gummerer · lore

[PATCH v2 1/2] stash: get git_stash_config at the top level

In the next commit we're adding another config variable to be read from 'git_stash_config', that is valid for the top level command instead of just a subset. Move the 'git_config' invocation for 'git_stash_config' to the top-level to prepare for that.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 builtin/stash.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index 879fc5f368..f371db270c 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -712,7 +712,7 @@ static int git_stash_config(const char *var, const char *value, void *cb)
 		show_patch = git_config_bool(var, value);
 		return 0;
 	}
-	return git_default_config(var, value, cb);
+	return git_diff_basic_config(var, value, cb);
 }
 
 static int show_stash(int argc, const char **argv, const char *prefix)
@@ -749,7 +749,6 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 	 * any options.
 	 */
 	if (revision_args.argc == 1) {
-		git_config(git_stash_config, NULL);
 		if (show_stat)
 			rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT;
 
@@ -1573,7 +1572,7 @@ int cmd_stash(int argc, const char **argv, const char *prefix)
 	trace_repo_setup(prefix);
 	setup_work_tree();
 
-	git_config(git_diff_basic_config, NULL);
+	git_config(git_stash_config, NULL);
 
 	argc = parse_options(argc, argv, prefix, options, git_stash_usage,
 			     PARSE_OPT_KEEP_UNKNOWN | PARSE_OPT_KEEP_DASHDASH);

-- 
2.25.1.460.g2f268890c2

Thomas Gummerer· Mar 3, 2020, 17:46 UTC · re: Thomas Gummerer · lore

[PATCH v2 2/2] stash: remove the stash.useBuiltin setting

Remove the stash.useBuiltin setting which was added as an escape hatch to disable the builtin version of stash first released with Git 2.22.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 .gitignore                     |   1 -
 Documentation/config/stash.txt |  18 +-
 Makefile                       |   1 -
 builtin/stash.c                |  47 +-
 git-legacy-stash.sh            | 798 ---------------------------------
 git.c                          |   7 +-
 t/README                       |   4 -
 t/t3903-stash.sh               |  14 +
 8 files changed, 30 insertions(+), 860 deletions(-)
 delete mode 100755 git-legacy-stash.sh

diff --git a/.gitignore b/.gitignore
index aebe7c0908..188bd1c3de 100644
--- a/.gitignore
+++ b/.gitignore
@@ -83,7 +83,6 @@
 /git-init-db
 /git-interpret-trailers
 /git-instaweb
-/git-legacy-stash
 /git-log
 /git-ls-files
 /git-ls-remote
diff --git a/Documentation/config/stash.txt b/Documentation/config/stash.txt
index abc7ef4a3a..0f3db821f6 100644
--- a/Documentation/config/stash.txt
+++ b/Documentation/config/stash.txt
@@ -1,17 +1,9 @@
 stash.useBuiltin::
-       Set to `false` to use the legacy shell script implementation of
-       linkgit:git-stash[1]. Is `true` by default, which means use
-       the built-in rewrite of it in C.
-+
-The C rewrite is first included with Git version 2.22 (and Git for Windows
-version 2.19). This option serves as an escape hatch to re-enable the
-legacy version in case any bugs are found in the rewrite. This option and
-the shell script version of linkgit:git-stash[1] will be removed in some
-future release.
-+
-If you find some reason to set this option to `false`, other than
-one-off testing, you should report the behavior difference as a bug in
-Git (see https://git-scm.com/community for details).
+	Unused configuration Variable.  Used in Git versions 2.22 to
+	2.26 as an escape hatch to enable the legacy shellscript
+	implementation of rebase. 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.showPatch::
 	If this is set to true, the `git stash show` command without an
diff --git a/Makefile b/Makefile
index 9804a0758b..096c6d1fbb 100644
--- a/Makefile
+++ b/Makefile
@@ -609,7 +609,6 @@ SCRIPT_SH += git-merge-one-file.sh
 SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
 SCRIPT_SH += git-quiltimport.sh
-SCRIPT_SH += git-legacy-stash.sh
 SCRIPT_SH += git-request-pull.sh
 SCRIPT_SH += git-submodule.sh
 SCRIPT_SH += git-web--browse.sh
diff --git a/builtin/stash.c b/builtin/stash.c
index f371db270c..ba908a6834 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -701,6 +701,7 @@ static int list_stash(int argc, const char **argv, const char *prefix)
 
 static int show_stat = 1;
 static int show_patch;
+static int use_legacy_stash;
 
 static int git_stash_config(const char *var, const char *value, void *cb)
 {
@@ -712,6 +713,10 @@ static int git_stash_config(const char *var, const char *value, void *cb)
 		show_patch = 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);
 }
 
@@ -1524,29 +1529,6 @@ static int save_stash(int argc, const char **argv, const char *prefix)
 	return ret;
 }
 
-static int use_builtin_stash(void)
-{
-	struct child_process cp = CHILD_PROCESS_INIT;
-	struct strbuf out = STRBUF_INIT;
-	int ret, env = git_env_bool("GIT_TEST_STASH_USE_BUILTIN", -1);
-
-	if (env != -1)
-		return env;
-
-	argv_array_pushl(&cp.args,
-			 "config", "--bool", "stash.usebuiltin", NULL);
-	cp.git_cmd = 1;
-	if (capture_command(&cp, &out, 6)) {
-		strbuf_release(&out);
-		return 1;
-	}
-
-	strbuf_trim(&out);
-	ret = !strcmp("true", out.buf);
-	strbuf_release(&out);
-	return ret;
-}
-
 int cmd_stash(int argc, const char **argv, const char *prefix)
 {
 	int i = -1;
@@ -1558,22 +1540,13 @@ int cmd_stash(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	if (!use_builtin_stash()) {
-		const char *path = mkpath("%s/git-legacy-stash",
-					  git_exec_path());
-
-		if (sane_execvp(path, (char **)argv) < 0)
-			die_errno(_("could not exec %s"), path);
-		else
-			BUG("sane_execvp() returned???");
-	}
-
-	prefix = setup_git_directory();
-	trace_repo_setup(prefix);
-	setup_work_tree();
-
 	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);
 
diff --git a/git-legacy-stash.sh b/git-legacy-stash.sh
deleted file mode 100755
index 4d4ebb4f2b..0000000000
--- a/git-legacy-stash.sh
+++ /dev/null
@@ -1,798 +0,0 @@
-#!/bin/sh
-# Copyright (c) 2007, Nanako Shiraishi
-
-dashless=$(basename "$0" | sed -e 's/-/ /')
-USAGE="list [<options>]
-   or: $dashless show [<stash>]
-   or: $dashless drop [-q|--quiet] [<stash>]
-   or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
-   or: $dashless branch <branchname> [<stash>]
-   or: $dashless save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
-		      [-u|--include-untracked] [-a|--all] [<message>]
-   or: $dashless [push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
-		       [-u|--include-untracked] [-a|--all] [-m <message>]
-		       [-- <pathspec>...]]
-   or: $dashless clear"
-
-SUBDIRECTORY_OK=Yes
-OPTIONS_SPEC=
-START_DIR=$(pwd)
-. git-sh-setup
-require_work_tree
-prefix=$(git rev-parse --show-prefix) || exit 1
-cd_to_toplevel
-
-TMP="$GIT_DIR/.git-stash.$$"
-TMPindex=${GIT_INDEX_FILE-"$(git rev-parse --git-path index)"}.stash.$$
-trap 'rm -f "$TMP-"* "$TMPindex"' 0
-
-ref_stash=refs/stash
-
-if git config --get-colorbool color.interactive; then
-       help_color="$(git config --get-color color.interactive.help 'red bold')"
-       reset_color="$(git config --get-color '' reset)"
-else
-       help_color=
-       reset_color=
-fi
-
-no_changes () {
-	git diff-index --quiet --cached HEAD --ignore-submodules -- "$@" &&
-	git diff-files --quiet --ignore-submodules -- "$@" &&
-	(test -z "$untracked" || test -z "$(untracked_files "$@")")
-}
-
-untracked_files () {
-	if test "$1" = "-z"
-	then
-		shift
-		z=-z
-	else
-		z=
-	fi
-	excl_opt=--exclude-standard
-	test "$untracked" = "all" && excl_opt=
-	git ls-files -o $z $excl_opt -- "$@"
-}
-
-prepare_fallback_ident () {
-	if ! git -c user.useconfigonly=yes var GIT_COMMITTER_IDENT >/dev/null 2>&1
-	then
-		GIT_AUTHOR_NAME="git stash"
-		GIT_AUTHOR_EMAIL=git@stash
-		GIT_COMMITTER_NAME="git stash"
-		GIT_COMMITTER_EMAIL=git@stash
-		export GIT_AUTHOR_NAME
-		export GIT_AUTHOR_EMAIL
-		export GIT_COMMITTER_NAME
-		export GIT_COMMITTER_EMAIL
-	fi
-}
-
-clear_stash () {
-	if test $# != 0
-	then
-		die "$(gettext "git stash clear with parameters is unimplemented")"
-	fi
-	if current=$(git rev-parse --verify --quiet $ref_stash)
-	then
-		git update-ref -d $ref_stash $current
-	fi
-}
-
-maybe_quiet () {
-	case "$1" in
-	--keep-stdout)
-		shift
-		if test -n "$GIT_QUIET"
-		then
-			"$@" 2>/dev/null
-		else
-			"$@"
-		fi
-		;;
-	*)
-		if test -n "$GIT_QUIET"
-		then
-			"$@" >/dev/null 2>&1
-		else
-			"$@"
-		fi
-		;;
-	esac
-}
-
-create_stash () {
-
-	prepare_fallback_ident
-
-	stash_msg=
-	untracked=
-	while test $# != 0
-	do
-		case "$1" in
-		-m|--message)
-			shift
-			stash_msg=${1?"BUG: create_stash () -m requires an argument"}
-			;;
-		-m*)
-			stash_msg=${1#-m}
-			;;
-		--message=*)
-			stash_msg=${1#--message=}
-			;;
-		-u|--include-untracked)
-			shift
-			untracked=${1?"BUG: create_stash () -u requires an argument"}
-			;;
-		--)
-			shift
-			break
-			;;
-		esac
-		shift
-	done
-
-	git update-index -q --refresh
-	if maybe_quiet no_changes "$@"
-	then
-		exit 0
-	fi
-
-	# state of the base commit
-	if b_commit=$(maybe_quiet --keep-stdout git rev-parse --verify HEAD)
-	then
-		head=$(git rev-list --oneline -n 1 HEAD --)
-	elif test -n "$GIT_QUIET"
-	then
-		exit 1
-	else
-		die "$(gettext "You do not have the initial commit yet")"
-	fi
-
-	if branch=$(git symbolic-ref -q HEAD)
-	then
-		branch=${branch#refs/heads/}
-	else
-		branch='(no branch)'
-	fi
-	msg=$(printf '%s: %s' "$branch" "$head")
-
-	# state of the index
-	i_tree=$(git write-tree) &&
-	i_commit=$(printf 'index on %s\n' "$msg" |
-		git commit-tree $i_tree -p $b_commit) ||
-		die "$(gettext "Cannot save the current index state")"
-
-	if test -n "$untracked"
-	then
-		# Untracked files are stored by themselves in a parentless commit, for
-		# ease of unpacking later.
-		u_commit=$(
-			untracked_files -z "$@" | (
-				GIT_INDEX_FILE="$TMPindex" &&
-				export GIT_INDEX_FILE &&
-				rm -f "$TMPindex" &&
-				git update-index -z --add --remove --stdin &&
-				u_tree=$(git write-tree) &&
-				printf 'untracked files on %s\n' "$msg" | git commit-tree $u_tree  &&
-				rm -f "$TMPindex"
-		) ) || die "$(gettext "Cannot save the untracked files")"
-
-		untracked_commit_option="-p $u_commit";
-	else
-		untracked_commit_option=
-	fi
-
-	if test -z "$patch_mode"
-	then
-
-		# state of the working tree
-		w_tree=$( (
-			git read-tree --index-output="$TMPindex" -m $i_tree &&
-			GIT_INDEX_FILE="$TMPindex" &&
-			export GIT_INDEX_FILE &&
-			git diff-index --name-only -z HEAD -- "$@" >"$TMP-stagenames" &&
-			git update-index --ignore-skip-worktree-entries \
-				-z --add --remove --stdin <"$TMP-stagenames" &&
-			git write-tree &&
-			rm -f "$TMPindex"
-		) ) ||
-			die "$(gettext "Cannot save the current worktree state")"
-
-	else
-
-		rm -f "$TMP-index" &&
-		GIT_INDEX_FILE="$TMP-index" git read-tree HEAD &&
-
-		# find out what the user wants
-		GIT_INDEX_FILE="$TMP-index" \
-			git add --legacy-stash-p -- "$@" &&
-
-		# state of the working tree
-		w_tree=$(GIT_INDEX_FILE="$TMP-index" git write-tree) ||
-		die "$(gettext "Cannot save the current worktree state")"
-
-		git diff-tree -p HEAD $w_tree -- >"$TMP-patch" &&
-		test -s "$TMP-patch" ||
-		die "$(gettext "No changes selected")"
-
-		rm -f "$TMP-index" ||
-		die "$(gettext "Cannot remove temporary index (can't happen)")"
-
-	fi
-
-	# create the stash
-	if test -z "$stash_msg"
-	then
-		stash_msg=$(printf 'WIP on %s' "$msg")
-	else
-		stash_msg=$(printf 'On %s: %s' "$branch" "$stash_msg")
-	fi
-	w_commit=$(printf '%s\n' "$stash_msg" |
-	git commit-tree $w_tree -p $b_commit -p $i_commit $untracked_commit_option) ||
-	die "$(gettext "Cannot record working tree state")"
-}
-
-store_stash () {
-	while test $# != 0
-	do
-		case "$1" in
-		-m|--message)
-			shift
-			stash_msg="$1"
-			;;
-		-m*)
-			stash_msg=${1#-m}
-			;;
-		--message=*)
-			stash_msg=${1#--message=}
-			;;
-		-q|--quiet)
-			quiet=t
-			;;
-		*)
-			break
-			;;
-		esac
-		shift
-	done
-	test $# = 1 ||
-	die "$(eval_gettext "\"$dashless store\" requires one <commit> argument")"
-
-	w_commit="$1"
-	if test -z "$stash_msg"
-	then
-		stash_msg="Created via \"git stash store\"."
-	fi
-
-	git update-ref --create-reflog -m "$stash_msg" $ref_stash $w_commit
-	ret=$?
-	test $ret != 0 && test -z "$quiet" &&
-	die "$(eval_gettext "Cannot update \$ref_stash with \$w_commit")"
-	return $ret
-}
-
-push_stash () {
-	keep_index=
-	patch_mode=
-	untracked=
-	stash_msg=
-	while test $# != 0
-	do
-		case "$1" in
-		-k|--keep-index)
-			keep_index=t
-			;;
-		--no-keep-index)
-			keep_index=n
-			;;
-		-p|--patch)
-			patch_mode=t
-			# only default to keep if we don't already have an override
-			test -z "$keep_index" && keep_index=t
-			;;
-		-q|--quiet)
-			GIT_QUIET=t
-			;;
-		-u|--include-untracked)
-			untracked=untracked
-			;;
-		-a|--all)
-			untracked=all
-			;;
-		-m|--message)
-			shift
-			test -z ${1+x} && usage
-			stash_msg=$1
-			;;
-		-m*)
-			stash_msg=${1#-m}
-			;;
-		--message=*)
-			stash_msg=${1#--message=}
-			;;
-		--help)
-			show_help
-			;;
-		--)
-			shift
-			break
-			;;
-		-*)
-			option="$1"
-			eval_gettextln "error: unknown option for 'stash push': \$option"
-			usage
-			;;
-		*)
-			break
-			;;
-		esac
-		shift
-	done
-
-	eval "set $(git rev-parse --sq --prefix "$prefix" -- "$@")"
-
-	if test -n "$patch_mode" && test -n "$untracked"
-	then
-		die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
-	fi
-
-	test -n "$untracked" || git ls-files --error-unmatch -- "$@" >/dev/null || exit 1
-
-	git update-index -q --refresh
-	if maybe_quiet no_changes "$@"
-	then
-		say "$(gettext "No local changes to save")"
-		exit 0
-	fi
-
-	git reflog exists $ref_stash ||
-		clear_stash || die "$(gettext "Cannot initialize stash")"
-
-	create_stash -m "$stash_msg" -u "$untracked" -- "$@"
-	store_stash -m "$stash_msg" -q $w_commit ||
-	die "$(gettext "Cannot save the current status")"
-	say "$(eval_gettext "Saved working directory and index state \$stash_msg")"
-
-	if test -z "$patch_mode"
-	then
-		test "$untracked" = "all" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=
-		if test -n "$untracked" && test $# = 0
-		then
-			git clean --force --quiet -d $CLEAN_X_OPTION
-		fi
-
-		if test $# != 0
-		then
-			test -z "$untracked" && UPDATE_OPTION="-u" || UPDATE_OPTION=
-			test "$untracked" = "all" && FORCE_OPTION="--force" || FORCE_OPTION=
-			git add $UPDATE_OPTION $FORCE_OPTION -- "$@"
-			git diff-index -p --cached --binary HEAD -- "$@" |
-			git apply --index -R
-		else
-			git reset --hard -q --no-recurse-submodules
-		fi
-
-		if test "$keep_index" = "t" && test -n "$i_tree"
-		then
-			git read-tree --reset $i_tree
-			git ls-files -z --modified -- "$@" |
-			git checkout-index -z --force --stdin
-		fi
-	else
-		git apply -R < "$TMP-patch" ||
-		die "$(gettext "Cannot remove worktree changes")"
-
-		if test "$keep_index" != "t"
-		then
-			git reset -q -- "$@"
-		fi
-	fi
-}
-
-save_stash () {
-	push_options=
-	while test $# != 0
-	do
-		case "$1" in
-		-q|--quiet)
-			GIT_QUIET=t
-			;;
-		--)
-			shift
-			break
-			;;
-		-*)
-			# pass all options through to push_stash
-			push_options="$push_options $1"
-			;;
-		*)
-			break
-			;;
-		esac
-		shift
-	done
-
-	stash_msg="$*"
-
-	if test -z "$stash_msg"
-	then
-		push_stash $push_options
-	else
-		push_stash $push_options -m "$stash_msg"
-	fi
-}
-
-have_stash () {
-	git rev-parse --verify --quiet $ref_stash >/dev/null
-}
-
-list_stash () {
-	have_stash || return 0
-	git log --format="%gd: %gs" -g --first-parent -m "$@" $ref_stash --
-}
-
-show_stash () {
-	ALLOW_UNKNOWN_FLAGS=t
-	assert_stash_like "$@"
-
-	if test -z "$FLAGS"
-	then
-		if test "$(git config --bool stash.showStat || echo true)" = "true"
-		then
-			FLAGS=--stat
-		fi
-
-		if test "$(git config --bool stash.showPatch || echo false)" = "true"
-		then
-			FLAGS=${FLAGS}${FLAGS:+ }-p
-		fi
-
-		if test -z "$FLAGS"
-		then
-			return 0
-		fi
-	fi
-
-	git diff ${FLAGS} $b_commit $w_commit
-}
-
-show_help () {
-	exec git help stash
-	exit 1
-}
-
-#
-# Parses the remaining options looking for flags and
-# at most one revision defaulting to ${ref_stash}@{0}
-# if none found.
-#
-# Derives related tree and commit objects from the
-# revision, if one is found.
-#
-# stash records the work tree, and is a merge between the
-# base commit (first parent) and the index tree (second parent).
-#
-#   REV is set to the symbolic version of the specified stash-like commit
-#   IS_STASH_LIKE is non-blank if ${REV} looks like a stash
-#   IS_STASH_REF is non-blank if the ${REV} looks like a stash ref
-#   s is set to the SHA1 of the stash commit
-#   w_commit is set to the commit containing the working tree
-#   b_commit is set to the base commit
-#   i_commit is set to the commit containing the index tree
-#   u_commit is set to the commit containing the untracked files tree
-#   w_tree is set to the working tree
-#   b_tree is set to the base tree
-#   i_tree is set to the index tree
-#   u_tree is set to the untracked files tree
-#
-#   GIT_QUIET is set to t if -q is specified
-#   INDEX_OPTION is set to --index if --index is specified.
-#   FLAGS is set to the remaining flags (if allowed)
-#
-# dies if:
-#   * too many revisions specified
-#   * no revision is specified and there is no stash stack
-#   * a revision is specified which cannot be resolve to a SHA1
-#   * a non-existent stash reference is specified
-#   * unknown flags were set and ALLOW_UNKNOWN_FLAGS is not "t"
-#
-
-parse_flags_and_rev()
-{
-	test "$PARSE_CACHE" = "$*" && return 0 # optimisation
-	PARSE_CACHE="$*"
-
-	IS_STASH_LIKE=
-	IS_STASH_REF=
-	INDEX_OPTION=
-	s=
-	w_commit=
-	b_commit=
-	i_commit=
-	u_commit=
-	w_tree=
-	b_tree=
-	i_tree=
-	u_tree=
-
-	FLAGS=
-	REV=
-	for opt
-	do
-		case "$opt" in
-			-q|--quiet)
-				GIT_QUIET=-t
-			;;
-			--index)
-				INDEX_OPTION=--index
-			;;
-			--help)
-				show_help
-			;;
-			-*)
-				test "$ALLOW_UNKNOWN_FLAGS" = t ||
-					die "$(eval_gettext "unknown option: \$opt")"
-				FLAGS="${FLAGS}${FLAGS:+ }$opt"
-			;;
-			*)
-				REV="${REV}${REV:+ }'$opt'"
-			;;
-		esac
-	done
-
-	eval set -- $REV
-
-	case $# in
-		0)
-			have_stash || die "$(gettext "No stash entries found.")"
-			set -- ${ref_stash}@{0}
-		;;
-		1)
-			:
-		;;
-		*)
-			die "$(eval_gettext "Too many revisions specified: \$REV")"
-		;;
-	esac
-
-	case "$1" in
-		*[!0-9]*)
-			:
-		;;
-		*)
-			set -- "${ref_stash}@{$1}"
-		;;
-	esac
-
-	REV=$(git rev-parse --symbolic --verify --quiet "$1") || {
-		reference="$1"
-		die "$(eval_gettext "\$reference is not a valid reference")"
-	}
-
-	i_commit=$(git rev-parse --verify --quiet "$REV^2") &&
-	set -- $(git rev-parse "$REV" "$REV^1" "$REV:" "$REV^1:" "$REV^2:" 2>/dev/null) &&
-	s=$1 &&
-	w_commit=$1 &&
-	b_commit=$2 &&
-	w_tree=$3 &&
-	b_tree=$4 &&
-	i_tree=$5 &&
-	IS_STASH_LIKE=t &&
-	test "$ref_stash" = "$(git rev-parse --symbolic-full-name "${REV%@*}")" &&
-	IS_STASH_REF=t
-
-	u_commit=$(git rev-parse --verify --quiet "$REV^3") &&
-	u_tree=$(git rev-parse "$REV^3:" 2>/dev/null)
-}
-
-is_stash_like()
-{
-	parse_flags_and_rev "$@"
-	test -n "$IS_STASH_LIKE"
-}
-
-assert_stash_like() {
-	is_stash_like "$@" || {
-		args="$*"
-		die "$(eval_gettext "'\$args' is not a stash-like commit")"
-	}
-}
-
-is_stash_ref() {
-	is_stash_like "$@" && test -n "$IS_STASH_REF"
-}
-
-assert_stash_ref() {
-	is_stash_ref "$@" || {
-		args="$*"
-		die "$(eval_gettext "'\$args' is not a stash reference")"
-	}
-}
-
-apply_stash () {
-
-	assert_stash_like "$@"
-
-	git update-index -q --refresh || die "$(gettext "unable to refresh index")"
-
-	# current index state
-	c_tree=$(git write-tree) ||
-		die "$(gettext "Cannot apply a stash in the middle of a merge")"
-
-	unstashed_index_tree=
-	if test -n "$INDEX_OPTION" && test "$b_tree" != "$i_tree" &&
-			test "$c_tree" != "$i_tree"
-	then
-		git diff-tree --binary $s^2^..$s^2 | git apply --cached
-		test $? -ne 0 &&
-			die "$(gettext "Conflicts in index. Try without --index.")"
-		unstashed_index_tree=$(git write-tree) ||
-			die "$(gettext "Could not save index tree")"
-		git reset
-	fi
-
-	if test -n "$u_tree"
-	then
-		GIT_INDEX_FILE="$TMPindex" git read-tree "$u_tree" &&
-		GIT_INDEX_FILE="$TMPindex" git checkout-index --all &&
-		rm -f "$TMPindex" ||
-		die "$(gettext "Could not restore untracked files from stash entry")"
-	fi
-
-	eval "
-		GITHEAD_$w_tree='Stashed changes' &&
-		GITHEAD_$c_tree='Updated upstream' &&
-		GITHEAD_$b_tree='Version stash was based on' &&
-		export GITHEAD_$w_tree GITHEAD_$c_tree GITHEAD_$b_tree
-	"
-
-	if test -n "$GIT_QUIET"
-	then
-		GIT_MERGE_VERBOSITY=0 && export GIT_MERGE_VERBOSITY
-	fi
-	if git merge-recursive $b_tree -- $c_tree $w_tree
-	then
-		# No conflict
-		if test -n "$unstashed_index_tree"
-		then
-			git read-tree "$unstashed_index_tree"
-		else
-			a="$TMP-added" &&
-			git diff-index --cached --name-only --diff-filter=A $c_tree >"$a" &&
-			git read-tree --reset $c_tree &&
-			git update-index --add --stdin <"$a" ||
-				die "$(gettext "Cannot unstage modified files")"
-			rm -f "$a"
-		fi
-		squelch=
-		if test -n "$GIT_QUIET"
-		then
-			squelch='>/dev/null 2>&1'
-		fi
-		(cd "$START_DIR" && eval "git status $squelch") || :
-	else
-		# Merge conflict; keep the exit status from merge-recursive
-		status=$?
-		git rerere
-		if test -n "$INDEX_OPTION"
-		then
-			gettextln "Index was not unstashed." >&2
-		fi
-		exit $status
-	fi
-}
-
-pop_stash() {
-	assert_stash_ref "$@"
-
-	if apply_stash "$@"
-	then
-		drop_stash "$@"
-	else
-		status=$?
-		say "$(gettext "The stash entry is kept in case you need it again.")"
-		exit $status
-	fi
-}
-
-drop_stash () {
-	assert_stash_ref "$@"
-
-	git reflog delete --updateref --rewrite "${REV}" &&
-		say "$(eval_gettext "Dropped \${REV} (\$s)")" ||
-		die "$(eval_gettext "\${REV}: Could not drop stash entry")"
-
-	# clear_stash if we just dropped the last stash entry
-	git rev-parse --verify --quiet "$ref_stash@{0}" >/dev/null ||
-	clear_stash
-}
-
-apply_to_branch () {
-	test -n "$1" || die "$(gettext "No branch name specified")"
-	branch=$1
-	shift 1
-
-	set -- --index "$@"
-	assert_stash_like "$@"
-
-	git checkout -b $branch $REV^ &&
-	apply_stash "$@" && {
-		test -z "$IS_STASH_REF" || drop_stash "$@"
-	}
-}
-
-test "$1" = "-p" && set "push" "$@"
-
-PARSE_CACHE='--not-parsed'
-# The default command is "push" if nothing but options are given
-seen_non_option=
-for opt
-do
-	case "$opt" in
-	--) break ;;
-	-*) ;;
-	*) seen_non_option=t; break ;;
-	esac
-done
-
-test -n "$seen_non_option" || set "push" "$@"
-
-# Main command set
-case "$1" in
-list)
-	shift
-	list_stash "$@"
-	;;
-show)
-	shift
-	show_stash "$@"
-	;;
-save)
-	shift
-	save_stash "$@"
-	;;
-push)
-	shift
-	push_stash "$@"
-	;;
-apply)
-	shift
-	apply_stash "$@"
-	;;
-clear)
-	shift
-	clear_stash "$@"
-	;;
-create)
-	shift
-	create_stash -m "$*" && echo "$w_commit"
-	;;
-store)
-	shift
-	store_stash "$@"
-	;;
-drop)
-	shift
-	drop_stash "$@"
-	;;
-pop)
-	shift
-	pop_stash "$@"
-	;;
-branch)
-	shift
-	apply_to_branch "$@"
-	;;
-*)
-	case $# in
-	0)
-		push_stash &&
-		say "$(gettext "(To restore them type \"git stash apply\")")"
-		;;
-	*)
-		usage
-	esac
-	;;
-esac
diff --git a/git.c b/git.c
index 7be7ad34bd..b07198fe03 100644
--- a/git.c
+++ b/git.c
@@ -574,12 +574,7 @@ static struct cmd_struct commands[] = {
 	{ "show-ref", cmd_show_ref, RUN_SETUP },
 	{ "sparse-checkout", cmd_sparse_checkout, RUN_SETUP | NEED_WORK_TREE },
 	{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
-	/*
-	 * NEEDSWORK: Until the builtin stash is thoroughly robust and no
-	 * longer needs redirection to the stash shell script this is kept as
-	 * is, then should be changed to RUN_SETUP | NEED_WORK_TREE
-	 */
-	{ "stash", cmd_stash },
+	{ "stash", cmd_stash, RUN_SETUP | NEED_WORK_TREE },
 	{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
 	{ "stripspace", cmd_stripspace },
 	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX | NO_PARSEOPT },
diff --git a/t/README b/t/README
index 9afd61e3ca..369e3a9ded 100644
--- a/t/README
+++ b/t/README
@@ -393,10 +393,6 @@ the --no-sparse command-line argument.
 GIT_TEST_PRELOAD_INDEX=<boolean> exercises the preload-index code path
 by overriding the minimum number of cache entries required per thread.
 
-GIT_TEST_STASH_USE_BUILTIN=<boolean>, when false, disables the
-built-in version of git-stash. See 'stash.useBuiltin' in
-git-config(1).
-
 GIT_TEST_ADD_I_USE_BUILTIN=<boolean>, when true, enables the
 built-in version of git add -i. See 'add.interactive.useBuiltin' in
 git-config(1).
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index ea56e85e70..b84d55a832 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1285,4 +1285,18 @@ 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_done

-- 
2.25.1.460.g2f268890c2

Junio C Hamano· Mar 3, 2020, 18:45 UTC · re: Thomas Gummerer · lore

Re: [PATCH v2 2/2] stash: remove the stash.useBuiltin setting

Thomas Gummerer <t.gummerer@gmail.com> writes:

> +	Unused configuration Variable.  Used in Git versions 2.22 to

s/Variable/variable/;

> +	2.26 as an escape hatch to enable the legacy shellscript
> +	implementation of rebase. 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.

I am a bit torn between "just document it here as a stale variable that can safely be removed from your configuration, and do not warn" and what you did here. Even if we warn for a few releases, this entry cannot be removed from the documentation for a lot longer time, if ever, anyway. It's not like having an extra entry that is unused in the configuration file would hurt users, anyway, and it is not obvious that the warning helps more than it disrupts them by bugging users to remove it from their configuration file.

Thomas Gummerer· Mar 4, 2020, 11:49 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 2/2] stash: remove the stash.useBuiltin setting

On 03/03, Junio C Hamano wrote:

Show 5 quoted lines

> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > +	Unused configuration Variable.  Used in Git versions 2.22 to
> 
> s/Variable/variable/;

Whoops. Do you want me to re-send an updated version, or could you fix this up locally?

Show 13 quoted lines

> > +	2.26 as an escape hatch to enable the legacy shellscript
> > +	implementation of rebase. 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.
> 
> I am a bit torn between "just document it here as a stale variable
> that can safely be removed from your configuration, and do not warn"
> and what you did here.  Even if we warn for a few releases, this
> entry cannot be removed from the documentation for a lot longer
> time, if ever, anyway.  It's not like having an extra entry that is
> unused in the configuration file would hurt users, anyway, and it is
> not obvious that the warning helps more than it disrupts them by
> bugging users to remove it from their configuration file.

Hmm I'm thinking of the warning mostly as preventing the "I have legacy stash enabled, why do I see a new bug all of a sudden?" question, as opposed to it helping removing this from the docs. I suspect we have very few people in the wild that did enable legacy stash, but if they did it might be better for them to know about the config variable not having any effect anymore.

FWIW we went with this same approach with removing the 'rebase.useBuiltin' config, and I don't think we heard any complaints about it in that transition.

Junio C Hamano· Mar 4, 2020, 16:25 UTC · re: Thomas Gummerer · lore

Re: [PATCH v2 2/2] stash: remove the stash.useBuiltin setting

Thomas Gummerer <t.gummerer@gmail.com> writes:

Show 9 quoted lines

> On 03/03, Junio C Hamano wrote:
>> Thomas Gummerer <t.gummerer@gmail.com> writes:
>> 
>> > +	Unused configuration Variable.  Used in Git versions 2.22 to
>> 
>> s/Variable/variable/;
>
> Whoops.  Do you want me to re-send an updated version, or could you
> fix this up locally?

After reading your response to the end, it appears that there is no other changes needed, so let me do the honors ;-)

Show 12 quoted lines

>> I am a bit torn between "just document it here as a stale variable
>> that can safely be removed from your configuration, and do not warn"
>> and what you did here.  Even if we warn for a few releases, this
>> entry cannot be removed from the documentation for a lot longer
>> time, if ever, anyway.  It's not like having an extra entry that is
>> unused in the configuration file would hurt users, anyway, and it is
>> not obvious that the warning helps more than it disrupts them by
>> bugging users to remove it from their configuration file.
>
> Hmm I'm thinking of the warning mostly as preventing the "I have
> legacy stash enabled, why do I see a new bug all of a sudden?"
> question, as opposed to it helping removing this from the docs.

OK.  Thanks.

Thomas Gummerer· Mar 5, 2020, 17:04 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 2/2] stash: remove the stash.useBuiltin setting

On 03/04, Junio C Hamano wrote:

Show 14 quoted lines

> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > On 03/03, Junio C Hamano wrote:
> >> Thomas Gummerer <t.gummerer@gmail.com> writes:
> >> 
> >> > +	Unused configuration Variable.  Used in Git versions 2.22 to
> >> 
> >> s/Variable/variable/;
> >
> > Whoops.  Do you want me to re-send an updated version, or could you
> > fix this up locally?
> 
> After reading your response to the end, it appears that there is no
> other changes needed, so let me do the honors ;-)

Thanks!

Son Luong Ngoc· Mar 1, 2020, 09:59 UTC · lore

Re: [External] Git Rebase: test failing with GIT_TEST_STASH_USE_BUILTIN=false

(this is a resent due to git@vger.kernel.org blocked HTML content, sry for the noises) (following up on https://public-inbox.org/git/xmqq36ayob9a.fsf@gitster-ct.c.googlers.com/T/#t )

Hi folks,

I ran a simple git-bisect on this to try to figure-out whats wrong:

$ cat run-test.sh #!/bin/bash

make -j8 prefix=/usr all
(
  cd ./t
  GIT_TEST_STASH_USE_BUILTIN=false ./t3903-stash.sh --run='103'
)
$ git bisect start master v2.22.2
$ git bisect run ./run-test.sh
...
$ git bisect log
(b932f6a5e8...)|BISECTING ~/work/booking/git/git> git bisect log
# bad: [2d2118b814c11f509e1aa76cb07110f7231668dc] The seventh batch for 2.26
# good: [d9589d4051537c387b70dc76e430c61b4c85a86d] Git 2.22.2
git bisect start 'HEAD' 'v2.22.2'
# bad: [22dd22dce050f042b3eec165440966186691db42] Merge branch 'wb/fsmonitor-bitmap-fix'
git bisect bad 22dd22dce050f042b3eec165440966186691db42
# bad: [fa9e7934c780bc804a09bfc88a93825096b3155e] Merge branch 'bm/repository-layout-typofix'
git bisect bad fa9e7934c780bc804a09bfc88a93825096b3155e
# good: [3a94cb31d52f061c315b00bfc005f1b1c42ac92d] bin-wrappers: append `.exe` to target paths if necessary
git bisect good 3a94cb31d52f061c315b00bfc005f1b1c42ac92d
# bad: [7b70d46ca410f9d37045558329c3143570d47ba6] Merge branch 'bb/grep-pcre2-bug-message-fix'
git bisect bad 7b70d46ca410f9d37045558329c3143570d47ba6
# good: [d60dc1a0b3829f3c4d69696f43f1c178c0701cdb] Merge branch 'ew/repack-with-bitmaps-by-default'
git bisect good d60dc1a0b3829f3c4d69696f43f1c178c0701cdb
# good: [43ba21cb574ee3f9a1acf4580868982f4c883ac6] Merge branch 'tg/range-diff-output-update'
git bisect good 43ba21cb574ee3f9a1acf4580868982f4c883ac6
# good: [080af915a3ee4d9511dc288b29143b9958ac0adc] Merge branch 'mt/dir-iterator-updates'
git bisect good 080af915a3ee4d9511dc288b29143b9958ac0adc
# bad: [75ce48674889df6a2bb493fb5d6bef0ef60ca7ae] Merge branch 'di/readme-markup-fix'
git bisect bad 75ce48674889df6a2bb493fb5d6bef0ef60ca7ae
# good: [984da7f8d2589b53cca7c920e597eab30d4c1b36] Merge branch 'sr/gpg-interface-stop-at-the-end'
git bisect good 984da7f8d2589b53cca7c920e597eab30d4c1b36
# bad: [f8aee8576ac5e01fa993c80b5b888af214c03758] Merge branch 'tg/stash-keep-index-with-removed-paths'
git bisect bad f8aee8576ac5e01fa993c80b5b888af214c03758
# good: [b932f6a5e8cdbb33eff4563fdfb1eae9ebf70a65] stash: fix handling removed files with --keep-index
git bisect good b932f6a5e8cdbb33eff4563fdfb1eae9ebf70a65
# first bad commit: [f8aee8576ac5e01fa993c80b5b888af214c03758] Merge branch 'tg/stash-keep-index-with-removed-paths'

Which pinned point the failure starting from the moment the test was added at https://github.com/git/git/commit/b932f6a5e8cdbb33eff4563fdfb1eae9ebf70a65 by @t.gummerer

I would appreciate if we can either deprecate the GIT_TEST_STASH_USE_BUILTIN flag entirely or wrap the test with an 'if' so that we auto skip it when the flag is enabled.

Thanks, Son Luong.

Show 27 quoted lines

> On Feb 25, 2020, at 17:57, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Son Luong Ngoc <son.luong@booking.com> writes:
> 
>> I have been trying to build git from source and noticing that some
>> tests have been failing since 2.25 with the flag
>> "GIT_TEST_STASH_USE_BUILTIN=false"
>> 
>> I think in 2.25 t3903.103 started to fail (rebase related) and
>> current master t3904 may be failing also.
>> 
>> Is "GIT_TEST_STASH_USE_BUILTIN=false" is still being tested with
>> or are we totally deprecating this flag?
> 
> In the longer term, when "git stash" gains new features that did not
> exist in the original scripted version, tests that observe how these
> features work would start failing when using the scripted version.
> 
> I picked some people from "git shortlog --no-merges builtin/stash.c"
> and placed them on the CC line---perhaps they may know more.  It
> happens that Johannes is also familiar with "rebase", which you
> said is involved in the test failure, so I'd imagine he would be the
> best person to ask.
> 
> Thanks for a report.
> 
>

Son Luong Ngoc· Mar 1, 2020, 10:40 UTC · re: Son Luong Ngoc · lore

Re: [External] Git Rebase: test failing with GIT_TEST_STASH_USE_BUILTIN=false

Similar strategy was used for t.3904, which has been failing with this flag since 2.25.0

$ cat run-test.sh #!/bin/bash

make -j8 prefix=/usr all
(
  cd ./t && GIT_TEST_STASH_USE_BUILTIN=false ./t3904-stash-patch.sh
)

$ git bisect log # bad: [2d2118b814c11f509e1aa76cb07110f7231668dc] The seventh batch for 2.26 # good: [d9589d4051537c387b70dc76e430c61b4c85a86d] Git 2.22.2 git bisect start 'HEAD' 'v2.22.2' # good: [22dd22dce050f042b3eec165440966186691db42] Merge branch 'wb/fsmonitor-bitmap-fix' git bisect good 22dd22dce050f042b3eec165440966186691db42 # good: [6514ad40a1a3cf80b2c25e3318dbf0252599fb8d] Merge branch 'ra/t5150-depends-on-perl' git bisect good 6514ad40a1a3cf80b2c25e3318dbf0252599fb8d # bad: [f52ab33616ee1d241f2292f1c1e47ba84a263523] Merge branch 'bc/hash-independent-tests-part-7' git bisect bad f52ab33616ee1d241f2292f1c1e47ba84a263523 # good: [4d924528d8bfe947abfc54ee9bd3892ab509c8cd] Revert "Merge branch 'ra/rebase-i-more-options'" git bisect good 4d924528d8bfe947abfc54ee9bd3892ab509c8cd # good: [f0940743facd619f251009e0307d8d6452cc582e] Merge branch 'js/builtin-add-i-cmds' git bisect good f0940743facd619f251009e0307d8d6452cc582e # good: [381e8e9de142b636e4a25b6df113d70168e21a34] Merge branch 'dl/test-must-fail-fixes' git bisect good 381e8e9de142b636e4a25b6df113d70168e21a34 # bad: [d0e70cd32e95df3be2250536f9089c858a298874] Merge branch 'am/checkout-file-and-ref-ref-ambiguity' git bisect bad d0e70cd32e95df3be2250536f9089c858a298874 # bad: [94ac3c31f730ab278e1373a942fb4503829f4279] terminal: make the code of disable_echo() reusable git bisect bad 94ac3c31f730ab278e1373a942fb4503829f4279 # bad: [52628f94fc35f57f0b3c54e4f849e490bfa44449] built-in add -p: implement the "checkout" patch modes git bisect bad 52628f94fc35f57f0b3c54e4f849e490bfa44449 # good: [36bae1dc0ee777aa529dd955f2e619281265f262] built-in add -p: implement the "stash" and "reset" patch modes git bisect good 36bae1dc0ee777aa529dd955f2e619281265f262 # bad: [6610e4628ac12396efc20201fe85d67591bed247] built-in stash: use the built-in `git add -p` if so configured git bisect bad 6610e4628ac12396efc20201fe85d67591bed247 # bad: [90a6bb98d11a664f729dbb86c90d9c7a38ea825a] legacy stash -p: respect the add.interactive.usebuiltin setting git bisect bad 90a6bb98d11a664f729dbb86c90d9c7a38ea825a # first bad commit: [90a6bb98d11a664f729dbb86c90d9c7a38ea825a] legacy stash -p: respect the add.interactive.usebuiltin setting

Which was merged in https://github.com/gitgitgadget/git/commit/9a5315edfdf662c4d9bf444ebc297bc802fa5e04 Author was Johannes Schindelin

Thanks, Son Luong.

Show 85 quoted lines

> On Mar 1, 2020, at 10:59, Son Luong Ngoc <sluongng@gmail.com> wrote:
> 
> (this is a resent due to git@vger.kernel.org blocked HTML content, sry for the noises)
> (following up on https://public-inbox.org/git/xmqq36ayob9a.fsf@gitster-ct.c.googlers.com/T/#t )
> 
> Hi folks,
> 
> I ran a simple git-bisect on this to try to figure-out whats wrong:
> 
> $ cat run-test.sh
> #!/bin/bash
> 
> make -j8 prefix=/usr all
> (
>  cd ./t
>  GIT_TEST_STASH_USE_BUILTIN=false ./t3903-stash.sh --run='103'
> )
> $ git bisect start master v2.22.2
> $ git bisect run ./run-test.sh
> ...
> $ git bisect log
> (b932f6a5e8...)|BISECTING ~/work/booking/git/git> git bisect log
> # bad: [2d2118b814c11f509e1aa76cb07110f7231668dc] The seventh batch for 2.26
> # good: [d9589d4051537c387b70dc76e430c61b4c85a86d] Git 2.22.2
> git bisect start 'HEAD' 'v2.22.2'
> # bad: [22dd22dce050f042b3eec165440966186691db42] Merge branch 'wb/fsmonitor-bitmap-fix'
> git bisect bad 22dd22dce050f042b3eec165440966186691db42
> # bad: [fa9e7934c780bc804a09bfc88a93825096b3155e] Merge branch 'bm/repository-layout-typofix'
> git bisect bad fa9e7934c780bc804a09bfc88a93825096b3155e
> # good: [3a94cb31d52f061c315b00bfc005f1b1c42ac92d] bin-wrappers: append `.exe` to target paths if necessary
> git bisect good 3a94cb31d52f061c315b00bfc005f1b1c42ac92d
> # bad: [7b70d46ca410f9d37045558329c3143570d47ba6] Merge branch 'bb/grep-pcre2-bug-message-fix'
> git bisect bad 7b70d46ca410f9d37045558329c3143570d47ba6
> # good: [d60dc1a0b3829f3c4d69696f43f1c178c0701cdb] Merge branch 'ew/repack-with-bitmaps-by-default'
> git bisect good d60dc1a0b3829f3c4d69696f43f1c178c0701cdb
> # good: [43ba21cb574ee3f9a1acf4580868982f4c883ac6] Merge branch 'tg/range-diff-output-update'
> git bisect good 43ba21cb574ee3f9a1acf4580868982f4c883ac6
> # good: [080af915a3ee4d9511dc288b29143b9958ac0adc] Merge branch 'mt/dir-iterator-updates'
> git bisect good 080af915a3ee4d9511dc288b29143b9958ac0adc
> # bad: [75ce48674889df6a2bb493fb5d6bef0ef60ca7ae] Merge branch 'di/readme-markup-fix'
> git bisect bad 75ce48674889df6a2bb493fb5d6bef0ef60ca7ae
> # good: [984da7f8d2589b53cca7c920e597eab30d4c1b36] Merge branch 'sr/gpg-interface-stop-at-the-end'
> git bisect good 984da7f8d2589b53cca7c920e597eab30d4c1b36
> # bad: [f8aee8576ac5e01fa993c80b5b888af214c03758] Merge branch 'tg/stash-keep-index-with-removed-paths'
> git bisect bad f8aee8576ac5e01fa993c80b5b888af214c03758
> # good: [b932f6a5e8cdbb33eff4563fdfb1eae9ebf70a65] stash: fix handling removed files with --keep-index
> git bisect good b932f6a5e8cdbb33eff4563fdfb1eae9ebf70a65
> # first bad commit: [f8aee8576ac5e01fa993c80b5b888af214c03758] Merge branch 'tg/stash-keep-index-with-removed-paths'
> 
> Which pinned point the failure starting from the moment the test was added at https://github.com/git/git/commit/b932f6a5e8cdbb33eff4563fdfb1eae9ebf70a65 by @t.gummerer
> 
> I would appreciate if we can either deprecate the GIT_TEST_STASH_USE_BUILTIN flag entirely or wrap the test with an 'if' so that we auto skip it when the flag is enabled.
> 
> Thanks,
> Son Luong.
> 
>> On Feb 25, 2020, at 17:57, Junio C Hamano <gitster@pobox.com> wrote:
>> 
>> Son Luong Ngoc <son.luong@booking.com> writes:
>> 
>>> I have been trying to build git from source and noticing that some
>>> tests have been failing since 2.25 with the flag
>>> "GIT_TEST_STASH_USE_BUILTIN=false"
>>> 
>>> I think in 2.25 t3903.103 started to fail (rebase related) and
>>> current master t3904 may be failing also.
>>> 
>>> Is "GIT_TEST_STASH_USE_BUILTIN=false" is still being tested with
>>> or are we totally deprecating this flag?
>> 
>> In the longer term, when "git stash" gains new features that did not
>> exist in the original scripted version, tests that observe how these
>> features work would start failing when using the scripted version.
>> 
>> I picked some people from "git shortlog --no-merges builtin/stash.c"
>> and placed them on the CC line---perhaps they may know more.  It
>> happens that Johannes is also familiar with "rebase", which you
>> said is involved in the test failure, so I'd imagine he would be the
>> best person to ask.
>> 
>> Thanks for a report.
>> 
>> 
> 
>

Johannes Schindelin· Mar 6, 2020, 14:11 UTC · lore

Re: [PATCH 2/2] stash: remove the stash.useBuiltin setting

Hi Jacob,

On Thu, 5 Mar 2020, Jacob Keller wrote:

> Sorry this is on phone and might not hit the list since Gmail forces
> html.... but the docs say git-rebase when you meant git-stash.
>
> I didn't want to forget this before I got to a desktop

Good point. You mean this part, right?

Show 25 quoted lines

> On Mon, Mar 2, 2020, 10:20 AM Thomas Gummerer <t.gummerer@gmail.com> wrote:
>
> > diff --git a/Documentation/config/stash.txt
> > b/Documentation/config/stash.txt
> > index abc7ef4a3a..0f3db821f6 100644
> > --- a/Documentation/config/stash.txt
> > +++ b/Documentation/config/stash.txt
> > @@ -1,17 +1,9 @@
> >  stash.useBuiltin::
> > -       Set to `false` to use the legacy shell script implementation of
> > -       linkgit:git-stash[1]. Is `true` by default, which means use
> > -       the built-in rewrite of it in C.
> > -+
> > -The C rewrite is first included with Git version 2.22 (and Git for Windows
> > -version 2.19). This option serves as an escape hatch to re-enable the
> > -legacy version in case any bugs are found in the rewrite. This option and
> > -the shell script version of linkgit:git-stash[1] will be removed in some
> > -future release.
> > -+
> > -If you find some reason to set this option to `false`, other than
> > -one-off testing, you should report the behavior difference as a bug in
> > -Git (see https://git-scm.com/community for details).
> > +       Unused configuration Variable.  Used in Git versions 2.22 to
> > +       2.26 as an escape hatch to enable the legacy shellscript
> > +       implementation of rebase. Now the built-in rewrite of it in C

                              ^^^^^^

If so, I agree, that should be s/rebase/stash/.

Thanks for paying apparently more attention than I did! Dscho

Show 5 quoted lines

> > +       is always used. Setting this will emit a warning, to alert any
> > +       remaining users that setting this now does nothing.
> >
> >  stash.showPatch::
> >         If this is set to true, the `git stash show` command without an

Thomas Gummerer· Mar 6, 2020, 17:29 UTC · lore

Re: [PATCH 2/2] stash: remove the stash.useBuiltin setting

On 03/05, Jacob Keller wrote:

> Sorry this is on phone and might not hit the list since Gmail forces
> html.... but the docs say git-rebase when you meant git-stash.

Embarrassing, thanks for noticing it!

Show 1063 quoted lines

> I didn't want to forget this before I got to a desktop
> 
> On Mon, Mar 2, 2020, 10:20 AM Thomas Gummerer <t.gummerer@gmail.com> wrote:
> 
> > Remove the stash.useBuiltin setting which was added as an escape hatch
> > to disable the builtin version of stash first released with Git 2.22.
> >
> > Carrying the legacy version is a maintenance burden, and has in fact
> > become out of date failing a test since the 2.23 release, without
> > anyone noticing until now.  So users would be getting a hint to fall
> > back to a potentially buggy version of the tool.
> >
> > We used to shell out to git config to get the useBuiltin configuration
> > to avoid changing any global state before spawning legacy-stash.
> > However that is no longer necessary, so just use the 'git_config'
> > function to get the setting instead.
> >
> > Similar to what we've done in d03ebd411c ("rebase: remove the
> > rebase.useBuiltin setting", 2019-03-18), where we remove the
> > corresponding setting for rebase, we leave the documentation in place,
> > so people can refer back to it when searching for it online, and so we
> > can refer to it in the commit message.
> >
> > Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> > ---
> >  .gitignore                     |   1 -
> >  Documentation/config/stash.txt |  18 +-
> >  Makefile                       |   1 -
> >  builtin/stash.c                |  44 +-
> >  git-legacy-stash.sh            | 798 ---------------------------------
> >  t/README                       |   4 -
> >  t/t3903-stash.sh               |  14 +
> >  7 files changed, 30 insertions(+), 850 deletions(-)
> >  delete mode 100755 git-legacy-stash.sh
> >
> > diff --git a/.gitignore b/.gitignore
> > index aebe7c0908..188bd1c3de 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -83,7 +83,6 @@
> >  /git-init-db
> >  /git-interpret-trailers
> >  /git-instaweb
> > -/git-legacy-stash
> >  /git-log
> >  /git-ls-files
> >  /git-ls-remote
> > diff --git a/Documentation/config/stash.txt
> > b/Documentation/config/stash.txt
> > index abc7ef4a3a..0f3db821f6 100644
> > --- a/Documentation/config/stash.txt
> > +++ b/Documentation/config/stash.txt
> > @@ -1,17 +1,9 @@
> >  stash.useBuiltin::
> > -       Set to `false` to use the legacy shell script implementation of
> > -       linkgit:git-stash[1]. Is `true` by default, which means use
> > -       the built-in rewrite of it in C.
> > -+
> > -The C rewrite is first included with Git version 2.22 (and Git for Windows
> > -version 2.19). This option serves as an escape hatch to re-enable the
> > -legacy version in case any bugs are found in the rewrite. This option and
> > -the shell script version of linkgit:git-stash[1] will be removed in some
> > -future release.
> > -+
> > -If you find some reason to set this option to `false`, other than
> > -one-off testing, you should report the behavior difference as a bug in
> > -Git (see https://git-scm.com/community for details).
> > +       Unused configuration Variable.  Used in Git versions 2.22 to
> > +       2.26 as an escape hatch to enable the legacy shellscript
> > +       implementation of rebase. 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.showPatch::
> >         If this is set to true, the `git stash show` command without an
> > diff --git a/Makefile b/Makefile
> > index 9804a0758b..096c6d1fbb 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -609,7 +609,6 @@ SCRIPT_SH += git-merge-one-file.sh
> >  SCRIPT_SH += git-merge-resolve.sh
> >  SCRIPT_SH += git-mergetool.sh
> >  SCRIPT_SH += git-quiltimport.sh
> > -SCRIPT_SH += git-legacy-stash.sh
> >  SCRIPT_SH += git-request-pull.sh
> >  SCRIPT_SH += git-submodule.sh
> >  SCRIPT_SH += git-web--browse.sh
> > diff --git a/builtin/stash.c b/builtin/stash.c
> > index f371db270c..5f4bd693df 100644
> > --- a/builtin/stash.c
> > +++ b/builtin/stash.c
> > @@ -701,6 +701,7 @@ static int list_stash(int argc, const char **argv,
> > const char *prefix)
> >
> >  static int show_stat = 1;
> >  static int show_patch;
> > +static int use_legacy_stash;
> >
> >  static int git_stash_config(const char *var, const char *value, void *cb)
> >  {
> > @@ -712,6 +713,10 @@ static int git_stash_config(const char *var, const
> > char *value, void *cb)
> >                 show_patch = 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);
> >  }
> >
> > @@ -1524,29 +1529,6 @@ static int save_stash(int argc, const char **argv,
> > const char *prefix)
> >         return ret;
> >  }
> >
> > -static int use_builtin_stash(void)
> > -{
> > -       struct child_process cp = CHILD_PROCESS_INIT;
> > -       struct strbuf out = STRBUF_INIT;
> > -       int ret, env = git_env_bool("GIT_TEST_STASH_USE_BUILTIN", -1);
> > -
> > -       if (env != -1)
> > -               return env;
> > -
> > -       argv_array_pushl(&cp.args,
> > -                        "config", "--bool", "stash.usebuiltin", NULL);
> > -       cp.git_cmd = 1;
> > -       if (capture_command(&cp, &out, 6)) {
> > -               strbuf_release(&out);
> > -               return 1;
> > -       }
> > -
> > -       strbuf_trim(&out);
> > -       ret = !strcmp("true", out.buf);
> > -       strbuf_release(&out);
> > -       return ret;
> > -}
> > -
> >  int cmd_stash(int argc, const char **argv, const char *prefix)
> >  {
> >         int i = -1;
> > @@ -1558,22 +1540,18 @@ int cmd_stash(int argc, const char **argv, const
> > char *prefix)
> >                 OPT_END()
> >         };
> >
> > -       if (!use_builtin_stash()) {
> > -               const char *path = mkpath("%s/git-legacy-stash",
> > -                                         git_exec_path());
> > -
> > -               if (sane_execvp(path, (char **)argv) < 0)
> > -                       die_errno(_("could not exec %s"), path);
> > -               else
> > -                       BUG("sane_execvp() returned???");
> > -       }
> > -
> >         prefix = setup_git_directory();
> >         trace_repo_setup(prefix);
> >         setup_work_tree();
> >
> >         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);
> >
> > diff --git a/git-legacy-stash.sh b/git-legacy-stash.sh
> > deleted file mode 100755
> > index 4d4ebb4f2b..0000000000
> > --- a/git-legacy-stash.sh
> > +++ /dev/null
> > @@ -1,798 +0,0 @@
> > -#!/bin/sh
> > -# Copyright (c) 2007, Nanako Shiraishi
> > -
> > -dashless=$(basename "$0" | sed -e 's/-/ /')
> > -USAGE="list [<options>]
> > -   or: $dashless show [<stash>]
> > -   or: $dashless drop [-q|--quiet] [<stash>]
> > -   or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]
> > -   or: $dashless branch <branchname> [<stash>]
> > -   or: $dashless save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
> > -                     [-u|--include-untracked] [-a|--all] [<message>]
> > -   or: $dashless [push [--patch] [-k|--[no-]keep-index] [-q|--quiet]
> > -                      [-u|--include-untracked] [-a|--all] [-m <message>]
> > -                      [-- <pathspec>...]]
> > -   or: $dashless clear"
> > -
> > -SUBDIRECTORY_OK=Yes
> > -OPTIONS_SPEC=
> > -START_DIR=$(pwd)
> > -. git-sh-setup
> > -require_work_tree
> > -prefix=$(git rev-parse --show-prefix) || exit 1
> > -cd_to_toplevel
> > -
> > -TMP="$GIT_DIR/.git-stash.$$"
> > -TMPindex=${GIT_INDEX_FILE-"$(git rev-parse --git-path index)"}.stash.$$
> > -trap 'rm -f "$TMP-"* "$TMPindex"' 0
> > -
> > -ref_stash=refs/stash
> > -
> > -if git config --get-colorbool color.interactive; then
> > -       help_color="$(git config --get-color color.interactive.help 'red
> > bold')"
> > -       reset_color="$(git config --get-color '' reset)"
> > -else
> > -       help_color=
> > -       reset_color=
> > -fi
> > -
> > -no_changes () {
> > -       git diff-index --quiet --cached HEAD --ignore-submodules -- "$@" &&
> > -       git diff-files --quiet --ignore-submodules -- "$@" &&
> > -       (test -z "$untracked" || test -z "$(untracked_files "$@")")
> > -}
> > -
> > -untracked_files () {
> > -       if test "$1" = "-z"
> > -       then
> > -               shift
> > -               z=-z
> > -       else
> > -               z=
> > -       fi
> > -       excl_opt=--exclude-standard
> > -       test "$untracked" = "all" && excl_opt=
> > -       git ls-files -o $z $excl_opt -- "$@"
> > -}
> > -
> > -prepare_fallback_ident () {
> > -       if ! git -c user.useconfigonly=yes var GIT_COMMITTER_IDENT
> > >/dev/null 2>&1
> > -       then
> > -               GIT_AUTHOR_NAME="git stash"
> > -               GIT_AUTHOR_EMAIL=git@stash
> > -               GIT_COMMITTER_NAME="git stash"
> > -               GIT_COMMITTER_EMAIL=git@stash
> > -               export GIT_AUTHOR_NAME
> > -               export GIT_AUTHOR_EMAIL
> > -               export GIT_COMMITTER_NAME
> > -               export GIT_COMMITTER_EMAIL
> > -       fi
> > -}
> > -
> > -clear_stash () {
> > -       if test $# != 0
> > -       then
> > -               die "$(gettext "git stash clear with parameters is
> > unimplemented")"
> > -       fi
> > -       if current=$(git rev-parse --verify --quiet $ref_stash)
> > -       then
> > -               git update-ref -d $ref_stash $current
> > -       fi
> > -}
> > -
> > -maybe_quiet () {
> > -       case "$1" in
> > -       --keep-stdout)
> > -               shift
> > -               if test -n "$GIT_QUIET"
> > -               then
> > -                       "$@" 2>/dev/null
> > -               else
> > -                       "$@"
> > -               fi
> > -               ;;
> > -       *)
> > -               if test -n "$GIT_QUIET"
> > -               then
> > -                       "$@" >/dev/null 2>&1
> > -               else
> > -                       "$@"
> > -               fi
> > -               ;;
> > -       esac
> > -}
> > -
> > -create_stash () {
> > -
> > -       prepare_fallback_ident
> > -
> > -       stash_msg=
> > -       untracked=
> > -       while test $# != 0
> > -       do
> > -               case "$1" in
> > -               -m|--message)
> > -                       shift
> > -                       stash_msg=${1?"BUG: create_stash () -m requires an
> > argument"}
> > -                       ;;
> > -               -m*)
> > -                       stash_msg=${1#-m}
> > -                       ;;
> > -               --message=*)
> > -                       stash_msg=${1#--message=}
> > -                       ;;
> > -               -u|--include-untracked)
> > -                       shift
> > -                       untracked=${1?"BUG: create_stash () -u requires an
> > argument"}
> > -                       ;;
> > -               --)
> > -                       shift
> > -                       break
> > -                       ;;
> > -               esac
> > -               shift
> > -       done
> > -
> > -       git update-index -q --refresh
> > -       if maybe_quiet no_changes "$@"
> > -       then
> > -               exit 0
> > -       fi
> > -
> > -       # state of the base commit
> > -       if b_commit=$(maybe_quiet --keep-stdout git rev-parse --verify
> > HEAD)
> > -       then
> > -               head=$(git rev-list --oneline -n 1 HEAD --)
> > -       elif test -n "$GIT_QUIET"
> > -       then
> > -               exit 1
> > -       else
> > -               die "$(gettext "You do not have the initial commit yet")"
> > -       fi
> > -
> > -       if branch=$(git symbolic-ref -q HEAD)
> > -       then
> > -               branch=${branch#refs/heads/}
> > -       else
> > -               branch='(no branch)'
> > -       fi
> > -       msg=$(printf '%s: %s' "$branch" "$head")
> > -
> > -       # state of the index
> > -       i_tree=$(git write-tree) &&
> > -       i_commit=$(printf 'index on %s\n' "$msg" |
> > -               git commit-tree $i_tree -p $b_commit) ||
> > -               die "$(gettext "Cannot save the current index state")"
> > -
> > -       if test -n "$untracked"
> > -       then
> > -               # Untracked files are stored by themselves in a parentless
> > commit, for
> > -               # ease of unpacking later.
> > -               u_commit=$(
> > -                       untracked_files -z "$@" | (
> > -                               GIT_INDEX_FILE="$TMPindex" &&
> > -                               export GIT_INDEX_FILE &&
> > -                               rm -f "$TMPindex" &&
> > -                               git update-index -z --add --remove --stdin
> > &&
> > -                               u_tree=$(git write-tree) &&
> > -                               printf 'untracked files on %s\n' "$msg" |
> > git commit-tree $u_tree  &&
> > -                               rm -f "$TMPindex"
> > -               ) ) || die "$(gettext "Cannot save the untracked files")"
> > -
> > -               untracked_commit_option="-p $u_commit";
> > -       else
> > -               untracked_commit_option=
> > -       fi
> > -
> > -       if test -z "$patch_mode"
> > -       then
> > -
> > -               # state of the working tree
> > -               w_tree=$( (
> > -                       git read-tree --index-output="$TMPindex" -m
> > $i_tree &&
> > -                       GIT_INDEX_FILE="$TMPindex" &&
> > -                       export GIT_INDEX_FILE &&
> > -                       git diff-index --name-only -z HEAD -- "$@"
> > >"$TMP-stagenames" &&
> > -                       git update-index --ignore-skip-worktree-entries \
> > -                               -z --add --remove --stdin
> > <"$TMP-stagenames" &&
> > -                       git write-tree &&
> > -                       rm -f "$TMPindex"
> > -               ) ) ||
> > -                       die "$(gettext "Cannot save the current worktree
> > state")"
> > -
> > -       else
> > -
> > -               rm -f "$TMP-index" &&
> > -               GIT_INDEX_FILE="$TMP-index" git read-tree HEAD &&
> > -
> > -               # find out what the user wants
> > -               GIT_INDEX_FILE="$TMP-index" \
> > -                       git add --legacy-stash-p -- "$@" &&
> > -
> > -               # state of the working tree
> > -               w_tree=$(GIT_INDEX_FILE="$TMP-index" git write-tree) ||
> > -               die "$(gettext "Cannot save the current worktree state")"
> > -
> > -               git diff-tree -p HEAD $w_tree -- >"$TMP-patch" &&
> > -               test -s "$TMP-patch" ||
> > -               die "$(gettext "No changes selected")"
> > -
> > -               rm -f "$TMP-index" ||
> > -               die "$(gettext "Cannot remove temporary index (can't
> > happen)")"
> > -
> > -       fi
> > -
> > -       # create the stash
> > -       if test -z "$stash_msg"
> > -       then
> > -               stash_msg=$(printf 'WIP on %s' "$msg")
> > -       else
> > -               stash_msg=$(printf 'On %s: %s' "$branch" "$stash_msg")
> > -       fi
> > -       w_commit=$(printf '%s\n' "$stash_msg" |
> > -       git commit-tree $w_tree -p $b_commit -p $i_commit
> > $untracked_commit_option) ||
> > -       die "$(gettext "Cannot record working tree state")"
> > -}
> > -
> > -store_stash () {
> > -       while test $# != 0
> > -       do
> > -               case "$1" in
> > -               -m|--message)
> > -                       shift
> > -                       stash_msg="$1"
> > -                       ;;
> > -               -m*)
> > -                       stash_msg=${1#-m}
> > -                       ;;
> > -               --message=*)
> > -                       stash_msg=${1#--message=}
> > -                       ;;
> > -               -q|--quiet)
> > -                       quiet=t
> > -                       ;;
> > -               *)
> > -                       break
> > -                       ;;
> > -               esac
> > -               shift
> > -       done
> > -       test $# = 1 ||
> > -       die "$(eval_gettext "\"$dashless store\" requires one <commit>
> > argument")"
> > -
> > -       w_commit="$1"
> > -       if test -z "$stash_msg"
> > -       then
> > -               stash_msg="Created via \"git stash store\"."
> > -       fi
> > -
> > -       git update-ref --create-reflog -m "$stash_msg" $ref_stash $w_commit
> > -       ret=$?
> > -       test $ret != 0 && test -z "$quiet" &&
> > -       die "$(eval_gettext "Cannot update \$ref_stash with \$w_commit")"
> > -       return $ret
> > -}
> > -
> > -push_stash () {
> > -       keep_index=
> > -       patch_mode=
> > -       untracked=
> > -       stash_msg=
> > -       while test $# != 0
> > -       do
> > -               case "$1" in
> > -               -k|--keep-index)
> > -                       keep_index=t
> > -                       ;;
> > -               --no-keep-index)
> > -                       keep_index=n
> > -                       ;;
> > -               -p|--patch)
> > -                       patch_mode=t
> > -                       # only default to keep if we don't already have an
> > override
> > -                       test -z "$keep_index" && keep_index=t
> > -                       ;;
> > -               -q|--quiet)
> > -                       GIT_QUIET=t
> > -                       ;;
> > -               -u|--include-untracked)
> > -                       untracked=untracked
> > -                       ;;
> > -               -a|--all)
> > -                       untracked=all
> > -                       ;;
> > -               -m|--message)
> > -                       shift
> > -                       test -z ${1+x} && usage
> > -                       stash_msg=$1
> > -                       ;;
> > -               -m*)
> > -                       stash_msg=${1#-m}
> > -                       ;;
> > -               --message=*)
> > -                       stash_msg=${1#--message=}
> > -                       ;;
> > -               --help)
> > -                       show_help
> > -                       ;;
> > -               --)
> > -                       shift
> > -                       break
> > -                       ;;
> > -               -*)
> > -                       option="$1"
> > -                       eval_gettextln "error: unknown option for 'stash
> > push': \$option"
> > -                       usage
> > -                       ;;
> > -               *)
> > -                       break
> > -                       ;;
> > -               esac
> > -               shift
> > -       done
> > -
> > -       eval "set $(git rev-parse --sq --prefix "$prefix" -- "$@")"
> > -
> > -       if test -n "$patch_mode" && test -n "$untracked"
> > -       then
> > -               die "$(gettext "Can't use --patch and --include-untracked
> > or --all at the same time")"
> > -       fi
> > -
> > -       test -n "$untracked" || git ls-files --error-unmatch -- "$@"
> > >/dev/null || exit 1
> > -
> > -       git update-index -q --refresh
> > -       if maybe_quiet no_changes "$@"
> > -       then
> > -               say "$(gettext "No local changes to save")"
> > -               exit 0
> > -       fi
> > -
> > -       git reflog exists $ref_stash ||
> > -               clear_stash || die "$(gettext "Cannot initialize stash")"
> > -
> > -       create_stash -m "$stash_msg" -u "$untracked" -- "$@"
> > -       store_stash -m "$stash_msg" -q $w_commit ||
> > -       die "$(gettext "Cannot save the current status")"
> > -       say "$(eval_gettext "Saved working directory and index state
> > \$stash_msg")"
> > -
> > -       if test -z "$patch_mode"
> > -       then
> > -               test "$untracked" = "all" && CLEAN_X_OPTION=-x ||
> > CLEAN_X_OPTION=
> > -               if test -n "$untracked" && test $# = 0
> > -               then
> > -                       git clean --force --quiet -d $CLEAN_X_OPTION
> > -               fi
> > -
> > -               if test $# != 0
> > -               then
> > -                       test -z "$untracked" && UPDATE_OPTION="-u" ||
> > UPDATE_OPTION=
> > -                       test "$untracked" = "all" &&
> > FORCE_OPTION="--force" || FORCE_OPTION=
> > -                       git add $UPDATE_OPTION $FORCE_OPTION -- "$@"
> > -                       git diff-index -p --cached --binary HEAD -- "$@" |
> > -                       git apply --index -R
> > -               else
> > -                       git reset --hard -q --no-recurse-submodules
> > -               fi
> > -
> > -               if test "$keep_index" = "t" && test -n "$i_tree"
> > -               then
> > -                       git read-tree --reset $i_tree
> > -                       git ls-files -z --modified -- "$@" |
> > -                       git checkout-index -z --force --stdin
> > -               fi
> > -       else
> > -               git apply -R < "$TMP-patch" ||
> > -               die "$(gettext "Cannot remove worktree changes")"
> > -
> > -               if test "$keep_index" != "t"
> > -               then
> > -                       git reset -q -- "$@"
> > -               fi
> > -       fi
> > -}
> > -
> > -save_stash () {
> > -       push_options=
> > -       while test $# != 0
> > -       do
> > -               case "$1" in
> > -               -q|--quiet)
> > -                       GIT_QUIET=t
> > -                       ;;
> > -               --)
> > -                       shift
> > -                       break
> > -                       ;;
> > -               -*)
> > -                       # pass all options through to push_stash
> > -                       push_options="$push_options $1"
> > -                       ;;
> > -               *)
> > -                       break
> > -                       ;;
> > -               esac
> > -               shift
> > -       done
> > -
> > -       stash_msg="$*"
> > -
> > -       if test -z "$stash_msg"
> > -       then
> > -               push_stash $push_options
> > -       else
> > -               push_stash $push_options -m "$stash_msg"
> > -       fi
> > -}
> > -
> > -have_stash () {
> > -       git rev-parse --verify --quiet $ref_stash >/dev/null
> > -}
> > -
> > -list_stash () {
> > -       have_stash || return 0
> > -       git log --format="%gd: %gs" -g --first-parent -m "$@" $ref_stash --
> > -}
> > -
> > -show_stash () {
> > -       ALLOW_UNKNOWN_FLAGS=t
> > -       assert_stash_like "$@"
> > -
> > -       if test -z "$FLAGS"
> > -       then
> > -               if test "$(git config --bool stash.showStat || echo true)"
> > = "true"
> > -               then
> > -                       FLAGS=--stat
> > -               fi
> > -
> > -               if test "$(git config --bool stash.showPatch || echo
> > false)" = "true"
> > -               then
> > -                       FLAGS=${FLAGS}${FLAGS:+ }-p
> > -               fi
> > -
> > -               if test -z "$FLAGS"
> > -               then
> > -                       return 0
> > -               fi
> > -       fi
> > -
> > -       git diff ${FLAGS} $b_commit $w_commit
> > -}
> > -
> > -show_help () {
> > -       exec git help stash
> > -       exit 1
> > -}
> > -
> > -#
> > -# Parses the remaining options looking for flags and
> > -# at most one revision defaulting to ${ref_stash}@{0}
> > -# if none found.
> > -#
> > -# Derives related tree and commit objects from the
> > -# revision, if one is found.
> > -#
> > -# stash records the work tree, and is a merge between the
> > -# base commit (first parent) and the index tree (second parent).
> > -#
> > -#   REV is set to the symbolic version of the specified stash-like commit
> > -#   IS_STASH_LIKE is non-blank if ${REV} looks like a stash
> > -#   IS_STASH_REF is non-blank if the ${REV} looks like a stash ref
> > -#   s is set to the SHA1 of the stash commit
> > -#   w_commit is set to the commit containing the working tree
> > -#   b_commit is set to the base commit
> > -#   i_commit is set to the commit containing the index tree
> > -#   u_commit is set to the commit containing the untracked files tree
> > -#   w_tree is set to the working tree
> > -#   b_tree is set to the base tree
> > -#   i_tree is set to the index tree
> > -#   u_tree is set to the untracked files tree
> > -#
> > -#   GIT_QUIET is set to t if -q is specified
> > -#   INDEX_OPTION is set to --index if --index is specified.
> > -#   FLAGS is set to the remaining flags (if allowed)
> > -#
> > -# dies if:
> > -#   * too many revisions specified
> > -#   * no revision is specified and there is no stash stack
> > -#   * a revision is specified which cannot be resolve to a SHA1
> > -#   * a non-existent stash reference is specified
> > -#   * unknown flags were set and ALLOW_UNKNOWN_FLAGS is not "t"
> > -#
> > -
> > -parse_flags_and_rev()
> > -{
> > -       test "$PARSE_CACHE" = "$*" && return 0 # optimisation
> > -       PARSE_CACHE="$*"
> > -
> > -       IS_STASH_LIKE=
> > -       IS_STASH_REF=
> > -       INDEX_OPTION=
> > -       s=
> > -       w_commit=
> > -       b_commit=
> > -       i_commit=
> > -       u_commit=
> > -       w_tree=
> > -       b_tree=
> > -       i_tree=
> > -       u_tree=
> > -
> > -       FLAGS=
> > -       REV=
> > -       for opt
> > -       do
> > -               case "$opt" in
> > -                       -q|--quiet)
> > -                               GIT_QUIET=-t
> > -                       ;;
> > -                       --index)
> > -                               INDEX_OPTION=--index
> > -                       ;;
> > -                       --help)
> > -                               show_help
> > -                       ;;
> > -                       -*)
> > -                               test "$ALLOW_UNKNOWN_FLAGS" = t ||
> > -                                       die "$(eval_gettext "unknown
> > option: \$opt")"
> > -                               FLAGS="${FLAGS}${FLAGS:+ }$opt"
> > -                       ;;
> > -                       *)
> > -                               REV="${REV}${REV:+ }'$opt'"
> > -                       ;;
> > -               esac
> > -       done
> > -
> > -       eval set -- $REV
> > -
> > -       case $# in
> > -               0)
> > -                       have_stash || die "$(gettext "No stash entries
> > found.")"
> > -                       set -- ${ref_stash}@{0}
> > -               ;;
> > -               1)
> > -                       :
> > -               ;;
> > -               *)
> > -                       die "$(eval_gettext "Too many revisions specified:
> > \$REV")"
> > -               ;;
> > -       esac
> > -
> > -       case "$1" in
> > -               *[!0-9]*)
> > -                       :
> > -               ;;
> > -               *)
> > -                       set -- "${ref_stash}@{$1}"
> > -               ;;
> > -       esac
> > -
> > -       REV=$(git rev-parse --symbolic --verify --quiet "$1") || {
> > -               reference="$1"
> > -               die "$(eval_gettext "\$reference is not a valid
> > reference")"
> > -       }
> > -
> > -       i_commit=$(git rev-parse --verify --quiet "$REV^2") &&
> > -       set -- $(git rev-parse "$REV" "$REV^1" "$REV:" "$REV^1:" "$REV^2:"
> > 2>/dev/null) &&
> > -       s=$1 &&
> > -       w_commit=$1 &&
> > -       b_commit=$2 &&
> > -       w_tree=$3 &&
> > -       b_tree=$4 &&
> > -       i_tree=$5 &&
> > -       IS_STASH_LIKE=t &&
> > -       test "$ref_stash" = "$(git rev-parse --symbolic-full-name "${REV%@*}")"
> > &&
> > -       IS_STASH_REF=t
> > -
> > -       u_commit=$(git rev-parse --verify --quiet "$REV^3") &&
> > -       u_tree=$(git rev-parse "$REV^3:" 2>/dev/null)
> > -}
> > -
> > -is_stash_like()
> > -{
> > -       parse_flags_and_rev "$@"
> > -       test -n "$IS_STASH_LIKE"
> > -}
> > -
> > -assert_stash_like() {
> > -       is_stash_like "$@" || {
> > -               args="$*"
> > -               die "$(eval_gettext "'\$args' is not a stash-like commit")"
> > -       }
> > -}
> > -
> > -is_stash_ref() {
> > -       is_stash_like "$@" && test -n "$IS_STASH_REF"
> > -}
> > -
> > -assert_stash_ref() {
> > -       is_stash_ref "$@" || {
> > -               args="$*"
> > -               die "$(eval_gettext "'\$args' is not a stash reference")"
> > -       }
> > -}
> > -
> > -apply_stash () {
> > -
> > -       assert_stash_like "$@"
> > -
> > -       git update-index -q --refresh || die "$(gettext "unable to refresh
> > index")"
> > -
> > -       # current index state
> > -       c_tree=$(git write-tree) ||
> > -               die "$(gettext "Cannot apply a stash in the middle of a
> > merge")"
> > -
> > -       unstashed_index_tree=
> > -       if test -n "$INDEX_OPTION" && test "$b_tree" != "$i_tree" &&
> > -                       test "$c_tree" != "$i_tree"
> > -       then
> > -               git diff-tree --binary $s^2^..$s^2 | git apply --cached
> > -               test $? -ne 0 &&
> > -                       die "$(gettext "Conflicts in index. Try without
> > --index.")"
> > -               unstashed_index_tree=$(git write-tree) ||
> > -                       die "$(gettext "Could not save index tree")"
> > -               git reset
> > -       fi
> > -
> > -       if test -n "$u_tree"
> > -       then
> > -               GIT_INDEX_FILE="$TMPindex" git read-tree "$u_tree" &&
> > -               GIT_INDEX_FILE="$TMPindex" git checkout-index --all &&
> > -               rm -f "$TMPindex" ||
> > -               die "$(gettext "Could not restore untracked files from
> > stash entry")"
> > -       fi
> > -
> > -       eval "
> > -               GITHEAD_$w_tree='Stashed changes' &&
> > -               GITHEAD_$c_tree='Updated upstream' &&
> > -               GITHEAD_$b_tree='Version stash was based on' &&
> > -               export GITHEAD_$w_tree GITHEAD_$c_tree GITHEAD_$b_tree
> > -       "
> > -
> > -       if test -n "$GIT_QUIET"
> > -       then
> > -               GIT_MERGE_VERBOSITY=0 && export GIT_MERGE_VERBOSITY
> > -       fi
> > -       if git merge-recursive $b_tree -- $c_tree $w_tree
> > -       then
> > -               # No conflict
> > -               if test -n "$unstashed_index_tree"
> > -               then
> > -                       git read-tree "$unstashed_index_tree"
> > -               else
> > -                       a="$TMP-added" &&
> > -                       git diff-index --cached --name-only
> > --diff-filter=A $c_tree >"$a" &&
> > -                       git read-tree --reset $c_tree &&
> > -                       git update-index --add --stdin <"$a" ||
> > -                               die "$(gettext "Cannot unstage modified
> > files")"
> > -                       rm -f "$a"
> > -               fi
> > -               squelch=
> > -               if test -n "$GIT_QUIET"
> > -               then
> > -                       squelch='>/dev/null 2>&1'
> > -               fi
> > -               (cd "$START_DIR" && eval "git status $squelch") || :
> > -       else
> > -               # Merge conflict; keep the exit status from merge-recursive
> > -               status=$?
> > -               git rerere
> > -               if test -n "$INDEX_OPTION"
> > -               then
> > -                       gettextln "Index was not unstashed." >&2
> > -               fi
> > -               exit $status
> > -       fi
> > -}
> > -
> > -pop_stash() {
> > -       assert_stash_ref "$@"
> > -
> > -       if apply_stash "$@"
> > -       then
> > -               drop_stash "$@"
> > -       else
> > -               status=$?
> > -               say "$(gettext "The stash entry is kept in case you need
> > it again.")"
> > -               exit $status
> > -       fi
> > -}
> > -
> > -drop_stash () {
> > -       assert_stash_ref "$@"
> > -
> > -       git reflog delete --updateref --rewrite "${REV}" &&
> > -               say "$(eval_gettext "Dropped \${REV} (\$s)")" ||
> > -               die "$(eval_gettext "\${REV}: Could not drop stash entry")"
> > -
> > -       # clear_stash if we just dropped the last stash entry
> > -       git rev-parse --verify --quiet "$ref_stash@{0}" >/dev/null ||
> > -       clear_stash
> > -}
> > -
> > -apply_to_branch () {
> > -       test -n "$1" || die "$(gettext "No branch name specified")"
> > -       branch=$1
> > -       shift 1
> > -
> > -       set -- --index "$@"
> > -       assert_stash_like "$@"
> > -
> > -       git checkout -b $branch $REV^ &&
> > -       apply_stash "$@" && {
> > -               test -z "$IS_STASH_REF" || drop_stash "$@"
> > -       }
> > -}
> > -
> > -test "$1" = "-p" && set "push" "$@"
> > -
> > -PARSE_CACHE='--not-parsed'
> > -# The default command is "push" if nothing but options are given
> > -seen_non_option=
> > -for opt
> > -do
> > -       case "$opt" in
> > -       --) break ;;
> > -       -*) ;;
> > -       *) seen_non_option=t; break ;;
> > -       esac
> > -done
> > -
> > -test -n "$seen_non_option" || set "push" "$@"
> > -
> > -# Main command set
> > -case "$1" in
> > -list)
> > -       shift
> > -       list_stash "$@"
> > -       ;;
> > -show)
> > -       shift
> > -       show_stash "$@"
> > -       ;;
> > -save)
> > -       shift
> > -       save_stash "$@"
> > -       ;;
> > -push)
> > -       shift
> > -       push_stash "$@"
> > -       ;;
> > -apply)
> > -       shift
> > -       apply_stash "$@"
> > -       ;;
> > -clear)
> > -       shift
> > -       clear_stash "$@"
> > -       ;;
> > -create)
> > -       shift
> > -       create_stash -m "$*" && echo "$w_commit"
> > -       ;;
> > -store)
> > -       shift
> > -       store_stash "$@"
> > -       ;;
> > -drop)
> > -       shift
> > -       drop_stash "$@"
> > -       ;;
> > -pop)
> > -       shift
> > -       pop_stash "$@"
> > -       ;;
> > -branch)
> > -       shift
> > -       apply_to_branch "$@"
> > -       ;;
> > -*)
> > -       case $# in
> > -       0)
> > -               push_stash &&
> > -               say "$(gettext "(To restore them type \"git stash
> > apply\")")"
> > -               ;;
> > -       *)
> > -               usage
> > -       esac
> > -       ;;
> > -esac
> > diff --git a/t/README b/t/README
> > index 9afd61e3ca..369e3a9ded 100644
> > --- a/t/README
> > +++ b/t/README
> > @@ -393,10 +393,6 @@ the --no-sparse command-line argument.
> >  GIT_TEST_PRELOAD_INDEX=<boolean> exercises the preload-index code path
> >  by overriding the minimum number of cache entries required per thread.
> >
> > -GIT_TEST_STASH_USE_BUILTIN=<boolean>, when false, disables the
> > -built-in version of git-stash. See 'stash.useBuiltin' in
> > -git-config(1).
> > -
> >  GIT_TEST_ADD_I_USE_BUILTIN=<boolean>, when true, enables the
> >  built-in version of git add -i. See 'add.interactive.useBuiltin' in
> >  git-config(1).
> > diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> > index ea56e85e70..b84d55a832 100755
> > --- a/t/t3903-stash.sh
> > +++ b/t/t3903-stash.sh
> > @@ -1285,4 +1285,18 @@ 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_done
> > --
> > 2.25.1.377.g2d2118b814
> >
> >