On Tue, Dec 15, 2020 at 6:31 AM Derrick Stolee <stolee@xxxxxxxxx> wrote:
>
> On 12/14/2020 11:21 AM, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@xxxxxxxxx>
> >
> > Implement cases where renames are involved in type changes (i.e. the
> > side of history that didn't rename the file changed its type from a
> > regular file to a symlink or submodule). There was some code to handle
> > this in merge-recursive but only in the special case when the renamed
> > file had no content changes. The code here works differently -- it
> > knows process_entry() can handle mode conflicts, so it does a few
> > minimal tweaks to ensure process_entry() can just finish the job as
> > needed.
> >
> > Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
> > ---
> > merge-ort.c | 33 +++++++++++++++++++++++++++++++--
> > 1 file changed, 31 insertions(+), 2 deletions(-)
> >
> > diff --git a/merge-ort.c b/merge-ort.c
> > index 9aac33c8e31..11e33f56edf 100644
> > --- a/merge-ort.c
> > +++ b/merge-ort.c
> > @@ -778,7 +778,32 @@ static int process_renames(struct merge_options *opt,
> > S_ISREG(newinfo->stages[target_index].mode));
> > if (type_changed && collision) {
> > /* special handling so later blocks can handle this */
>
> Perhaps drop this comment, or incorporate it into the lower one?
Will do.
> > - die("Not yet implemented");
> > + /*
> > + * if type_changed && collision are both true, then this
> > + * was really a double rename, but one side wasn't
> > + * detected due to lack of break detection. I.e.
> > + * something like
> > + * orig: has normal file 'foo'
> > + * side1: renames 'foo' to 'bar', adds 'foo' symlink
> > + * side2: renames 'foo' to 'bar'
> > + * In this case, the foo->bar rename on side1 won't be
> > + * detected because the new symlink named 'foo' is
> > + * there and we don't do break detection. But we detect
> > + * this here because we don't want to merge the content
> > + * of the foo symlink with the foo->bar file, so we
> > + * have some logic to handle this special case. The
> > + * easiest way to do that is make 'bar' on side1 not
> > + * be considered a colliding file but the other part
> > + * of a normal rename. If the file is very different,
> > + * well we're going to get content merge conflicts
> > + * anyway so it doesn't hurt. And if the colliding
> > + * file also has a different type, that'll be handled
> > + * by the content merge logic in process_entry() too.
> > + *
> > + * See also t6430, 'rename vs. rename/symlink'
>
> I appreciate the callout to a test that exercises this behavior.
>
> > + */
> > + collision = 0;
> > + }
>
> Here, we regain that closing curly brace, fixing the compiler errors from
> earlier.
So embarrassing. I was pretty sure I tested the individual patches,
but maybe I somehow missed this series?? Anyway, yeah, I'll fix it
up.
>
> > if (source_deleted) {
> > if (target_index == 1) {
> > rename_branch = opt->branch1;
> > @@ -858,7 +883,11 @@ static int process_renames(struct merge_options *opt,
> > newinfo->pathnames[0] = oldpath;
> > if (type_changed) {
> > /* rename vs. typechange */
> > - die("Not yet implemented");
> > + /* Mark the original as resolved by removal */
> > + memcpy(&oldinfo->stages[0].oid, &null_oid,
> > + sizeof(oldinfo->stages[0].oid));
> > + oldinfo->stages[0].mode = 0;
> > + oldinfo->filemask &= 0x06;
>
> This matches your explanation in the comment above. I wonder if 0x06
> could be less magical, but we are really deep in the weeds here already.
>
> Thanks,
> -Stolee
>
