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

[Sean Farley]

FUJIWARA Katsunori writes:

> At Wed, 02 Jul 2014 14:33:01 -0500,
> Sean Farley wrote:
>> 
>> FUJIWARA Katsunori writes:
>
>> > 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.
>
> Yes, I have a question about your work around "__contains__".
>
>
> (background)
>
> With a little more researching, I found that my explanation in the
> previous mail was not correct :-<

Well, it was onto something :-) I used an idea from Sid to set
substate=None in the __init__ method of memctx.

>> > 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"
>
> "memctx" uses the "__contains__" below derived from "committablectx":
>
>     def __contains__(self, key):
>         return self._repo.dirstate[key] not in "?r"
>
> Then, "f in ctx" on "memctx" returns the result value unexpectedly
> according to current dirstate.

Oops. memctx shouldn't be worrying about the dirstate.

>> >   - "_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.
>
> I confirmed that making working directory empty by "hg update null"
> and "hg debugrebuilddirstate" before "hg convert" can suppress
> unexpected failure in the case of this patch series.
>
>
> (question)
>
> Actions using "memctx" below may allow such dirstate dependent
> behavior:
>
>   - "collapse" of "hg histedit", and
>   - "hg import" wihtout "--bypass": these use working directory

There are a lot of commands that use the working directory. I view this
as an implementation detail for the most part. Take localrepo.commit,
for instance. It is completely dependent on the working directory but
doesn't need to be. My current approach to decouple this is:

- lift the code that makes a context object from the working directory
- separate that code out of localrepo.commit
- only have context.commit be the object that actually does the commit
  transaction

> But it may cause unexpected file handling in actions below:
>
>   - "hg lfconvert" of largefiles, and
>   - incremantal "hg convert" of convert: working directory may be in use
>   - "hg import --bypass": working directory should be ignored absolutely

That is wayyyyy off in the future for me (working on in-memory merge first).

> I think that current "__contains__" of "committablectx" should be
> defined in "workingctx", to make "memctx" use "__contains__" of
> "basectx", which was moved from "changectx" in your work below (of
> course, "handling removed files, too" should be needed for correct
> manifest calculation).
>
>     changeset:   19550:0c8ad779eb36
>     user:        Sean Farley <sean.michael.farley at gmail.com>
>     date:        Mon Aug 05 17:21:38 2013 -0500
>     summary:     basectx: move __contains__ from changectx
>
> But this replacement is a just backing out of your work below:
>
>     changeset:   19668:9d56a3359011
>     user:        Sean Farley <sean.michael.farley at gmail.com>
>     date:        Wed Aug 14 15:29:09 2013 -0500
>     summary:     commitablectx: move __contains__ from workingctx
>
>
> Is current "__contains__" hierarchy between "basectx",
> "committablectx", "workingctx" and "memctx" intentional ?

I'm pretty sure that was unintentional on my part. That might make the
substate=None patch unneeded, right?