[PATCH STABLE] convert: detect removal of ".gitmodules" at git source revisions correctly

[Sean Farley]

FUJIWARA Katsunori writes:


[...]

> "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 ?)

Yes, this must be on default.

> +    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".

Yes, this is definitely my fault. Both hg-git and hgsubversion have
suffered from this as well.

> 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.

Thanks for the great explanation!

> 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.

I've started work on something like this. Basically, manipulate the
manifest directly to indicate modified, added, or removed files. This is
needed for in-memory merge but will also help get rid of
localrepository.commitctx and the like. In turn, this will help revamp
the record interface.

If you have any input, please do chime in. I'm eager to get my work on
the list and get some feedback.