Re: [PATCH v12 25/26] stash: optionally use the scripted version again

[Thomas Gummerer <t.gummerer@gmail.com>]

On 12/20, Paul-Sebastian Ungureanu wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> We recently converted the `git stash` command from Unix shell scripts
> to builtins.
> 
> Let's end users a way out when they discover a bug in the

s/Let's/& give/ maybe?

The rest of the patch looks good to me.

> builtin command: `stash.useBuiltin`.
> 
> As the file name `git-stash` is already in use, let's rename the
> scripted backend to `git-legacy-stash`.
> 
> To make the test suite pass with `stash.useBuiltin=false`, this commit
> also backports rudimentary support for `-q` (but only *just* enough
> to appease the test suite), and adds a super-ugly hack to force exit
> code 129 for `git stash -h`.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  .gitignore                          |  1 +
>  Makefile                            |  1 +
>  builtin/stash.c                     | 35 +++++++++++++++++++++++++++++
>  git-stash.sh => git-legacy-stash.sh | 34 +++++++++++++++++++++++++---
>  git-sh-setup.sh                     |  1 +
>  git.c                               |  7 +++++-
>  6 files changed, 75 insertions(+), 4 deletions(-)
>  rename git-stash.sh => git-legacy-stash.sh (97%)
> 
> diff --git a/.gitignore b/.gitignore
> index 0d77ea5894..7b0164675e 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -82,6 +82,7 @@
>  /git-interpret-trailers
>  /git-instaweb
>  /git-legacy-rebase
> +/git-legacy-stash
>  /git-log
>  /git-ls-files
>  /git-ls-remote
> diff --git a/Makefile b/Makefile
> index 8cee2731aa..810231a0b5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -617,6 +617,7 @@ SCRIPT_SH += git-merge-resolve.sh
>  SCRIPT_SH += git-mergetool.sh
>  SCRIPT_SH += git-quiltimport.sh
>  SCRIPT_SH += git-legacy-rebase.sh
> +SCRIPT_SH += git-legacy-stash.sh
>  SCRIPT_SH += git-remote-testgit.sh
>  SCRIPT_SH += git-request-pull.sh
>  SCRIPT_SH += git-submodule.sh
> diff --git a/builtin/stash.c b/builtin/stash.c
> index fe32ff42fd..346c9d2bb1 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -13,6 +13,7 @@
>  #include "revision.h"
>  #include "log-tree.h"
>  #include "diffcore.h"
> +#include "exec-cmd.h"
>  
>  #define INCLUDE_ALL_FILES 2
>  
> @@ -1513,6 +1514,26 @@ 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;
> +
> +	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;
> @@ -1524,6 +1545,20 @@ 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_diff_basic_config, NULL);
>  
>  	argc = parse_options(argc, argv, prefix, options, git_stash_usage,
> diff --git a/git-stash.sh b/git-legacy-stash.sh
> similarity index 97%
> rename from git-stash.sh
> rename to git-legacy-stash.sh
> index 789ce2f41d..8a8c4a9270 100755
> --- a/git-stash.sh
> +++ b/git-legacy-stash.sh
> @@ -80,6 +80,28 @@ clear_stash () {
>  	fi
>  }
>  
> +maybe_quiet () {
> +	case "$1" in
> +	--keep-stdout)
> +		shift
> +		if test -n "$GIT_QUIET"
> +		then
> +			eval "$@" 2>/dev/null
> +		else
> +			eval "$@"
> +		fi
> +		;;
> +	*)
> +		if test -n "$GIT_QUIET"
> +		then
> +			eval "$@" >/dev/null 2>&1
> +		else
> +			eval "$@"
> +		fi
> +		;;
> +	esac
> +}
> +
>  create_stash () {
>  
>  	prepare_fallback_ident
> @@ -112,15 +134,18 @@ create_stash () {
>  	done
>  
>  	git update-index -q --refresh
> -	if no_changes "$@"
> +	if maybe_quiet no_changes "$@"
>  	then
>  		exit 0
>  	fi
>  
>  	# state of the base commit
> -	if b_commit=$(git rev-parse --verify HEAD)
> +	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
> @@ -315,7 +340,7 @@ push_stash () {
>  	test -n "$untracked" || git ls-files --error-unmatch -- "$@" >/dev/null || exit 1
>  
>  	git update-index -q --refresh
> -	if no_changes "$@"
> +	if maybe_quiet no_changes "$@"
>  	then
>  		say "$(gettext "No local changes to save")"
>  		exit 0
> @@ -370,6 +395,9 @@ save_stash () {
>  	while test $# != 0
>  	do
>  		case "$1" in
> +		-q|--quiet)
> +			GIT_QUIET=t
> +			;;
>  		--)
>  			shift
>  			break
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index 378928518b..10d9764185 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -101,6 +101,7 @@ $LONG_USAGE")"
>  	case "$1" in
>  		-h)
>  		echo "$LONG_USAGE"
> +		case "$0" in *git-legacy-stash) exit 129;; esac
>  		exit
>  	esac
>  fi
> diff --git a/git.c b/git.c
> index 8a20909eae..591ebe9409 100644
> --- a/git.c
> +++ b/git.c
> @@ -554,7 +554,12 @@ static struct cmd_struct commands[] = {
>  	{ "show-index", cmd_show_index },
>  	{ "show-ref", cmd_show_ref, RUN_SETUP },
>  	{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
> -	{ "stash", cmd_stash, 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 },
>  	{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
>  	{ "stripspace", cmd_stripspace },
>  	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX | NO_PARSEOPT },
> -- 
> 2.20.1.441.g764a526393
>