[PATCH 3 of 3] archive: support 'wdir()'

[Yuya Nishihara]

On Mon, 15 Jun 2015 21:59:22 -0400, Matt Harbison wrote:
> On Mon, 15 Jun 2015 20:56:59 -0400, Matt Harbison <mharbison72 at gmail.com>  
> wrote:
> 
> > On Mon, 15 Jun 2015 11:46:09 -0400, Yuya Nishihara <yuya at tcha.org> wrote:
> >
> >> On Mon, 15 Jun 2015 00:28:59 -0400, Matt Harbison wrote:
> >>> # HG changeset patch
> >>> # User Matt Harbison <matt_harbison at yahoo.com>
> >>> # Date 1434328768 14400
> >>> #      Sun Jun 14 20:39:28 2015 -0400
> >>> # Node ID 18d376a661ffabe8121ab5d57b245f4f347f8cd1
> >>> # Parent  f5f5e4ae488d9cae8111e9a212f647ed1430a019
> >>> archive: support 'wdir()'
> >>
> >>> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> >>> --- a/mercurial/subrepo.py
> >>> +++ b/mercurial/subrepo.py
> >>> @@ -751,8 +751,8 @@
> >>>      def archive(self, archiver, prefix, match=None):
> >>>          self._get(self._state + ('hg',))
> >>>          total = abstractsubrepo.archive(self, archiver, prefix, match)
> >>> -        rev = self._state[1]
> >>> -        ctx = self._repo[rev]
> >>> +        ctx = self._getctx()
> >>> +
> >>>          for subpath in ctx.substate:
> >>>              s = subrepo(ctx, subpath)
> >>>              submatch = matchmod.narrowmatcher(subpath, match)
> >>> @@ -918,18 +918,13 @@
> >>>
> >>>      @annotatesubrepoerror
> >>>      def files(self):
> >>> -        rev = self._state[1]
> >>> -        ctx = self._repo[rev]
> >>> -        return ctx.manifest().keys()
> >>> +        return self._getctx().manifest().keys()
> >>>
> >>>      def filedata(self, name):
> >>> -        rev = self._state[1]
> >>> -        return self._repo[rev][name].data()
> >>> +        return self._getctx()[name].data()
> >>>
> >>>      def fileflags(self, name):
> >>> -        rev = self._state[1]
> >>> -        ctx = self._repo[rev]
> >>> -        return ctx.flags(name)
> >>> +        return self._getctx().flags(name)
> >>
> >> Can't we switch to the hgsubrepo object representing wdir?
> >
> > We don't have such a thing- subrepo is always created based on the  
> > .hgsubstate file.  Do you mean do a hack like this, to patch state[1] to  
> > be None?
> >
> >    https://www.selenic.com/pipermail/mercurial-devel/2015-June/070763.html
> >
> > I know previously when I tried getting subrepo.subrepo() to return a  
> > subrepo with state[1] == None if the calling context was wdir(), status  
> > and diff broke.  I can't seem to get it to fail the same way now, so  
> > I'll fiddle with that some more.
> 
> A bit of followup:
> 
> 1) sub._substate[1] must reflect what is written to .hgsubstate in p1(),  
> otherwise sub.dirty() is wrong.  Archive doesn't care about dirty, but  
> this detail precludes a general solution I think.

True. subrepo shouldn't reflect the working directory changes in general.
Perhaps "archive" is one of exceptional cases.

> 2) If we were to create a local subrepo object in archive that is hacked  
> up to represent wdir(), I think we have to do it each time a subrepo is  
> entered into.  It isn't a matter of redirecting once at the top and  
> everything just works.  And since creating a subrepo involves pathauditor  
> (or if we call the constructor directly, creating a new repo object and  
> another path.exists()), it seems that the 'if rev is None' duplication  
> below is actually cheaper.

Hmm, I thought something like the following. But I agree, it wasn't nice
because sub.archive() had to know it want a wdir subrepo anyway.

diff --git a/mercurial/archival.py b/mercurial/archival.py
--- a/mercurial/archival.py
+++ b/mercurial/archival.py
@@ -315,7 +315,7 @@ def archive(repo, dest, node, kind, deco
 
     if subrepos:
         for subpath in sorted(ctx.substate):
-            sub = ctx.sub(subpath)
+            sub = ctx.workingsub(subpath)
             submatch = matchmod.narrowmatcher(subpath, matchfn)
             total += sub.archive(archiver, prefix, submatch)
 
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -254,6 +254,11 @@ class basectx(object):
     def nullsub(self, path, pctx):
         return subrepo.nullsubrepo(self, path, pctx)
 
+    def workingsub(self, path):
+        state = self.substate[path]
+        state = (state[0], self.subrev(path), state[2])  # XXX hack
+        return subrepo.subrepo(self, path, state)
+
     def match(self, pats=[], include=None, exclude=None, default='glob',
               listsubrepos=False, badfn=None):
         r = self._repo
diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -324,7 +324,7 @@ def _sanitize(ui, vfs, ignore):
                           "in '%s'\n") % vfs.join(dirname))
                 vfs.unlink(vfs.reljoin(dirname, f))
 
-def subrepo(ctx, path):
+def subrepo(ctx, path, state=None):
     """return instance of the right subrepo class for subrepo in path"""
     # subrepo inherently violates our import layering rules
     # because it wants to make repo objects from deep inside the stack
@@ -335,7 +335,8 @@ def subrepo(ctx, path):
     hg = h
 
     pathutil.pathauditor(ctx.repo().root)(path)
-    state = ctx.substate[path]
+    if not state:  # XXX hack
+        state = ctx.substate[path]
     if state[2] not in types:
         raise util.Abort(_('unknown subrepo type %s') % state[2])
     return types[state[2]](ctx, path, state[:2])
@@ -754,7 +755,7 @@ class hgsubrepo(abstractsubrepo):
         rev = self._state[1]
         ctx = self._repo[rev]
         for subpath in ctx.substate:
-            s = subrepo(ctx, subpath)
+            s = ctx.workingsub(subpath)  # XXX smells bad
             submatch = matchmod.narrowmatcher(subpath, match)
             total += s.archive(archiver, prefix + self._path + '/', submatch)
         return total