Thread (37 messages) 37 messages, 4 authors, 2021-09-09

Re: [PATCH v2 3/6] merge-ort: expand only for out-of-cone conflicts

flat view

From: Elijah Newren <hidden>
Date: 2021-08-27 22:47:56

On Tue, Aug 24, 2021 at 2:52 PM Derrick Stolee via GitGitGadget
[off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Derrick Stolee <redacted>

Merge conflicts happen often enough to want to avoid expanding a sparse
index when they happen, as long as those conflicts are within the
sparse-checkout cone. If a conflict exists outside of the
sparse-checkout cone, then we still need to expand before iterating over
the index entries. This is critical to do in advance because of how the
original_cache_nr is tracked to allow inserting and replacing cache
entries.

Iterate over the conflicted files and check if any paths are outside of
the sparse-checkout cone. If so, then expand the full index.

Add a test that demonstrates that we do not expand the index, even when
we hit a conflict within the sparse-checkout cone.

Signed-off-by: Derrick Stolee <redacted>
---
 merge-ort.c                              | 13 +++++++---
 t/t1092-sparse-checkout-compatibility.sh | 30 ++++++++++++++++++++++--
 2 files changed, 38 insertions(+), 5 deletions(-)
diff --git a/merge-ort.c b/merge-ort.c
index 8e754b769e1..805f7c41397 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -4060,11 +4060,18 @@ static int record_conflicted_index_entries(struct merge_options *opt)

        /*
         * We are in a conflicted state. These conflicts might be inside
-        * sparse-directory entries, so expand the index preemtively.
-        * Also, we set original_cache_nr below, but that might change if
+        * sparse-directory entries, so check if any entries are outside
+        * of the sparse-checkout cone preemptively.
+        *
+        * We set original_cache_nr below, but that might change if
         * index_name_pos() calls ask for paths within sparse directories.
         */
-       ensure_full_index(index);
+       strmap_for_each_entry(&opt->priv->conflicted, &iter, e) {
+               if (!path_in_sparse_checkout(e->key, index)) {
+                       ensure_full_index(index);
+                       break;
+               }
+       }

        /* If any entries have skip_worktree set, we'll have to check 'em out */
        state.force = 1;
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index dc56252865c..38afdf689a2 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -617,8 +617,17 @@ test_expect_success 'sparse-index is expanded and converted back' '
 ensure_not_expanded () {
        rm -f trace2.txt &&
        echo >>sparse-index/untracked.txt &&
-       GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
-               git -C sparse-index "$@" &&
+
+       if test "$1" = "!"
+       then
+               shift &&
+               test_must_fail env \
+                       GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
+                       git -C sparse-index "$@" || return 1
+       else
+               GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
+                       git -C sparse-index "$@" || return 1
+       fi &&
        test_region ! index ensure_full_index trace2.txt
 }
@@ -658,6 +667,23 @@ test_expect_success 'sparse-index is not expanded' '
        )
 '

+test_expect_success 'sparse-index is not expanded: merge conflict in cone' '
+       init_repos &&
+
+       for side in right left
+       do
+               git -C sparse-index checkout -b expand-$side base &&
+               echo $side >sparse-index/deep/a &&
+               git -C sparse-index commit -a -m "$side" || return 1
+       done &&
+
+       (
+               sane_unset GIT_TEST_MERGE_ALGORITHM &&
+               git -C sparse-index config pull.twohead ort &&
+               ensure_not_expanded ! merge -m merged expand-right
+       )
These last five lines could just be replaced with the fourth, if you
just set GIT_TEST_MERGE_ALGORITHM=ort at the beginning of the file.

Are you worrying about testing with recursive in some of the testcases?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help