Patchwork [4,of,4] transaction: remove the 'onabort' mechanism

login
register
mail settings
Submitter Pierre-Yves David
Date Dec. 9, 2014, 1:26 a.m.
Message ID <980580522580073e8c8e.1418088375@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/7024/
State Accepted
Commit 4c7ea2d9bad93fc120f3ab84dda0cf023cf7e036
Headers show

Comments

Pierre-Yves David - Dec. 9, 2014, 1:26 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1417729966 28800
#      Thu Dec 04 13:52:46 2014 -0800
# Node ID 980580522580073e8c8e5ea1b7f765dca8abd53a
# Parent  6187b0dbeaf5ce313b6dae85f1f01c3ec67e866f
transaction: remove the 'onabort' mechanism

It has no known users. If someones needs similar functionality, a new 'addabort'
method similar to 'addfinalize' should be added.
Durham Goode - Dec. 9, 2014, 7:36 p.m.
On 12/8/14 5:26 PM, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1417729966 28800
> #      Thu Dec 04 13:52:46 2014 -0800
> # Node ID 980580522580073e8c8e5ea1b7f765dca8abd53a
> # Parent  6187b0dbeaf5ce313b6dae85f1f01c3ec67e866f
> transaction: remove the 'onabort' mechanism
>
> It has no known users. If someones needs similar functionality, a new 'addabort'
> method similar to 'addfinalize' should be added.
>
Series looks sane.  Queued to the clowncopter.
Gregory Szorc - Dec. 10, 2014, 4:26 p.m.
On 12/8/14 5:26 PM, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1417729966 28800
> #      Thu Dec 04 13:52:46 2014 -0800
> # Node ID 980580522580073e8c8e5ea1b7f765dca8abd53a
> # Parent  6187b0dbeaf5ce313b6dae85f1f01c3ec67e866f
> transaction: remove the 'onabort' mechanism
>
> It has no known users. If someones needs similar functionality, a new 'addabort'
> method similar to 'addfinalize' should be added.

You just broke Facebook's hgsql extension: 
https://bitbucket.org/facebook/hgsql/src/b8ec4d48df21025f74d3b892408af91200d91ac6/hgsql.py?at=default#cl-289

On a related note, Mozilla also wants to hook other transaction-based 
systems up to Mercurial's transaction object. I was kinda relying on the 
existence of _abort() to handle rollback. I guess I'll have to write 
'addabort' patches now :/
Pierre-Yves David - Dec. 11, 2014, 12:34 a.m.
On 12/10/2014 08:26 AM, Gregory Szorc wrote:
> On 12/8/14 5:26 PM, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>> # Date 1417729966 28800
>> #      Thu Dec 04 13:52:46 2014 -0800
>> # Node ID 980580522580073e8c8e5ea1b7f765dca8abd53a
>> # Parent  6187b0dbeaf5ce313b6dae85f1f01c3ec67e866f
>> transaction: remove the 'onabort' mechanism
>>
>> It has no known users. If someones needs similar functionality, a new
>> 'addabort'
>> method similar to 'addfinalize' should be added.
>
> You just broke Facebook's hgsql extension:
> https://bitbucket.org/facebook/hgsql/src/b8ec4d48df21025f74d3b892408af91200d91ac6/hgsql.py?at=default#cl-289

Good catch, thanks.

> On a related note, Mozilla also wants to hook other transaction-based
> systems up to Mercurial's transaction object. I was kinda relying on the
> existence of _abort() to handle rollback. I guess I'll have to write
> 'addabort' patches now :/

Yup, I think it is a more consistent and powerful path.
Durham Goode - Dec. 11, 2014, 1:05 a.m.
On 12/10/14 4:34 PM, Pierre-Yves David wrote:
>
> On 12/10/2014 08:26 AM, Gregory Szorc wrote:
>> On 12/8/14 5:26 PM, Pierre-Yves David wrote:
>>> # HG changeset patch
>>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>>> # Date 1417729966 28800
>>> #      Thu Dec 04 13:52:46 2014 -0800
>>> # Node ID 980580522580073e8c8e5ea1b7f765dca8abd53a
>>> # Parent  6187b0dbeaf5ce313b6dae85f1f01c3ec67e866f
>>> transaction: remove the 'onabort' mechanism
>>>
>>> It has no known users. If someones needs similar functionality, a new
>>> 'addabort'
>>> method similar to 'addfinalize' should be added.
>>
>> You just broke Facebook's hgsql extension:
>> https://urldefense.proofpoint.com/v1/url?u=https://bitbucket.org/facebook/hgsql/src/b8ec4d48df21025f74d3b892408af91200d91ac6/hgsql.py?at%3Ddefault%23cl-289&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=pHOG6Hz51SkYmYr%2FxoTFzw%3D%3D%0A&m=CijdtF%2BO4%2FdAmOJ1rMXlyKKE7tw6BqyH%2F4%2FP3fRQ7uU%3D%0A&s=78f2dbce723513eeb22dbc3bdcc9be6a6a049c462a23345b57a483099ba466fd 
>>
>
> Good catch, thanks.
I don't think it broke hgsql.  hgsql wraps transaction._abort which 
wasn't touched by your changes.  Though I haven't actually tried to run 
hgsql with the new mercurial bits yet.

Patch

diff --git a/mercurial/transaction.py b/mercurial/transaction.py
--- a/mercurial/transaction.py
+++ b/mercurial/transaction.py
@@ -82,20 +82,18 @@  def _playback(journal, report, opener, v
         # only pure backup file remains, it is sage to ignore any error
         pass
 
 class transaction(object):
     def __init__(self, report, opener, vfsmap, journal, after=None,
-                 createmode=None, onabort=None):
+                 createmode=None):
         """Begin a new transaction
 
         Begins a new transaction that allows rolling back writes in the event of
         an exception.
 
         * `after`: called after the transaction has been committed
         * `createmode`: the mode of the journal file that will be created
-        * `onabort`: called as the transaction is aborting, but before any files
-        have been truncated
         """
         self.count = 1
         self.usages = 1
         self.report = report
         # a vfs to the store content
@@ -103,11 +101,10 @@  class transaction(object):
         # a map to access file in various {location -> vfs}
         vfsmap = vfsmap.copy()
         vfsmap[''] = opener  # set default value
         self._vfsmap = vfsmap
         self.after = after
-        self.onabort = onabort
         self.entries = []
         self.map = {}
         self.journal = journal
         self._queue = []
         # a dict of arguments to be passed to hooks
@@ -434,13 +431,10 @@  class transaction(object):
         self.count = 0
         self.usages = 0
         self.file.close()
         self._backupsfile.close()
 
-        if self.onabort is not None:
-            self.onabort()
-
         try:
             if not self.entries and not self._backupentries:
                 if self.journal:
                     self.opener.unlink(self.journal)
                 if self._backupjournal: