[PATCH 2/5] Treat D/F conflict entry more carefully in unpack-trees.c::threeway_merge()

Subsystems: the rest

DORMANTno replies

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH 2/5] Treat D/F conflict entry more carefully in unpack-trees.c::threeway_merge()

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:03

This fixes three buglets in threeway_merge() regarding D/F
conflict entries.

* After finishing with path D and handling path D/F, some stages
  have D/F conflict entry which are obviously non-NULL.  For the
  purpose of determining if the path D/F is missing in the
  ancestor, they should not be taken into account.

* D/F conflict entry is a phony entry and does not record the
  path being processed, so do not pick up the name from there.

* D/F conflict entry is a marker to say "this stage does _not_
  have the path", so do not send them to keep_entry().

There might be more glitches, but I am slowly digging this mess
through, which unfortunately was made even more work since
merge-recursive is a built-in now.

Signed-off-by: Junio C Hamano <redacted>
---
 unpack-trees.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index a0b6769..4039653 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -675,10 +675,10 @@ int threeway_merge(struct cache_entry **stages,
 	int i;
 
 	for (i = 1; i < o->head_idx; i++) {
-		if (!stages[i])
+		if (!stages[i] || stages[i] == o->df_conflict_entry)
 			any_anc_missing = 1;
 		else {
-			if (!path)
+			if (!path && stages[i] != o->df_conflict_entry)
 				path = stages[i]->name;
 			no_anc_exists = 0;
 		}
@@ -790,7 +790,7 @@ int threeway_merge(struct cache_entry **stages,
 	count = 0;
 	if (!head_match || !remote_match) {
 		for (i = 1; i < o->head_idx; i++) {
-			if (stages[i]) {
+			if (stages[i] && stages[i] != o->df_conflict_entry) {
 				keep_entry(stages[i], o);
 				count++;
 				break;
-- 
1.5.1.81.gee969

Re: [PATCH 2/5] Treat D/F conflict entry more carefully in unpack-trees.c::threeway_merge()

From: Daniel Barkalow <hidden>
Date: 2016-06-15 22:43:03

On Sat, 7 Apr 2007, Junio C Hamano wrote:
This fixes three buglets in threeway_merge() regarding D/F
conflict entries.

* After finishing with path D and handling path D/F, some stages
  have D/F conflict entry which are obviously non-NULL.  For the
  purpose of determining if the path D/F is missing in the
  ancestor, they should not be taken into account.

* D/F conflict entry is a phony entry and does not record the
  path being processed, so do not pick up the name from there.
This bit is unnecessary, because the first bit means we treat D/F conflict 
as missing in that conditional, and don't count it as an entry at all, let 
alone one with a useful name.
* D/F conflict entry is a marker to say "this stage does _not_
  have the path", so do not send them to keep_entry().

There might be more glitches, but I am slowly digging this mess
through, which unfortunately was made even more work since
merge-recursive is a built-in now.
Looks good, although it might be wise to add an "exists" function that 
returns false for df_conflict_entry and for NULL, to make the tests 
clearer, and to get a comment to point out the meaning of 
df_conflict_entry. Also, I think df_conflict_entry can be static bss in 
unpack-trees and not accessed through o->df_conflict_entry, since it's 
always the same value (being now universally initialized to a static 
pointer to heap...).

	-Daniel
*This .sig left intentionally blank*
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help