[v2] stash: avoid recursive hard reset on submodules
diff mbox series

Message ID 20191011001114.GB640501@OJAN-PAAVO.localdomain
State New
Headers show
Series
  • [v2] stash: avoid recursive hard reset on submodules
Related show

Commit Message

Jakob Jarmar Oct. 11, 2019, 12:11 a.m. UTC
git stash push does not recursively stash submodules, but if
submodule.recurse is set, it may recursively reset --hard them. Having
only the destructive action recurse is likely to be surprising
behaviour, and unlikely to be desirable, so the easiest fix should be to
ensure that the call to git reset --hard never recurses into submodules.

This matches the behavior of check_changes_tracked_files, which ignores
submodules.

Signed-off-by: Jakob Jarmar <jakob@jarmar.se>
---

Sorry for sending a patch with messed up whitespace. This one should be
correct.

 builtin/stash.c     | 2 +-
 git-legacy-stash.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Junio C Hamano Oct. 11, 2019, 6:02 a.m. UTC | #1
Jakob Jarmar <jakob.j@gmail.com> writes:

> git stash push does not recursively stash submodules, but if
> submodule.recurse is set, it may recursively reset --hard them. Having
> only the destructive action recurse is likely to be surprising
> behaviour, and unlikely to be desirable, so the easiest fix should be to
> ensure that the call to git reset --hard never recurses into submodules.
>
> This matches the behavior of check_changes_tracked_files, which ignores
> submodules.

Makes sense.  Can we demonstrate an existing breakage and the fix in
a new test or two in perhaps t3906-stash-submodule.sh?

Thanks.

>
> Signed-off-by: Jakob Jarmar <jakob@jarmar.se>
> ---
>
> Sorry for sending a patch with messed up whitespace. This one should be
> correct.
>
>  builtin/stash.c     | 2 +-
>  git-legacy-stash.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/stash.c b/builtin/stash.c
> index b5a301f24d..c986c258f2 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -1383,7 +1383,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
>  			struct child_process cp = CHILD_PROCESS_INIT;
>  			cp.git_cmd = 1;
>  			argv_array_pushl(&cp.args, "reset", "--hard", "-q",
> -					 NULL);
> +					 "--no-recurse-submodules", NULL);
>  			if (run_command(&cp)) {
>  				ret = -1;
>  				goto done;
> diff --git a/git-legacy-stash.sh b/git-legacy-stash.sh
> index f60e9b3e87..07ad4a5459 100755
> --- a/git-legacy-stash.sh
> +++ b/git-legacy-stash.sh
> @@ -370,7 +370,7 @@ push_stash () {
>  			git diff-index -p --cached --binary HEAD -- "$@" |
>  			git apply --index -R
>  		else
> -			git reset --hard -q
> +			git reset --hard -q --no-recurse-submodules
>  		fi
>  
>  		if test "$keep_index" = "t" && test -n "$i_tree"

Patch
diff mbox series

diff --git a/builtin/stash.c b/builtin/stash.c
index b5a301f24d..c986c258f2 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1383,7 +1383,7 @@  static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 			struct child_process cp = CHILD_PROCESS_INIT;
 			cp.git_cmd = 1;
 			argv_array_pushl(&cp.args, "reset", "--hard", "-q",
-					 NULL);
+					 "--no-recurse-submodules", NULL);
 			if (run_command(&cp)) {
 				ret = -1;
 				goto done;
diff --git a/git-legacy-stash.sh b/git-legacy-stash.sh
index f60e9b3e87..07ad4a5459 100755
--- a/git-legacy-stash.sh
+++ b/git-legacy-stash.sh
@@ -370,7 +370,7 @@  push_stash () {
 			git diff-index -p --cached --binary HEAD -- "$@" |
 			git apply --index -R
 		else
-			git reset --hard -q
+			git reset --hard -q --no-recurse-submodules
 		fi
 
 		if test "$keep_index" = "t" && test -n "$i_tree"