[PATCH] repair: forbid strip from inside a transaction

[Pierre-Yves David]

On 05/27/2015 08:15 AM, Augie Fackler wrote:
> On Tue, May 26, 2015 at 06:40:41PM -0700, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david at fb.com>
>> # Date 1432441127 25200
>> #      Sat May 23 21:18:47 2015 -0700
>> # Node ID 0c71ff45e361550948fcc3cbfa03898c8ba9fd96
>> # Parent  61b3529e23778f542044d24bb1ccd5688516a7f0
>> repair: forbid strip from inside a transaction
>>
>> Striping inside a transaction will, at best, crash or , at worst, result in very
>
> Spellcheck betrayed you: stripping (as opposed to striping).
>
> (copyediting here because there's one sentence in a comment I can't parse.)
>
>> unexpected result. We explicitly forbid it early.
>>
>> diff --git a/mercurial/repair.py b/mercurial/repair.py
>> --- a/mercurial/repair.py
>> +++ b/mercurial/repair.py
>> @@ -149,10 +149,16 @@ def strip(ui, repo, nodelist, backup=Tru
>>           chgrpfile = _bundle(repo, savebases, saveheads, node, 'temp',
>>                               compress=False)
>>
>>       mfst = repo.manifest
>>
>> +    curtr = repo.currenttransaction()
>> +    if curtr is not None:
>> +        del curtr  # avoid carrying reference to transaction for nothing
>> +        msg = _('programming error: cannot strip from inside a transaction')
>> +        raise util.Abort(msg, hint=_('contact your extensions maintainer'))
>
> "contact your extension maintainer"
>
>> +
>>       tr = repo.transaction("strip")
>>       offset = len(tr.entries)
>>
>>       try:
>>           tr.startgroup()
>> diff --git a/tests/test-devel-warnings.t b/tests/test-devel-warnings.t
>> --- a/tests/test-devel-warnings.t
>> +++ b/tests/test-devel-warnings.t
>> @@ -1,11 +1,11 @@
>>
>>     $ cat << EOF > buggylocking.py
>>     > """A small extension that acquire locks in the wrong order
>>     > """
>>     >
>> -  > from mercurial import cmdutil
>> +  > from mercurial import cmdutil, repair
>>     >
>>     > cmdtable = {}
>>     > command = cmdutil.command(cmdtable)
>>     >
>>     > @command('buggylocking', [], '')
>> @@ -36,10 +36,17 @@
>>     > def nowaitlocking(ui, repo):
>>     >     lo = repo.lock()
>>     >     wl = repo.wlock(wait=False)
>>     >     wl.release()
>>     >     lo.release()
>> +  >
>> +  > @command('stripintr', [], '')
>> +  > def stripintr(ui, repo):
>> +  >     lo = repo.lock()
>> +  >     # not a warning no worth testing.
>
> Not sure what this sentence should be (the "no" doesn't make sense)

me neither, lets drop the line.

-- 
Pierre-Yves David