All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: derrickstolee@github.com, newren@gmail.com, gitster@pobox.com,
	Victoria Dye <vdye@github.com>, Victoria Dye <vdye@github.com>
Subject: [PATCH v3 5/6] stash: apply stash using 'merge_ort_nonrecursive()'
Date: Tue, 10 May 2022 23:32:31 +0000	[thread overview]
Message-ID: <e733c2fd9f497a8d80555126ec2e166e182ab8db.1652225552.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1171.v3.git.1652225552.gitgitgadget@gmail.com>

From: Victoria Dye <vdye@github.com>

Update 'stash' to use 'merge_ort_nonrecursive()' to apply a stash to the
current working tree. When 'git stash apply' was converted from its shell
script implementation to a builtin in 8a0fc8d19d (stash: convert apply to
builtin, 2019-02-25), 'merge_recursive_generic()' was used to merge a stash
into the working tree as part of 'git stash (apply|pop)'. However, with the
single merge base used in 'do_apply_stash()', the commit wrapping done by
'merge_recursive_generic()' is not only unnecessary, but misleading (the
*real* merge base is labeled "constructed merge base"). Therefore, a
non-recursive merge of the working tree, stashed tree, and stash base tree
is more appropriate.

There are two options for a non-recursive merge-then-update-worktree
function: 'merge_trees()' and 'merge_ort_nonrecursive()'. Use
'merge_ort_nonrecursive()' to align with the default merge strategy used by
'git merge' (6a5fb96672 (Change default merge backend from recursive to ort,
2021-08-04)) and, because merge-ort does not operate in-place on the index,
avoid unnecessary index expansion. Update tests in 't1092' verifying index
expansion for 'git stash' accordingly.

Signed-off-by: Victoria Dye <vdye@github.com>
---
 builtin/stash.c                          | 30 +++++++++++++++++++-----
 t/t1092-sparse-checkout-compatibility.sh |  4 ++--
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index 1bfba532044..3fe549f7d3c 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -7,6 +7,7 @@
 #include "cache-tree.h"
 #include "unpack-trees.h"
 #include "merge-recursive.h"
+#include "merge-ort-wrappers.h"
 #include "strvec.h"
 #include "run-command.h"
 #include "dir.h"
