( )⚙ D12125 transaction: do not rely on a global variable to post_finalize file

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12125

transaction: do not rely on a global variable to post_finalize file
ClosedPublic

Authored by marmoute on Jan 31 2022, 1:48 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG21ac6aedd5e5: transaction: do not rely on a global variable to post_finalize file
Summary

We can just add a new argument to the addfilegenerator function. This is more
explicit and therefor clearer and less error prone.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

marmoute created this revision.Jan 31 2022, 1:48 PM
Alphare accepted this revision.Feb 3 2022, 5:36 AM
This revision is now accepted and ready to land.Feb 3 2022, 5:36 AM

Revision Contents
Changeset List

Diff 32021

mercurial/bookmarks.py

self._recordchange(tr) self._recordchange(tr)
def _recordchange(self, tr): def _recordchange(self, tr):
"""record that bookmarks have been changed in a transaction """record that bookmarks have been changed in a transaction
The transaction is then responsible for updating the file content.""" The transaction is then responsible for updating the file content."""
location = b'' if bookmarksinstore(self._repo) else b'plain' location = b'' if bookmarksinstore(self._repo) else b'plain'
tr.addfilegenerator( tr.addfilegenerator(
b'bookmarks', (b'bookmarks',), self._write, location=location b'bookmarks',
(b'bookmarks',),
self._write,
location=location,
post_finalize=True,
) )
tr.hookargs[b'bookmark_moved'] = b'1' tr.hookargs[b'bookmark_moved'] = b'1'
def _writerepo(self, repo): def _writerepo(self, repo):
"""Factored out for extensibility""" """Factored out for extensibility"""
rbm = repo._bookmarks rbm = repo._bookmarks
if rbm.active not in self._refmap: if rbm.active not in self._refmap:
rbm.active = None rbm.active = None

mercurial/dirstate.py

if tr: if tr:
# delay writing in-memory changes out # delay writing in-memory changes out
if write_key: if write_key:
tr.addfilegenerator( tr.addfilegenerator(
b'dirstate-0-key-pre', b'dirstate-0-key-pre',
(self._filename_tk,), (self._filename_tk,),
lambda f: self._write_tracked_key(tr, f), lambda f: self._write_tracked_key(tr, f),
location=b'plain', location=b'plain',
post_finalize=True,
) )
tr.addfilegenerator( tr.addfilegenerator(
b'dirstate-1-main', b'dirstate-1-main',
(self._filename,), (self._filename,),
lambda f: self._writedirstate(tr, f), lambda f: self._writedirstate(tr, f),
location=b'plain', location=b'plain',
post_finalize=True,
) )
if write_key: if write_key:
tr.addfilegenerator( tr.addfilegenerator(
b'dirstate-2-key-post', b'dirstate-2-key-post',
(self._filename_tk,), (self._filename_tk,),
lambda f: self._write_tracked_key(tr, f), lambda f: self._write_tracked_key(tr, f),
location=b'plain', location=b'plain',
post_finalize=True,
) )
return return
file = lambda f: self._opener(f, b"w", atomictemp=True, checkambig=True) file = lambda f: self._opener(f, b"w", atomictemp=True, checkambig=True)
if write_key: if write_key:
# we change the key-file before changing the dirstate to make sure # we change the key-file before changing the dirstate to make sure
# reading invalidate there cache before we start writing # reading invalidate there cache before we start writing
with file(self._filename_tk) as f: with file(self._filename_tk) as f:
# ensure that subsequent tr.writepending returns True for # ensure that subsequent tr.writepending returns True for
# changes written out above, even if dirstate is never # changes written out above, even if dirstate is never
# changed after this # changed after this
tr.addfilegenerator( tr.addfilegenerator(
b'dirstate-1-main', b'dirstate-1-main',
(self._filename,), (self._filename,),
lambda f: self._writedirstate(tr, f), lambda f: self._writedirstate(tr, f),
location=b'plain', location=b'plain',
post_finalize=True,
) )
# ensure that pending file written above is unlinked at # ensure that pending file written above is unlinked at
# failure, even if tr.writepending isn't invoked until the # failure, even if tr.writepending isn't invoked until the
# end of this transaction # end of this transaction
tr.registertmp(filename, location=b'plain') tr.registertmp(filename, location=b'plain')
self._opener.tryunlink(backupname) self._opener.tryunlink(backupname)

mercurial/transaction.py

