[PATCH 2 of 2] obsstore: move delete function from obsstore class to repair module

[Pierre-Yves David]

On 04/13/2016 02:54 AM, Kostia Balytskyi wrote:
> On 4/13/16 6:49 AM, Pierre-Yves David wrote:
>>
>>
>> On 04/12/2016 04:10 AM, Kostia Balytskyi wrote:
>>> # HG changeset patch
>>> # User Kostia Balytskyi <ikostia at fb.com>
>>> # Date 1460459210 25200
>>> #      Tue Apr 12 04:06:50 2016 -0700
>>> # Node ID cd55c17037a71173ec5572b32b951f8007ea164f
>>> # Parent  33e53380588a5953e3f25d4ff66d1b2e27ec7820
>>> obsstore: move delete function from obsstore class to repair module
>>
>> Pushed thanks
>>
>> Can you move the transaction check and the locking inside the repair
>> function too?
> Why would we want to do so? My plan was to reuse this function in 
> future for commit+marker deletion. We might want to open a transaction 
> in an outer function that would later call deleteobsmarkers and we 
> don't want deleteobsmarkers to fail in that case. Am I wrong?

The rewrite of the obsstore file is incompatible with the way Mercurial 
transaction work (record the size of the file at opening time and 
truncate if the transaction rollback). The transaction assume the file 
is untouched and happened only. Changing file content under the 
transaction feet will lead to disaster.

The stripping of changesets (repair.strip) is also incompatible with a 
transaction and already explicitly check for open transaction within the 
function, same logic and rules should applies to markers stripping.

>> I've renamed "deletemarkers" to "deleteobsmarkers" because markers is a
>> bit too generic.
>>
>> The first patch would have won to be sliced with one cleanup par patch.
>> If one of the cleanup were wrong that would have avoided a global 
>> resend.
> Sorry, I did not get it. Can you re-phrase?

You first patch is doing different things:
1) add faulty entry in ValueError message,
2) update the transaction in progress message,
3) update locking
4) update delete docstring.

Please make this different patches next time you do such clean up.

Cheers,

-- 
Pierre-Yves David