This is an archive of the discontinued Mercurial Phabricator instance.

[RFC] mergestate: store about files resolved in favour of other

Authored by pulkit on Apr 10 2020, 6:43 AM.



Committing a merge sometimes wrongly creates a new filenode where it can re-use
an existing one. This happens because the commit code does it's own calculation
and does not know what happened on merge.

This starts storing information in mergestate about files which were
automatically merged and the other/remote version of file was used.
We need this information at commit to pick the filenode parent for the new

This issue was found by Pierre-Yves David and idea to store the relevant parts
in mergestate is also suggested by him.

Somethings which can be further investigated are:

  1. refactoring of commit logic more to depend on this information
  2. maybe a more generic solution?

Diff Detail

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

Event Timeline

pulkit created this revision.Apr 10 2020, 6:43 AM

This attempts to fix the issue which D8243 tries to fix by storing extra information in mergestate.

marmoute accepted this revision.Apr 10 2020, 7:56 AM
marmoute added a subscriber: marmoute.

The approach seems good and the test change seems consistent with D8243 (@pulkit can you confirm there are no difference?)

This looks good to me.

pulkit edited the summary of this revision. (Show Details)Apr 10 2020, 10:28 AM
durin42 accepted this revision as: durin42.

I like it, but want to get @martinvonz to look too.

martinvonz accepted this revision.Apr 10 2020, 12:11 PM
This revision is now accepted and ready to land.Apr 10 2020, 12:11 PM
This revision was automatically updated to reflect the committed changes.
martinvonz added inline comments.Oct 6 2020, 4:28 PM

Could you send a follow-up updating this?


The hash no longer exists, and maybe it's no longer wrong?


And I don't think this is wrong anymore (i.e. the comment is now wrong)


Would be clearer to update this too. Either simply delete it or say what it should show in order to be correct (e.g. "log output should not include the merge commit").


Is this still true?




And this comment also seems wrong now


As above, delete to clarify

pulkit added a comment.Oct 7 2020, 3:59 AM

@martinvonz sent D9168 as followup for all the comments.