| Submitter | Yuya Nishihara |
|---|---|
| Date | June 16, 2015, 3:05 p.m. |
| Message ID | <20150617000552.97026a6b27f624d1f7f4de7d@tcha.org> |
| Download | mbox | patch |
| Permalink | /patch/9662/ |
| State | Superseded |
| Commit | 3ec8351fa6ed66ccfea9c9754abb4fe59fca959f |
| Headers | show |
Comments
On Tue, 16 Jun 2015 11:05:52 -0400, Yuya Nishihara <yuya@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@gmail.com> >> wrote: >> >> > On Mon, 15 Jun 2015 11:46:09 -0400, Yuya Nishihara <yuya@tcha.org> >> wrote: >> > >> >> On Mon, 15 Jun 2015 00:28:59 -0400, Matt Harbison wrote: >> >>> # HG changeset patch >> >>> # User Matt Harbison <matt_harbison@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.
Patch
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