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

login
register
mail settings
Submitter Matt Harbison
Date Feb. 9, 2015, 4:01 a.m.
Message ID <9c4f27e5c804662d2d25.1423454497@Envy>
Download mbox | patch
Permalink /patch/7770/
State Changes Requested
Headers show

Comments

Matt Harbison - Feb. 9, 2015, 4:01 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1342370590 14400
#      Sun Jul 15 12:43:10 2012 -0400
# Node ID 9c4f27e5c804662d2d25581ad3856ef6da3729ec
# Parent  6abceaa1a49f82cebd3a4f141f69558e2bb3cec4
extdiff: use -S to archive the full repo

The working copy snapshot into the subrepo(s) is still missing.  (The hangup is
how to pass the 'wfn not in ctx' check, and it would be nice to get all types of
subrepo for free.  Maybe the solution is to change archive.archival to accept a
context instead of a node, which would allow it to archive the working copy too.
The subrepos would have to be changed to support this, but that doesn't mean the
command line needs to see this capability.)

There should be tests without the largefiles extension loaded too.
Mathias De Maré - Feb. 9, 2015, 7:28 a.m.
On Mon, Feb 9, 2015 at 5:01 AM, Matt Harbison <mharbison72@gmail.com> wrote:

> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1342370590 14400
> #      Sun Jul 15 12:43:10 2012 -0400
> # Node ID 9c4f27e5c804662d2d25581ad3856ef6da3729ec
> # Parent  6abceaa1a49f82cebd3a4f141f69558e2bb3cec4
> extdiff: use -S to archive the full repo
>
> The working copy snapshot into the subrepo(s) is still missing.  (The
> hangup is
> how to pass the 'wfn not in ctx' check, and it would be nice to get all
> types of
> subrepo for free.  Maybe the solution is to change archive.archival to
> accept a
> context instead of a node, which would allow it to archive the working
> copy too.
> The subrepos would have to be changed to support this, but that doesn't
> mean the
> command line needs to see this capability.)
>

I guess both options are possible:
1. Extending archival to support the working directory.
2. Special-casing snapshot to copy the files.

I'm not sure what's the best choice here.

Other than that dilemma, this patch looks good to me.
Augie Fackler - Feb. 9, 2015, 11:02 p.m.
On Sun, Feb 08, 2015 at 11:01:37PM -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1342370590 14400
> #      Sun Jul 15 12:43:10 2012 -0400
> # Node ID 9c4f27e5c804662d2d25581ad3856ef6da3729ec
> # Parent  6abceaa1a49f82cebd3a4f141f69558e2bb3cec4
> extdiff: use -S to archive the full repo
>
> The working copy snapshot into the subrepo(s) is still missing.

This sentence makes no sense to me. Can you reprhase it?

> (The hangup is
> how to pass the 'wfn not in ctx' check, and it would be nice to get all types of
> subrepo for free.  Maybe the solution is to change archive.archival to accept a
> context instead of a node, which would allow it to archive the working copy too.
> The subrepos would have to be changed to support this, but that doesn't mean the
> command line needs to see this capability.)
>
> There should be tests without the largefiles extension loaded too.
>
> diff --git a/hgext/extdiff.py b/hgext/extdiff.py
> --- a/hgext/extdiff.py
> +++ b/hgext/extdiff.py
> @@ -70,7 +70,7 @@
>  command = cmdutil.command(cmdtable)
>  testedwith = 'internal'
>
> -def snapshot(ui, repo, files, node, tmproot):
> +def snapshot(ui, repo, files, node, tmproot, listsubrepos):
>      '''snapshot files as of some revision
>      if not using snapshot, -I/-X does not work and recursive diff
>      in tools like kdiff3 and meld displays too many files.'''
> @@ -93,7 +93,8 @@
>              repo.ui.setconfig("ui", "archivemeta", False)
>
>              archival.archive(repo, base, node, 'files',
> -                             matchfn=scmutil.matchfiles(repo, files))
> +                             matchfn=scmutil.matchfiles(repo, files),
> +                             subrepos=listsubrepos)
>      else:
>          ui.note(_('making snapshot of %d files from working directory\n') %
>              (len(files)))
> @@ -154,10 +155,14 @@
>          if node1b == nullid:
>              do3way = False
>
> +    subrepos=opts.get('subrepos')
> +
>      matcher = scmutil.match(repo[node2], pats, opts)
> -    mod_a, add_a, rem_a = map(set, repo.status(node1a, node2, matcher)[:3])
> +    mod_a, add_a, rem_a = map(set, repo.status(node1a, node2, matcher,
> +                                               listsubrepos=subrepos)[:3])
>      if do3way:
> -        mod_b, add_b, rem_b = map(set, repo.status(node1b, node2, matcher)[:3])
> +        mod_b, add_b, rem_b = map(set, repo.status(node1b, node2, matcher,
> +                                                   listsubrepos=subrepos)[:3])
>      else:
>          mod_b, add_b, rem_b = set(), set(), set()
>      modadd = mod_a | add_a | mod_b | add_b
> @@ -169,11 +174,12 @@
>      try:
>          # Always make a copy of node1a (and node1b, if applicable)
>          dir1a_files = mod_a | rem_a | ((mod_b | add_b) - add_a)
> -        dir1a = snapshot(ui, repo, dir1a_files, node1a, tmproot)[0]
> +        dir1a = snapshot(ui, repo, dir1a_files, node1a, tmproot, subrepos)[0]
>          rev1a = '@%d' % repo[node1a].rev()
>          if do3way:
>              dir1b_files = mod_b | rem_b | ((mod_a | add_a) - add_b)
> -            dir1b = snapshot(ui, repo, dir1b_files, node1b, tmproot)[0]
> +            dir1b = snapshot(ui, repo, dir1b_files, node1b, tmproot,
> +                             subrepos)[0]
>              rev1b = '@%d' % repo[node1b].rev()
>          else:
>              dir1b = None
> @@ -185,14 +191,15 @@
>          dir2root = ''
>          rev2 = ''
>          if node2:
> -            dir2 = snapshot(ui, repo, modadd, node2, tmproot)[0]
> +            dir2 = snapshot(ui, repo, modadd, node2, tmproot, subrepos)[0]
>              rev2 = '@%d' % repo[node2].rev()
>          elif len(common) > 1:
>              #we only actually need to get the files to copy back to
>              #the working dir in this case (because the other cases
>              #are: diffing 2 revisions or single file -- in which case
>              #the file is already directly passed to the diff tool).
> -            dir2, fns_and_mtime = snapshot(ui, repo, modadd, None, tmproot)
> +            dir2, fns_and_mtime = snapshot(ui, repo, modadd, None, tmproot,
> +                                           subrepos)
>          else:
>              # This lets the diff tool open the changed file directly
>              dir2 = ''
> @@ -260,7 +267,7 @@
>       _('pass option to comparison program'), _('OPT')),
>      ('r', 'rev', [], _('revision'), _('REV')),
>      ('c', 'change', '', _('change made by revision'), _('REV')),
> -    ] + commands.walkopts,
> +    ] + commands.walkopts + commands.subrepoopts,
>      _('hg extdiff [OPT]... [FILE]...'),
>      inferrepo=True)
>  def extdiff(ui, repo, *pats, **opts):
> diff --git a/tests/test-subrepo-deep-nested-change.t b/tests/test-subrepo-deep-nested-change.t
> --- a/tests/test-subrepo-deep-nested-change.t
> +++ b/tests/test-subrepo-deep-nested-change.t
> @@ -427,7 +427,7 @@
>
>  Interaction with extdiff, largefiles and subrepos
>
> -  $ hg --config extensions.extdiff= extdiff
> +  $ hg --config extensions.extdiff= extdiff -S
>    diff -Npru cloned.6e907bf12afc/.hglf/a.dat cloned/.hglf/a.dat
>    --- cloned.6e907bf12afc/.hglf/a.dat	1970-01-01 00:00:00 +0000
>    +++ cloned/.hglf/a.dat	* (glob)
> @@ -435,7 +435,7 @@
>    +
>    [1]
>
> -  $ hg --config extensions.extdiff= extdiff -r .^
> +  $ hg --config extensions.extdiff= extdiff -r .^ -S
>    diff -Npru cloned.1f79fbad5d0f/.hglf/a.dat cloned/.hglf/a.dat
>    --- cloned.1f79fbad5d0f/.hglf/a.dat	1970-01-01 00:00:00 +0000
>    +++ cloned/.hglf/a.dat	* (glob)
> @@ -453,13 +453,30 @@
>    +changed
>    [1]
>
> -  $ hg --config extensions.extdiff= extdiff -r 0 -r .^
> +  $ hg --config extensions.extdiff= extdiff -r 0 -r .^ -S
>    diff -Npru cloned.7f491f53a367/.hgsubstate cloned.1f79fbad5d0f/.hgsubstate
>    --- cloned.7f491f53a367/.hgsubstate	* (glob)
>    +++ cloned.1f79fbad5d0f/.hgsubstate	* (glob)
>    @@ -1 +1 @@
>    -fc3b4ce2696f7741438c79207583768f2ce6b0dd sub1
>    +7a36fa02b66e61f27f3d4a822809f159479b8ab2 sub1
> +  diff -Npru cloned.7f491f53a367/sub1/.hgsubstate cloned.1f79fbad5d0f/sub1/.hgsubstate
> +  --- cloned.7f491f53a367/sub1/.hgsubstate	* (glob)
> +  +++ cloned.1f79fbad5d0f/sub1/.hgsubstate	* (glob)
> +  @@ -1 +1 @@
> +  -c57a0840e3badd667ef3c3ef65471609acb2ba3c sub2
> +  +c77908c81ccea3794a896c79e98b0e004aee2e9e sub2
> +  diff -Npru cloned.7f491f53a367/sub1/sub2/folder/test.txt cloned.1f79fbad5d0f/sub1/sub2/folder/test.txt
> +  --- cloned.7f491f53a367/sub1/sub2/folder/test.txt	* (glob)
> +  +++ cloned.1f79fbad5d0f/sub1/sub2/folder/test.txt	* (glob)
> +  @@ -0,0 +1 @@
> +  +subfolder
> +  diff -Npru cloned.7f491f53a367/sub1/sub2/sub2 cloned.1f79fbad5d0f/sub1/sub2/sub2
> +  --- cloned.7f491f53a367/sub1/sub2/sub2	* (glob)
> +  +++ cloned.1f79fbad5d0f/sub1/sub2/sub2	* (glob)
> +  @@ -1 +1 @@
> +  -sub2
> +  +modified
>    [1]
>
>    $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Matt Harbison - Feb. 10, 2015, 12:52 a.m.
On Mon, 09 Feb 2015 18:02:25 -0500, Augie Fackler <raf@durin42.com> wrote:

> On Sun, Feb 08, 2015 at 11:01:37PM -0500, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1342370590 14400
>> #      Sun Jul 15 12:43:10 2012 -0400
>> # Node ID 9c4f27e5c804662d2d25581ad3856ef6da3729ec
>> # Parent  6abceaa1a49f82cebd3a4f141f69558e2bb3cec4
>> extdiff: use -S to archive the full repo
>>
>> The working copy snapshot into the subrepo(s) is still missing.
>
> This sentence makes no sense to me. Can you reprhase it?

Basically, archive -S can extract the necessary files into the snapshot  
directory, from both the parent and the subrepo.  But archive doesn't work  
on 'repo[None]', so I left the old code in there to (partially) handle  
that case.  The old code doesn't recurse into subrepos though.  So this  
meant "working copy snapshot *of* the subrepo(s) is still missing- only  
the parent repo is in the snapshot of the working directory".

I'm not sure what the right thing is here.  A revset symbol for the  
working directory was recently mentioned on the ML.  Presumably archive  
should support it.  But it doesn't look like the subrepo code supports  
"give me a subrepo containing its working context".  It looks like the  
state tuple can only contain a committed revision identifier?  So archive  
-S wouldn't be able to support such a symbol without changes to subrepo.

OTOH, Mathias pointed out util.copyfile(), which is probably easier, and  
looks like it would work with largefiles without any issues.  But then we  
lose the 'wfn not in ctx' check.

>> (The hangup is
>> how to pass the 'wfn not in ctx' check, and it would be nice to get all  
>> types of
>> subrepo for free.  Maybe the solution is to change archive.archival to  
>> accept a
>> context instead of a node, which would allow it to archive the working  
>> copy too.
>> The subrepos would have to be changed to support this, but that doesn't  
>> mean the
>> command line needs to see this capability.)
>>
>> There should be tests without the largefiles extension loaded too.
>>
>> diff --git a/hgext/extdiff.py b/hgext/extdiff.py
>> --- a/hgext/extdiff.py
>> +++ b/hgext/extdiff.py
>> @@ -70,7 +70,7 @@
>>  command = cmdutil.command(cmdtable)
>>  testedwith = 'internal'
>>
>> -def snapshot(ui, repo, files, node, tmproot):
>> +def snapshot(ui, repo, files, node, tmproot, listsubrepos):
>>      '''snapshot files as of some revision
>>      if not using snapshot, -I/-X does not work and recursive diff
>>      in tools like kdiff3 and meld displays too many files.'''
>> @@ -93,7 +93,8 @@
>>              repo.ui.setconfig("ui", "archivemeta", False)
>>
>>              archival.archive(repo, base, node, 'files',
>> -                             matchfn=scmutil.matchfiles(repo, files))
>> +                             matchfn=scmutil.matchfiles(repo, files),
>> +                             subrepos=listsubrepos)
>>      else:
>>          ui.note(_('making snapshot of %d files from working  
>> directory\n') %
>>              (len(files)))
>> @@ -154,10 +155,14 @@
>>          if node1b == nullid:
>>              do3way = False
>>
>> +    subrepos=opts.get('subrepos')
>> +
>>      matcher = scmutil.match(repo[node2], pats, opts)
>> -    mod_a, add_a, rem_a = map(set, repo.status(node1a, node2,  
>> matcher)[:3])
>> +    mod_a, add_a, rem_a = map(set, repo.status(node1a, node2, matcher,
>> +                                                
>> listsubrepos=subrepos)[:3])
>>      if do3way:
>> -        mod_b, add_b, rem_b = map(set, repo.status(node1b, node2,  
>> matcher)[:3])
>> +        mod_b, add_b, rem_b = map(set, repo.status(node1b, node2,  
>> matcher,
>> +                                                    
>> listsubrepos=subrepos)[:3])
>>      else:
>>          mod_b, add_b, rem_b = set(), set(), set()
>>      modadd = mod_a | add_a | mod_b | add_b
>> @@ -169,11 +174,12 @@
>>      try:
>>          # Always make a copy of node1a (and node1b, if applicable)
>>          dir1a_files = mod_a | rem_a | ((mod_b | add_b) - add_a)
>> -        dir1a = snapshot(ui, repo, dir1a_files, node1a, tmproot)[0]
>> +        dir1a = snapshot(ui, repo, dir1a_files, node1a, tmproot,  
>> subrepos)[0]
>>          rev1a = '@%d' % repo[node1a].rev()
>>          if do3way:
>>              dir1b_files = mod_b | rem_b | ((mod_a | add_a) - add_b)
>> -            dir1b = snapshot(ui, repo, dir1b_files, node1b, tmproot)[0]
>> +            dir1b = snapshot(ui, repo, dir1b_files, node1b, tmproot,
>> +                             subrepos)[0]
>>              rev1b = '@%d' % repo[node1b].rev()
>>          else:
>>              dir1b = None
>> @@ -185,14 +191,15 @@
>>          dir2root = ''
>>          rev2 = ''
>>          if node2:
>> -            dir2 = snapshot(ui, repo, modadd, node2, tmproot)[0]
>> +            dir2 = snapshot(ui, repo, modadd, node2, tmproot,  
>> subrepos)[0]
>>              rev2 = '@%d' % repo[node2].rev()
>>          elif len(common) > 1:
>>              #we only actually need to get the files to copy back to
>>              #the working dir in this case (because the other cases
>>              #are: diffing 2 revisions or single file -- in which case
>>              #the file is already directly passed to the diff tool).
>> -            dir2, fns_and_mtime = snapshot(ui, repo, modadd, None,  
>> tmproot)
>> +            dir2, fns_and_mtime = snapshot(ui, repo, modadd, None,  
>> tmproot,
>> +                                           subrepos)
>>          else:
>>              # This lets the diff tool open the changed file directly
>>              dir2 = ''
>> @@ -260,7 +267,7 @@
>>       _('pass option to comparison program'), _('OPT')),
>>      ('r', 'rev', [], _('revision'), _('REV')),
>>      ('c', 'change', '', _('change made by revision'), _('REV')),
>> -    ] + commands.walkopts,
>> +    ] + commands.walkopts + commands.subrepoopts,
>>      _('hg extdiff [OPT]... [FILE]...'),
>>      inferrepo=True)
>>  def extdiff(ui, repo, *pats, **opts):
>> diff --git a/tests/test-subrepo-deep-nested-change.t  
>> b/tests/test-subrepo-deep-nested-change.t
>> --- a/tests/test-subrepo-deep-nested-change.t
>> +++ b/tests/test-subrepo-deep-nested-change.t
>> @@ -427,7 +427,7 @@
>>
>>  Interaction with extdiff, largefiles and subrepos
>>
>> -  $ hg --config extensions.extdiff= extdiff
>> +  $ hg --config extensions.extdiff= extdiff -S
>>    diff -Npru cloned.6e907bf12afc/.hglf/a.dat cloned/.hglf/a.dat
>>    --- cloned.6e907bf12afc/.hglf/a.dat	1970-01-01 00:00:00 +0000
>>    +++ cloned/.hglf/a.dat	* (glob)
>> @@ -435,7 +435,7 @@
>>    +
>>    [1]
>>
>> -  $ hg --config extensions.extdiff= extdiff -r .^
>> +  $ hg --config extensions.extdiff= extdiff -r .^ -S
>>    diff -Npru cloned.1f79fbad5d0f/.hglf/a.dat cloned/.hglf/a.dat
>>    --- cloned.1f79fbad5d0f/.hglf/a.dat	1970-01-01 00:00:00 +0000
>>    +++ cloned/.hglf/a.dat	* (glob)
>> @@ -453,13 +453,30 @@
>>    +changed
>>    [1]
>>
>> -  $ hg --config extensions.extdiff= extdiff -r 0 -r .^
>> +  $ hg --config extensions.extdiff= extdiff -r 0 -r .^ -S
>>    diff -Npru cloned.7f491f53a367/.hgsubstate  
>> cloned.1f79fbad5d0f/.hgsubstate
>>    --- cloned.7f491f53a367/.hgsubstate	* (glob)
>>    +++ cloned.1f79fbad5d0f/.hgsubstate	* (glob)
>>    @@ -1 +1 @@
>>    -fc3b4ce2696f7741438c79207583768f2ce6b0dd sub1
>>    +7a36fa02b66e61f27f3d4a822809f159479b8ab2 sub1
>> +  diff -Npru cloned.7f491f53a367/sub1/.hgsubstate  
>> cloned.1f79fbad5d0f/sub1/.hgsubstate
>> +  --- cloned.7f491f53a367/sub1/.hgsubstate	* (glob)
>> +  +++ cloned.1f79fbad5d0f/sub1/.hgsubstate	* (glob)
>> +  @@ -1 +1 @@
>> +  -c57a0840e3badd667ef3c3ef65471609acb2ba3c sub2
>> +  +c77908c81ccea3794a896c79e98b0e004aee2e9e sub2
>> +  diff -Npru cloned.7f491f53a367/sub1/sub2/folder/test.txt  
>> cloned.1f79fbad5d0f/sub1/sub2/folder/test.txt
>> +  --- cloned.7f491f53a367/sub1/sub2/folder/test.txt	* (glob)
>> +  +++ cloned.1f79fbad5d0f/sub1/sub2/folder/test.txt	* (glob)
>> +  @@ -0,0 +1 @@
>> +  +subfolder
>> +  diff -Npru cloned.7f491f53a367/sub1/sub2/sub2  
>> cloned.1f79fbad5d0f/sub1/sub2/sub2
>> +  --- cloned.7f491f53a367/sub1/sub2/sub2	* (glob)
>> +  +++ cloned.1f79fbad5d0f/sub1/sub2/sub2	* (glob)
>> +  @@ -1 +1 @@
>> +  -sub2
>> +  +modified
>>    [1]
>>
>>    $ cd ..
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
Matt Harbison - Feb. 10, 2015, 3:39 a.m.
On Mon, 09 Feb 2015 19:52:56 -0500, Matt Harbison <mharbison72@gmail.com>  
wrote:

> On Mon, 09 Feb 2015 18:02:25 -0500, Augie Fackler <raf@durin42.com>  
> wrote:
>
>> On Sun, Feb 08, 2015 at 11:01:37PM -0500, Matt Harbison wrote:
>>> # HG changeset patch
>>> # User Matt Harbison <matt_harbison@yahoo.com>
>>> # Date 1342370590 14400
>>> #      Sun Jul 15 12:43:10 2012 -0400
>>> # Node ID 9c4f27e5c804662d2d25581ad3856ef6da3729ec
>>> # Parent  6abceaa1a49f82cebd3a4f141f69558e2bb3cec4
>>> extdiff: use -S to archive the full repo
>>>
>>> The working copy snapshot into the subrepo(s) is still missing.
>>
>> This sentence makes no sense to me. Can you reprhase it?
>
> Basically, archive -S can extract the necessary files into the snapshot  
> directory, from both the parent and the subrepo.  But archive doesn't  
> work on 'repo[None]', so I left the old code in there to (partially)  
> handle that case.  The old code doesn't recurse into subrepos though.   
> So this meant "working copy snapshot *of* the subrepo(s) is still  
> missing- only the parent repo is in the snapshot of the working  
> directory".
>
> I'm not sure what the right thing is here.  A revset symbol for the  
> working directory was recently mentioned on the ML.  Presumably archive  
> should support it.  But it doesn't look like the subrepo code supports  
> "give me a subrepo containing its working context".  It looks like the  
> state tuple can only contain a committed revision identifier?  So  
> archive -S wouldn't be able to support such a symbol without changes to  
> subrepo.
>
> OTOH, Mathias pointed out util.copyfile(), which is probably easier, and  
> looks like it would work with largefiles without any issues.  But then  
> we lose the 'wfn not in ctx' check.

Scratch that.  Archive *does* support taking the working directory (the  
command line doesn't), and it looks like it even grabs the uncommitted  
files in hgsubrepo too.  But largefiles aborts when trying to archive the  
working copy, so I think that needs to be fixed first to avoid breaking  
something that works now.  I'm not sure why I special cased 'node is None'  
2 years ago, and I didn't double check yesterday.

Mathias- can you test with dirty files in git?  You just need pull the new  
archiving lines out of the 'if node is not None' check, and drop the  
opener and for loop left over in the else case.  I did a quick comparison  
against 'diff -S' output, and it looks sane.

--Matt
Mathias De Maré - Feb. 10, 2015, 7:36 p.m.
On Tue, Feb 10, 2015 at 4:39 AM, Matt Harbison <mharbison72@gmail.com>
wrote:

> On Mon, 09 Feb 2015 19:52:56 -0500, Matt Harbison <mharbison72@gmail.com>
> wrote:
>
>  On Mon, 09 Feb 2015 18:02:25 -0500, Augie Fackler <raf@durin42.com>
>> wrote:
>>
>>  On Sun, Feb 08, 2015 at 11:01:37PM -0500, Matt Harbison wrote:
>>>
>>>> # HG changeset patch
>>>> # User Matt Harbison <matt_harbison@yahoo.com>
>>>> # Date 1342370590 14400
>>>> #      Sun Jul 15 12:43:10 2012 -0400
>>>> # Node ID 9c4f27e5c804662d2d25581ad3856ef6da3729ec
>>>> # Parent  6abceaa1a49f82cebd3a4f141f69558e2bb3cec4
>>>> extdiff: use -S to archive the full repo
>>>>
>>>> The working copy snapshot into the subrepo(s) is still missing.
>>>>
>>>
>>> This sentence makes no sense to me. Can you reprhase it?
>>>
>>
>> Basically, archive -S can extract the necessary files into the snapshot
>> directory, from both the parent and the subrepo.  But archive doesn't work
>> on 'repo[None]', so I left the old code in there to (partially) handle that
>> case.  The old code doesn't recurse into subrepos though.  So this meant
>> "working copy snapshot *of* the subrepo(s) is still missing- only the
>> parent repo is in the snapshot of the working directory".
>>
>> I'm not sure what the right thing is here.  A revset symbol for the
>> working directory was recently mentioned on the ML.  Presumably archive
>> should support it.  But it doesn't look like the subrepo code supports
>> "give me a subrepo containing its working context".  It looks like the
>> state tuple can only contain a committed revision identifier?  So archive
>> -S wouldn't be able to support such a symbol without changes to subrepo.
>>
>> OTOH, Mathias pointed out util.copyfile(), which is probably easier, and
>> looks like it would work with largefiles without any issues.  But then we
>> lose the 'wfn not in ctx' check.
>>
>
> Scratch that.  Archive *does* support taking the working directory (the
> command line doesn't), and it looks like it even grabs the uncommitted
> files in hgsubrepo too.  But largefiles aborts when trying to archive the
> working copy, so I think that needs to be fixed first to avoid breaking
> something that works now.  I'm not sure why I special cased 'node is None'
> 2 years ago, and I didn't double check yesterday.
>
> Mathias- can you test with dirty files in git?  You just need pull the new
> archiving lines out of the 'if node is not None' check, and drop the opener
> and for loop left over in the else case.  I did a quick comparison against
> 'diff -S' output, and it looks sane.
>

Adding a single file in the working directory worked fine (since it only
does the snapshot for the old (non-working dir) context).
When I add a file and remove another, I get 'abort: no files match the
archive pattern' when the snapshot of the working directory is done. I had
a further look, and it seems (archive.py:307-310) like for the
subrepositories, the revision is always extracted by checking the substate.
As a result, the last revision is used (instead of the working directory).
Specifically for Git, I have the impression there's additionally the
problem that 'git archive' does not support archiving the working directory.

If I then commit the added and removed file and run extdiff on those, it
works fine.

So to get the working directory working, we'll also need to modify the
support for archiving subrepos in general and git subrepos in particular.

Greetings,
Mathias

>
> --Matt
>

Patch

diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -70,7 +70,7 @@ 
 command = cmdutil.command(cmdtable)
 testedwith = 'internal'
 
-def snapshot(ui, repo, files, node, tmproot):
+def snapshot(ui, repo, files, node, tmproot, listsubrepos):
     '''snapshot files as of some revision
     if not using snapshot, -I/-X does not work and recursive diff
     in tools like kdiff3 and meld displays too many files.'''
@@ -93,7 +93,8 @@ 
             repo.ui.setconfig("ui", "archivemeta", False)
 
             archival.archive(repo, base, node, 'files',
-                             matchfn=scmutil.matchfiles(repo, files))
+                             matchfn=scmutil.matchfiles(repo, files),
+                             subrepos=listsubrepos)
     else:
         ui.note(_('making snapshot of %d files from working directory\n') %
             (len(files)))
@@ -154,10 +155,14 @@ 
         if node1b == nullid:
             do3way = False
 
+    subrepos=opts.get('subrepos')
+
     matcher = scmutil.match(repo[node2], pats, opts)
-    mod_a, add_a, rem_a = map(set, repo.status(node1a, node2, matcher)[:3])
+    mod_a, add_a, rem_a = map(set, repo.status(node1a, node2, matcher,
+                                               listsubrepos=subrepos)[:3])
     if do3way:
-        mod_b, add_b, rem_b = map(set, repo.status(node1b, node2, matcher)[:3])
+        mod_b, add_b, rem_b = map(set, repo.status(node1b, node2, matcher,
+                                                   listsubrepos=subrepos)[:3])
     else:
         mod_b, add_b, rem_b = set(), set(), set()
     modadd = mod_a | add_a | mod_b | add_b
@@ -169,11 +174,12 @@ 
     try:
         # Always make a copy of node1a (and node1b, if applicable)
         dir1a_files = mod_a | rem_a | ((mod_b | add_b) - add_a)
-        dir1a = snapshot(ui, repo, dir1a_files, node1a, tmproot)[0]
+        dir1a = snapshot(ui, repo, dir1a_files, node1a, tmproot, subrepos)[0]
         rev1a = '@%d' % repo[node1a].rev()
         if do3way:
             dir1b_files = mod_b | rem_b | ((mod_a | add_a) - add_b)
-            dir1b = snapshot(ui, repo, dir1b_files, node1b, tmproot)[0]
+            dir1b = snapshot(ui, repo, dir1b_files, node1b, tmproot,
+                             subrepos)[0]
             rev1b = '@%d' % repo[node1b].rev()
         else:
             dir1b = None
@@ -185,14 +191,15 @@ 
         dir2root = ''
         rev2 = ''
         if node2:
-            dir2 = snapshot(ui, repo, modadd, node2, tmproot)[0]
+            dir2 = snapshot(ui, repo, modadd, node2, tmproot, subrepos)[0]
             rev2 = '@%d' % repo[node2].rev()
         elif len(common) > 1:
             #we only actually need to get the files to copy back to
             #the working dir in this case (because the other cases
             #are: diffing 2 revisions or single file -- in which case
             #the file is already directly passed to the diff tool).
-            dir2, fns_and_mtime = snapshot(ui, repo, modadd, None, tmproot)
+            dir2, fns_and_mtime = snapshot(ui, repo, modadd, None, tmproot,
+                                           subrepos)
         else:
             # This lets the diff tool open the changed file directly
             dir2 = ''
@@ -260,7 +267,7 @@ 
      _('pass option to comparison program'), _('OPT')),
     ('r', 'rev', [], _('revision'), _('REV')),
     ('c', 'change', '', _('change made by revision'), _('REV')),
-    ] + commands.walkopts,
+    ] + commands.walkopts + commands.subrepoopts,
     _('hg extdiff [OPT]... [FILE]...'),
     inferrepo=True)
 def extdiff(ui, repo, *pats, **opts):
diff --git a/tests/test-subrepo-deep-nested-change.t b/tests/test-subrepo-deep-nested-change.t
--- a/tests/test-subrepo-deep-nested-change.t
+++ b/tests/test-subrepo-deep-nested-change.t
@@ -427,7 +427,7 @@ 
 
 Interaction with extdiff, largefiles and subrepos
 
-  $ hg --config extensions.extdiff= extdiff
+  $ hg --config extensions.extdiff= extdiff -S
   diff -Npru cloned.6e907bf12afc/.hglf/a.dat cloned/.hglf/a.dat
   --- cloned.6e907bf12afc/.hglf/a.dat	1970-01-01 00:00:00 +0000
   +++ cloned/.hglf/a.dat	* (glob)
@@ -435,7 +435,7 @@ 
   +
   [1]
 
-  $ hg --config extensions.extdiff= extdiff -r .^
+  $ hg --config extensions.extdiff= extdiff -r .^ -S
   diff -Npru cloned.1f79fbad5d0f/.hglf/a.dat cloned/.hglf/a.dat
   --- cloned.1f79fbad5d0f/.hglf/a.dat	1970-01-01 00:00:00 +0000
   +++ cloned/.hglf/a.dat	* (glob)
@@ -453,13 +453,30 @@ 
   +changed
   [1]
 
-  $ hg --config extensions.extdiff= extdiff -r 0 -r .^
+  $ hg --config extensions.extdiff= extdiff -r 0 -r .^ -S
   diff -Npru cloned.7f491f53a367/.hgsubstate cloned.1f79fbad5d0f/.hgsubstate
   --- cloned.7f491f53a367/.hgsubstate	* (glob)
   +++ cloned.1f79fbad5d0f/.hgsubstate	* (glob)
   @@ -1 +1 @@
   -fc3b4ce2696f7741438c79207583768f2ce6b0dd sub1
   +7a36fa02b66e61f27f3d4a822809f159479b8ab2 sub1
+  diff -Npru cloned.7f491f53a367/sub1/.hgsubstate cloned.1f79fbad5d0f/sub1/.hgsubstate
+  --- cloned.7f491f53a367/sub1/.hgsubstate	* (glob)
+  +++ cloned.1f79fbad5d0f/sub1/.hgsubstate	* (glob)
+  @@ -1 +1 @@
+  -c57a0840e3badd667ef3c3ef65471609acb2ba3c sub2
+  +c77908c81ccea3794a896c79e98b0e004aee2e9e sub2
+  diff -Npru cloned.7f491f53a367/sub1/sub2/folder/test.txt cloned.1f79fbad5d0f/sub1/sub2/folder/test.txt
+  --- cloned.7f491f53a367/sub1/sub2/folder/test.txt	* (glob)
+  +++ cloned.1f79fbad5d0f/sub1/sub2/folder/test.txt	* (glob)
+  @@ -0,0 +1 @@
+  +subfolder
+  diff -Npru cloned.7f491f53a367/sub1/sub2/sub2 cloned.1f79fbad5d0f/sub1/sub2/sub2
+  --- cloned.7f491f53a367/sub1/sub2/sub2	* (glob)
+  +++ cloned.1f79fbad5d0f/sub1/sub2/sub2	* (glob)
+  @@ -1 +1 @@
+  -sub2
+  +modified
   [1]
 
   $ cd ..