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

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


New version of the fix for issue3056, more or less along the lines of Matt's
suggestions:

 * These patches no longer use optional keyword args, and in fact no longer
   touch the functions to which they had been passing those args at all.
 * I replaced 'itersubrepos' with a new function 'itersubsdeltas'. Matt had
   suggested that 'itersubrepos' be changed to accept only one arg, but I
   didn't find it to be at all useful in that form (the entire purpose of
   'itersubrepos' seems to have been to deal with *two* contexts, using one as
   a fallback for the other). The new function 'itersubsdeltas' returns all the
   information necessary for comparing a subrepo as it appeared (or didn't) in
   rev1 with its state in rev2.
 * Using the new function, the code calling it (which used to be calling
   itersubrepos) can now be simplified, since it no longer has to handle the
   case in which a subrepo is not found at the given subpath in one of the revs
   --- 'itersubsdeltas' now deos that internally.
 * I didn't really understand the bit about the "null constructor"...

Reminder: 

The problem statement: subrepo 'sub' containing a single file 'subfile' has
been added to 'repo' sometime between rev1 and rev2:

           rev1                  rev2
          ------                ------

      repo                   repo
      +-- file               +-- file
                             +-- sub
                                 +-- subfile

I would expect:

  $ hg status -S --rev rev1 --rev rev2
  A .hgsub
  A .hgsubstate
  A sub/subfile

However, the actual output is:

  A .hgsub
  A .hgsubstate

with no mention of the contents of 'sub'. 'diff' behaves in the same way.

Searching the bts, I found that this is issue 3056 [1]. That issue seems to
have culminated in a thread [2] in which everyone agrees that the issue should
be fixed, but that's where things were left.

The attached patch series fixes the issue.

I've added some tests for this to test-subrepo.t. I've also fixed some existing
tests which inadvertently display this issue; note, though, that not all cases
covered by my new tests are covered by the existing ones. If reducing the
number of new tests is an issue, I would suggest perhaps adding the new tests
in exchange for removing the existing tests for issue3153 [3] (which is
actually a subset of issue3056)?

[1] http://mercurial.selenic.com/bts/issue3056
[2] http://www.selenic.com/pipermail/mercurial-devel/2011-October/035194.html
[3] http://mercurial.selenic.com/bts/issue3153


More information about the Mercurial-devel mailing list