Re: [PATCH v2] unpack-trees: fix oneway_merge accidentally carry over stage index

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Duy

Thanks for doing this, one minor comment - I try to use phillip.wood@xxxxxxxxxxxxx for git as it wont change if I change my email provider.

Best Wishes

Phillip

On 18/03/2019 11:38, Nguyễn Thái Ngọc Duy wrote:
Phillip found out that 'git checkout -f <branch>' does not restore
conflict/unmerged files correctly. All tracked files should be taken
from <branch> and all non-zero stages removed. Most of this is true,
except that the final file could be in stage one instead of zero.

"checkout -f" (among other commands) does this with one-way merge, which
is supposed to take stat info from the index and everything else from
the given tree. The add_entry(.., old, ...) call in oneway_merge()
though will keep stage index from the index.

This is normally not a problem if the entry from the index is
normal (stage #0). But if there is a conflict, stage #0 does not exist
and we'll get stage #1 entry as "old" variable, which gets recorded in
the final index. Fix it by clearing stage mask.

This bug probably comes from b5b425074e (git-read-tree: make one-way
merge also honor the "update" flag, 2005-06-07). Before this commit, we
may create the final ("dst") index entry from the one in index, but we
do clear CE_STAGEMASK.

I briefly checked two- and three-way merge functions. I think we don't
have the same problem in those.

Reported-by: Phillip Wood <phillip.wood123@xxxxxxxxx>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
---
  v2 updates log message to describe the problem and moves the test to
  t2023-checkout.m.sh

  t/t2023-checkout-m.sh | 24 ++++++++++++++++++++++++
  unpack-trees.c        |  2 +-
  2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/t/t2023-checkout-m.sh b/t/t2023-checkout-m.sh
index 7e18985134..fca3f85824 100755
--- a/t/t2023-checkout-m.sh
+++ b/t/t2023-checkout-m.sh
@@ -46,4 +46,28 @@ test_expect_success '-m restores 3-way conflicted+resolved file' '
  	test_cmp both.txt.conflicted.cleaned both.txt.cleaned
  '
+test_expect_success 'force checkout a conflict file creates stage zero entry' '
+	git init co-force &&
+	(
+		cd co-force &&
+		echo a >a &&
+		git add a &&
+		git commit -ama &&
+		A_OBJ=$(git rev-parse :a) &&
+		git branch topic &&
+		echo b >a &&
+		git commit -amb &&
+		B_OBJ=$(git rev-parse :a) &&
+		git checkout topic &&
+		echo c >a &&
+		C_OBJ=$(git hash-object a) &&
+		git checkout -m master &&
+		test_cmp_rev :1:a $A_OBJ &&
+		test_cmp_rev :2:a $B_OBJ &&
+		test_cmp_rev :3:a $C_OBJ &&
+		git checkout -f topic &&
+		test_cmp_rev :0:a $A_OBJ
+	)
+'
+
  test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 22c41a3ba8..1ccd343cad 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -2386,7 +2386,7 @@ int oneway_merge(const struct cache_entry * const *src,
  		if (o->update && S_ISGITLINK(old->ce_mode) &&
  		    should_update_submodules() && !verify_uptodate(old, o))
  			update |= CE_UPDATE;
-		add_entry(o, old, update, 0);
+		add_entry(o, old, update, CE_STAGEMASK);
  		return 0;
  	}
  	return merged_entry(a, old, o);




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux