Patchwork [RFC] revset: add new predicates for finding merge revisions

login
register
mail settings
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

Simon Farnsworth - Feb. 29, 2016, 6:14 p.m.
# 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.
Simon Farnsworth - Feb. 29, 2016, 6:19 p.m.
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=
Augie Fackler - March 1, 2016, 6:26 p.m.
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 ..