Re: A note on merging conflicts..

[Junio C Hamano <junkio@cox.net>]

Junio C Hamano <junkio@cox.net> writes:

> Linus Torvalds <torvalds@osdl.org> writes:
>
>> On Sat, 1 Jul 2006, Rene Scharfe wrote:
>>> 
>>> I wonder why the two clear_commit_marks() calls at the end of
>>> get_merge_bases() are not sufficient, though.
>>
>> Why does that thing check for "parent->object.parsed"?
>
> Oh, I thought I fixed that up when I merged.  Sorry.
>
>> Also, it only clears commit marks if they are contiguous, but some commit 
>> marking may not be dense (eg, the "UNINTERESTING" mark may have been set 
>> by (PARENT1 && PARENT2) triggering, but is not set in the commits that 
>> reach it.
>
> That is true.

I'll be offline for a few hours, but if anybody is inclined to
fix this up, I'd appreciate it to be based on top of 7c6f8aa
commit (which is the tip of js/merge-base topic branch).

Perhaps as a starter (not even compile tested)... 

diff --git a/commit.c b/commit.c
index 0431027..23ac210 100644
--- a/commit.c
+++ b/commit.c
@@ -397,13 +397,13 @@ void clear_commit_marks(struct commit *c
 {
 	struct commit_list *parents;
 
-	parents = commit->parents;
+	if (!commit)
+		return;
 	commit->object.flags &= ~mark;
+	parents = commit->parents;
 	while (parents) {
 		struct commit *parent = parents->item;
-		if (parent && parent->object.parsed &&
-		    (parent->object.flags & mark))
-			clear_commit_marks(parent, mark);
+		clear_commit_marks(parent, mark);
 		parents = parents->next;
 	}
 }
@@ -1012,7 +1012,8 @@ static void mark_reachable_commits(struc
 	}
 }
 
-struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2)
+struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2,
+				    int cleanup_needed)
 {
 	struct commit_list *list = NULL;
 	struct commit_list *result = NULL;
@@ -1081,8 +1082,10 @@ struct commit_list *get_merge_bases(stru
 	}
 
 	/* reset flags */
-	clear_commit_marks(rev1, PARENT1 | PARENT2 | UNINTERESTING);
-	clear_commit_marks(rev2, PARENT1 | PARENT2 | UNINTERESTING);
+	if (cleanup_needed) {
+		clear_commit_marks(rev1, PARENT1 | PARENT2 | UNINTERESTING);
+		clear_commit_marks(rev2, PARENT1 | PARENT2 | UNINTERESTING);
+	}
 
 	return result;
 }
diff --git a/commit.h b/commit.h
index 89b9dad..53bc697 100644
--- a/commit.h
+++ b/commit.h
@@ -105,6 +105,6 @@ struct commit_graft *read_graft_line(cha
 int register_commit_graft(struct commit_graft *, int);
 int read_graft_file(const char *graft_file);
 
-extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2);
+extern struct commit_list *get_merge_bases(struct commit *rev1, struct commit *rev2, int cleanup_needed);
 
 #endif /* COMMIT_H */
diff --git a/merge-base.c b/merge-base.c
index b41f76c..59f723f 100644
--- a/merge-base.c
+++ b/merge-base.c
@@ -6,7 +6,7 @@ static int show_all = 0;
 
 static int merge_base(struct commit *rev1, struct commit *rev2)
 {
-	struct commit_list *result = get_merge_bases(rev1, rev2);
+	struct commit_list *result = get_merge_bases(rev1, rev2, 0);
 
 	if (!result)
 		return 1;