[darcs-users] darcs patch: test that tentative leftover is cleared (and 2 more)

Jason Dagit dagit at codersbase.com
Sun Sep 6 19:18:32 UTC 2009


On Sun, Sep 6, 2009 at 11:48 AM, Petr Rockai<me at mornfall.net> wrote:
> Hi,
>
> Jason Dagit <dagit at codersbase.com> writes:
>>> Tentative state corresponds to the real state of the repository. The
>>> representation is the same except for the tentative_pristine, which is
>>> stored as a sequence of patches, a diff from the real pristine.
>>
>> Sure.  That makes sense.
> Actually, I don't know about old-fashioned (there, this would indeed make
> sense), but for hashed repositories, tentative_pristine is actually an
> alternative root pointer into pristine.hashed. No patches involved at all.

This bug and patch only apply to old-fashioned.

>>> That would break consistency. remove_from_tentative_inventory should
>>> not assume that it is the only or first operation in a transaction
>>> (which actually holds, but nevertheless). There might have been an
>>> arbitrary number of operations on the tentative state before it, each
>>> of which could have modified the tentative_pristine. In other words,
>>> we should update the tentative_pristine, not create it based on the
>>> real pristine, which would drop all the changes previous operations in
>>> the transaction made.
>>
>> In that case, the patches we want to remove from the
>> tentative_pristine need to be removed carefully.  For example,
>> commuting them to the end and discarding them.  The code as written
>> writes the inverses to the tentative_pristine.  Do we make any effort
>> to later collapse inverses and remove the resulting identity patches?
>> The ideal case is that patches to be removed simply do not exist in
>> the tentative_pristine when we're done.  Failing that, we need to
>> simulate a 'rollback' which is what applying their inverse is all
>> about.
> This is tentative_*pristine*, not tentative patches. Even if we store patches
> in there, these will never materialise as actual patches in the repository,
> they exist solely for the purpose of application. (Hopefully.)

Yes, but if you look at the code and read over my description from the
most recent patch review, you'll see that with these changes we would
call write_patch on changes that are tentative.  Priory to that
write_patch was only called on patches that already existed in the
repository but which had been changed via a commute with the patches
to_remove.  With the change, write_patch is also called on patches
read from the tentative inventory.

Jason


More information about the darcs-users mailing list