[PATCH 11 of 11 sparse V3] sparse: move post commit actions into core

[Sean Farley]

Gregory Szorc <gregory.szorc at gmail.com> writes:

> On Thu, Jul 6, 2017 at 10:16 PM, Martin von Zweigbergk <
> martinvonz at google.com> wrote:
>
>> On Thu, Jul 6, 2017 at 6:18 PM, Gregory Szorc <gregory.szorc at gmail.com>
>> wrote:
>> > # HG changeset patch
>> > # User Gregory Szorc <gregory.szorc at gmail.com>
>> > # Date 1499379254 25200
>> > #      Thu Jul 06 15:14:14 2017 -0700
>> > # Node ID fe47ebda0b384aa1d3002c6db3c4bace60472337
>> > # Parent  1c1a25d842854cbb2a7516136f7bcaab0d9f4539
>> > sparse: move post commit actions into core
>> >
>> > Instead of wrapping committablectx.markcommitted(), we inline
>> > the call into localrepository.commit(), which is the only place
>> > we call markcommitted().
>> >
>> > The APIs and distribution of duties between
>> > localrepository.commit() and committablecontext.markcommitted()
>> > are unclear to me. As is the proper order for certain operations
>> > to be performed. There is room to improve this code.
>>
>> 4fb92f14a97a (commit: factor out post-commit cleanup into workingctx,
>> 2013-02-08) says that markcommitted() was created to prepare for
>> in-memory merge. Given that, it seems more appropriate to leave the
>> call to sparse.aftercommit() at the end of
>> committablectx.markcommitted(), no? (I say "leave" because that's
>> where it was essentially called from in the wrapper.)
>>
>> I'll queue 1-9 and let you think about this one a bit. Thanks!
>>
>
> Looking at this more, I'm not sure committablectx is the proper place for
> it. This sparse code interacts with working directory state. If
> committablectx (which is the base for memctx) is used outside of a working
> directory, it doesn't make a lot of sense to be touching the working
> directory from an in-memory commit. localrepository.commit() does, however,
> only operate on working directories.
>
> Perhaps this code belongs in workingctx.markcommitted()? But then again,
> committablectx.markcommitted() touches dirstate, which is specific to the
> working directory. So now I'm just confused as to what these various
> contexts are for and what each should and should not do.

Yes, anything touching the dirstate should be in workingctx. It was a
misunderstanding on my part that committablectx ever got dirstate stuff.

> I suppose I'll just preserve the code as part of
> commitablectx.markcommitted() in the next patchbomb. We can always clean it
> up later if it is wrong.

I have a series (I thought I sent it but it seems not) that cleans up
all that mess. So, for clarity,

- workingctx: anything touching the disk
- memctx: anything in memory
- committablectx: anything that is shared
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170707/e3e6180f/attachment.sig>