git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Jakob Jarmar <jakob.j@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: [PATCH v4] stash: avoid recursive hard reset on submodules
Date: Sat, 12 Oct 2019 17:38:29 +0200
Message-ID: <20191012153829.GB743557@OJAN-PAAVO.localdomain> (raw)
In-Reply-To: <xmqqsgnyenwj.fsf@gitster-ct.c.googlers.com>

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>
---

Notes:
    1. Added space between function name and parentheses
    2. Moved test_when_finished cleanup to top of setup_basic

 builtin/stash.c            |  2 +-
 git-legacy-stash.sh        |  2 +-
 t/t3906-stash-submodule.sh | 42 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 43 insertions(+), 3 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"
diff --git a/t/t3906-stash-submodule.sh b/t/t3906-stash-submodule.sh
index d7219d6f8f..b93d1d74da 100755
--- a/t/t3906-stash-submodule.sh
+++ b/t/t3906-stash-submodule.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='stash apply can handle submodules'
+test_description='stash can handle submodules'
 
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-submodule-update.sh
@@ -21,4 +21,44 @@ KNOWN_FAILURE_CHERRY_PICK_SEES_EMPTY_COMMIT=1
 KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
 test_submodule_switch "git_stash"
 
+setup_basic () {
+	test_when_finished "rm -rf main sub" &&
+	git init sub &&
+	(
+		cd sub &&
+		test_commit sub_file
+	) &&
+	git init main &&
+	(
+		cd main &&
+		git submodule add ../sub &&
+		test_commit main_file
+	)
+}
+
+test_expect_success 'stash push with submodule.recurse=true preserves dirty submodule worktree' '
+	setup_basic &&
+	(
+		cd main &&
+		git config submodule.recurse true &&
+		echo "x" >main_file.t &&
+		echo "y" >sub/sub_file.t &&
+		git stash push &&
+		test_must_fail git -C sub diff --quiet
+	)
+'
+
+test_expect_success 'stash push and pop with submodule.recurse=true preserves dirty submodule worktree' '
+	setup_basic &&
+	(
+		cd main &&
+		git config submodule.recurse true &&
+		echo "x" >main_file.t &&
+		echo "y" >sub/sub_file.t &&
+		git stash push &&
+		git stash pop &&
+		test_must_fail git -C sub diff --quiet
+	)
+'
+
 test_done
-- 
2.23.0


      parent reply index

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-10 23:34 [PATCH] " Jakob Jarmar
2019-10-11  0:11 ` [PATCH v2] " Jakob Jarmar
2019-10-11  6:02   ` Junio C Hamano
2019-10-11 22:24     ` [PATCH v3] " Jakob Jarmar
2019-10-12  1:46       ` Junio C Hamano
2019-10-12 15:24         ` Jakob Jarmar
2019-10-12 15:38         ` Jakob Jarmar [this message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191012153829.GB743557@OJAN-PAAVO.localdomain \
    --to=jakob.j@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox