changeset 46332:25be21ec6c65

share: rework config options to be much clearer and easier Recently I implemented various boolean configs which control how to behave when there is a share-safe mismatch between source and share repository. Mismatch means that source supports share-safe where as share does not or vice versa. However, while discussion and documentation we realized that it's too complicated and there are some combinations of values which makes no sense. We decided to introduce a config option with 4 possible values which makes controlling and understanding things easier. The config option `share.safe-mismatch.source-{not-}safe` can have following 4 values: * abort (default): error out if there is mismatch * allow: allow to work with respecting share source configuration * {up|down}grade-abort: try to {up|down}grade, if it fails, abort * {up|down}grade-allow: try to {up|down}grade, if it fails, continue in allow mode I am not sure if I can explain 3 config options which I deleted right now in just 5 lines which is a sign of how complex they became. No test changes demonstrate that functionality is same, only names have changed. Differential Revision: https://phab.mercurial-scm.org/D9785
author Pulkit Goyal <7895pulkit@gmail.com>
date Mon, 18 Jan 2021 21:37:20 +0530
parents 8788981c95f8
children 4f17773fc6b5
files mercurial/configitems.py mercurial/helptext/config.txt mercurial/localrepo.py mercurial/upgrade.py tests/test-share-safe.t
diffstat 5 files changed, 119 insertions(+), 34 deletions(-) [+]
line wrap: on
line diff
--- a/mercurial/configitems.py	Thu Jan 14 21:34:12 2021 +0530
+++ b/mercurial/configitems.py	Mon Jan 18 21:37:20 2021 +0530
@@ -1098,21 +1098,6 @@
 )
 coreconfigitem(
     b'experimental',
-    b'sharesafe-auto-downgrade-shares',
-    default=False,
-)
-coreconfigitem(
-    b'experimental',
-    b'sharesafe-auto-upgrade-shares',
-    default=False,
-)
-coreconfigitem(
-    b'experimental',
-    b'sharesafe-auto-upgrade-fail-error',
-    default=False,
-)
-coreconfigitem(
-    b'experimental',
     b'sharesafe-warn-outdated-shares',
     default=True,
 )
@@ -1926,6 +1911,16 @@
     default=b'identity',
 )
 coreconfigitem(
+    b'share',
+    b'safe-mismatch.source-not-safe',
+    default=b'abort',
+)
+coreconfigitem(
+    b'share',
+    b'safe-mismatch.source-safe',
+    default=b'abort',
+)
+coreconfigitem(
     b'shelve',
     b'maxbackups',
     default=10,
--- a/mercurial/helptext/config.txt	Thu Jan 14 21:34:12 2021 +0530
+++ b/mercurial/helptext/config.txt	Mon Jan 18 21:37:20 2021 +0530
@@ -1932,6 +1932,46 @@
     Currently, only the rebase and absorb commands consider this configuration.
     (EXPERIMENTAL)
 
+``share``
+---------
+
+``safe-mismatch.source-safe``
+
+    Controls what happens when the shared repository does not use the
+    share-safe mechanism but its source repository does.
+
+    Possible values are `abort` (default), `allow`, `upgrade-abort` and
+    `upgrade-abort`.
+
+    ``abort``
+    Disallows running any command and aborts
+    ``allow``
+    Respects the feature presence in the share source
+    ``upgrade-abort``
+    tries to upgrade the share to use share-safe; if it fails, aborts
+    ``upgrade-allow``
+    tries to upgrade the share; if it fails, continue by
+    respecting the share source setting
+
+``safe-mismatch.source-not-safe``
+
+    Controls what happens when the shared repository uses the share-safe
+    mechanism but its source does not.
+
+    Possible values are `abort` (default), `allow`, `downgrade-abort` and
+    `downgrade-abort`.
+
+    ``abort``
+    Disallows running any command and aborts
+    ``allow``
+    Respects the feature presence in the share source
+    ``downgrade-abort``
+    tries to downgrade the share to not use share-safe; if it fails, aborts
+    ``downgrade-allow``
+    tries to downgrade the share to not use share-safe;
+    if it fails, continue by respecting the shared source setting
+
+
 ``storage``
 -----------
 
--- a/mercurial/localrepo.py	Thu Jan 14 21:34:12 2021 +0530
+++ b/mercurial/localrepo.py	Mon Jan 18 21:37:20 2021 +0530
@@ -575,8 +575,13 @@
             and requirementsmod.SHARESAFE_REQUIREMENT
             not in _readrequires(sharedvfs, True)
         ):
