This is an archive of the discontinued Mercurial Phabricator instance.

drawdag: include files from both parents in merge commits
ClosedPublic

Authored by martinvonz on Jul 15 2017, 2:13 AM.

Details

Summary

Consider a graph like this:

D
|\
B C
|/
A

drawdag will add a file called A in commit A, file B in B, file C in
C. That's fine and expected. In merge commits like D, I would expect
the files and their contents to be taken from the parent commits, so
commit D in this example would have files A, B, and C. However,
drawdag will instead add the file D compared to the first
parent. Depending on whether B or C got a smaller nodeid, the contents
of D would be {A, B, D} or {A, C, D}. This patch changes it to to be
{A, B, C}.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

martinvonz created this revision.Jul 15 2017, 2:13 AM
quark accepted this revision.Jul 15 2017, 12:48 PM
quark added a subscriber: quark.

Thanks for doing this! It's a missing feature when I was writing the first version of drawdag.

I read through repo.commitctx and double checked the {files} field in changelog is correct.

quark added a comment.Jul 15 2017, 6:13 PM

I think you can self-push these so I can rebase my work on top of it. Otherwise we can defer multi-dest feature to after the freeze.

martinvonz updated this revision to Diff 183.Jul 16 2017, 12:46 AM
In D92#1383, @quark wrote:

I think you can self-push these so I can rebase my work on top of it.

I don't think I've ever pushed my own patches, so I'm not comfortable doing that for these either.

Otherwise we can defer multi-dest feature to after the freeze.

I would honestly prefer that anyway. It's mostly the defineparents patch I'm concerned with, as I've said before. After having spent a day trying to understand the old code, it's clear that there are bugs in it. I'm sure your rewrite fixes most or all of them. It's still just too large for me to be comfortable accepting it.

durin42 accepted this revision.Jul 17 2017, 6:16 PM
This revision is now accepted and ready to land.Jul 17 2017, 6:16 PM
This revision was automatically updated to reflect the committed changes.