Patchwork [2,of,2,RFC] extdiff: use -S to archive the full repo

login
register
mail settings
Submitter Mathias De Maré
Date Feb. 18, 2015, 7:42 p.m.
Message ID <CADzL+YjQhM=NNc9Q3J4hO1T9NfKN+TMUCmdvqzaptZxYVMTJ1w@mail.gmail.com>
Download mbox | patch
Permalink /patch/7810/
State Changes Requested
Headers show

Comments

Mathias De Maré - Feb. 18, 2015, 7:42 p.m.
On Fri, Feb 13, 2015 at 4:45 AM, Matt Harbison <mharbison72@gmail.com>
wrote:

>
> Sorry for the delayed response.  I had to reboot last night, and Windows
> decided to install 11 updates... and it still wasn't done 12 hours later
> when I left for work this morning.
>
As you might have noticed by now, I'm a bit strapped for time myself (so
apologies for an even longer delay).

>
> I put together some tests to see if I could figure out the differences
> between what I was seeing and what you were.  I had assumed that adding a
> file to a subrepo and modifying one in a subrepo wouldn't cause a
> difference in how the recursion is handled- but that doesn't appear to be
> the case.  This can be applied on top of the 3rd patch in this chain.
> There's either a subtle subrepo bug, or (more likely), I'm missing
> something.
>
I've modified your below tests to clarify the bug a bit. Have a look at the
difference between my patch below and your patch to see. The issue does not
appear for only one file, because extdiff behaves differently for one file
in the working directory versus multiple!
I only modified your patch for hg subrepos, but same goes for git and
subversion repos.

# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1423795917 18000
#      Don Feb 12 21:51:57 2015 -0500
# Node ID ed6dfa605f0451605fc42658c5492d29e570eac7
# Parent  e7b1097a59713d2eb813c09013d71f7e1bb676ff
extdiff: basic hg subrepo testing

What extdiff sees when a subrepo has a modified file, and when it has an
added
file is different for reasons unknown.  A modified file in a git subrepo
also
works as we would like (i.e. the modified file is archived), but apparently
it
doesn't archive an added file either.

Changes by Mathias: modified files are NOT seen by extdiff either! They are
when
it's just one, but that's because archive is only called for MORE than one
file!


Side note: I was thinking about setting up a Kallithea instance to work on
this together without polluting the mailing list too much. What's your
opinion?

Greetings,
Mathias
Matt Harbison - Feb. 22, 2015, 10:46 p.m.
On Wed, 18 Feb 2015 14:42:17 -0500, Mathias De Maré  
<mathias.demare@gmail.com> wrote:

> On Fri, Feb 13, 2015 at 4:45 AM, Matt Harbison <mharbison72@gmail.com>
> wrote:
>
>>
>> Sorry for the delayed response.  I had to reboot last night, and Windows
>> decided to install 11 updates... and it still wasn't done 12 hours later
>> when I left for work this morning.
>>
> As you might have noticed by now, I'm a bit strapped for time myself (so
> apologies for an even longer delay).
>
>>
>> I put together some tests to see if I could figure out the differences
>> between what I was seeing and what you were.  I had assumed that adding  
>> a
>> file to a subrepo and modifying one in a subrepo wouldn't cause a
>> difference in how the recursion is handled- but that doesn't appear to  
>> be
>> the case.  This can be applied on top of the 3rd patch in this chain.
>> There's either a subtle subrepo bug, or (more likely), I'm missing
>> something.
>>
> I've modified your below tests to clarify the bug a bit. Have a look at  
> the
> difference between my patch below and your patch to see. The issue does  
> not
> appear for only one file, because extdiff behaves differently for one  
> file
> in the working directory versus multiple!
> I only modified your patch for hg subrepos, but same goes for git and
> subversion repos.

Yep, good point.

> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1423795917 18000
> #      Don Feb 12 21:51:57 2015 -0500
> # Node ID ed6dfa605f0451605fc42658c5492d29e570eac7
> # Parent  e7b1097a59713d2eb813c09013d71f7e1bb676ff
> extdiff: basic hg subrepo testing
>
> What extdiff sees when a subrepo has a modified file, and when it has an
> added
> file is different for reasons unknown.  A modified file in a git subrepo
> also
> works as we would like (i.e. the modified file is archived), but  
> apparently
> it
> doesn't archive an added file either.
>
> Changes by Mathias: modified files are NOT seen by extdiff either! They  
> are
> when
> it's just one, but that's because archive is only called for MORE than  
> one
> file!
>
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -658,6 +658,8 @@ class hgsubrepo(abstractsubrepo):
>      def diff(self, ui, diffopts, node2, match, prefix, **opts):
>          try:
>              node1 = node.bin(self._state[1])
> +#            print('hgsub diff: "%s"' % self._state[1])
> +
>              # We currently expect node2 to come from substate and be
>              # in hex format
>              if node2 is not None:
> @@ -676,6 +678,7 @@ class hgsubrepo(abstractsubrepo):
>          total = abstractsubrepo.archive(self, archiver, prefix, match)
>          rev = self._state[1]
>          ctx = self._repo[rev]
> +#        print('hgsub archive: "%s, ctx type is %s"' % (rev, type(ctx)))
>          for subpath in ctx.substate:
>              s = subrepo(ctx, subpath)
>              submatch = matchmod.narrowmatcher(subpath, match)
> diff --git a/tests/test-subrepo-extdiff.t b/tests/test-subrepo-extdiff.t
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-subrepo-extdiff.t
> @@ -0,0 +1,111 @@
> +  $ cat >> $HGRCPATH <<EOF
> +  > [extensions]
> +  > extdiff=
> +  > EOF
> +
> +  $ hg init root
> +  $ echo "hgsub = hgsub" > root/.hgsub
> +  $ cd root
> +  $ hg init hgsub
> +  $ hg add .hgsub
> +  $ echo test > hgsub/modified.txt
> +  $ echo test2 > hgsub/modified2.txt
> +  $ hg -R hgsub add hgsub/modified.txt
> +  $ hg -R hgsub add hgsub/modified2.txt
> +
> +-------------------------------------------------------------------------------
> +For hg subrepos that have not been locked in, changes are visible to  
> diff,
> but
> +NOT extdiff.  self._state[1] is ''
> +
> +  $ hg status -S
> +  A .hgsub
> +  A hgsub/modified.txt
> +  A hgsub/modified2.txt
> +
> +  $ hg diff -S
> +  diff -r 000000000000 .hgsub
> +  --- /dev/null    Thu Jan 01 00:00:00 1970 +0000
> +  +++ b/.hgsub    * (glob)
> +  @@ -0,0 +1,1 @@
> +  +hgsub = hgsub
> +  diff -r 000000000000 hgsub/modified.txt
> +  --- /dev/null    Thu Jan 01 00:00:00 1970 +0000
> +  +++ b/hgsub/modified.txt    * (glob)
> +  @@ -0,0 +1,1 @@
> +  +test
> +  diff -r 000000000000 hgsub/modified2.txt
> +  --- /dev/null    Thu Jan 01 00:00:00 1970 +0000
> +  +++ b/hgsub/modified2.txt    * (glob)
> +  @@ -0,0 +1,1 @@
> +  +test2
> +
> +  $ hg extdiff -S
> +  diff -Npru root.000000000000/.hgsub root/.hgsub
> +  --- root.000000000000/.hgsub    1970-01-01 00:00:00* +0000 (glob)
> +  +++ root/.hgsub    * (glob)
> +  @@ -0,0 +1 @@
> +  +hgsub = hgsub
> +  [1]
> +
> +  $ hg ci -qm "modified.txt -> test" -S
> +
> +-------------------------------------------------------------------------------
> +For hg subrepos that *have* been locked in, changes are visible to diff
> +AND extdiff.  self._state[1] is 50a11ebc6587
> +
> +  $ echo 'second mod' > hgsub/modified.txt
> +  $ echo 'second mod' > hgsub/modified2.txt
> +
> +  $ hg status -S
> +  M hgsub/modified.txt
> +  M hgsub/modified2.txt
> +  $ hg diff -S
> +  diff -r 4034090a5971 hgsub/modified.txt
> +  --- a/hgsub/modified.txt    Thu Jan 01 00:00:00 1970 +0000
> +  +++ b/hgsub/modified.txt    * (glob)
> +  @@ -1,1 +1,1 @@
> +  -test
> +  +second mod
> +  diff -r 4034090a5971 hgsub/modified2.txt
> +  --- a/hgsub/modified2.txt    Thu Jan 01 00:00:00 1970 +0000
> +  +++ b/hgsub/modified2.txt    * (glob)
> +  @@ -1,1 +1,1 @@
> +  -test2
> +  +second mod
> +  $ hg extdiff -S
> +  [1]
> +
> +-------------------------------------------------------------------------------
> +But adding a file to hgsubrepo causes extdiff to see *nothing*.
> self_state[1]
> +is still 50a11ebc6587, as it was when we had better results in the last
> test.
> +
> +  $ echo 'added' > hgsub/added.txt
> +#  $ hg -R hgsub add hgsub/added.txt
> +  $ hg add hgsub/added.txt
> +
> +  $ hg status -S
> +  M hgsub/modified.txt
> +  M hgsub/modified2.txt
> +  A hgsub/added.txt
> +
> +  $ hg diff -S
> +  diff -r 4034090a5971 hgsub/added.txt
> +  --- /dev/null    Thu Jan 01 00:00:00 1970 +0000
> +  +++ b/hgsub/added.txt    * (glob)
> +  @@ -0,0 +1,1 @@
> +  +added
> +  diff -r 4034090a5971 hgsub/modified.txt
> +  --- a/hgsub/modified.txt    Thu Jan 01 00:00:00 1970 +0000
> +  +++ b/hgsub/modified.txt    * (glob)
> +  @@ -1,1 +1,1 @@
> +  -test
> +  +second mod
> +  diff -r 4034090a5971 hgsub/modified2.txt
> +  --- a/hgsub/modified2.txt    Thu Jan 01 00:00:00 1970 +0000
> +  +++ b/hgsub/modified2.txt    * (glob)
> +  @@ -1,1 +1,1 @@
> +  -test2
> +  +second mod
> +
> +  $ hg extdiff -S
> +  [1]
> diff --git a/tests/test-subrepo-git.t b/tests/test-subrepo-git.t
> --- a/tests/test-subrepo-git.t
> +++ b/tests/test-subrepo-git.t
> @@ -698,6 +698,16 @@ check differences made by most recent ch
>    +foo
>    +bar
>
> +  $ hg extdiff -S --config extensions.extdiff=
> +  --- nul      1970-01-01 00:00:00 +0000
> +  +++ $TESTTMP/tc/s/foobar     * (glob)
> +  @@ -0,0 +1,4 @@
> +  +woopwoop
> +  +
> +  +foo
> +  +bar
> +  [1]
> +
>    $ hg commit --subrepos -m "Added foobar"
>    committing subrepository s
>    created new head
>
>
>> After further digging, the only time I've seen this case triggered is if
>> the subrepo hasn't been committed to the parent repo yet.  I was hoping
>> this would be used as 'give me the wctx of this subrepo', but it's  
>> not.  I
>> can see how it gets there in subrepo:145.
>>
>> I wonder if context.substate() should be overridden in workingctx to set
>> the state to None, so that repo[None].substate always yields subrepo
>> working directories.  We would need it for a revset symbol that  
>> indicates
>> the working directory too.  It's different from repo['.'].substate, so I
>> don't think it will break anything.  I'll try it and see what happens,  
>> but
>> maybe a subrepo expert can chime in.
>>
>
> I agree that we probably need to move in that direction. Not sure who the
> subrepo experts are, but remarks welcome.
> Below is an example hack that can be applied to the above patch, it shows
> what happens when the substate is also set to None (extdiff works, yay!).
> This of course has nice side-effects like 'hg status -S' not working
> correctly anymore, so I think we're in for a bit more work.