@@ -492,13 +493,13 @@ static void unstage_changes_unless_new(struct object_id *orig_tree)
 static int do_apply_stash(const char *prefix, struct stash_info *info,
 			  int index, int quiet)
 {
-	int ret;
+	int clean, ret;
 	int has_index = index;
 	struct merge_options o;
 	struct object_id c_tree;
 	struct object_id index_tree;
-	struct commit *result;
-	const struct object_id *bases[1];
+	struct tree *head, *merge, *merge_base;
+	struct lock_file lock = LOCK_INIT;
 
 	read_cache_preload(NULL);
 	if (refresh_and_write_cache(REFRESH_QUIET, 0, 0))
@@ -541,6 +542,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
 
 	o.branch1 = "Updated upstream";
 	o.branch2 = "Stashed changes";
+	o.ancestor = "Stash base";
 
 	if (oideq(&info->b_tree, &c_tree))
 		o.branch1 = "Version stash was based on";
@@ -551,10 +553,26 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
 	if (o.verbosity >= 3)
 		printf_ln(_("Merging %s with %s"), o.branch1, o.branch2);
 
-	bases[0] = &info->b_tree;
+	head = lookup_tree(o.repo, &c_tree);
+	merge = lookup_tree(o.repo, &info->w_tree);
+	merge_base = lookup_tree(o.repo, &info->b_tree);
+
+	repo_hold_locked_index(o.repo, &lock, LOCK_DIE_ON_ERROR);
+	clean = merge_ort_nonrecursive(&o, head, merge, merge_base);
+
+	/*
+	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
+	 * merge was clean, and nonzero if the merge was unclean or encountered
+	 * an error.
+	 */
+	ret = clean >= 0 ? !clean : clean;
+
+	if (ret < 0)
+		rollback_lock_file(&lock);
+	else if (write_locked_index(o.repo->index, &lock,
+				      COMMIT_LOCK | SKIP_IF_UNCHANGED))
+		ret = error(_("could not write index"));
 
-	ret = merge_recursive_generic(&o, &c_tree, &info->w_tree, 1, bases,
-				      &result);
 	if (ret) {
 		rerere(0);
 
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index 85c6a56f1b7..aaf4d880dbc 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -1385,7 +1385,7 @@ test_expect_success 'sparse-index is not expanded: stash' '
 	ensure_not_expanded stash &&
 	ensure_not_expanded stash list &&
 	ensure_not_expanded stash show stash@{0} &&
-	! ensure_not_expanded stash apply stash@{0} &&
+	ensure_not_expanded stash apply stash@{0} &&
 	ensure_not_expanded stash drop stash@{0} &&
 
 	echo >>sparse-index/deep/new &&
@@ -1399,7 +1399,7 @@ test_expect_success 'sparse-index is not expanded: stash' '
 	oid=$(git -C sparse-index stash create) &&
 	ensure_not_expanded stash store -m "test" $oid &&
 	ensure_not_expanded reset --hard &&
-	! ensure_not_expanded stash pop
+	ensure_not_expanded stash pop
 '
 
 test_expect_success 'sparse index is not expanded: diff' '
-- 
gitgitgadget


  parent reply	other threads:[~2022-05-10 23:32 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-25 17:49 [PATCH 0/7] Sparse index: integrate with 'git stash' Victoria Dye via GitGitGadget
2022-04-25 17:49 ` [PATCH 1/7] stash: expand sparse-checkout compatibility testing Victoria Dye via GitGitGadget
2022-04-25 17:49 ` [PATCH 2/7] stash: integrate with sparse index Victoria Dye via GitGitGadget
2022-04-25 21:34   ` Junio C Hamano
2022-04-26 12:53   ` Derrick Stolee
2022-04-26 15:26     ` Victoria Dye
2022-04-26 16:21     ` Junio C Hamano
2022-04-25 17:49 ` [PATCH 3/7] sparse-index: expose 'is_sparse_index_allowed()' Victoria Dye via GitGitGadget
2022-04-25 17:49 ` [PATCH 4/7] read-cache: set sparsity when index is new Victoria Dye via GitGitGadget
2022-04-25 21:35   ` Junio C Hamano
2022-04-25 17:49 ` [PATCH 5/7] merge-recursive: add merge function arg to 'merge_recursive_generic' Victoria Dye via GitGitGadget
2022-04-25 21:38   ` Junio C Hamano
2022-04-26 12:57     ` Derrick Stolee
2022-04-25 17:49 ` [PATCH 6/7] stash: merge applied stash with merge-ort Victoria Dye via GitGitGadget
2022-04-26 13:02   ` Derrick Stolee
2022-04-25 17:49 ` [PATCH 7/7] unpack-trees: preserve index sparsity Victoria Dye via GitGitGadget
2022-04-26 12:49 ` [PATCH 0/7] Sparse index: integrate with 'git stash' Derrick Stolee
2022-04-26 13:09   ` Derrick Stolee
2022-04-27 18:16 ` [PATCH v2 " Victoria Dye via GitGitGadget
2022-04-27 18:16   ` [PATCH v2 1/7] stash: expand sparse-checkout compatibility testing Victoria Dye via GitGitGadget
2022-04-27 18:16   ` [PATCH v2 2/7] stash: integrate with sparse index Victoria Dye via GitGitGadget
2022-04-27 18:16   ` [PATCH v2 3/7] sparse-index: expose 'is_sparse_index_allowed()' Victoria Dye via GitGitGadget
2022-04-27 18:16   ` [PATCH v2 4/7] read-cache: set sparsity when index is new Victoria Dye via GitGitGadget
2022-04-27 18:16   ` [PATCH v2 5/7] merge-recursive: add merge function arg to 'merge_recursive_generic' Victoria Dye via GitGitGadget
2022-05-06  7:23     ` Elijah Newren
2022-05-09 19:24       ` Victoria Dye
2022-05-10  7:06         ` Elijah Newren
2022-04-27 18:16   ` [PATCH v2 6/7] stash: merge applied stash with merge-ort Victoria Dye via GitGitGadget
2022-04-27 18:16   ` [PATCH v2 7/7] unpack-trees: preserve index sparsity Victoria Dye via GitGitGadget
2022-05-06  7:46   ` [PATCH v2 0/7] Sparse index: integrate with 'git stash' Elijah Newren
2022-05-10 23:32   ` [PATCH v3 0/6] " Victoria Dye via GitGitGadget
2022-05-10 23:32     ` [PATCH v3 1/6] stash: expand sparse-checkout compatibility testing Victoria Dye via GitGitGadget
2022-05-10 23:32     ` [PATCH v3 2/6] stash: integrate with sparse index Victoria Dye via GitGitGadget
2022-05-10 23:32     ` [PATCH v3 3/6] sparse-index: expose 'is_sparse_index_allowed()' Victoria Dye via GitGitGadget
2022-05-10 23:32     ` [PATCH v3 4/6] read-cache: set sparsity when index is new Victoria Dye via GitGitGadget
2022-05-10 23:32     ` Victoria Dye via GitGitGadget [this message]
2022-05-11  0:26       ` [PATCH v3 5/6] stash: apply stash using 'merge_ort_nonrecursive()' Junio C Hamano
2022-05-12  1:01       ` Jonathan Tan
2022-05-12 14:52         ` Elijah Newren
2022-05-12 16:55           ` Jonathan Tan
2022-05-12 14:51       ` Elijah Newren
2022-05-10 23:32     ` [PATCH v3 6/6] unpack-trees: preserve index sparsity Victoria Dye via GitGitGadget

Reply instructions:

You may reply publicly 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

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

  git send-email \
    --in-reply-to=e733c2fd9f497a8d80555126ec2e166e182ab8db.1652225552.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --cc=vdye@github.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.