| Submitter | Simon Farnsworth |
|---|---|
| Date | Feb. 29, 2016, 6:14 p.m. |
| Message ID | <fbb5d06a6aed2d2e0d9a.1456769667@simonfar-macbookpro.local> |
| Download | mbox | patch |
| Permalink | /patch/13472/ |
| State | RFC, archived |
| Headers | show |
Comments
This patch isn't safe to apply as-is, because it introduces import loops. I can fix most of them by code motion, but there's one I'm stuck on, so sending this out for advice. Specifically, I can't work out a good way to break "Import cycle: mercurial.filemerge -> mercurial.templater -> mercurial.revset -> mercurial.merge -> mercurial.filemerge". filemerge needs templater because it uses templater to write conflict markers. In turn, templater needs revset because there's a revset template function. I want revset to pull in merge so that it can reuse merge's code for reading merge state. Am I best off creating a new mergestate.py file, and pulling merge state into there to break the loop? If not, have I missed something extremely obvious? Simon On 29/02/2016, 18:14, "Mercurial-devel on behalf of Simon Farnsworth" <mercurial-devel-bounces@mercurial-scm.org on behalf of simonfar@fb.com> wrote: ># HG changeset patch ># User Simon Farnsworth <simonfar@fb.com> ># Date 1456564536 0 ># Sat Feb 27 09:15:36 2016 +0000 ># Node ID fbb5d06a6aed2d2e0d9af3a63d3c251dcb72dbcb ># Parent 41dcd754526612c43b9695df8851557c851828ef >revset: add new predicates for finding merge revisions > >When you've merged a large stack of changesets in a fast moving repository, >the conflict markers can be apparent nonsense, as they're based on the heads >of the two trees that were merged. > >Provide three new revset predicates (conflictbase, conflictother and >conflictlocal) to help you explore the relevant parts of history; these are >the introducing revisions of the "base", "other" and "local" commits for >each file that has conflicts (restricted by a pattern). The user can build >appropriate views of history from these three predicates for their >particular situation. > >diff --git a/mercurial/merge.py b/mercurial/merge.py >--- a/mercurial/merge.py >+++ b/mercurial/merge.py >@@ -434,6 +434,25 @@ > if entry[0] == 'u': > yield f > >+ def ancestorfilectx(self, dfile): >+ extras = self.extras(dfile) >+ anccommitnode = extras.get('ancestorlinknode') >+ if anccommitnode: >+ actx = self._repo[anccommitnode] >+ else: >+ actx = None >+ >+ entry = self._state[dfile] >+ return self._repo.filectx(entry[3], fileid=entry[4], changeid=actx) >+ >+ def otherfilectx(self, dfile): >+ entry = self._state[dfile] >+ return self.otherctx.filectx(entry[5], fileid=entry[6]) >+ >+ def localfilectx(self, dfile): >+ entry = self._state[dfile] >+ return self.localctx.filectx(entry[2]) >+ > def driverresolved(self): > """Obtain the paths of driver-resolved files.""" > >diff --git a/mercurial/revset.py b/mercurial/revset.py >--- a/mercurial/revset.py >+++ b/mercurial/revset.py >@@ -17,6 +17,7 @@ > error, > hbisect, > match as matchmod, >+ merge as mergemod, > node, > obsolete as obsmod, > parser, >@@ -816,6 +817,93 @@ > getargs(x, 0, 0, _("closed takes no arguments")) > return subset.filter(lambda r: repo[r].closesbranch()) > >+@predicate('conflictbase(pattern)') >+def conflictbase(repo, subset, x): >+ """The base revision for any merge conflict matching pattern. >+ See :hg:`help patterns` for information about file patterns. >+ >+ The pattern without explicit kind like ``glob:`` is expected to be >+ relative to the current directory and match against a file exactly >+ for efficiency. >+ """ >+ # i18n: "conflictbase" is a keyword >+ pat = getstring(x, _("conflictbase requires a pattern")) >+ ms = mergemod.mergestate.read(repo) >+ >+ if not matchmod.patkind(pat): >+ f = pathutil.canonpath(repo.root, repo.getcwd(), pat) >+ if f in ms: >+ files = [f] >+ else: >+ files = [] >+ else: >+ m = matchmod.match(repo.root, repo.getcwd(), [pat], ctx=repo[None]) >+ files = (f for f in ms if m(f)) >+ >+ s = set() >+ for f in files: >+ s.add(ms.ancestorfilectx(f).introrev()) >+ >+ return subset & s >+ >+@predicate('conflictlocal(pattern)') >+def conflictlocal(repo, subset, x): >+ """The local revision for any merge conflict matching pattern. >+ See :hg:`help patterns` for information about file patterns. >+ >+ The pattern without explicit kind like ``glob:`` is expected to be >+ relative to the current directory and match against a file exactly >+ for efficiency. >+ """ >+ # i18n: "conflictlocal" is a keyword >+ pat = getstring(x, _("conflictlocal requires a pattern")) >+ ms = mergemod.mergestate.read(repo) >+ >+ if not matchmod.patkind(pat): >+ f = pathutil.canonpath(repo.root, repo.getcwd(), pat) >+ if f in ms: >+ files = [f] >+ else: >+ files = [] >+ else: >+ m = matchmod.match(repo.root, repo.getcwd(), [pat], ctx=repo[None]) >+ files = (f for f in ms if m(f)) >+ >+ s = set() >+ for f in files: >+ s.add(ms.localfilectx(f).introrev()) >+ >+ return subset & s >+ >+@predicate('conflictother(pattern)') >+def conflictother(repo, subset, x): >+ """The other revision for any merge conflict matching pattern. >+ See :hg:`help patterns` for information about file patterns. >+ >+ The pattern without explicit kind like ``glob:`` is expected to be >+ relative to the current directory and match against a file exactly >+ for efficiency. >+ """ >+ # i18n: "conflictother" is a keyword >+ pat = getstring(x, _("conflictother requires a pattern")) >+ ms = mergemod.mergestate.read(repo) >+ >+ if not matchmod.patkind(pat): >+ f = pathutil.canonpath(repo.root, repo.getcwd(), pat) >+ if f in ms: >+ files = [f] >+ else: >+ files = [] >+ else: >+ m = matchmod.match(repo.root, repo.getcwd(), [pat], ctx=repo[None]) >+ files = (f for f in ms if m(f)) >+ >+ s = set() >+ for f in files: >+ s.add(ms.otherfilectx(f).introrev()) >+ >+ return subset & s >+ > @predicate('contains(pattern)') > def contains(repo, subset, x): > """The revision's manifest contains a file matching pattern (but might not >diff --git a/tests/test-revset.t b/tests/test-revset.t >--- a/tests/test-revset.t >+++ b/tests/test-revset.t >@@ -2230,3 +2230,62 @@ > 2 > > $ cd .. >+ >+Test merge conflict predicates >+ >+ $ hg init conflictrepo >+ $ cd conflictrepo >+ $ echo file1 > file1 >+ $ echo file2 > file2 >+ $ hg commit -qAm first >+ $ echo line2 >> file1 >+ $ hg commit -qAm second >+ $ hg bookmark base >+ $ hg bookmark tree1 >+ $ echo line1 > file1 >+ $ hg commit -qAm tree1-file1 >+ $ echo tree1-file2 > file2 >+ $ hg commit -qAm tree1-file2 >+ $ echo file3 > file3 >+ $ hg commit -qAm tree1-file3 >+ $ hg update base >+ 2 files updated, 0 files merged, 1 files removed, 0 files unresolved >+ (activating bookmark base) >+ $ hg bookmark tree2 >+ $ echo lineA > file1 >+ $ echo line2 >> file1 >+ $ hg commit -qAm tree2 >+ $ hg bookmark tree2 >+ $ echo tree2-file2 > file2 >+ $ hg commit -qAm tree2-file2 >+ $ echo file4 > file4 >+ $ hg commit -qAm tree2-file4 >+ $ hg merge --rev tree1 --tool :fail >+ 1 files updated, 0 files merged, 0 files removed, 2 files unresolved >+ use 'hg resolve' to retry unresolved file merges or 'hg update -C .' to abandon >+ [1] >+ $ hg resolve --list >+ U file1 >+ U file2 >+ $ hg debugrevspec 'conflictbase("glob:*")' >+ 0 >+ 1 >+ $ hg debugrevspec 'conflictlocal("glob:*")' >+ 5 >+ 6 >+ $ hg debugrevspec 'conflictother("glob:*")' >+ 2 >+ 3 >+ $ hg debugrevspec 'conflictbase("file1")' >+ 1 >+ $ hg debugrevspec 'conflictlocal("file1")' >+ 5 >+ $ hg debugrevspec 'conflictother("file1")' >+ 2 >+ $ hg debugrevspec 'conflictbase("file2")' >+ 0 >+ $ hg debugrevspec 'conflictlocal("file2")' >+ 6 >+ $ hg debugrevspec 'conflictother("file2")' >+ 3 >+ $ cd .. >_______________________________________________ >Mercurial-devel mailing list >Mercurial-devel@mercurial-scm.org >https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=CwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=mEgSWILcY4c4W3zjApBQLA&m=MfnfL18BLlALJ5-H51eJzbhtGOy9GWUJnFcSF-MfQoQ&s=wvLikMa9gCtpYuQCRpdbKVBKdFKOVGtB05Vi3pT3x_4&e=
On Mon, Feb 29, 2016 at 06:19:09PM +0000, Simon Farnsworth wrote: > This patch isn't safe to apply as-is, because it introduces import loops. > > I can fix most of them by code motion, but there's one I'm stuck on, > so sending this out for advice. > > Specifically, I can't work out a good way to break "Import cycle: > mercurial.filemerge -> mercurial.templater -> mercurial.revset -> > mercurial.merge -> mercurial.filemerge". > > > > filemerge needs templater because it uses templater to write > conflict markers. In turn, templater needs revset because there's a > revset template function. I want revset to pull in merge so that it > can reuse merge's code for reading merge state. > > Am I best off creating a new mergestate.py file, and pulling merge > state into there to break the loop? If not, have I missed something > extremely obvious? A mergestate.py file sounds reasoanble. Maybe give that a try.
Patch
diff --git a/mercurial/merge.py b/mercurial/merge.py --- a/mercurial/merge.py +++ b/mercurial/merge.py @@ -434,6 +434,25 @@ if entry[0] == 'u': yield f + def ancestorfilectx(self, dfile): + extras = self.extras(dfile) + anccommitnode = extras.get('ancestorlinknode') + if anccommitnode: + actx = self._repo[anccommitnode] + else: + actx = None + + entry = self._state[dfile] + return self._repo.filectx(entry[3], fileid=entry[4], changeid=actx) + + def otherfilectx(self, dfile): + entry = self._state[dfile] + return self.otherctx.filectx(entry[5], fileid=entry[6]) + + def localfilectx(self, dfile): + entry = self._state[dfile] + return self.localctx.filectx(entry[2]) + def driverresolved(self): """Obtain the paths of driver-resolved files.""" diff --git a/mercurial/revset.py b/mercurial/revset.py --- a/mercurial/revset.py +++ b/mercurial/revset.py @@ -17,6 +17,7 @@ error, hbisect, match as matchmod, + merge as mergemod, node, obsolete as obsmod, parser, @@ -816,6 +817,93 @@ getargs(x, 0, 0, _("closed takes no arguments")) return subset.filter(lambda r: repo[r].closesbranch()) +@predicate('conflictbase(pattern)') +def conflictbase(repo, subset, x): + """The base revision for any merge conflict matching pattern. + See :hg:`help patterns` for information about file patterns. + + The pattern without explicit kind like ``glob:`` is expected to be + relative to the current directory and match against a file exactly + for efficiency. + """ + # i18n: "conflictbase" is a keyword + pat = getstring(x, _("conflictbase requires a pattern")) + ms = mergemod.mergestate.read(repo) + + if not matchmod.patkind(pat): + f = pathutil.canonpath(repo.root, repo.getcwd(), pat) + if f in ms: + files = [f] + else: + files = [] + else: + m = matchmod.match(repo.root, repo.getcwd(), [pat], ctx=repo[None]) + files = (f for f in ms if m(f)) + + s = set() + for f in files: + s.add(ms.ancestorfilectx(f).introrev()) + + return subset & s + +@predicate('conflictlocal(pattern)') +def conflictlocal(repo, subset, x): + """The local revision for any merge conflict matching pattern. + See :hg:`help patterns` for information about file patterns. + + The pattern without explicit kind like ``glob:`` is expected to be + relative to the current directory and match against a file exactly + for efficiency. + """ + # i18n: "conflictlocal" is a keyword + pat = getstring(x, _("conflictlocal requires a pattern")) + ms = mergemod.mergestate.read(repo) + + if not matchmod.patkind(pat): + f = pathutil.canonpath(repo.root, repo.getcwd(), pat) + if f in ms: + files = [f] + else: + files = [] + else: + m = matchmod.match(repo.root, repo.getcwd(), [pat], ctx=repo[None]) + files = (f for f in ms if m(f)) + + s = set() + for f in files: + s.add(ms.localfilectx(f).introrev()) + + return subset & s + +@predicate('conflictother(pattern)') +def conflictother(repo, subset, x): + """The other revision for any merge conflict matching pattern. + See :hg:`help patterns` for information about file patterns. + + The pattern without explicit kind like ``glob:`` is expected to be + relative to the current directory and match against a file exactly + for efficiency. + """ + # i18n: "conflictother" is a keyword + pat = getstring(x, _("conflictother requires a pattern")) + ms = mergemod.mergestate.read(repo) + + if not matchmod.patkind(pat): + f = pathutil.canonpath(repo.root, repo.getcwd(), pat) + if f in ms: + files = [f] + else: + files = [] + else: + m = matchmod.match(repo.root, repo.getcwd(), [pat], ctx=repo[None]) + files = (f for f in ms if m(f)) + + s = set() + for f in files: + s.add(ms.otherfilectx(f).introrev()) + + return subset & s + @predicate('contains(pattern)') def contains(repo, subset, x): """The revision's manifest contains a file matching pattern (but might not diff --git a/tests/test-revset.t b/tests/test-revset.t --- a/tests/test-revset.t +++ b/tests/test-revset.t @@ -2230,3 +2230,62 @@ 2 $ cd .. + +Test merge conflict predicates + + $ hg init conflictrepo + $ cd conflictrepo + $ echo file1 > file1 + $ echo file2 > file2 + $ hg commit -qAm first + $ echo line2 >> file1 + $ hg commit -qAm second + $ hg bookmark base + $ hg bookmark tree1 + $ echo line1 > file1 + $ hg commit -qAm tree1-file1 + $ echo tree1-file2 > file2 + $ hg commit -qAm tree1-file2 + $ echo file3 > file3 + $ hg commit -qAm tree1-file3 + $ hg update base + 2 files updated, 0 files merged, 1 files removed, 0 files unresolved + (activating bookmark base) + $ hg bookmark tree2 + $ echo lineA > file1 + $ echo line2 >> file1 + $ hg commit -qAm tree2 + $ hg bookmark tree2 + $ echo tree2-file2 > file2 + $ hg commit -qAm tree2-file2 + $ echo file4 > file4 + $ hg commit -qAm tree2-file4 + $ hg merge --rev tree1 --tool :fail + 1 files updated, 0 files merged, 0 files removed, 2 files unresolved + use 'hg resolve' to retry unresolved file merges or 'hg update -C .' to abandon + [1] + $ hg resolve --list + U file1 + U file2 + $ hg debugrevspec 'conflictbase("glob:*")' + 0 + 1 + $ hg debugrevspec 'conflictlocal("glob:*")' + 5 + 6 + $ hg debugrevspec 'conflictother("glob:*")' + 2 + 3 + $ hg debugrevspec 'conflictbase("file1")' + 1 + $ hg debugrevspec 'conflictlocal("file1")' + 5 + $ hg debugrevspec 'conflictother("file1")' + 2 + $ hg debugrevspec 'conflictbase("file2")' + 0 + $ hg debugrevspec 'conflictlocal("file2")' + 6 + $ hg debugrevspec 'conflictother("file2")' + 3 + $ cd ..