Skip to contentSkip to conversation

git-stash: add flag to skip "git reset --hard"

PATCH In review Tom Anderson 2 participants 6 messages Jun 16, 2016 Documentation

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 git-stash: add flag to skip "git reset --hard"

Add the "--no-reset" (same as "-r") flag to git-stash.sh. A common workflow is to use "git-stash save" and "git-stash apply" to save your work without modifying your changed files. This forces users to reload their changed files in text editors like emacs, and touches the modify time on all changed files, potentially making incremental builds slower if many files in the build depend on your changed files.

Add documentation in Documentation/git-stash.txt and tests in t/t3903-stash.sh.

---
 Documentation/git-stash.txt |  9 ++++++---
 git-stash.sh                | 15 ++++++++++-----
 t/t3903-stash.sh            | 27 +++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 8 deletions(-)
Documentation/git-stash.txt +2 -2
index 92df596..ba5ecf2 100644
@@ -13,8 +13,8 @@ SYNOPSIS
 'git stash' drop [-q|--quiet] [<stash>]
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
-'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
-	     [-u|--include-untracked] [-a|--all] [<message>]]
+'git stash' [save [-p|--patch] [-r|--no-reset] [-k|--[no-]keep-index]
+	     [-q|--quiet] [-u|--include-untracked] [-a|--all] [<message>]]
 'git stash' clear
 'git stash' create [<message>]
 'git stash' store [-m|--message <message>] [-q|--quiet] <commit>
@@ -44,7 +44,7 @@ is also possible).
 OPTIONS
 -------
 -save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] 

[-a|--all] [-q|--quiet] [<message>]::
+save [-p|--patch] [-r|--no-reset] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
Save your local modifications to a new 'stash', and run `git reset
--hard` to revert them. The <message> part is optional and gives
@@ -61,6 +61,9 @@ stashed and then cleaned up with git clean, leaving the working directory
in a very clean state. If the --all option is used instead then the
ignored files are stashed and cleaned in addition to the untracked files.
+
+If the --no-reset option is used, git reset --hard is skipped and the
+--[no-]keep-index, --include-untracked, and --all flags are ignored.
++
With --patch, you can interactively select hunks from the diff
between HEAD and the working tree to be stashed. The stash entry is
constructed such that its index state is the same as the index state

git-stash.sh +10 -4
index c7c65e2..aebebb0 100755
@@ -7,8 +7,9 @@ USAGE="list [<options>]
    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 [save [--patch] [-r|--no-reset] [-k|--[no-]keep-index]
+		       [-q|--quiet] [-u|--include-untracked] [-a|--all]
+		       [<message>]]
    or: $dashless clear"
  SUBDIRECTORY_OK=Yes
@@ -194,9 +195,13 @@ save_stash () {
 	keep_index=
 	patch_mode=
 	untracked=
+	no_reset=
 	while test $# != 0
 	do
 		case "$1" in
+		-r|--no-reset)
+			no_reset=t
+			;;
 		-k|--keep-index)
 			keep_index=t
 			;;
