On Fri, Jun 15, 2018 at 07:42:51AM +0300, Max Kirillov wrote:
> After modify/delete merge conflict happens in a file skipped by sparse
> checkout, "git reset --merge", which implements the "--abort" actions, and
> "git reset --hard" fail with message "Entry * not uptodate. Cannot update
> sparse checkout." The reason is that the entry is verified in
> apply_sparse_checkout() for being up-to-date even when it has a conflict.
Conflicted entries should not be skipped by design. Even if you
specify sparse patterns to ignore them, they must be checked out. When
a conflicted entry appears in apply_sparse_checkout() something else
is already wrong.
I think this is a better fix along that line. As you can see we
already un-skip staged entries. But I think I forgot (or did not know)
about CE_CONFLICTED. This change passes your new tests, but I didn't
try to run the whole test suite to see if I broke anything else.
-- 8< --
diff --git a/unpack-trees.c b/unpack-trees.c
index 3a85a02a77..eb544ee1b3 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1246,7 +1246,7 @@ static void mark_new_skip_worktree(struct exclude_list *el,
if (select_flag && !(ce->ce_flags & select_flag))
continue;
- if (!ce_stage(ce))
+ if (!ce_stage(ce) && !(ce->ce_flags & CE_CONFLICTED))
ce->ce_flags |= skip_wt_flag;
else
ce->ce_flags &= ~skip_wt_flag;
-- 8< --
> Checking conflicted entry for being up-to-date is not performed in other
> cases. One obvious reason to not check it is that it is already modified
> by inserting conflict marks.
>
> Fix by not checking conflicted entries before performing reset.
> Also, add test case which verifies the issue is fixed.
>
> Signed-off-by: Max Kirillov <max@xxxxxxxxxx>
> ---
> I have tried to use sparse-checkout for merging and cherrypicking, to save on IO
> and disk space. It works, mostly, but there are issues here and there.
> This one was low hanging, and also pretty annoying.
>
> t/t3035-merge-sparse.sh | 46 +++++++++++++++++++++++++++++++++++++++++
> unpack-trees.c | 2 +-
> 2 files changed, 47 insertions(+), 1 deletion(-)
> create mode 100755 t/t3035-merge-sparse.sh
>
> diff --git a/t/t3035-merge-sparse.sh b/t/t3035-merge-sparse.sh
> new file mode 100755
> index 0000000000..c6b2b0b82a
> --- /dev/null
> +++ b/t/t3035-merge-sparse.sh
> @@ -0,0 +1,46 @@
> +#!/bin/sh
> +
> +test_description='merge with sparse files'
> +
> +. ./test-lib.sh
> +
> +# test_file $filename $content
> +test_file () {
> + echo "$2" > "$1" &&
> + git add "$1"
> +}
> +
> +# test_commit_this $message_and_tag
> +test_commit_this () {
> + git commit -m "$1" &&
> + git tag "$1"
> +}
> +
> +test_expect_success 'setup' '
> + test_file checked-out init &&
> + test_file modify_delete modify_delete_init &&
> + test_commit_this init &&
> + test_file modify_delete modify_delete_theirs &&
> + test_commit_this theirs &&
> + git reset --hard init &&
> + git rm modify_delete &&
> + test_commit_this ours &&
> + git config core.sparseCheckout true &&
> + echo "/checked-out" >.git/info/sparse-checkout &&
> + git reset --hard &&
> + ! git merge theirs
> +'
> +
> +test_expect_success 'reset --hard works after the conflict' '
> + git reset --hard
> +'
> +
> +test_expect_success 'setup: conflict back' '
> + ! git merge theirs
> +'
> +
> +test_expect_success 'Merge abort works after the conflict' '
> + git merge --abort
> +'
> +
> +test_done
> diff --git a/unpack-trees.c b/unpack-trees.c
> index e73745051e..65ae0721a6 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -468,7 +468,7 @@ static int apply_sparse_checkout(struct index_state *istate,
> * also stat info may have lost after merged_entry() so calling
> * verify_uptodate() again may fail
> */
> - if (!(ce->ce_flags & CE_UPDATE) && verify_uptodate_sparse(ce, o))
> + if (!(ce->ce_flags & CE_UPDATE) && !(ce->ce_flags & CE_CONFLICTED) && verify_uptodate_sparse(ce, o))
> return -1;
> ce->ce_flags |= CE_WT_REMOVE;
> ce->ce_flags &= ~CE_UPDATE;
> --
> 2.17.0.1185.g782057d875
>
