[PATCH 1 of 2] merge: separate read-only mergestate into separate module

Simon Farnsworth simonfar at fb.com
Mon Mar 7 10:20:26 EST 2016


On 07/03/2016, 14:58, "Yuya Nishihara" <youjah at gmail.com on behalf of yuya at tcha.org> wrote:


>On Fri, 4 Mar 2016 12:12:04 -0800, Simon Farnsworth wrote:
>> # HG changeset patch
>> # User Simon Farnsworth <simonfar at fb.com>
>> # Date 1456843967 0
>> #      Tue Mar 01 14:52:47 2016 +0000
>> # Node ID 37fe1f9d08245f7540cb6137c312ae30dbcde688
>> # Parent  c7f89ad87baef87f00c507545dfd4cc824bc3131
>> merge: separate read-only mergestate into separate module
>> 
>> This is code motion to prepare for revset predicates that read the
>> mergestate; without this code motion, there would be an import loop.
>
>Why did you separate a read-only mergestate class? It doesn't make sense to
>separate read/write functions of the same storage format.
>
>I guess the reason would be to cut the import loop from mergestate._resolve()
>to filemerge, which I think worth mentioning in a commit message. Someone can
>find a better way.

That's the import loop through scmutil that's hardest to break if revset imports merge directly. However, without the code motion, there are the following import loops:

+  Import cycle: mercurial.merge -> mercurial.scmutil -> mercurial.revset -> mercurial.merge
+  Import cycle: mercurial.copies -> mercurial.scmutil -> mercurial.revset -> mercurial.merge -> mercurial.copies
+  Import cycle: mercurial.cmdutil -> mercurial.revset -> mercurial.merge -> mercurial.subrepo -> mercurial.cmdutil
+  Import cycle: mercurial.filemerge -> mercurial.scmutil -> mercurial.revset -> mercurial.merge -> mercurial.filemerge
+  Import cycle: mercurial.exchange -> mercurial.scmutil -> mercurial.revset -> mercurial.merge -> mercurial.subrepo -> mercurial.exchange
+  Import cycle: mercurial.branchmap -> mercurial.scmutil -> mercurial.revset -> mercurial.merge -> mercurial.subrepo -> mercurial.exchange -> mercurial.discovery -> mercurial.branchmap
+  Import cycle: mercurial.changelog -> mercurial.revlog -> mercurial.templatefilters -> mercurial.templatekw -> mercurial.scmutil -> mercurial.revset -> mercurial.merge -> mercurial.subrepo -> mercurial.cmdutil -> mercurial.changelog


I can put these lines in the commit message if that'd help.

To fix these needs a mixture of code motion to reduce the number of places that import scmutil, plus removing mergestate.resolve, mergestate.preresolve, mergestate.recordactions and their helper functions from class mergestate.

Having removed these three functions from the mergestate class, I end up with a read-only mergestate, a set of methods for queueing merge driver actions, and a read-write mergestate; it seems simplest to me to split into read-only and read-write classes. I'm happy to split a different way if there's a preference for a better split.

Simon


More information about the Mercurial-devel mailing list