error, error,
pycompat, pycompat,
util, util,
) )
from .utils import stringutil from .utils import stringutil
version = 2 version = 2
# These are the file generators that should only be executed after the
# finalizers are done, since they rely on the output of the finalizers (like
# the changelog having been written).
postfinalizegenerators = {
b'bookmarks',
b'dirstate-0-key-pre',
b'dirstate-1-main',
b'dirstate-2-key-post',
}
GEN_GROUP_ALL = b'all' GEN_GROUP_ALL = b'all'
GEN_GROUP_PRE_FINALIZE = b'prefinalize' GEN_GROUP_PRE_FINALIZE = b'prefinalize'
GEN_GROUP_POST_FINALIZE = b'postfinalize' GEN_GROUP_POST_FINALIZE = b'postfinalize'
def active(func): def active(func):
def _active(self, *args, **kwds): def _active(self, *args, **kwds):
if self._count == 0: if self._count == 0:
Such files will be deleted when the transaction exits (on both Such files will be deleted when the transaction exits (on both
failure and success). failure and success).
""" """
self._addbackupentry((location, b'', tmpfile, False)) self._addbackupentry((location, b'', tmpfile, False))
@active @active
def addfilegenerator( def addfilegenerator(
self, genid, filenames, genfunc, order=0, location=b'' self,
genid,
filenames,
genfunc,
order=0,
location=b'',
post_finalize=False,
): ):
"""add a function to generates some files at transaction commit """add a function to generates some files at transaction commit
The `genfunc` argument is a function capable of generating proper The `genfunc` argument is a function capable of generating proper
content of each entry in the `filename` tuple. content of each entry in the `filename` tuple.
At transaction close time, `genfunc` will be called with one file At transaction close time, `genfunc` will be called with one file
object argument per entries in `filenames`. object argument per entries in `filenames`.
The transaction itself is responsible for the backup, creation and The transaction itself is responsible for the backup, creation and
final write of such file. final write of such file.
The `genid` argument is used to ensure the same set of file is only The `genid` argument is used to ensure the same set of file is only
generated once. Call to `addfilegenerator` for a `genid` already generated once. Call to `addfilegenerator` for a `genid` already
present will overwrite the old entry. present will overwrite the old entry.
The `order` argument may be used to control the order in which multiple The `order` argument may be used to control the order in which multiple
generator will be executed. generator will be executed.
The `location` arguments may be used to indicate the files are located The `location` arguments may be used to indicate the files are located
outside of the the standard directory for transaction. It should match outside of the the standard directory for transaction. It should match
one of the key of the `transaction.vfsmap` dictionary. one of the key of the `transaction.vfsmap` dictionary.
The `post_finalize` argument can be set to `True` for file generation
that must be run after the transaction has been finalized.
""" """
# For now, we are unable to do proper backup and restore of custom vfs # For now, we are unable to do proper backup and restore of custom vfs
# but for bookmarks that are handled outside this mechanism. # but for bookmarks that are handled outside this mechanism.
self._filegenerators[genid] = (order, filenames, genfunc, location) entry = (order, filenames, genfunc, location, post_finalize)
self._filegenerators[genid] = entry
@active @active
def removefilegenerator(self, genid): def removefilegenerator(self, genid):
"""reverse of addfilegenerator, remove a file generator function""" """reverse of addfilegenerator, remove a file generator function"""
if genid in self._filegenerators: if genid in self._filegenerators:
del self._filegenerators[genid] del self._filegenerators[genid]
def _generatefiles(self, suffix=b'', group=GEN_GROUP_ALL): def _generatefiles(self, suffix=b'', group=GEN_GROUP_ALL):
# write files registered for generation # write files registered for generation
any = False any = False
if group == GEN_GROUP_ALL: if group == GEN_GROUP_ALL:
skip_post = skip_pre = False skip_post = skip_pre = False
else: else:
skip_pre = group == GEN_GROUP_POST_FINALIZE skip_pre = group == GEN_GROUP_POST_FINALIZE
skip_post = group == GEN_GROUP_PRE_FINALIZE skip_post = group == GEN_GROUP_PRE_FINALIZE
for id, entry in sorted(pycompat.iteritems(self._filegenerators)): for id, entry in sorted(pycompat.iteritems(self._filegenerators)):
any = True any = True
order, filenames, genfunc, location = entry order, filenames, genfunc, location, post_finalize = entry
# for generation at closing, check if it's before or after finalize # for generation at closing, check if it's before or after finalize
is_post = id in postfinalizegenerators if skip_post and post_finalize:
if skip_post and is_post:
continue continue
elif skip_pre and not is_post: elif skip_pre and not post_finalize:
continue continue
vfs = self._vfsmap[location] vfs = self._vfsmap[location]
files = [] files = []
try: try:
for name in filenames: for name in filenames:
name += suffix name += suffix
if suffix: if suffix:

tests/testlib/crash_transaction_late.py

# tiny extension to abort a transaction very late during test # tiny extension to abort a transaction very late during test
# #
# Copyright 2020 Pierre-Yves David <pierre-yves.david@octobus.net> # Copyright 2020 Pierre-Yves David <pierre-yves.david@octobus.net>
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
from mercurial import ( from mercurial import (
error, error,
transaction,
) )
def abort(fp): def abort(fp):
raise error.Abort(b"This is a late abort") raise error.Abort(b"This is a late abort")
def reposetup(ui, repo): def reposetup(ui, repo):
transaction.postfinalizegenerators.add(b'late-abort')
class LateAbortRepo(repo.__class__): class LateAbortRepo(repo.__class__):
def transaction(self, *args, **kwargs): def transaction(self, *args, **kwargs):
tr = super(LateAbortRepo, self).transaction(*args, **kwargs) tr = super(LateAbortRepo, self).transaction(*args, **kwargs)
tr.addfilegenerator( tr.addfilegenerator(
b'late-abort', [b'late-abort'], abort, order=9999999 b'late-abort',
[b'late-abort'],
abort,
order=9999999,
post_finalize=True,
) )
return tr return tr
repo.__class__ = LateAbortRepo repo.__class__ = LateAbortRepo