Page MenuHomePhabricator

merge: check for dir rename dest before adding ACTION_KEEP
Changes PlannedPublic

Authored by pulkit on Wed, Sep 2, 7:54 AM.

Details

Reviewers
marmoute
Group Reviewers
hg-reviewers
Summary

A previous patch in the series blindly uses ACTION_KEEP if the file is not
present on both remote and ancestor. This was wrong.

We tries to detect directory renames and in some graft cases, it can be possible
that file is not present on both sides but is created in rename destination of
other directory which exists on remote. In such cases, we need to merge the
rename source from remote with rename dest from local.

This patch makes sure we checks for such rename cases before falling back to
ACTION_KEEP.

Action for rename destination can be added while processing rename destination
or processing rename source. In some cases, when an optimization only diffing
files changes between m2-vs-ma are in play, either of rename dest or rename
source might not be processed.
Hence we need to be extra sure about adding action related to rename
destination.

This issue of missing to check dir rename dest was spotted by a future change
where some tests were failing.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

pulkit created this revision.Wed, Sep 2, 7:54 AM
marmoute requested changes to this revision.Fri, Sep 11, 8:54 AM
marmoute added a subscriber: marmoute.

Why don't we get a test change with this ?

This revision now requires changes to proceed.Fri, Sep 11, 8:54 AM

Why don't we get a test change with this ?

The test changes are visible after disabling the m2-vs-ma optimization. I just plugged it before to minimize the test changes in that patch which disables the optimization.

Why don't we get a test change with this ?

The test changes are visible after disabling the m2-vs-ma optimization. I just plugged it before to minimize the test changes in that patch which disables the optimization.

Can you mention it in the changeset description ?

pulkit planned changes to this revision.Fri, Sep 18, 9:04 AM

I have some changes which need to be made to this patch according to Yuya's review on mailing list.