| 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
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.
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 :/
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.
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: