This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9822

upgrade: re-read current requirements after taking lock
ClosedPublic

Authored by pulkit on Jan 18 2021, 10:19 AM.

Details

Reviewers
marmoute
mharbison72
Group Reviewers
hg-reviewers
Commits
rHG02f3badf9011: upgrade: re-read current requirements after taking lock
Summary

Since we are writing to repository, it's better to re-read after taking the
lock.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit created this revision.Jan 18 2021, 10:19 AM
pulkit updated this revision to Diff 25120.Jan 18 2021, 10:41 AM
marmoute accepted this revision.Jan 18 2021, 10:49 AM
marmoute added a subscriber: marmoute.

This looks "fine" as it fix the previous race window. However do not hesitate to follow up with another patch to reduce the code duplication.

pulkit updated this revision to Diff 25125.Jan 18 2021, 11:12 AM
marmoute accepted this revision.Jan 18 2021, 11:45 AM
mharbison72 accepted this revision.Jan 18 2021, 9:08 PM
This revision is now accepted and ready to land.Jan 18 2021, 9:08 PM
pulkit added a commit: rHG02f3badf9011: upgrade: re-read current requirements after taking lock.Jan 18 2021, 9:12 PM
Closed by commit rHG02f3badf9011: upgrade: re-read current requirements after taking lock (authored by pulkit). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/upgrade.py (26 lines)

Diff 25146

mercurial/upgrade.py

upgrade_op.print_post_op_messages() upgrade_op.print_post_op_messages()
def upgrade_share_to_safe(ui, hgvfs, storevfs, current_requirements): def upgrade_share_to_safe(ui, hgvfs, storevfs, current_requirements):
"""Upgrades a share to use share-safe mechanism""" """Upgrades a share to use share-safe mechanism"""
wlock = None wlock = None
store_requirements = localrepo._readrequires(storevfs, False) store_requirements = localrepo._readrequires(storevfs, False)
original_crequirements = current_requirements.copy()
# after upgrade, store requires will be shared, so lets find # after upgrade, store requires will be shared, so lets find
# the requirements which are not present in store and # the requirements which are not present in store and
# write them to share's .hg/requires # write them to share's .hg/requires
diffrequires = current_requirements - store_requirements diffrequires = current_requirements - store_requirements
# add share-safe requirement as it will mark the share as share-safe # add share-safe requirement as it will mark the share as share-safe
diffrequires.add(requirementsmod.SHARESAFE_REQUIREMENT) diffrequires.add(requirementsmod.SHARESAFE_REQUIREMENT)
current_requirements.add(requirementsmod.SHARESAFE_REQUIREMENT) current_requirements.add(requirementsmod.SHARESAFE_REQUIREMENT)
try: try:
wlock = lockmod.trylock(ui, hgvfs, b'wlock', 0, 0) wlock = lockmod.trylock(ui, hgvfs, b'wlock', 0, 0)
# some process might change the requirement in between, re-read
# and update current_requirements
locked_requirements = localrepo._readrequires(hgvfs, True)
if locked_requirements != original_crequirements:
removed = current_requirements - locked_requirements
# update current_requirements in place because it's passed
# as reference
current_requirements -= removed
current_requirements |= locked_requirements
diffrequires = current_requirements - store_requirements
# add share-safe requirement as it will mark the share as share-safe
diffrequires.add(requirementsmod.SHARESAFE_REQUIREMENT)
current_requirements.add(requirementsmod.SHARESAFE_REQUIREMENT)
scmutil.writerequires(hgvfs, diffrequires) scmutil.writerequires(hgvfs, diffrequires)
ui.warn(_(b'repository upgraded to use share-safe mode\n')) ui.warn(_(b'repository upgraded to use share-safe mode\n'))
except error.LockError as e: except error.LockError as e:
if ui.configbool(b'experimental', b'sharesafe-auto-upgrade-fail-error'): if ui.configbool(b'experimental', b'sharesafe-auto-upgrade-fail-error'):
raise error.Abort( raise error.Abort(
_(b'failed to upgrade share, got error: %s') _(b'failed to upgrade share, got error: %s')
% stringutil.forcebytestr(e.strerror) % stringutil.forcebytestr(e.strerror)
) )
ui, ui,
hgvfs, hgvfs,
sharedvfs, sharedvfs,
current_requirements, current_requirements,
): ):
"""Downgrades a share which use share-safe to not use it""" """Downgrades a share which use share-safe to not use it"""
wlock = None wlock = None
source_requirements = localrepo._readrequires(sharedvfs, True) source_requirements = localrepo._readrequires(sharedvfs, True)
original_crequirements = current_requirements.copy()
# we cannot be 100% sure on which requirements were present in store when # we cannot be 100% sure on which requirements were present in store when
# the source supported share-safe. However, we do know that working # the source supported share-safe. However, we do know that working
# directory requirements were not there. Hence we remove them # directory requirements were not there. Hence we remove them
source_requirements -= requirementsmod.WORKING_DIR_REQUIREMENTS source_requirements -= requirementsmod.WORKING_DIR_REQUIREMENTS
current_requirements |= source_requirements current_requirements |= source_requirements
current_requirements.remove(requirementsmod.SHARESAFE_REQUIREMENT) current_requirements.remove(requirementsmod.SHARESAFE_REQUIREMENT)
try: try:
wlock = lockmod.trylock(ui, hgvfs, b'wlock', 0, 0) wlock = lockmod.trylock(ui, hgvfs, b'wlock', 0, 0)
# some process might change the requirement in between, re-read
# and update current_requirements
locked_requirements = localrepo._readrequires(hgvfs, True)
if locked_requirements != original_crequirements:
removed = current_requirements - locked_requirements
# update current_requirements in place because it's passed
# as reference
current_requirements -= removed
current_requirements |= locked_requirements
current_requirements |= source_requirements
current_requirements -= set(requirementsmod.SHARESAFE_REQUIREMENT)
scmutil.writerequires(hgvfs, current_requirements) scmutil.writerequires(hgvfs, current_requirements)
ui.warn(_(b'repository downgraded to not use share-safe mode\n')) ui.warn(_(b'repository downgraded to not use share-safe mode\n'))
except error.LockError as e: except error.LockError as e:
# raise error right away because if downgrade failed, we cannot load # raise error right away because if downgrade failed, we cannot load
# the repository because it does not have complete set of requirements # the repository because it does not have complete set of requirements
raise error.Abort( raise error.Abort(
_(b'failed to downgrade share, got error: %s') _(b'failed to downgrade share, got error: %s')
% stringutil.forcebytestr(e.strerror) % stringutil.forcebytestr(e.strerror)
) )
finally: finally:
if wlock: if wlock:
wlock.release() wlock.release()