[PATCH 2 of 3] subrepo: fix diff/status -S of added/removed subrepo (issue3056)

Dov Feldstern dovdevel at gmail.com
Sun May 20 13:27:50 CDT 2012


# HG changeset patch
# User Dov Feldstern <dfeldstern at gmail.com>
# Date 1337537636 -10800
# Branch stable
# Node ID 5114a02e115b5d7176226fa624d0326a527d938a
# Parent  4585b54fbedcc83df50088ed37a3d5ade772032a
subrepo: fix diff/status -S of added/removed subrepo (issue3056)

When comparing revisions between which a subrepo was added (i.e., subrepo
doesn't yet exist in rev1, but does exist in rev2), diff/status -S should
report the new subrepo's files as newly added files (because, from the parent
repo's point of view, the contents of the new subrepo is new content). However,
until now, the subrepo's contents was not reported at all in the comparison of
rev1 and rev2.

The problem was due to the fact that when iterating over the subrepos in the
two revisions being compared, if at a given subpath there was no subrepo in
rev1, then we would return the subrepo for that subpath in rev2. This, however,
does not allow us to consistently show the differences going from rev1 -> rev2,
because we are now left with a set of subrepos which are from either rev1 or
rev2,and we have no way of knowing which it is.

To solve this, we replace 'itersubrepos' with a new function 'itersubsdeltas'
which returns enough information to allow the comparison to be done
consistently. Specifically, for each subpath, two additional values are
returned: the subrepo in the state that the subrepo was in at rev1, and that
subrepo's state in rev2.

Given this function, the callers can now be simplified, since they need not
worry about the case in which a subrepo isn't found at subpath in one of the
revs --- 'itersubsdeltas' has already handled that internally.

This changeset also fixes some existing tests which until now were displaying
the incorrect behavior.

diff -r 4585b54fbedc -r 5114a02e115b mercurial/cmdutil.py
--- a/mercurial/cmdutil.py	Sun May 20 21:11:34 2012 +0300
+++ b/mercurial/cmdutil.py	Sun May 20 21:13:56 2012 +0300
@@ -607,18 +607,11 @@
     if listsubrepos:
         ctx1 = repo[node1]
         ctx2 = repo[node2]
-        for subpath, sub in subrepo.itersubrepos(ctx1, ctx2):
-            tempnode2 = node2
-            try:
-                if node2 is not None:
-                    tempnode2 = ctx2.substate[subpath][1]
-            except KeyError:
-                # A subrepo that existed in node1 was deleted between node1 and
-                # node2 (inclusive). Thus, ctx2's substate won't contain that
-                # subpath. The best we can do is to ignore it.
-                tempnode2 = None
+        for subpath, sub1, substate2 in subrepo.itersubsdeltas(ctx1, ctx2):
+            if node2 is None:
+                substate2 = None
             submatch = matchmod.narrowmatcher(subpath, match)
-            sub.diff(diffopts, tempnode2, submatch, changes=changes,
+            sub1.diff(diffopts, substate2, submatch, changes=changes,
                      stat=stat, fp=fp, prefix=prefix)
 
 class changeset_printer(object):
diff -r 4585b54fbedc -r 5114a02e115b mercurial/localrepo.py
--- a/mercurial/localrepo.py	Sun May 20 21:11:34 2012 +0300
+++ b/mercurial/localrepo.py	Sun May 20 21:13:56 2012 +0300
@@ -1456,16 +1456,14 @@
         r = modified, added, removed, deleted, unknown, ignored, clean
 
         if listsubrepos:
-            for subpath, sub in subrepo.itersubrepos(ctx1, ctx2):
+            for subpath, sub1, rev2 in subrepo.itersubsdeltas(ctx1, ctx2):
                 if working:
                     rev2 = None
-                else:
-                    rev2 = ctx2.substate[subpath][1]
                 try:
                     submatch = matchmod.narrowmatcher(subpath, match)
-                    s = sub.status(rev2, match=submatch, ignored=listignored,
-                                   clean=listclean, unknown=listunknown,
-                                   listsubrepos=True)
+                    s = sub1.status(rev2, match=submatch, ignored=listignored,
+                                    clean=listclean, unknown=listunknown,
+                                    listsubrepos=True)
                     for rfiles, sfiles in zip(r, s):
                         rfiles.extend("%s/%s" % (subpath, f) for f in sfiles)
                 except error.LookupError:
diff -r 4585b54fbedc -r 5114a02e115b mercurial/subrepo.py
--- a/mercurial/subrepo.py	Sun May 20 21:11:34 2012 +0300
+++ b/mercurial/subrepo.py	Sun May 20 21:13:56 2012 +0300
@@ -246,15 +246,29 @@
         raise util.Abort(_("default path for subrepository %s not found") %
             reporelpath(repo))
 
-def itersubrepos(ctx1, ctx2):
-    """find subrepos in ctx1 or ctx2"""
-    # Create a (subpath, ctx) mapping where we prefer subpaths from
-    # ctx1. The subpaths from ctx2 are important when the .hgsub file
-    # has been modified (in ctx2) but not yet committed (in ctx1).
-    subpaths = dict.fromkeys(ctx2.substate, ctx2)
-    subpaths.update(dict.fromkeys(ctx1.substate, ctx1))
-    for subpath, ctx in sorted(subpaths.iteritems()):
-        yield subpath, ctx.sub(subpath)
+def itersubsdeltas(ctx1, ctx2):
+    """yield (subpath, subrepo1, substate2) tuples for each subrepo in either
+    ctx1 or ctx2, such that:
+    - 'subpath' is the path at which the subrepo resides;
+    - 'subrepo1' is a subrepo of the correct type, whose state is in the
+      subrepo's state in ctx1 (or the null state, if there was no subrepo at
+      subpath in ctx1);
+    - 'substate2' is the state of the subrepo in ctx2."""
+    subpaths = set(ctx1.substate).union(set(ctx2.substate))
+    for subpath in sorted(subpaths):
+        try:
+            subrepo1 = ctx1.sub(subpath)
+        except KeyError:
+            sub2source, sub2state, sub2type = ctx2.substate[subpath]
+            # subrepo1 should be like subrepo2, but belong to ctx1 and be in
+            # the null state
+            subrepo1 = types[sub2type](ctx1, subpath,
+                                       (sub2source, node.hex(node.nullid)))
+        try:
+            substate2 = ctx2.substate[subpath][1]
+        except KeyError:
+            substate2 = node.hex(node.nullid)
+        yield subpath, subrepo1, substate2
 
 def subrepo(ctx, path):
     """return instance of the right subrepo class for subrepo in path"""
diff -r 4585b54fbedc -r 5114a02e115b tests/test-mq-subrepo.t
--- a/tests/test-mq-subrepo.t	Sun May 20 21:11:34 2012 +0300
+++ b/tests/test-mq-subrepo.t	Sun May 20 21:13:56 2012 +0300
@@ -104,6 +104,7 @@
   [255]
   % update substate when adding .hgsub w/clean updated subrepo
   A .hgsub
+  A sub/a
   % qnew -m0 0.diff
   path sub
    source   sub
@@ -119,6 +120,7 @@
   [255]
   % update substate when modifying .hgsub w/clean updated subrepo
   M .hgsub
+  A sub2/a
   % qnew -m1 1.diff
   path sub
    source   sub
@@ -163,6 +165,7 @@
   [255]
   % update substate when adding .hgsub w/clean updated subrepo
   A .hgsub
+  A sub/a
   % qrefresh
   path sub
    source   sub
@@ -179,6 +182,7 @@
   [255]
   % update substate when modifying .hgsub w/clean updated subrepo
   M .hgsub
+  A sub2/a
   % qrefresh
   path sub
    source   sub
@@ -268,6 +272,7 @@
   [255]
   % update substate when adding .hgsub w/clean updated subrepo
   A .hgsub
+  A sub/a
   % qrecord --config ui.interactive=1 -m0 0.diff
   diff --git a/.hgsub b/.hgsub
   new file mode 100644
@@ -296,6 +301,7 @@
   [255]
   % update substate when modifying .hgsub w/clean updated subrepo
   M .hgsub
+  A sub2/a
   % qrecord --config ui.interactive=1 -m1 1.diff
   diff --git a/.hgsub b/.hgsub
   1 hunks, 1 lines changed
diff -r 4585b54fbedc -r 5114a02e115b tests/test-subrepo.t
--- a/tests/test-subrepo.t	Sun May 20 21:11:34 2012 +0300
+++ b/tests/test-subrepo.t	Sun May 20 21:13:56 2012 +0300
@@ -891,6 +891,31 @@
   @@ -1,2 +0,0 @@
   -fc627a69481fcbe5f1135069e8a3881c023e4cf5 s
   -e95bcfa18a358dc4936da981ebf4147b4cad1362 t
+  diff -r fc627a69481f -r 000000000000 s/.hgsub
+  --- a/s/.hgsub
+  +++ /dev/null
+  @@ -1,1 +0,0 @@
+  -ss = ss
+  diff -r fc627a69481f -r 000000000000 s/.hgsubstate
+  --- a/s/.hgsubstate
+  +++ /dev/null
+  @@ -1,1 +0,0 @@
+  -c7be3c1adb1112f8e167cb6b59b9b8ff342ec25d ss
+  diff -r fc627a69481f -r 000000000000 s/a
+  --- a/s/a
+  +++ /dev/null
+  @@ -1,1 +0,0 @@
+  -b
+  diff -r c7be3c1adb11 -r 000000000000 s/ss/a
+  --- a/s/ss/a
+  +++ /dev/null
+  @@ -1,1 +0,0 @@
+  -a
+  diff -r e95bcfa18a35 -r 000000000000 t/t
+  --- a/t/t
+  +++ /dev/null
+  @@ -1,1 +0,0 @@
+  -bah
 
 Test behavior of add for explicit path in subrepo:
   $ cd ..


More information about the Mercurial-devel mailing list