[PATCH STABLE] convert: detect removal of ".gitmodules" at git source revisions correctly
FUJIWARA Katsunori
foozy at lares.dti.ne.jp
Wed Jul 2 13:19:39 UTC 2014
At Wed, 02 Jul 2014 12:44:04 +0900,
FUJIWARA Katsunori wrote:
> At Tue, 01 Jul 2014 18:46:08 -0500,
> Matt Mackall wrote:
> >
> > On Tue, 2014-07-01 at 15:47 -0500, Matt Mackall wrote:
> > > On Sun, 2014-06-29 at 00:11 +0900, FUJIWARA Katsunori wrote:
> > > > # HG changeset patch
> > > > # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> > > > # Date 1403966784 -32400
> > > > # Sat Jun 28 23:46:24 2014 +0900
> > > > # Branch stable
> > > > # Node ID cb8567d0c76d7c0e66d2fe32607acc8621d2389f
> > > > # Parent a4b67bf1f0a5051736c14d9c13fae50fd5f5e464
> > > > convert: detect removal of ".gitmodules" at git source revisions correctly
> > >
> > > Queued for stable, thanks.
> >
> > Breaks tests:
> >
> > $ hg convert -q git-repo6 git-repo6-hg
> > + transaction abort!
> > + rollback completed
> > + Traceback (most recent call last):
> > + File "/dev/shm/hgtests.uBEOuH/install/bin/hg", line 43, in <module>
> > + mercurial.dispatch.run()
> > + File "/dev/shm/hgtests.uBEOuH/install/lib/python/mercurial/dispatch.py", line 28, in run
> > + sys.exit((dispatch(request(sys.argv[1:])) or 0) & 255)
> > + File "/dev/shm/hgtests.uBEOuH/install/lib/python/mercurial/dispatch.py", line 69, in dispatch
> > + ret = _runcatch(req)
> > + File "/dev/shm/hgtests.uBEOuH/install/lib/python/mercurial/dispatch.py", line 228, in _runcatch
> > + elif util.safehasattr(inst, "args") and inst.args[0] == errno.EPIPE:
> > + IndexError: tuple index out of range
> > + [1]
> > $ hg -R git-repo6-hg tip -T "{desc|firstline}\n"
> > - remove .gitmodules and submodule git-repo5
> > + addsubmodule
> > $ hg -R git-repo6-hg tip -T "{file_dels}\n"
> > - .hgsub .hgsubstate
> > +
>
> "IndexError: tuple index out of range" itself seems to occurs, because
> IOError raised in "getfile()" in "hgext/convert/git.py" has empty
> "args".
>
>
> With a little monkey patching, I could get actual back trace of this
> unexpected failure.
>
> (BTW, this is a failure on "default" branch, isn't it ?)
>
> + File "/tmp/hgtests.Ac881r/install/lib/python/hgext/convert/hg.py", line 178, in putcommit
> + self.repo.commitctx(ctx)
> + File "/tmp/hgtests.Ac881r/install/lib/python/mercurial/localrepo.py", line 63, in wrapper
> + return orig(repo.unfiltered(), *args, **kwargs)
> + File "/tmp/hgtests.Ac881r/install/lib/python/mercurial/localrepo.py", line 1430, in commitctx
> + targetphase = subrepo.newcommitphase(self.ui, ctx)
> + File "/tmp/hgtests.Ac881r/install/lib/python/mercurial/subrepo.py", line 349, in newcommitphase
> + substate = getattr(ctx, "substate", None)
> + File "/tmp/hgtests.Ac881r/install/lib/python/mercurial/util.py", line 287, in __get__
> + result = self.func(obj)
> + File "/tmp/hgtests.Ac881r/install/lib/python/mercurial/context.py", line 136, in substate
> + return subrepo.state(self, self._repo.ui)
> + File "/tmp/hgtests.Ac881r/install/lib/python/mercurial/subrepo.py", line 86, in state
> + read('.hgsub')
> + File "/tmp/hgtests.Ac881r/install/lib/python/mercurial/subrepo.py", line 74, in read
> + data = ctx[f].data()
> + File "/tmp/hgtests.Ac881r/install/lib/python/mercurial/context.py", line 1594, in __getitem__
> + return self.filectx(key)
> + File "/tmp/hgtests.Ac881r/install/lib/python/mercurial/context.py", line 1636, in filectx
> + return self._filectxfn(self._repo, self, path)
> + File "/tmp/hgtests.Ac881r/install/lib/python/hgext/convert/hg.py", line 136, in getfilectx
> + data, mode = source.getfile(f, v)
> + File "/tmp/hgtests.Ac881r/install/lib/python/hgext/convert/convcmd.py", line 88, in getfile
> + return self.source.getfile(file, rev)
> + File "/tmp/hgtests.Ac881r/install/lib/python/hgext/convert/git.py", line 137, in getfile
> + raise IOError
> + IOError
>
>
> According to my "hg bisect", the revision below is the first revision,
> which causes this failure with this series.
>
> changeset: 21665:d2743be1bb06
> user: Sean Farley <sean.michael.farley at gmail.com>
> date: Thu Aug 15 15:00:03 2013 -0500
> summary: memctx: inherit from committablectx
>
>
> Before Sean's work above, "subrepo.newcommitphase()" returns
> immediately, because "memctx" (derived from "object") doesn't have
> "substate" property.
>
> ================
> def newcommitphase(ui, ctx):
> commitphase = phases.newcommitphase(ui)
> substate = getattr(ctx, "substate", None)
> if not substate:
> return commitphase
> ================
>
> In the other hand, after Sean's work, "memctxt" has "substate"
> inherited from "basectx" via "committablectx", and causes invocation
> of "subrepo.state" via "basectx.substate".
>
>
> After that, "'.hgsub' in ctx" examination on "memctx" in
> "subrepo.state" fails to detect removal of ".hgsub", because:
>
> - "f in ctx" invokes "f in self._manifest" via "__contains__" of
> "basectx"
>
> - "_manifest" of "committablectx" can detect removal of files, only
> if "self._status" contains also removed files correctly, but
>
> - "memctx" contains only modified (status[0]) files
> (removal of files are only recognized in
> "localrepository.commitctx" layer)
>
>
> Finally, "subrepo.state()" tries to get already removed ".hgsub" and
> causes unexpected failure.
>
>
> Then, what should we do to fix this problem ?
I found the another way to resolve this problem, suppressing subrepo
phase checking while converting. I'll send revised series.
> 1. make "memctx" take new "removed files" argument
>
> this is not reasonable (at least for stable), because this
> requires many changes around "memctx" construction.
>
>
> 2. make "localrepository.commitctx" put removed files into "memctx"
>
> "detecting additional removal of files" in "commitctx" means that
> the specified "ctx" should be "memctx", because other than "memctx"
> contain removed files in "ctx.removed()" (= "self._status[2]").
>
> ================
> removed = list(ctx.removed())
> :
> :
> try:
> fctx = ctx[f]
> :
> :
> except IOError, inst:
> errcode = getattr(inst, 'errno', errno.ENOENT)
> if error or errcode and errcode != errno.ENOENT:
> self.ui.warn(_("trouble committing %s!\n") % f)
> raise
> else:
> removed.append(f) # ctx should be "memctx" in this case
> :
> :
>
> # add code path below ....
> if not ctx.removed() and removed: # or using "isinstance()" ????
> ctx.setremoved(removed) # or "ctx._status[2] = removed" ????
> ================
>
> this is simple, but not smart.
>
>
> 3. examine "'.hgsub' in ctx" with try/except for IOError, before
> getting "substate" from ctx, in "newcommitphase"
>
> this is simple, but not smart.
>
> 4. and so on (any other good solutions ?)
>
> ----------------------------------------------------------------------
> [FUJIWARA Katsunori] foozy at lares.dti.ne.jp
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
----------------------------------------------------------------------
[FUJIWARA Katsunori] foozy at lares.dti.ne.jp
More information about the Mercurial-devel
mailing list