@@ -268,7 +273,7 @@ save_stash () {
 	die "$(gettext "Cannot save the current status")"
 	say Saved working directory and index state "$stash_msg"
 -	if test -z "$patch_mode"
+	if test -z "$patch_mode" -a -z "$no_reset"
 	then
 		git reset --hard ${GIT_QUIET:+-q}
 		test "$untracked" = "all" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION=
@@ -276,12 +281,12 @@ save_stash () {
 		then
 			git clean --force --quiet -d $CLEAN_X_OPTION
 		fi
-
 		if test "$keep_index" = "t" && test -n $i_tree
 		then
 			git read-tree --reset -u $i_tree
 		fi
-	else
+	elif -n "$patch_mode"
+	then
 		git apply -R < "$TMP-patch" ||
 		die "$(gettext "Cannot remove worktree changes")"
 diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh

index 2142c1f..3eba19a 100755

t/t3903-stash.sh +26 -0
@@ -248,6 +248,33 @@ test_expect_success 'stash --no-keep-index' '
 	test bar,bar2 = $(cat file),$(cat file2)
 '
 +test_expect_success 'stash -r' '
+	git reset &&
+	echo foo420 > file &&
+	echo bar420 > file2 &&
+	git add file2 &&
+	git stash -r &&
+	test foo420,bar420 = $(cat file),$(cat file2)
+'
+
+test_expect_success 'stash --no-reset' '
+	git reset &&
+	echo bar33 > file &&
+	echo bar44 > file2 &&
+	git add file2 &&
+	git stash --no-reset &&
+	test bar33,bar44 = $(cat file),$(cat file2)
+'
+
+test_expect_success 'stash -r with ignored options -k -u -a' '
+	git reset &&
+	echo foo55 > file &&
+	echo bar66 > file2 &&
+	git add file2 &&
+	git stash -r &&
+	test foo55,bar66 = $(cat file),$(cat file2)
+'
+
 test_expect_success 'stash --invalid-option' '
 	echo bar5 > file &&
 	echo bar6 > file2 &&
Signature · 1 line
2.8.0.rc2
#1 lore raw
Quoted 8 lines · Tom Anderson
(quoted diff) +0 -0
@@ -61,6 +61,9 @@ stashed and then cleaned up with `git clean`,

leaving the working directory
in a very clean state. If the --all option is used instead then the ignored files are stashed and cleaned in addition to the untracked files. +
+If the --no-reset option is used, git reset --hard is skipped and the +--[no-]keep-index, --include-untracked, and --all flags are ignored. ++

I am afraid that a reader who does not read git-stash.sh script would not know what you are talking about. They do not know (or particularly care) where "git reset --hard", how often and for what purpose. They can tell that this option affects only "save", because that is where it is described, but they would not know what it means to "skip reset --hard", other than that they cannot use the three features listed there.

It is unclear what problem you are trying to solve from this text, and the log message's mention of mtime and rebuilding makes it sound like an X-Y problem.

It could very well be that what you are trying to implement makes perfect sense and the new option is named with a stress on a wrong aspect (i.e. named after what it uses to achieve things, rather than saying what you are trying to achieve).

The workhorse used in "git stash save" (which is what you are touching) is "git stash create", and that is only responsible for recording a new stash entry without touch the working tree. Is that what you are after, perhaps?

#2 lore raw
Quoted 17 lines · Junio C Hamano
Quoted 8 lines · Tom Anderson
(quoted diff) +0 -0
@@ -61,6 +61,9 @@ stashed and then cleaned up with `git clean`,

leaving the working directory
in a very clean state. If the --all option is used instead then the ignored files are stashed and cleaned in addition to the untracked files. +
+If the --no-reset option is used, git reset --hard is skipped and the +--[no-]keep-index, --include-untracked, and --all flags are ignored. ++

I am afraid that a reader who does not read git-stash.sh script would not know what you are talking about. They do not know (or particularly care) where "git reset --hard", how often and for what purpose. They can tell that this option affects only "save", because that is where it is described, but they would not know what it means to "skip reset --hard", other than that they cannot use the three features listed there.

The man page for git-stash unfortunately already makes a reference to git reset --hard:

save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]
Save your local modifications to a new stash, and run git reset --hard to revert them

Quoted 9 lines

It is unclear what problem you are trying to solve from this text, and the log message's mention of mtime and rebuilding makes it sound like an X-Y problem.

It could very well be that what you are trying to implement makes perfect sense and the new option is named with a stress on a wrong aspect (i.e. named after what it uses to achieve things, rather than saying what you are trying to achieve).

Perhaps it would be better to rename --no-reset to something more appropriate?

Quoted 5 lines

The workhorse used in "git stash save" (which is what you are touching) is "git stash create", and that is only responsible for recording a new stash entry without touch the working tree. Is that what you are after, perhaps?

Yes, but I like my stashes to be saved in the ref namespace. Maybe an alternative if it's a bad idea to add this new flag to "git stash save" is to add a new flag to "git stash create"? Or make a new git-stash command entirely?

#3 lore raw
Quoted 1 line · Tom Anderson

Yes, but I like my stashes to be saved in the ref namespace.

Isn't that something you can do so yourself with store_stash?

#4 lore raw

What I want can be achieved using "git stash store git stash create"

Quoted 4 lines · Junio C Hamano
Quoted 1 line · Tom Anderson

Yes, but I like my stashes to be saved in the ref namespace.

Isn't that something you can do so yourself with store_stash?

#5 lore raw
Quoted 7 lines · Tom Anderson
Quoted 4 lines · Junio C Hamano
Quoted 1 line · Tom Anderson

Yes, but I like my stashes to be saved in the ref namespace.

Isn't that something you can do so yourself with store_stash?

What I want can be achieved using "git stash store git stash create"

OK.

I am not strongly opposed to either a new option to create (i.e. "create --store" stores a new stash entry) or a new subcommand.

It just felt that a new option to "save" that deliberately breaks the basic premise of the command, i.e. "my working tree is a mess, and I want to revert it to the pristine state quickly to work on something else that is urgent, but I want to store the mess away instead of discarding, so that I can come back to it later", was totally out of place. I.e.

work work work to create a mess
git stash save
work on a quick and urgent stuff, knowing that you are building on a solidly committed state
git commit
git stash pop
continue building on the mess

is what "save" is about.

As your workflow is quite different (without "reset --hard", you no longer can rely on the resulting state to be pristine, suitable to work on something totally unrelated), it would have needed way way more explanation in the description section to describe this quite different mode of operation, as it is quite incompatible to the way traditional "stash save" users would want to use the command for if you do not revert the working tree to the pristine state. Instead you would do something else (which was not described--if the answer is "keep working, leading to the creation of the next commit", it is unclear what the resulting stash entry would be used for, as it obviously won't apply to that state, as the stashed change is already contained in that next commit).

"git stash save --keep" is bad enough already; it does not give the user pristine state wrt the current HEAD--what it does is a pristine state wrt the next HEAD that _would_ have resulted if the stuff you've been working on and decided are good by doing "git add" is all committed. In hindsight, it may have deserved a separate command to make it easier to explain to the end users.

Let's not make it worse.

This is a tangent, but I am not sure where your aversion to "reset --hard" comes from. If something was changed, mtime changes, and otherwise your build would break. It's not like "reset --hard" touches mtime for every path in the working tree including unchanged ones.