Re: [PATCH] Fix for a merge where a branch has an F->D transition

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

Re: [PATCH] Fix for a merge where a branch has an F->D transition

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:46:45 

Alex Riesen [off-list ref] writes:

Frankly, I'm not really sure. The solution came largely ... empirical
way. IOW, I tried more or less random things which looked like they
should fix the problem. So a review is very much appreciated. Please.
I've always thought that D/F conflict logic in merge-recursive is placed
at the wrong processing phase.  IIRC, it enumerates potential D/F
conflicting paths before even attempting to process renames, and it is
easy to miss a path that was previously file going away as the result of a
clean merge (in which case it is ok to have a directory there as the
result of a merge for other paths).  This breakage could be a small
example of it.

Regardless, I think your patch is a reasonable fix to go to the
maintenance track.  Thanks for looking into it.
quoted hunk
 merge-recursive.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index a3721ef..3c5420b 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -980,14 +980,15 @@ static int process_renames(struct merge_options *o,
 
 				if (mfi.clean &&
 				    sha_eq(mfi.sha, ren1->pair->two->sha1) &&
-				    mfi.mode == ren1->pair->two->mode)
+				    mfi.mode == ren1->pair->two->mode) {
 					/*
 					 * This messaged is part of
 					 * t6022 test. If you change
 					 * it update the test too.
 					 */
 					output(o, 3, "Skipped %s (merged same as existing)", ren1_dst);
-				else {
+					ren1->dst_entry->processed = 0;
+				} else {
 					if (mfi.merge || !mfi.clean)
 						output(o, 1, "Renaming %s => %s", ren1_src, ren1_dst);
 					if (mfi.merge)
-- 
1.6.3.28.ga852b

Re: [PATCH] Fix for a merge where a branch has an F->D transition

From: Alex Riesen <hidden>
Date: 2016-06-15 22:46:45 

2009/5/13 Junio C Hamano [off-list ref]:
Alex Riesen [off-list ref] writes:
quoted
Frankly, I'm not really sure. The solution came largely ... empirical
way. IOW, I tried more or less random things which looked like they
should fix the problem. So a review is very much appreciated. Please.
I've always thought that D/F conflict logic in merge-recursive is placed
at the wrong processing phase.  IIRC, it enumerates potential D/F
conflicting paths before even attempting to process renames, and it is
easy to miss a path that was previously file going away as the result of a
clean merge (in which case it is ok to have a directory there as the
result of a merge for other paths).  This breakage could be a small
example of it.

Regardless, I think your patch is a reasonable fix to go to the
maintenance track.  Thanks for looking into it.
I'm afraid the fix is not that simple: the "if" branch where the change
is placed supposed to prevent updating files in the working tree
which already have the same content as the merge's output.
My change may force them to be updated regardless. I think...
Johannes, you know this area the best, could you please have
a look?

Re: [PATCH] Fix for a merge where a branch has an F->D transition

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:46:45 

Hi,

On Wed, 13 May 2009, Alex Riesen wrote:

2009/5/13 Junio C Hamano [off-list ref]:
quoted
Alex Riesen [off-list ref] writes:
quoted
Frankly, I'm not really sure. The solution came largely ... empirical
way. IOW, I tried more or less random things which looked like they
should fix the problem. So a review is very much appreciated. Please.
I've always thought that D/F conflict logic in merge-recursive is placed
at the wrong processing phase.  IIRC, it enumerates potential D/F
conflicting paths before even attempting to process renames, and it is
easy to miss a path that was previously file going away as the result of a
clean merge (in which case it is ok to have a directory there as the
result of a merge for other paths).  This breakage could be a small
example of it.

Regardless, I think your patch is a reasonable fix to go to the
maintenance track.  Thanks for looking into it.
I'm afraid the fix is not that simple: the "if" branch where the change
is placed supposed to prevent updating files in the working tree
which already have the same content as the merge's output.
My change may force them to be updated regardless. I think...
Johannes, you know this area the best, could you please have
a look?
Sorry, no time at the moment...

Ciao,
Dscho

Re: [PATCH] Fix for a merge where a branch has an F->D transition

From: Alex Riesen <hidden>
Date: 2016-06-15 22:46:45 

2009/5/13 Johannes Schindelin [off-list ref]:
On Wed, 13 May 2009, Alex Riesen wrote:
quoted
2009/5/13 Junio C Hamano [off-list ref]:
quoted
Alex Riesen [off-list ref] writes:
quoted
Frankly, I'm not really sure. The solution came largely ... empirical
way. IOW, I tried more or less random things which looked like they
should fix the problem. So a review is very much appreciated. Please.
I've always thought that D/F conflict logic in merge-recursive is placed
at the wrong processing phase.  IIRC, it enumerates potential D/F
conflicting paths before even attempting to process renames, and it is
easy to miss a path that was previously file going away as the result of a
clean merge (in which case it is ok to have a directory there as the
result of a merge for other paths).  This breakage could be a small
example of it.

Regardless, I think your patch is a reasonable fix to go to the
maintenance track.  Thanks for looking into it.
I'm afraid the fix is not that simple: the "if" branch where the change
is placed supposed to prevent updating files in the working tree
which already have the same content as the merge's output.
My change may force them to be updated regardless. I think...
Johannes, you know this area the best, could you please have
a look?
Sorry, no time at the moment...
Maybe the patch could live on master or next for while? Many people
track master, so a performance degradation, if it happens, will be noticed.
And with master (or even better - next) we have an auditory which is at
least capable to complain.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help