[PATCH STABLE] convert: detect removal of ".gitmodules" at git source revisions correctly
FUJIWARA Katsunori
foozy at lares.dti.ne.jp
Tue Jul 1 22:44:04 CDT 2014
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 ?
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
More information about the Mercurial-devel
mailing list