Patchwork [1,of,3] copies: extract an explicit `computechangesetcopie` method from context

login
register
mail settings
Submitter Pierre-Yves David
Date Aug. 6, 2019, 9:50 a.m.
Message ID <fc8e461200c7262246eb.1565085038@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/41163/
State Superseded
Headers show

Comments

Pierre-Yves David - Aug. 6, 2019, 9:50 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@octobus.net>
# Date 1565054260 -7200
#      Tue Aug 06 03:17:40 2019 +0200
# Node ID fc8e461200c7262246ebd610100bcf9a75ded461
# Parent  f95b59ffc307c4549d9640a81d750a99bd75f423
# EXP-Topic extrameta
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r fc8e461200c7
copies: extract an explicit `computechangesetcopie` method from context

Right now, the logic around changeset centric copies data are buried into the
"changectx" code. We extract this code in a dedicated method (in the copies
module) for clarity. This clarity will help to explicitly compute and caches
these data in the future.
Yuya Nishihara - Aug. 6, 2019, 12:25 p.m.
On Tue, 06 Aug 2019 11:50:38 +0200, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> # Date 1565054260 -7200
> #      Tue Aug 06 03:17:40 2019 +0200
> # Node ID fc8e461200c7262246ebd610100bcf9a75ded461
> # Parent  f95b59ffc307c4549d9640a81d750a99bd75f423
> # EXP-Topic extrameta
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r fc8e461200c7
> copies: extract an explicit `computechangesetcopie` method from context

The idea seems fine, but the last two functions are unrelated to "copies",
and I feel the copies module is more like a set of copy-detection algorithms.

Maybe it's better to move to scmutil or new module.
Pierre-Yves David - Aug. 6, 2019, 12:38 p.m.
On 8/6/19 2:25 PM, Yuya Nishihara wrote:
> On Tue, 06 Aug 2019 11:50:38 +0200, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@octobus.net>
>> # Date 1565054260 -7200
>> #      Tue Aug 06 03:17:40 2019 +0200
>> # Node ID fc8e461200c7262246ebd610100bcf9a75ded461
>> # Parent  f95b59ffc307c4549d9640a81d750a99bd75f423
>> # EXP-Topic extrameta
>> # Available At https://bitbucket.org/octobus/mercurial-devel/
>> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r fc8e461200c7
>> copies: extract an explicit `computechangesetcopie` method from context
> 
> The idea seems fine, but the last two functions are unrelated to "copies",
> and I feel the copies module is more like a set of copy-detection algorithms.
> 
> Maybe it's better to move to scmutil or new module.

These two functions (added and removed) returns data that the changesets 
centric copy tracing algorithm use. This is why I moved them to the 
`copies` module.
Do you still want them in the scmutil module ?
Yuya Nishihara - Aug. 6, 2019, 12:54 p.m.
On Tue, 6 Aug 2019 14:38:23 +0200, Pierre-Yves David wrote:
> 
> 
> On 8/6/19 2:25 PM, Yuya Nishihara wrote:
> > On Tue, 06 Aug 2019 11:50:38 +0200, Pierre-Yves David wrote:
> >> # HG changeset patch
> >> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> >> # Date 1565054260 -7200
> >> #      Tue Aug 06 03:17:40 2019 +0200
> >> # Node ID fc8e461200c7262246ebd610100bcf9a75ded461
> >> # Parent  f95b59ffc307c4549d9640a81d750a99bd75f423
> >> # EXP-Topic extrameta
> >> # Available At https://bitbucket.org/octobus/mercurial-devel/
> >> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r fc8e461200c7
> >> copies: extract an explicit `computechangesetcopie` method from context
> > 
> > The idea seems fine, but the last two functions are unrelated to "copies",
> > and I feel the copies module is more like a set of copy-detection algorithms.
> > 
> > Maybe it's better to move to scmutil or new module.
> 
> These two functions (added and removed) returns data that the changesets 
> centric copy tracing algorithm use. This is why I moved them to the 
> `copies` module.

IIRC, filesadded/removed are the sources of the template keywords.

> Do you still want them in the scmutil module ?

Yes, it seems less bad. And I can't think of a name for new module which
will host helper functions of context.py

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -24,6 +24,7 @@  from .node import (
     wdirhex,
 )
 from . import (
+    copies,
     dagop,
     encoding,
     error,
@@ -274,23 +275,7 @@  class basectx(object):
 
     @propertycache
     def _copies(self):
-        p1copies = {}
-        p2copies = {}
-        p1 = self.p1()
-        p2 = self.p2()
-        narrowmatch = self._repo.narrowmatch()
-        for dst in self.files():
-            if not narrowmatch(dst) or dst not in self:
-                continue
-            copied = self[dst].renamed()
-            if not copied:
-                continue
-            src, srcnode = copied
-            if src in p1 and p1[src].filenode() == srcnode:
-                p1copies[dst] = src
-            elif src in p2 and p2[src].filenode() == srcnode:
-                p2copies[dst] = src
-        return p1copies, p2copies
+        return copies.computechangesetcopies(self)
     def p1copies(self):
         return self._copies[0]
     def p2copies(self):
diff --git a/mercurial/copies.py b/mercurial/copies.py
--- a/mercurial/copies.py
+++ b/mercurial/copies.py
@@ -809,3 +809,28 @@  def duplicatecopies(repo, wctx, rev, fro
             continue
         if dst in wctx:
             wctx[dst].markcopied(src)
+
+def computechangesetcopies(ctx):
+    """return the copies data for a changeset
+
+    The copies data are returned as a pair of dictionnary (p1copies, p2copies).
+
+    Each dictionnary are in the form: `{newname: oldname}`
+    """
+    p1copies = {}
+    p2copies = {}
+    p1 = ctx.p1()
+    p2 = ctx.p2()
+    narrowmatch = ctx._repo.narrowmatch()
+    for dst in ctx.files():
+        if not narrowmatch(dst) or dst not in ctx:
+            continue
+        copied = ctx[dst].renamed()
+        if not copied:
+            continue
+        src, srcnode = copied
+        if src in p1 and p1[src].filenode() == srcnode:
+            p1copies[dst] = src
+        elif src in p2 and p2[src].filenode() == srcnode:
+            p2copies[dst] = src
+    return p1copies, p2copies