-            if ui.configbool(
-                b'experimental', b'sharesafe-auto-downgrade-shares'
+            mismatch_config = ui.config(
+                b'share', b'safe-mismatch.source-not-safe'
+            )
+            if mismatch_config in (
+                b'downgrade-allow',
+                b'allow',
+                b'downgrade-abort',
             ):
                 # prevent cyclic import localrepo -> upgrade -> localrepo
                 from . import upgrade
@@ -586,19 +591,38 @@
                     hgvfs,
                     sharedvfs,
                     requirements,
+                    mismatch_config,
                 )
-            else:
+            elif mismatch_config == b'abort':
                 raise error.Abort(
                     _(
                         b"share source does not support exp-sharesafe requirement"
                     )
                 )
+            else:
+                hint = _(
+                    "run `hg help config.share.safe-mismatch.source-not-safe`"
+                )
+                raise error.Abort(
+                    _(
+                        b"share-safe mismatch with source.\nUnrecognized"
+                        b" value '%s' of `share.safe-mismatch.source-not-safe`"
+                        b" set."
+                    )
+                    % mismatch_config,
+                    hint=hint,
+                )
         else:
             requirements |= _readrequires(storevfs, False)
     elif shared:
         sourcerequires = _readrequires(sharedvfs, False)
         if requirementsmod.SHARESAFE_REQUIREMENT in sourcerequires:
-            if ui.configbool(b'experimental', b'sharesafe-auto-upgrade-shares'):
+            mismatch_config = ui.config(b'share', b'safe-mismatch.source-safe')
+            if mismatch_config in (
+                b'upgrade-allow',
+                b'allow',
+                b'upgrade-abort',
+            ):
                 # prevent cyclic import localrepo -> upgrade -> localrepo
                 from . import upgrade
 
@@ -607,14 +631,25 @@
                     hgvfs,
                     storevfs,
                     requirements,
+                    mismatch_config,
                 )
-            else:
+            elif mismatch_config == b'abort':
                 raise error.Abort(
                     _(
                         b'version mismatch: source uses share-safe'
                         b' functionality while the current share does not'
                     )
                 )
+            else:
+                hint = _("run `hg help config.share.safe-mismatch.source-safe`")
+                raise error.Abort(
+                    _(
+                        b"share-safe mismatch with source.\nUnrecognized"
+                        b" value '%s' of `share.safe-mismatch.source-safe` set."
+                    )
+                    % mismatch_config,
+                    hint=hint,
+                )
 
     # The .hg/hgrc file may load extensions or contain config options
     # that influence repository construction. Attempt to load it and
--- a/mercurial/upgrade.py	Thu Jan 14 21:34:12 2021 +0530
+++ b/mercurial/upgrade.py	Mon Jan 18 21:37:20 2021 +0530
@@ -241,7 +241,9 @@
             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, mismatch_config
+):
     """Upgrades a share to use share-safe mechanism"""
     wlock = None
     store_requirements = localrepo._readrequires(storevfs, False)
@@ -253,6 +255,10 @@
     # add share-safe requirement as it will mark the share as share-safe
     diffrequires.add(requirementsmod.SHARESAFE_REQUIREMENT)
     current_requirements.add(requirementsmod.SHARESAFE_REQUIREMENT)
+    # in `allow` case, we don't try to upgrade, we just respect the source
+    # state, update requirements and continue
+    if mismatch_config == b'allow':
+        return
     try:
         wlock = lockmod.trylock(ui, hgvfs, b'wlock', 0, 0)
         # some process might change the requirement in between, re-read
@@ -271,7 +277,7 @@
         scmutil.writerequires(hgvfs, diffrequires)
         ui.warn(_(b'repository upgraded to use share-safe mode\n'))
     except error.LockError as e:
-        if ui.configbool(b'experimental', b'sharesafe-auto-upgrade-fail-error'):
+        if mismatch_config == b'upgrade-abort':
             raise error.Abort(
                 _(b'failed to upgrade share, got error: %s')
                 % stringutil.forcebytestr(e.strerror)
@@ -291,6 +297,7 @@
     hgvfs,
     sharedvfs,
     current_requirements,
+    mismatch_config,
 ):
     """Downgrades a share which use share-safe to not use it"""
     wlock = None
@@ -302,6 +309,8 @@
     source_requirements -= requirementsmod.WORKING_DIR_REQUIREMENTS
     current_requirements |= source_requirements
     current_requirements.remove(requirementsmod.SHARESAFE_REQUIREMENT)
+    if mismatch_config == b'allow':
+        return
 
     try:
         wlock = lockmod.trylock(ui, hgvfs, b'wlock', 0, 0)
