Page MenuHomePhabricator

changing-files: add clean computation of changed file for merges
ClosedPublic

Authored by marmoute on Sep 30 2020, 9:11 AM.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

marmoute created this revision.Sep 30 2020, 9:11 AM
marmoute updated this revision to Diff 22958.Sep 30 2020, 9:36 PM
marmoute updated this revision to Diff 22987.Oct 2 2020, 3:14 AM
marmoute updated this revision to Diff 22995.Oct 2 2020, 5:09 AM
Alphare accepted this revision.Oct 2 2020, 8:55 AM
Alphare added a subscriber: Alphare.

Whew, that was a read.

The logic looks good to me, aside from the special case that I don't really understand, but we're not handling it yet.

I appreciate you taking the time to write a thorough docstring, though my suggestion could probably help in simplifying it.

mercurial/metadata.py
302

I found that listing the symmetrical cases as separate (B/G, C/H, D/I, E/J, F/K and N/O) rather than the same to be confusing. Maybe just reuse the same letter for each pair?

396

I'm not entirely sure I understand how sure a case happens.

For example repository have some commit where the *new* node is an ancestor of the node in parent-A

That sounds like a cycle, so I'm sure I'm just confused

parent-A and parent-B are two branches of the same file history, yet not merge-filenode were created

How is that even possible?

marmoute added inline comments.Oct 2 2020, 12:34 PM
mercurial/metadata.py
302

They are the same, but we arrive to them from slightly different angle. So I prefer to have a distinct letter for them because you never know what can hide in these angles.

396

I'm not entirely sure I understand how sure a case happens.

This happened when a bugged client, created a bugged manifest. This is bad, but this happened in the past.

They are no cycle in the filelog history.

marmoute updated this revision to Diff 23015.Oct 2 2020, 12:40 PM
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.