It looks like the latter two hunks in this file will need to be able to read the merge state from an overlayworkingctx. Maybe that's coming in a later patch, but I think it would be easier to follow if this code was changed only then (very much related to my comment on an earlier patch).

Yeah, I'm happy to reorder patches, this is still a bit "work-in-progress unfortunately.

@martinvonz do you mean that these patch break normal behavior if taken without other patch ?

In D8563#128618, @marmoute wrote:

@martinvonz do you mean that these patch break normal behavior if taken without other patch ?

No, but I think those two places should be able to accept an overlayworkingctx, so it might be better to leave them unchanged for now and to switch them directly to use wctx.mergestate() once we have a wctx passed in here.

Is your concern about the creation of new "independant" wctc (by each call of repo[None]) instead of reusing the same wctx for the full operation ?

In D8563#128630, @marmoute wrote:

Is your concern about the creation of new "independant" wctc (by each call of repo[None]) instead of reusing the same wctx for the full operation ?

Yes. I guess I'd rather have the pattern wctx.mergestate() indicate that it works with either a workingctx or a overlayworkingctx, while direct access to mergestate.read() indicates that we care about the actual working copy.

In D8563#128768, @martinvonz wrote:

In D8563#128630, @marmoute wrote:

Is your concern about the creation of new "independant" wctc (by each call of repo[None]) instead of reusing the same wctx for the full operation ?

Yes. I guess I'd rather have the pattern wctx.mergestate() indicate that it works with either a workingctx or a overlayworkingctx, while direct access to mergestate.read() indicates that we care about the actual working copy.

Is there any reason to distinct the two ? From afar, it would be better for the mergestate module to become an implementation details and every access to happen throught the context for consistency.

Do you have concrêt example where distincting the two seems better ?

In D8563#128829, @marmoute wrote:

In D8563#128768, @martinvonz wrote:

In D8563#128630, @marmoute wrote:

Is your concern about the creation of new "independant" wctc (by each call of repo[None]) instead of reusing the same wctx for the full operation ?

Yes. I guess I'd rather have the pattern wctx.mergestate() indicate that it works with either a workingctx or a overlayworkingctx, while direct access to mergestate.read() indicates that we care about the actual working copy.

Is there any reason to distinct the two ? From afar, it would be better for the mergestate module to become an implementation details and every access to happen throught the context for consistency.
Do you have concrêt example where distincting the two seems better ?

Should we have a brief email thread or IRC discussion about this? We seem to be conflating workingctx and overlayworkingctx here, and I'm not sure we're all on the same page.

I can probably do a VC tomorrow (US morning time) if that help

Okay, now localrepo correctly uses the same context across whole operations. This almost certainly avoids some bugs, given that committing a memctx was (surprise!) reading the mergestate from disk.