@@ -319,12 +328,13 @@
         scmutil.writerequires(hgvfs, current_requirements)
         ui.warn(_(b'repository downgraded to not use share-safe mode\n'))
     except error.LockError as e:
-        # raise error right away because if downgrade failed, we cannot load
-        # the repository because it does not have complete set of requirements
-        raise error.Abort(
-            _(b'failed to downgrade share, got error: %s')
-            % stringutil.forcebytestr(e.strerror)
-        )
+        # If upgrade-abort is set, abort when upgrade fails, else let the
+        # process continue as `upgrade-allow` is set
+        if mismatch_config == b'downgrade-abort':
+            raise error.Abort(
+                _(b'failed to downgrade share, got error: %s')
+                % stringutil.forcebytestr(e.strerror)
+            )
     finally:
         if wlock:
             wlock.release()
--- a/tests/test-share-safe.t	Thu Jan 14 21:34:12 2021 +0530
+++ b/tests/test-share-safe.t	Mon Jan 18 21:37:20 2021 +0530
@@ -486,12 +486,12 @@
 Testing automatic downgrade of shares when config is set
 
   $ touch ../ss-share/.hg/wlock
-  $ hg log -GT "{node}: {desc}\n" -R ../ss-share --config experimental.sharesafe-auto-downgrade-shares=true
+  $ hg log -GT "{node}: {desc}\n" -R ../ss-share --config share.safe-mismatch.source-not-safe=downgrade-abort
   abort: failed to downgrade share, got error: Lock held
   [255]
   $ rm ../ss-share/.hg/wlock
 
-  $ hg log -GT "{node}: {desc}\n" -R ../ss-share --config experimental.sharesafe-auto-downgrade-shares=true
+  $ hg log -GT "{node}: {desc}\n" -R ../ss-share --config share.safe-mismatch.source-not-safe=downgrade-abort
   repository downgraded to not use share-safe mode
   @  f63db81e6dde1d9c78814167f77fb1fb49283f4f: added bar
   |
@@ -533,26 +533,31 @@
   [255]
 
 Check that if lock is taken, upgrade fails but read operation are successful
+  $ hg log -GT "{node}: {desc}\n" -R ../nss-share --config share.safe-mismatch.source-safe=upgra
+  abort: share-safe mismatch with source.
+  Unrecognized value 'upgra' of `share.safe-mismatch.source-safe` set.
+  (run `hg help config.share.safe-mismatch.source-safe`)
+  [255]
   $ touch ../nss-share/.hg/wlock
-  $ hg log -GT "{node}: {desc}\n" -R ../nss-share --config experimental.sharesafe-auto-upgrade-shares=true
+  $ hg log -GT "{node}: {desc}\n" -R ../nss-share --config share.safe-mismatch.source-safe=upgrade-allow
   failed to upgrade share, got error: Lock held
   @  f63db81e6dde1d9c78814167f77fb1fb49283f4f: added bar
   |
   o  f3ba8b99bb6f897c87bbc1c07b75c6ddf43a4f77: added foo
   
 
-  $ hg log -GT "{node}: {desc}\n" -R ../nss-share --config experimental.sharesafe-auto-upgrade-shares=true --config experimental.sharesafe-warn-outdated-shares=false
+  $ hg log -GT "{node}: {desc}\n" -R ../nss-share --config share.safe-mismatch.source-safe=upgrade-allow --config experimental.sharesafe-warn-outdated-shares=false
   @  f63db81e6dde1d9c78814167f77fb1fb49283f4f: added bar
   |
   o  f3ba8b99bb6f897c87bbc1c07b75c6ddf43a4f77: added foo
   
 
-  $ hg log -GT "{node}: {desc}\n" -R ../nss-share --config experimental.sharesafe-auto-upgrade-shares=true --config experimental.sharesafe-auto-upgrade-fail-error=true
+  $ hg log -GT "{node}: {desc}\n" -R ../nss-share --config share.safe-mismatch.source-safe=upgrade-abort
   abort: failed to upgrade share, got error: Lock held
   [255]
 
   $ rm ../nss-share/.hg/wlock
-  $ hg log -GT "{node}: {desc}\n" -R ../nss-share --config experimental.sharesafe-auto-upgrade-shares=true
+  $ hg log -GT "{node}: {desc}\n" -R ../nss-share --config share.safe-mismatch.source-safe=upgrade-abort
   repository upgraded to use share-safe mode
   @  f63db81e6dde1d9c78814167f77fb1fb49283f4f: added bar
   |