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

[Matt Harbison]

On Tue, 16 Jun 2015 11:05:52 -0400, Yuya Nishihara <yuya at tcha.org> wrote:

> 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

Works for me.  The funny thing is I did something almost identical a few  
months ago and rejected it as too hacky.