Yeah, after I sent the email, I saw this.  And it crashes diff -S in my  
test case too.  I wasn't able to figure out why, but I only spent a few  
hours on it.  It looks like you hit it too from the patch below.  I'm  
starting to see why I never finished this in the first place...

> # HG changeset patch
> # User Mathias De Maré <mathias.demare@gmail.com>
> # Date 1424287953 -3600
> #      Mit Feb 18 20:32:33 2015 +0100
> # Node ID a1ec50dc0168e2e0798d74efaaf1e10df66f5a63
> # Parent  ed6dfa605f0451605fc42658c5492d29e570eac7
> subrepo: hack in propagation of top-level 'None' context
>
> Previously, subrepo context was determined by looking at
> substate. However, in the case where the context of
> the parent repository is the working directory (repo[None]),
> this leads to incorrect archiving.
>
> This patch is a hack, but it's purely meant to show what type of change
> would be required to fix subrepo archiving.
>
> (Note as a side-effect of this hack that 'hg st -S' is now messed up.
>
> diff --git a/mercurial/archival.py b/mercurial/archival.py
> --- a/mercurial/archival.py
> +++ b/mercurial/archival.py
> @@ -305,6 +305,7 @@ def archive(repo, dest, node, kind, deco
>
>      if subrepos:
>          for subpath in sorted(ctx.substate):
> +#            repo.ui.write("going to archive, toplevel rev: %s\n" %
> ctx.rev())
>              sub = ctx.sub(subpath)
>              submatch = matchmod.narrowmatcher(subpath, matchfn)
>              total += sub.archive(archiver, prefix, submatch)
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -334,8 +334,13 @@ def subrepo(ctx, path):
>
>      pathutil.pathauditor(ctx._repo.root)(path)
>      state = ctx.substate[path]
> +#    ctx._repo.ui.write("substate: %s\n" % str(state))
>      if state[2] not in types:
>          raise util.Abort(_('unknown subrepo type %s') % state[2])
> +    if ctx.rev() is None:
> +        state = list(state)
> +        state[1] = None
> +        state = tuple(state)
>      return types[state[2]](ctx, path, state[:2])
>
>  def newcommitphase(ui, ctx):
> @@ -657,7 +662,10 @@ class hgsubrepo(abstractsubrepo):
>      @annotatesubrepoerror
>      def diff(self, ui, diffopts, node2, match, prefix, **opts):
>          try:
> -            node1 = node.bin(self._state[1])
> +            if self._state[1]:
> +                node1 = node.bin(self._state[1])
> +            else:
> +                node1 = None
>  #            print('hgsub diff: "%s"' % self._state[1])
>
>              # We currently expect node2 to come from substate and be
> @@ -728,6 +736,8 @@ class hgsubrepo(abstractsubrepo):
>
>      def _get(self, state):
>          source, revision, kind = state
> +        if revision is None:
> +            return True
>          if revision in self._repo.unfiltered():
>              return True
>          self._repo._subsource = source
> diff --git a/tests/test-subrepo-extdiff.t b/tests/test-subrepo-extdiff.t
> --- a/tests/test-subrepo-extdiff.t
> +++ b/tests/test-subrepo-extdiff.t
> @@ -19,8 +19,6 @@ NOT extdiff.  self._state[1] is ''
>
>    $ hg status -S
>    A .hgsub
> -  A hgsub/modified.txt
> -  A hgsub/modified2.txt
>
>    $ hg diff -S
>    diff -r 000000000000 .hgsub
> @@ -40,9 +38,8 @@ NOT extdiff.  self._state[1] is ''
>    +test2
>
>    $ hg extdiff -S
> -  diff -Npru root.000000000000/.hgsub root/.hgsub
> -  --- root.000000000000/.hgsub    1970-01-01 00:00:00* +0000 (glob)
> -  +++ root/.hgsub    * (glob)
> +  --- /dev/null    * (glob)
> +  +++ $TESTTMP/root/.hgsub    * (glob)
>    @@ -0,0 +1 @@
>    +hgsub = hgsub
>    [1]
> @@ -73,6 +70,18 @@ AND extdiff.  self._state[1] is 50a11ebc
>    -test2
>    +second mod
>    $ hg extdiff -S
> +  diff -Npru root.a642cdd33cab/hgsub/modified.txt  
> root/hgsub/modified.txt
> +  --- root.a642cdd33cab/hgsub/modified.txt    * (glob)
> +  +++ root/hgsub/modified.txt    * (glob)
> +  @@ -1 +1 @@
> +  -test
> +  +second mod
> +  diff -Npru root.a642cdd33cab/hgsub/modified2.txt  
> root/hgsub/modified2.txt
> +  --- root.a642cdd33cab/hgsub/modified2.txt    * (glob)
> +  +++ root/hgsub/modified2.txt    * (glob)
> +  @@ -1 +1 @@
> +  -test2
> +  +second mod
>    [1]
>
>  -------------------------------------------------------------------------------
> @@ -108,4 +117,21 @@ is still 50a11ebc6587, as it was when we
>    +second mod
>
>    $ hg extdiff -S
> +  diff -Npru root.a642cdd33cab/hgsub/added.txt root/hgsub/added.txt
> +  --- root.a642cdd33cab/hgsub/added.txt    1970-01-01 00:00:00.000000000
> +0000
> +  +++ root/hgsub/added.txt    * (glob)
> +  @@ -0,0 +1 @@
> +  +added
> +  diff -Npru root.a642cdd33cab/hgsub/modified.txt  
> root/hgsub/modified.txt
> +  --- root.a642cdd33cab/hgsub/modified.txt    * (glob)
> +  +++ root/hgsub/modified.txt    * (glob)
> +  @@ -1 +1 @@
> +  -test
> +  +second mod
> +  diff -Npru root.a642cdd33cab/hgsub/modified2.txt  
> root/hgsub/modified2.txt
> +  --- root.a642cdd33cab/hgsub/modified2.txt    * (glob)
> +  +++ root/hgsub/modified2.txt    * (glob)
> +  @@ -1 +1 @@
> +  -test2
> +  +second mod
>    [1]
>
> Side note: I was thinking about setting up a Kallithea instance to work  
> on
> this together without polluting the mailing list too much. What's your
> opinion?

I think mpm wants to keep all discussions on the list, for current  
learning and future archeology purposes.  It's also more likely to get a  
helpful suggestion from a casual reader.

It looks like either queuebot had a hiccup today, or he dumped his entire  
inbox to catchup.  So I'm sort of waiting until crew gets pulled into the  
main repo, so I can rebase/evolve the current mess of branches I have and  
reevaluate.  Once that happens, I'll resend what I have to reboot the  
discussion- it's too hard to follow it all over a several week period of  
deadends and wrong turns.

BTW, a (slightly?) cleaner variant on what you have above is maybe (I  
didn't try this yet) inside subrepo.archive(), instead of going right to  
accessing substate[1], call self.ctx.subrev(path).  committablectx has an  
override that returns None, maybe workingctx should too.  It isn't nice  
that this has to be looked up again (and in each subrepo class), but it is  
a cached property, so no real harm for the non workingctx case?  That  
would sidestep the issue with breaking status and diff (and probably other  
stuff), until that can be sorted out.

The only hurtle that I see is hgsubrepo doesn't have a parent ctx yet.   
I've been meaning to add one, but I can't shake the feeling that it would  
need to be updated outside of __init__.  (Consider what happens if to a  
nested subrepo if you revert its parent subrepo- it seems that its ctx  
would be wrong.)  But by simply adding it, it wouldn't be any more wrong  
than git and svn currently are.

--Matt

> Greetings,
> Mathias

Patch

diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -658,6 +658,8 @@  class hgsubrepo(abstractsubrepo):
     def diff(self, ui, diffopts, node2, match, prefix, **opts):
         try:
             node1 = node.bin(self._state[1])
+#            print('hgsub diff: "%s"' % self._state[1])
+
             # We currently expect node2 to come from substate and be
             # in hex format
             if node2 is not None:
@@ -676,6 +678,7 @@  class hgsubrepo(abstractsubrepo):
         total = abstractsubrepo.archive(self, archiver, prefix, match)
         rev = self._state[1]
         ctx = self._repo[rev]
+#        print('hgsub archive: "%s, ctx type is %s"' % (rev, type(ctx)))
         for subpath in ctx.substate:
             s = subrepo(ctx, subpath)
             submatch = matchmod.narrowmatcher(subpath, match)
diff --git a/tests/test-subrepo-extdiff.t b/tests/test-subrepo-extdiff.t
new file mode 100644
--- /dev/null
+++ b/tests/test-subrepo-extdiff.t
@@ -0,0 +1,111 @@ 
+  $ cat >> $HGRCPATH <<EOF
+  > [extensions]
+  > extdiff=
+  > EOF
+
+  $ hg init root
+  $ echo "hgsub = hgsub" > root/.hgsub
+  $ cd root
+  $ hg init hgsub
+  $ hg add .hgsub
+  $ echo test > hgsub/modified.txt
+  $ echo test2 > hgsub/modified2.txt
+  $ hg -R hgsub add hgsub/modified.txt
+  $ hg -R hgsub add hgsub/modified2.txt
+
+-------------------------------------------------------------------------------
+For hg subrepos that have not been locked in, changes are visible to diff,
but
+NOT extdiff.  self._state[1] is ''
+
+  $ hg status -S
+  A .hgsub
+  A hgsub/modified.txt
+  A hgsub/modified2.txt
+
+  $ hg diff -S
+  diff -r 000000000000 .hgsub
+  --- /dev/null    Thu Jan 01 00:00:00 1970 +0000
+  +++ b/.hgsub    * (glob)
+  @@ -0,0 +1,1 @@
+  +hgsub = hgsub
+  diff -r 000000000000 hgsub/modified.txt
+  --- /dev/null    Thu Jan 01 00:00:00 1970 +0000
+  +++ b/hgsub/modified.txt    * (glob)
+  @@ -0,0 +1,1 @@
+  +test
+  diff -r 000000000000 hgsub/modified2.txt
+  --- /dev/null    Thu Jan 01 00:00:00 1970 +0000
+  +++ b/hgsub/modified2.txt    * (glob)
+  @@ -0,0 +1,1 @@
+  +test2
+
+  $ hg extdiff -S
+  diff -Npru root.000000000000/.hgsub root/.hgsub
+  --- root.000000000000/.hgsub    1970-01-01 00:00:00* +0000 (glob)
+  +++ root/.hgsub    * (glob)
+  @@ -0,0 +1 @@
+  +hgsub = hgsub
+  [1]
+
+  $ hg ci -qm "modified.txt -> test" -S
+
+-------------------------------------------------------------------------------
+For hg subrepos that *have* been locked in, changes are visible to diff
+AND extdiff.  self._state[1] is 50a11ebc6587
+
+  $ echo 'second mod' > hgsub/modified.txt
+  $ echo 'second mod' > hgsub/modified2.txt
+
+  $ hg status -S
+  M hgsub/modified.txt
+  M hgsub/modified2.txt
+  $ hg diff -S
+  diff -r 4034090a5971 hgsub/modified.txt
+  --- a/hgsub/modified.txt    Thu Jan 01 00:00:00 1970 +0000
+  +++ b/hgsub/modified.txt    * (glob)
+  @@ -1,1 +1,1 @@
+  -test
+  +second mod
+  diff -r 4034090a5971 hgsub/modified2.txt
+  --- a/hgsub/modified2.txt    Thu Jan 01 00:00:00 1970 +0000
+  +++ b/hgsub/modified2.txt    * (glob)
+  @@ -1,1 +1,1 @@
+  -test2
+  +second mod
+  $ hg extdiff -S
+  [1]
+
+-------------------------------------------------------------------------------
+But adding a file to hgsubrepo causes extdiff to see *nothing*.
self_state[1]
+is still 50a11ebc6587, as it was when we had better results in the last
test.
+
+  $ echo 'added' > hgsub/added.txt
+#  $ hg -R hgsub add hgsub/added.txt
+  $ hg add hgsub/added.txt
+
+  $ hg status -S
+  M hgsub/modified.txt
+  M hgsub/modified2.txt
+  A hgsub/added.txt
+
+  $ hg diff -S
+  diff -r 4034090a5971 hgsub/added.txt
+  --- /dev/null    Thu Jan 01 00:00:00 1970 +0000
+  +++ b/hgsub/added.txt    * (glob)
+  @@ -0,0 +1,1 @@
+  +added
+  diff -r 4034090a5971 hgsub/modified.txt
+  --- a/hgsub/modified.txt    Thu Jan 01 00:00:00 1970 +0000
+  +++ b/hgsub/modified.txt    * (glob)
+  @@ -1,1 +1,1 @@
+  -test
+  +second mod
+  diff -r 4034090a5971 hgsub/modified2.txt
+  --- a/hgsub/modified2.txt    Thu Jan 01 00:00:00 1970 +0000
+  +++ b/hgsub/modified2.txt    * (glob)
+  @@ -1,1 +1,1 @@
+  -test2
+  +second mod
+
+  $ hg extdiff -S
+  [1]
diff --git a/tests/test-subrepo-git.t b/tests/test-subrepo-git.t
--- a/tests/test-subrepo-git.t
+++ b/tests/test-subrepo-git.t
@@ -698,6 +698,16 @@  check differences made by most recent ch
   +foo
   +bar

+  $ hg extdiff -S --config extensions.extdiff=
+  --- nul      1970-01-01 00:00:00 +0000
+  +++ $TESTTMP/tc/s/foobar     * (glob)
+  @@ -0,0 +1,4 @@
+  +woopwoop
+  +
+  +foo
+  +bar
+  [1]
+
   $ hg commit --subrepos -m "Added foobar"
   committing subrepository s
   created new head


After further digging, the only time I've seen this case triggered is if
> the subrepo hasn't been committed to the parent repo yet.  I was hoping
> this would be used as 'give me the wctx of this subrepo', but it's not.  I
> can see how it gets there in subrepo:145.
>
> I wonder if context.substate() should be overridden in workingctx to set
> the state to None, so that repo[None].substate always yields subrepo
> working directories.  We would need it for a revset symbol that indicates
> the working directory too.  It's different from repo['.'].substate, so I
> don't think it will break anything.  I'll try it and see what happens, but
> maybe a subrepo expert can chime in.
>

I agree that we probably need to move in that direction. Not sure who the
subrepo experts are, but remarks welcome.
Below is an example hack that can be applied to the above patch, it shows
what happens when the substate is also set to None (extdiff works, yay!).
This of course has nice side-effects like 'hg status -S' not working
correctly anymore, so I think we're in for a bit more work.

# HG changeset patch
# User Mathias De Maré <mathias.demare@gmail.com>
# Date 1424287953 -3600
#      Mit Feb 18 20:32:33 2015 +0100
# Node ID a1ec50dc0168e2e0798d74efaaf1e10df66f5a63
# Parent  ed6dfa605f0451605fc42658c5492d29e570eac7
subrepo: hack in propagation of top-level 'None' context

Previously, subrepo context was determined by looking at
substate. However, in the case where the context of
the parent repository is the working directory (repo[None]),
this leads to incorrect archiving.

This patch is a hack, but it's purely meant to show what type of change
would be required to fix subrepo archiving.

(Note as a side-effect of this hack that 'hg st -S' is now messed up.

diff --git a/mercurial/archival.py b/mercurial/archival.py
--- a/mercurial/archival.py
+++ b/mercurial/archival.py
@@ -305,6 +305,7 @@  def archive(repo, dest, node, kind, deco

     if subrepos:
         for subpath in sorted(ctx.substate):
+#            repo.ui.write("going to archive, toplevel rev: %s\n" %
ctx.rev())
             sub = ctx.sub(subpath)
             submatch = matchmod.narrowmatcher(subpath, matchfn)
             total += sub.archive(archiver, prefix, submatch)
diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -334,8 +334,13 @@  def subrepo(ctx, path):

     pathutil.pathauditor(ctx._repo.root)(path)
     state = ctx.substate[path]
+#    ctx._repo.ui.write("substate: %s\n" % str(state))
     if state[2] not in types:
         raise util.Abort(_('unknown subrepo type %s') % state[2])
+    if ctx.rev() is None:
+        state = list(state)
+        state[1] = None
+        state = tuple(state)
     return types[state[2]](ctx, path, state[:2])

 def newcommitphase(ui, ctx):
@@ -657,7 +662,10 @@  class hgsubrepo(abstractsubrepo):
     @annotatesubrepoerror
     def diff(self, ui, diffopts, node2, match, prefix, **opts):
         try:
-            node1 = node.bin(self._state[1])
+            if self._state[1]:
+                node1 = node.bin(self._state[1])
+            else:
+                node1 = None
 #            print('hgsub diff: "%s"' % self._state[1])

             # We currently expect node2 to come from substate and be
@@ -728,6 +736,8 @@  class hgsubrepo(abstractsubrepo):

     def _get(self, state):
         source, revision, kind = state
+        if revision is None:
+            return True
         if revision in self._repo.unfiltered():
             return True
         self._repo._subsource = source
diff --git a/tests/test-subrepo-extdiff.t b/tests/test-subrepo-extdiff.t
--- a/tests/test-subrepo-extdiff.t
+++ b/tests/test-subrepo-extdiff.t
@@ -19,8 +19,6 @@  NOT extdiff.  self._state[1] is ''

   $ hg status -S
   A .hgsub
-  A hgsub/modified.txt
-  A hgsub/modified2.txt

   $ hg diff -S
   diff -r 000000000000 .hgsub
@@ -40,9 +38,8 @@  NOT extdiff.  self._state[1] is ''
   +test2

   $ hg extdiff -S
-  diff -Npru root.000000000000/.hgsub root/.hgsub
-  --- root.000000000000/.hgsub    1970-01-01 00:00:00* +0000 (glob)
-  +++ root/.hgsub    * (glob)
+  --- /dev/null    * (glob)
+  +++ $TESTTMP/root/.hgsub    * (glob)
   @@ -0,0 +1 @@
   +hgsub = hgsub
   [1]
@@ -73,6 +70,18 @@  AND extdiff.  self._state[1] is 50a11ebc
   -test2
   +second mod
   $ hg extdiff -S
+  diff -Npru root.a642cdd33cab/hgsub/modified.txt root/hgsub/modified.txt
+  --- root.a642cdd33cab/hgsub/modified.txt    * (glob)
+  +++ root/hgsub/modified.txt    * (glob)
+  @@ -1 +1 @@
+  -test
+  +second mod
+  diff -Npru root.a642cdd33cab/hgsub/modified2.txt root/hgsub/modified2.txt
+  --- root.a642cdd33cab/hgsub/modified2.txt    * (glob)
+  +++ root/hgsub/modified2.txt    * (glob)
+  @@ -1 +1 @@
+  -test2
+  +second mod
   [1]

 -------------------------------------------------------------------------------
@@ -108,4 +117,21 @@  is still 50a11ebc6587, as it was when we
   +second mod

   $ hg extdiff -S
+  diff -Npru root.a642cdd33cab/hgsub/added.txt root/hgsub/added.txt
+  --- root.a642cdd33cab/hgsub/added.txt    1970-01-01 00:00:00.000000000
+0000
+  +++ root/hgsub/added.txt    * (glob)
+  @@ -0,0 +1 @@
+  +added
+  diff -Npru root.a642cdd33cab/hgsub/modified.txt root/hgsub/modified.txt
+  --- root.a642cdd33cab/hgsub/modified.txt    * (glob)
+  +++ root/hgsub/modified.txt    * (glob)
+  @@ -1 +1 @@
+  -test
+  +second mod
+  diff -Npru root.a642cdd33cab/hgsub/modified2.txt root/hgsub/modified2.txt
+  --- root.a642cdd33cab/hgsub/modified2.txt    * (glob)
+  +++ root/hgsub/modified2.txt    * (glob)
+  @@ -1 +1 @@
+  -test2
+  +second mod
   [1]