diff mbox series

[01/10] merge-ort: handle D/F conflict where directory disappears due to merge

Message ID 382a009c18efc8a46a9c0210754f2266c3116ee4.1608270687.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series merge-ort: add more handling of basic conflict types | expand

Commit Message

Elijah Newren Dec. 18, 2020, 5:51 a.m. UTC 
From: Elijah Newren <newren@gmail.com>

When one side has a directory at a given path and the other side of
history has a file at the path, but the merge resolves the directory
away (e.g. because no path within that directory was modified and the
other side deleted it, or because renaming moved all the files
elsewhere), then we don't actually have a conflict anymore.  We just
need to clear away any information related to the relevant directory,
and then the subsequent process_entry() handling can handle the given
path.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-ort.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

Comments

Derrick Stolee Dec. 30, 2020, 2:06 p.m. UTC | #1 
On 12/18/2020 12:51 AM, Elijah Newren via GitGitGadget wrote:
> +	} else if (ci->df_conflict && ci->merged.result.mode != 0) {
>  		die("Not yet implemented.");
>  	}
>  
>  	/*
>  	 * NOTE: Below there is a long switch-like if-elseif-elseif... block
>  	 *       which the code goes through even for the df_conflict cases
> -	 *       above.  Well, it will once we don't die-not-implemented above.
> +	 *       above.
>  	 */

This comment change might be a bit premature.

Thanks,
-Stolee
Elijah Newren Dec. 30, 2020, 3:13 p.m. UTC | #2 
On Wed, Dec 30, 2020 at 6:06 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 12/18/2020 12:51 AM, Elijah Newren via GitGitGadget wrote:
> > +     } else if (ci->df_conflict && ci->merged.result.mode != 0) {
> >               die("Not yet implemented.");
> >       }
> >
> >       /*
> >        * NOTE: Below there is a long switch-like if-elseif-elseif... block
> >        *       which the code goes through even for the df_conflict cases
> > -      *       above.  Well, it will once we don't die-not-implemented above.
> > +      *       above.
> >        */
>
> This comment change might be a bit premature.

Or perhaps it should have been squashed into an earlier series that
was already merged to next.
Derrick Stolee Dec. 31, 2020, 11:17 a.m. UTC | #3 
On 12/30/2020 10:13 AM, Elijah Newren wrote:
> On Wed, Dec 30, 2020 at 6:06 AM Derrick Stolee <stolee@gmail.com> wrote:
>>
>> On 12/18/2020 12:51 AM, Elijah Newren via GitGitGadget wrote:
>>> +     } else if (ci->df_conflict && ci->merged.result.mode != 0) {
>>>               die("Not yet implemented.");
>>>       }
>>>
>>>       /*
>>>        * NOTE: Below there is a long switch-like if-elseif-elseif... block
>>>        *       which the code goes through even for the df_conflict cases
>>> -      *       above.  Well, it will once we don't die-not-implemented above.
>>> +      *       above.
>>>        */
>>
>> This comment change might be a bit premature.
> 
> Or perhaps it should have been squashed into an earlier series that
> was already merged to next.
 
I think it works with the next patch, which removes the die() from the
if-elseif-elseif from immediately before the comment.

-Stolee
Elijah Newren Dec. 31, 2020, 5:13 p.m. UTC | #4 
On Thu, Dec 31, 2020 at 3:17 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 12/30/2020 10:13 AM, Elijah Newren wrote:
> > On Wed, Dec 30, 2020 at 6:06 AM Derrick Stolee <stolee@gmail.com> wrote:
> >>
> >> On 12/18/2020 12:51 AM, Elijah Newren via GitGitGadget wrote:
> >>> +     } else if (ci->df_conflict && ci->merged.result.mode != 0) {
> >>>               die("Not yet implemented.");
> >>>       }
> >>>
> >>>       /*
> >>>        * NOTE: Below there is a long switch-like if-elseif-elseif... block
> >>>        *       which the code goes through even for the df_conflict cases
> >>> -      *       above.  Well, it will once we don't die-not-implemented above.
> >>> +      *       above.
> >>>        */
> >>
> >> This comment change might be a bit premature.
> >
> > Or perhaps it should have been squashed into an earlier series that
> > was already merged to next.
>
> I think it works with the next patch, which removes the die() from the
> if-elseif-elseif from immediately before the comment.

Oh, right, it's been long enough that I forgot the details and then I
read the patch backwards thinking it was adding the message.  Yeah, it
should go with the next patch.  I'll fix it up.
diff mbox series

Patch

diff --git a/merge-ort.c b/merge-ort.c
index 414e7b7eeac..f9a79eb25e6 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -976,14 +976,35 @@  static void process_entry(struct merge_options *opt,
 		assert(ci->df_conflict);
 	}
 
-	if (ci->df_conflict) {
+	if (ci->df_conflict && ci->merged.result.mode == 0) {
+		int i;
+
+		/*
+		 * directory no longer in the way, but we do have a file we
+		 * need to place here so we need to clean away the "directory
+		 * merges to nothing" result.
+		 */
+		ci->df_conflict = 0;
+		assert(ci->filemask != 0);
+		ci->merged.clean = 0;
+		ci->merged.is_null = 0;
+		/* and we want to zero out any directory-related entries */
+		ci->match_mask = (ci->match_mask & ~ci->dirmask);
+		ci->dirmask = 0;
+		for (i = MERGE_BASE; i <= MERGE_SIDE2; i++) {
+			if (ci->filemask & (1 << i))
+				continue;
+			ci->stages[i].mode = 0;
+			oidcpy(&ci->stages[i].oid, &null_oid);
+		}
+	} else if (ci->df_conflict && ci->merged.result.mode != 0) {
 		die("Not yet implemented.");
 	}
 
 	/*
 	 * NOTE: Below there is a long switch-like if-elseif-elseif... block
 	 *       which the code goes through even for the df_conflict cases
-	 *       above.  Well, it will once we don't die-not-implemented above.
+	 *       above.
 	 */
 	if (ci->match_mask) {
 		ci->merged.clean = 1;