Re: [PATCH v9 16/21] stash: convert push to builtin

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/26, Paul-Sebastian Ungureanu wrote:
> Add stash push to the helper.
> 
> Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@xxxxxxxxx>
> ---
>  builtin/stash--helper.c | 244 +++++++++++++++++++++++++++++++++++++++-
>  git-stash.sh            |   6 +-
>  2 files changed, 244 insertions(+), 6 deletions(-)
> 
> diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
> index 49b05f2458..d79233d7ec 100644
> --- a/builtin/stash--helper.c
> +++ b/builtin/stash--helper.c
> @@ -23,6 +23,9 @@ static const char * const git_stash_helper_usage[] = {
>  	N_("git stash--helper ( pop | apply ) [--index] [-q|--quiet] [<stash>]"),
>  	N_("git stash--helper branch <branchname> [<stash>]"),
>  	N_("git stash--helper clear"),
> +	N_("git stash--helper [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
> +	   "          [-u|--include-untracked] [-a|--all] [-m|--message <message>]\n"
> +	   "          [--] [<pathspec>...]]"),
>  	NULL
>  };
>  
> @@ -71,6 +74,13 @@ static const char * const git_stash_helper_create_usage[] = {
>  	NULL
>  };
>  
> +static const char * const git_stash_helper_push_usage[] = {
> +	N_("git stash--helper [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
> +	   "          [-u|--include-untracked] [-a|--all] [-m|--message <message>]\n"
> +	   "          [--] [<pathspec>...]]"),
> +	NULL
> +};
> +
>  static const char *ref_stash = "refs/stash";
>  static struct strbuf stash_index_path = STRBUF_INIT;
>  
> @@ -1088,7 +1098,7 @@ static int stash_working_tree(struct stash_info *info, struct pathspec ps)
>  
>  static int do_create_stash(struct pathspec ps, char **stash_msg,
>  			   int include_untracked, int patch_mode,
> -			   struct stash_info *info)
> +			   struct stash_info *info, struct strbuf *patch)
>  {
>  	int ret = 0;
>  	int flags = 0;
> @@ -1102,7 +1112,6 @@ static int do_create_stash(struct pathspec ps, char **stash_msg,
>  	struct strbuf commit_tree_label = STRBUF_INIT;
>  	struct strbuf untracked_files = STRBUF_INIT;
>  	struct strbuf stash_msg_buf = STRBUF_INIT;
> -	struct strbuf patch = STRBUF_INIT;
>  
>  	read_cache_preload(NULL);
>  	refresh_cache(REFRESH_QUIET);
> @@ -1152,7 +1161,7 @@ static int do_create_stash(struct pathspec ps, char **stash_msg,
>  		untracked_commit_option = 1;
>  	}
>  	if (patch_mode) {
> -		ret = stash_patch(info, ps, &patch);
> +		ret = stash_patch(info, ps, patch);
>  		*stash_msg = NULL;
>  		if (ret < 0) {
>  			fprintf_ln(stderr, _("Cannot save the current worktree state"));
> @@ -1221,7 +1230,8 @@ static int create_stash(int argc, const char **argv, const char *prefix)
>  			     0);
>  
>  	memset(&ps, 0, sizeof(ps));
> -	ret = do_create_stash(ps, &stash_msg, include_untracked, 0, &info);
> +	ret = do_create_stash(ps, &stash_msg, include_untracked, 0, &info,
> +			      NULL);
>  
>  	if (!ret)
>  		printf_ln("%s", oid_to_hex(&info.w_commit));
> @@ -1234,6 +1244,230 @@ static int create_stash(int argc, const char **argv, const char *prefix)
>  	return ret < 0;
>  }
>  
> +static int do_push_stash(struct pathspec ps, char *stash_msg, int quiet,
> +			 int keep_index, int patch_mode, int include_untracked)
> +{
> +	int ret = 0;
> +	struct stash_info info;
> +	struct strbuf patch = STRBUF_INIT;
> +
> +	if (patch_mode && keep_index == -1)
> +		keep_index = 1;
> +
> +	if (patch_mode && include_untracked) {
> +		fprintf_ln(stderr, _("Can't use --patch and --include-untracked"
> +				     " or --all at the same time"));
> +		ret = -1;

We should set "stash_msg" to NULL here, otherwise we'll get an invalid
free if these options are used together and a message is given.

In general I find this API and the do_create_stash API a bit
cumbersome to use.  Maybe it would help if we'd 'xstrdup' the string
before passing it in, so we can free it unconditionally, without
setting it to NULL everywhere.  As it's only a single strdup for each
command that's run that isn't very costly compared to what else we're
doing here, and I think the readability we're gaining would be worth
it.

> +		goto done;
> +	}
> +
> +	read_cache_preload(NULL);
> +	if (!include_untracked && ps.nr) {
> +		int i;
> +		char *ps_matched = xcalloc(ps.nr, 1);
> +
> +		for (i = 0; i < active_nr; i++)
> +			ce_path_match(&the_index, active_cache[i], &ps,
> +				      ps_matched);
> +
> +		if (report_path_error(ps_matched, &ps, NULL)) {
> +			fprintf_ln(stderr, _("Did you forget to 'git add'?"));
> +			stash_msg = NULL;
> +			ret = -1;

'ps_matched' is not being free'd in this error case.

> +			goto done;
> +		}
> +		free(ps_matched);
> +	}
> +
> +	if (refresh_cache(REFRESH_QUIET)) {
> +		stash_msg = NULL;
> +		ret = -1;
> +		goto done;
> +	}
> +
> +	if (!check_changes(ps, include_untracked)) {
> +		stash_msg = NULL;
> +		if (!quiet)
> +			printf_ln(_("No local changes to save"));
> +		goto done;
> +	}
> +
> +	if (!reflog_exists(ref_stash) && do_clear_stash()) {
> +		stash_msg = NULL;
> +		ret = -1;
> +		fprintf_ln(stderr, _("Cannot initialize stash"));
> +		goto done;
> +	}
> +
> +	if (do_create_stash(ps, &stash_msg, include_untracked, patch_mode,
> +			    &info, &patch)) {
> +		ret = -1;
> +		goto done;
> +	}
> +
> +	if (do_store_stash(&info.w_commit, stash_msg, 1)) {
> +		ret = -1;
> +		fprintf_ln(stderr, _("Cannot save the current status"));
> +		goto done;
> +	}
> +
> +	printf_ln(_("Saved working directory and index state %s"), stash_msg);
> +
> +	if (!patch_mode) {
> +		if (include_untracked && !ps.nr) {
> +			struct child_process cp = CHILD_PROCESS_INIT;
> +
> +			cp.git_cmd = 1;
> +			argv_array_pushl(&cp.args, "clean", "--force",
> +					 "--quiet", "-d", NULL);
> +			if (include_untracked == INCLUDE_ALL_FILES)
> +				argv_array_push(&cp.args, "-x");
> +			if (run_command(&cp)) {
> +				ret = -1;
> +				goto done;
> +			}
> +		}
> +		if (ps.nr) {
> +			struct child_process cp_add = CHILD_PROCESS_INIT;
> +			struct child_process cp_diff = CHILD_PROCESS_INIT;
> +			struct child_process cp_apply = CHILD_PROCESS_INIT;
> +			struct strbuf out = STRBUF_INIT;
> +
> +			cp_add.git_cmd = 1;
> +			argv_array_push(&cp_add.args, "add");
> +			if (!include_untracked)
> +				argv_array_push(&cp_add.args, "-u");
> +			if (include_untracked == INCLUDE_ALL_FILES)
> +				argv_array_push(&cp_add.args, "--force");
> +			argv_array_push(&cp_add.args, "--");
> +			add_pathspecs(&cp_add.args, ps);
> +			if (run_command(&cp_add)) {
> +				ret = -1;
> +				goto done;
> +			}
> +
> +			cp_diff.git_cmd = 1;
> +			argv_array_pushl(&cp_diff.args, "diff-index", "-p",
> +					 "--cached", "--binary", "HEAD", "--",
> +					 NULL);
> +			add_pathspecs(&cp_diff.args, ps);
> +			if (pipe_command(&cp_diff, NULL, 0, &out, 0, NULL, 0)) {
> +				ret = -1;
> +				goto done;
> +			}
> +
> +			cp_apply.git_cmd = 1;
> +			argv_array_pushl(&cp_apply.args, "apply", "--index",
> +					 "-R", NULL);
> +			if (pipe_command(&cp_apply, out.buf, out.len, NULL, 0,
> +					 NULL, 0)) {
> +				ret = -1;
> +				goto done;
> +			}
> +		} else {
> +			struct child_process cp = CHILD_PROCESS_INIT;
> +			cp.git_cmd = 1;
> +			argv_array_pushl(&cp.args, "reset", "--hard", "-q",
> +					 NULL);
> +			if (run_command(&cp)) {
> +				ret = -1;
> +				goto done;
> +			}
> +		}
> +
> +		if (keep_index == 1 && !is_null_oid(&info.i_tree)) {
> +			struct child_process cp_ls = CHILD_PROCESS_INIT;
> +			struct child_process cp_checkout = CHILD_PROCESS_INIT;
> +			struct strbuf out = STRBUF_INIT;
> +
> +			if (reset_tree(&info.i_tree, 0, 1)) {
> +				ret = -1;
> +				goto done;
> +			}
> +
> +			cp_ls.git_cmd = 1;
> +			argv_array_pushl(&cp_ls.args, "ls-files", "-z",
> +					 "--modified", "--", NULL);
> +
> +			add_pathspecs(&cp_ls.args, ps);
> +			if (pipe_command(&cp_ls, NULL, 0, &out, 0, NULL, 0)) {
> +				ret = -1;
> +				goto done;
> +			}
> +
> +			cp_checkout.git_cmd = 1;
> +			argv_array_pushl(&cp_checkout.args, "checkout-index",
> +					 "-z", "--force", "--stdin", NULL);
> +			if (pipe_command(&cp_checkout, out.buf, out.len, NULL,
> +					 0, NULL, 0)) {
> +				ret = -1;
> +				goto done;
> +			}
> +		}
> +		goto done;
> +	} else {
> +		struct child_process cp = CHILD_PROCESS_INIT;
> +
> +		cp.git_cmd = 1;
> +		argv_array_pushl(&cp.args, "apply", "-R", NULL);
> +
> +		if (pipe_command(&cp, patch.buf, patch.len, NULL, 0, NULL, 0)) {
> +			fprintf_ln(stderr, _("Cannot remove worktree changes"));
> +			ret = -1;
> +			goto done;
> +		}
> +
> +		if (keep_index < 1) {
> +			struct child_process cp = CHILD_PROCESS_INIT;
> +
> +			cp.git_cmd = 1;
> +			argv_array_pushl(&cp.args, "reset", "-q", "--", NULL);
> +			add_pathspecs(&cp.args, ps);
> +			if (run_command(&cp)) {
> +				ret = -1;
> +				goto done;
> +			}
> +		}
> +		goto done;
> +	}
> +
> +done:
> +	free(stash_msg);
> +	return ret;
> +}
> +
> +static int push_stash(int argc, const char **argv, const char *prefix)
> +{
> +	int keep_index = -1;
> +	int patch_mode = 0;
> +	int include_untracked = 0;
> +	int quiet = 0;
> +	char *stash_msg = NULL;
> +	struct pathspec ps;
> +	struct option options[] = {
> +		OPT_BOOL('k', "keep-index", &keep_index,
> +			 N_("keep index")),
> +		OPT_BOOL('p', "patch", &patch_mode,
> +			 N_("stash in patch mode")),
> +		OPT__QUIET(&quiet, N_("quiet mode")),
> +		OPT_BOOL('u', "include-untracked", &include_untracked,
> +			 N_("include untracked files in stash")),
> +		OPT_SET_INT('a', "all", &include_untracked,
> +			    N_("include ignore files"), 2),
> +		OPT_STRING('m', "message", &stash_msg, N_("message"),
> +			   N_("stash message")),
> +		OPT_END()
> +	};
> +
> +	argc = parse_options(argc, argv, prefix, options,
> +			     git_stash_helper_push_usage,
> +			     0);
> +
> +	parse_pathspec(&ps, 0, PATHSPEC_PREFER_FULL, prefix, argv);
> +	return do_push_stash(ps, stash_msg, quiet, keep_index, patch_mode,
> +			     include_untracked);
> +}
> +
>  int cmd_stash__helper(int argc, const char **argv, const char *prefix)
>  {
>  	pid_t pid = getpid();
> @@ -1272,6 +1506,8 @@ int cmd_stash__helper(int argc, const char **argv, const char *prefix)
>  		return !!store_stash(argc, argv, prefix);
>  	else if (!strcmp(argv[0], "create"))
>  		return !!create_stash(argc, argv, prefix);
> +	else if (!strcmp(argv[0], "push"))
> +		return !!push_stash(argc, argv, prefix);
>  
>  	usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), argv[0]),
>  		      git_stash_helper_usage, options);
> diff --git a/git-stash.sh b/git-stash.sh
> index ab06e4ffb8..c3146f62ab 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -412,7 +412,8 @@ save)
>  	;;
>  push)
>  	shift
> -	push_stash "$@"
> +	cd "$START_DIR"
> +	git stash--helper push "$@"
>  	;;
>  apply)
>  	shift
> @@ -448,7 +449,8 @@ branch)
>  *)
>  	case $# in
>  	0)
> -		push_stash &&
> +		cd "$START_DIR"
> +		git stash--helper push &&
>  		say "$(gettext "(To restore them type \"git stash apply\")")"
>  		;;
>  	*)
> -- 
> 2.19.0.rc0.23.g1fb9f40d88
> 



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux