<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 13, 2015 at 4:45 AM, Matt Harbison <span dir="ltr"><<a href="mailto:mharbison72@gmail.com" target="_blank">mharbison72@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class=""><div class="h5"><br></div></div>
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.<br></blockquote><div>As you might have noticed by now, I'm a bit strapped for time myself (so apologies for an even longer delay).<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
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.<span class=""><br></span></blockquote><div>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!<br></div><div>I only modified your patch for hg subrepos, but same goes for git and subversion repos.<br><br># HG changeset patch<br># User Matt Harbison <<a href="mailto:matt_harbison@yahoo.com">matt_harbison@yahoo.com</a>><br># Date 1423795917 18000<br># Don Feb 12 21:51:57 2015 -0500<br># Node ID ed6dfa605f0451605fc42658c5492d29e570eac7<br># Parent e7b1097a59713d2eb813c09013d71f7e1bb676ff<br>extdiff: basic hg subrepo testing<br><br>What extdiff sees when a subrepo has a modified file, and when it has an added<br>file is different for reasons unknown. A modified file in a git subrepo also<br>works as we would like (i.e. the modified file is archived), but apparently it<br>doesn't archive an added file either.<br><br>Changes by Mathias: modified files are NOT seen by extdiff either! They are when<br>it's just one, but that's because archive is only called for MORE than one file!<br><br>diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py<br>--- a/mercurial/subrepo.py<br>+++ b/mercurial/subrepo.py<br>@@ -658,6 +658,8 @@ class hgsubrepo(abstractsubrepo):<br> def diff(self, ui, diffopts, node2, match, prefix, **opts):<br> try:<br> node1 = node.bin(self._state[1])<br>+# print('hgsub diff: "%s"' % self._state[1])<br>+<br> # We currently expect node2 to come from substate and be<br> # in hex format<br> if node2 is not None:<br>@@ -676,6 +678,7 @@ class hgsubrepo(abstractsubrepo):<br> total = abstractsubrepo.archive(self, archiver, prefix, match)<br> rev = self._state[1]<br> ctx = self._repo[rev]<br>+# print('hgsub archive: "%s, ctx type is %s"' % (rev, type(ctx)))<br> for subpath in ctx.substate:<br> s = subrepo(ctx, subpath)<br> submatch = matchmod.narrowmatcher(subpath, match)<br>diff --git a/tests/test-subrepo-extdiff.t b/tests/test-subrepo-extdiff.t<br>new file mode 100644<br>--- /dev/null<br>+++ b/tests/test-subrepo-extdiff.t<br>@@ -0,0 +1,111 @@<br>+ $ cat >> $HGRCPATH <<EOF<br>+ > [extensions]<br>+ > extdiff=<br>+ > EOF<br>+<br>+ $ hg init root<br>+ $ echo "hgsub = hgsub" > root/.hgsub<br>+ $ cd root<br>+ $ hg init hgsub<br>+ $ hg add .hgsub<br>+ $ echo test > hgsub/modified.txt<br>+ $ echo test2 > hgsub/modified2.txt<br>+ $ hg -R hgsub add hgsub/modified.txt<br>+ $ hg -R hgsub add hgsub/modified2.txt<br>+<br>+-------------------------------------------------------------------------------<br>+For hg subrepos that have not been locked in, changes are visible to diff, but<br>+NOT extdiff. self._state[1] is ''<br>+<br>+ $ hg status -S<br>+ A .hgsub<br>+ A hgsub/modified.txt<br>+ A hgsub/modified2.txt<br>+<br>+ $ hg diff -S<br>+ diff -r 000000000000 .hgsub<br>+ --- /dev/null Thu Jan 01 00:00:00 1970 +0000<br>+ +++ b/.hgsub * (glob)<br>+ @@ -0,0 +1,1 @@<br>+ +hgsub = hgsub<br>+ diff -r 000000000000 hgsub/modified.txt<br>+ --- /dev/null Thu Jan 01 00:00:00 1970 +0000<br>+ +++ b/hgsub/modified.txt * (glob)<br>+ @@ -0,0 +1,1 @@<br>+ +test<br>+ diff -r 000000000000 hgsub/modified2.txt<br>+ --- /dev/null Thu Jan 01 00:00:00 1970 +0000<br>+ +++ b/hgsub/modified2.txt * (glob)<br>+ @@ -0,0 +1,1 @@<br>+ +test2<br>+<br>+ $ hg extdiff -S<br>+ diff -Npru root.000000000000/.hgsub root/.hgsub<br>+ --- root.000000000000/.hgsub 1970-01-01 00:00:00* +0000 (glob)<br>+ +++ root/.hgsub * (glob)<br>+ @@ -0,0 +1 @@<br>+ +hgsub = hgsub<br>+ [1]<br>+<br>+ $ hg ci -qm "modified.txt -> test" -S<br>+<br>+-------------------------------------------------------------------------------<br>+For hg subrepos that *have* been locked in, changes are visible to diff<br>+AND extdiff. self._state[1] is 50a11ebc6587<br>+<br>+ $ echo 'second mod' > hgsub/modified.txt<br>+ $ echo 'second mod' > hgsub/modified2.txt<br>+<br>+ $ hg status -S<br>+ M hgsub/modified.txt<br>+ M hgsub/modified2.txt<br>+ $ hg diff -S<br>+ diff -r 4034090a5971 hgsub/modified.txt<br>+ --- a/hgsub/modified.txt Thu Jan 01 00:00:00 1970 +0000<br>+ +++ b/hgsub/modified.txt * (glob)<br>+ @@ -1,1 +1,1 @@<br>+ -test<br>+ +second mod<br>+ diff -r 4034090a5971 hgsub/modified2.txt<br>+ --- a/hgsub/modified2.txt Thu Jan 01 00:00:00 1970 +0000<br>+ +++ b/hgsub/modified2.txt * (glob)<br>+ @@ -1,1 +1,1 @@<br>+ -test2<br>+ +second mod<br>+ $ hg extdiff -S<br>+ [1]<br>+<br>+-------------------------------------------------------------------------------<br>+But adding a file to hgsubrepo causes extdiff to see *nothing*. self_state[1]<br>+is still 50a11ebc6587, as it was when we had better results in the last test.<br>+<br>+ $ echo 'added' > hgsub/added.txt<br>+# $ hg -R hgsub add hgsub/added.txt<br>+ $ hg add hgsub/added.txt<br>+<br>+ $ hg status -S<br>+ M hgsub/modified.txt<br>+ M hgsub/modified2.txt<br>+ A hgsub/added.txt<br>+<br>+ $ hg diff -S<br>+ diff -r 4034090a5971 hgsub/added.txt<br>+ --- /dev/null Thu Jan 01 00:00:00 1970 +0000<br>+ +++ b/hgsub/added.txt * (glob)<br>+ @@ -0,0 +1,1 @@<br>+ +added<br>+ diff -r 4034090a5971 hgsub/modified.txt<br>+ --- a/hgsub/modified.txt Thu Jan 01 00:00:00 1970 +0000<br>+ +++ b/hgsub/modified.txt * (glob)<br>+ @@ -1,1 +1,1 @@<br>+ -test<br>+ +second mod<br>+ diff -r 4034090a5971 hgsub/modified2.txt<br>+ --- a/hgsub/modified2.txt Thu Jan 01 00:00:00 1970 +0000<br>+ +++ b/hgsub/modified2.txt * (glob)<br>+ @@ -1,1 +1,1 @@<br>+ -test2<br>+ +second mod<br>+<br>+ $ hg extdiff -S<br>+ [1]<br>diff --git a/tests/test-subrepo-git.t b/tests/test-subrepo-git.t<br>--- a/tests/test-subrepo-git.t<br>+++ b/tests/test-subrepo-git.t<br>@@ -698,6 +698,16 @@ check differences made by most recent ch<br> +foo<br> +bar<br> <br>+ $ hg extdiff -S --config extensions.extdiff=<br>+ --- nul 1970-01-01 00:00:00 +0000<br>+ +++ $TESTTMP/tc/s/foobar * (glob)<br>+ @@ -0,0 +1,4 @@<br>+ +woopwoop<br>+ +<br>+ +foo<br>+ +bar<br>+ [1]<br>+<br> $ hg commit --subrepos -m "Added foobar"<br> committing subrepository s<br> created new head<br></div><div> <br><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class=""></span>
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.<br>
<br>
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.<span class=""><br></span></blockquote><div><br></div><div>I agree that we probably need to move in that direction. Not sure who the subrepo experts are, but remarks welcome.<br></div><div>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.<br><br># HG changeset patch<br># User Mathias De Maré <<a href="mailto:mathias.demare@gmail.com">mathias.demare@gmail.com</a>><br># Date 1424287953 -3600<br># Mit Feb 18 20:32:33 2015 +0100<br># Node ID a1ec50dc0168e2e0798d74efaaf1e10df66f5a63<br># Parent ed6dfa605f0451605fc42658c5492d29e570eac7<br>subrepo: hack in propagation of top-level 'None' context<br><br>Previously, subrepo context was determined by looking at<br>substate. However, in the case where the context of<br>the parent repository is the working directory (repo[None]),<br>this leads to incorrect archiving.<br><br>This patch is a hack, but it's purely meant to show what type of change<br>would be required to fix subrepo archiving.<br><br>(Note as a side-effect of this hack that 'hg st -S' is now messed up.<br><br>diff --git a/mercurial/archival.py b/mercurial/archival.py<br>--- a/mercurial/archival.py<br>+++ b/mercurial/archival.py<br>@@ -305,6 +305,7 @@ def archive(repo, dest, node, kind, deco<br> <br> if subrepos:<br> for subpath in sorted(ctx.substate):<br>+# repo.ui.write("going to archive, toplevel rev: %s\n" % ctx.rev())<br> sub = ctx.sub(subpath)<br> submatch = matchmod.narrowmatcher(subpath, matchfn)<br> total += sub.archive(archiver, prefix, submatch)<br>diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py<br>--- a/mercurial/subrepo.py<br>+++ b/mercurial/subrepo.py<br>@@ -334,8 +334,13 @@ def subrepo(ctx, path):<br> <br> pathutil.pathauditor(ctx._repo.root)(path)<br> state = ctx.substate[path]<br>+# ctx._repo.ui.write("substate: %s\n" % str(state))<br> if state[2] not in types:<br> raise util.Abort(_('unknown subrepo type %s') % state[2])<br>+ if ctx.rev() is None:<br>+ state = list(state)<br>+ state[1] = None<br>+ state = tuple(state)<br> return types[state[2]](ctx, path, state[:2])<br> <br> def newcommitphase(ui, ctx):<br>@@ -657,7 +662,10 @@ class hgsubrepo(abstractsubrepo):<br> @annotatesubrepoerror<br> def diff(self, ui, diffopts, node2, match, prefix, **opts):<br> try:<br>- node1 = node.bin(self._state[1])<br>+ if self._state[1]:<br>+ node1 = node.bin(self._state[1])<br>+ else:<br>+ node1 = None<br> # print('hgsub diff: "%s"' % self._state[1])<br> <br> # We currently expect node2 to come from substate and be<br>@@ -728,6 +736,8 @@ class hgsubrepo(abstractsubrepo):<br> <br> def _get(self, state):<br> source, revision, kind = state<br>+ if revision is None:<br>+ return True<br> if revision in self._repo.unfiltered():<br> return True<br> self._repo._subsource = source<br>diff --git a/tests/test-subrepo-extdiff.t b/tests/test-subrepo-extdiff.t<br>--- a/tests/test-subrepo-extdiff.t<br>+++ b/tests/test-subrepo-extdiff.t<br>@@ -19,8 +19,6 @@ NOT extdiff. self._state[1] is ''<br> <br> $ hg status -S<br> A .hgsub<br>- A hgsub/modified.txt<br>- A hgsub/modified2.txt<br> <br> $ hg diff -S<br> diff -r 000000000000 .hgsub<br>@@ -40,9 +38,8 @@ NOT extdiff. self._state[1] is ''<br> +test2<br> <br> $ hg extdiff -S<br>- diff -Npru root.000000000000/.hgsub root/.hgsub<br>- --- root.000000000000/.hgsub 1970-01-01 00:00:00* +0000 (glob)<br>- +++ root/.hgsub * (glob)<br>+ --- /dev/null * (glob)<br>+ +++ $TESTTMP/root/.hgsub * (glob)<br> @@ -0,0 +1 @@<br> +hgsub = hgsub<br> [1]<br>@@ -73,6 +70,18 @@ AND extdiff. self._state[1] is 50a11ebc<br> -test2<br> +second mod<br> $ hg extdiff -S<br>+ diff -Npru root.a642cdd33cab/hgsub/modified.txt root/hgsub/modified.txt<br>+ --- root.a642cdd33cab/hgsub/modified.txt * (glob)<br>+ +++ root/hgsub/modified.txt * (glob)<br>+ @@ -1 +1 @@<br>+ -test<br>+ +second mod<br>+ diff -Npru root.a642cdd33cab/hgsub/modified2.txt root/hgsub/modified2.txt<br>+ --- root.a642cdd33cab/hgsub/modified2.txt * (glob)<br>+ +++ root/hgsub/modified2.txt * (glob)<br>+ @@ -1 +1 @@<br>+ -test2<br>+ +second mod<br> [1]<br> <br> -------------------------------------------------------------------------------<br>@@ -108,4 +117,21 @@ is still 50a11ebc6587, as it was when we<br> +second mod<br> <br> $ hg extdiff -S<br>+ diff -Npru root.a642cdd33cab/hgsub/added.txt root/hgsub/added.txt<br>+ --- root.a642cdd33cab/hgsub/added.txt 1970-01-01 00:00:00.000000000 +0000<br>+ +++ root/hgsub/added.txt * (glob)<br>+ @@ -0,0 +1 @@<br>+ +added<br>+ diff -Npru root.a642cdd33cab/hgsub/modified.txt root/hgsub/modified.txt<br>+ --- root.a642cdd33cab/hgsub/modified.txt * (glob)<br>+ +++ root/hgsub/modified.txt * (glob)<br>+ @@ -1 +1 @@<br>+ -test<br>+ +second mod<br>+ diff -Npru root.a642cdd33cab/hgsub/modified2.txt root/hgsub/modified2.txt<br>+ --- root.a642cdd33cab/hgsub/modified2.txt * (glob)<br>+ +++ root/hgsub/modified2.txt * (glob)<br>+ @@ -1 +1 @@<br>+ -test2<br>+ +second mod<br> [1]<br><br></div><div>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?<br><br></div><div>Greetings,<br>Mathias<br></div></div></div></div>