Patchwork D8148: revlog-compression: update the config to be a list

login
register
mail settings
Submitter phabricator
Date Feb. 25, 2020, 10:15 a.m.
Message ID <differential-rev-PHID-DREV-ptqsyvwlkq7bmu5j6eja-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/45310/
State Superseded
Headers show

Comments

phabricator - Feb. 25, 2020, 10:15 a.m.
marmoute created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  format.revlog-compression is now a list of engine, the first supported one is to
  be used. Doing this have several benefits:
  
  1. this is fully backward compatible, config using a single entry will be read
  
  as a single item list, not changing any behavior.
  
  2. This open the way to use zstd by default without impacting platform were it
  
  is not available. This will be done in a later changesets.
  
  Using zstd provide a significant performance boost explained in : bb271ec2fbfb <https://phab.mercurial-scm.org/rHGbb271ec2fbfbd456d1281c5dad21f92db6d30f7b>.
  However zstd is not available in some cases, A notable example is the `--pure`
  version of Mercurial which doesn't come with zstd support.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8148

AFFECTED FILES
  mercurial/configitems.py
  mercurial/helptext/config.txt
  mercurial/localrepo.py
  mercurial/upgrade.py
  tests/test-repo-compengines.t

CHANGE DETAILS




To: marmoute, #hg-reviewers
Cc: mercurial-devel
phabricator - Feb. 25, 2020, 11:12 a.m.
pulkit added a comment.


  Looks good to me, can you add an entry to `relnotes/next`?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8148/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8148

To: marmoute, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - Feb. 25, 2020, 5:45 p.m.
This revision now requires changes to proceed.
mharbison72 added inline comments.
mharbison72 requested changes to this revision.

INLINE COMMENTS

> config.txt:891
>      a newer format that is usually a net win over `zlib`, operating faster at
> -    better compression rates. Use `zstd` to reduce CPU usage.
> +    better compression rates. Use `zstd` to reduce CPU usage. Multiple value can
> +    be specified, the first available one wille be used.

s/value/values/

> config.txt:892
> +    better compression rates. Use `zstd` to reduce CPU usage. Multiple value can
> +    be specified, the first available one wille be used.
>  

s/wille/will/

> test-repo-compengines.t:25
>    $ hg --config format.revlog-compression=unknown init unknown
> -  abort: compression engine unknown defined by format.revlog-compression not available
> +  abort: compression engines unknown defined by format.revlog-compression not available
>    (run "hg debuginstall" to list available compression engines)

Nit: It might help readability (especially with a list) if the engines were delimited with quotes or parenthesis or something.  It's mostly obvious here, but not if it's in a config file.  (And actually, I missed that "unknown" was used above, and thought it was an awkward sentence at first.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8148/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8148

To: marmoute, #hg-reviewers, mharbison72
Cc: mharbison72, pulkit, mercurial-devel
phabricator - Feb. 25, 2020, 11:58 p.m.
mharbison72 added a comment.
mharbison72 accepted this revision.


  LGTM, thanks for the update.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8148/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8148

To: marmoute, #hg-reviewers, mharbison72
Cc: mharbison72, pulkit, mercurial-devel
phabricator - Feb. 26, 2020, 9:31 a.m.
pulkit added a comment.


  In D8148#121295 <https://phab.mercurial-scm.org/D8148#121295>, @pulkit wrote:
  
  > Looks good to me, can you add an entry to `relnotes/next`?
  
  still missing the release notes entry.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8148/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8148

To: marmoute, #hg-reviewers, mharbison72
Cc: mharbison72, pulkit, mercurial-devel

Patch

diff --git a/tests/test-repo-compengines.t b/tests/test-repo-compengines.t
--- a/tests/test-repo-compengines.t
+++ b/tests/test-repo-compengines.t
@@ -22,10 +22,15 @@ 
 Unknown compression engine to format.compression aborts
 
   $ hg --config format.revlog-compression=unknown init unknown
-  abort: compression engine unknown defined by format.revlog-compression not available
+  abort: compression engines unknown defined by format.revlog-compression not available
   (run "hg debuginstall" to list available compression engines)
   [255]
 
+unknown compression engine in a list with known one works fine
+
+  $ hg --config format.revlog-compression=zlib,unknown init zlib-before-unknow
+  $ hg --config format.revlog-compression=unknown,zlib init unknown-before-zlib
+
 A requirement specifying an unknown compression engine results in bail
 
   $ hg init unknownrequirement
diff --git a/mercurial/upgrade.py b/mercurial/upgrade.py
--- a/mercurial/upgrade.py
+++ b/mercurial/upgrade.py
@@ -449,7 +449,14 @@ 
 
     @classmethod
     def fromconfig(cls, repo):
-        return repo.ui.config(b'format', b'revlog-compression')
+        compengines = repo.ui.configlist(b'format', b'revlog-compression')
+        # return the first valid value as the selection code would do
+        for comp in compengines:
+            if comp in util.compengines:
+                return comp
+
+        # no valide compression found lets display it all for clarity
+        return b','.join(compengines)
 
 
 @registerformatvariant
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -3576,14 +3576,17 @@ 
             if ui.configbool(b'format', b'dotencode'):
                 requirements.add(b'dotencode')
 
-    compengine = ui.config(b'format', b'revlog-compression')
-    if compengine not in util.compengines:
+    compengines = ui.configlist(b'format', b'revlog-compression')
+    for compengine in compengines:
+        if compengine in util.compengines:
+            break
+    else:
         raise error.Abort(
             _(
-                b'compression engine %s defined by '
+                b'compression engines %s defined by '
                 b'format.revlog-compression not available'
             )
-            % compengine,
+            % b', '.join(compengines),
             hint=_(
                 b'run "hg debuginstall" to list available '
                 b'compression engines'
@@ -3591,7 +3594,7 @@ 
         )
 
     # zlib is the historical default and doesn't need an explicit requirement.
-    elif compengine == b'zstd':
+    if compengine == b'zstd':
         requirements.add(b'revlog-compression-zstd')
     elif compengine != b'zlib':
         requirements.add(b'exp-compression-%s' % compengine)
diff --git a/mercurial/helptext/config.txt b/mercurial/helptext/config.txt
--- a/mercurial/helptext/config.txt
+++ b/mercurial/helptext/config.txt
@@ -888,7 +888,8 @@ 
     Compression algorithm used by revlog. Supported values are `zlib` and
     `zstd`. The `zlib` engine is the historical default of Mercurial. `zstd` is
     a newer format that is usually a net win over `zlib`, operating faster at
-    better compression rates. Use `zstd` to reduce CPU usage.
+    better compression rates. Use `zstd` to reduce CPU usage. Multiple value can
+    be specified, the first available one wille be used.
 
     On some systems, the Mercurial installation may lack `zstd` support.
 
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -756,7 +756,7 @@ 
 coreconfigitem(
     b'format',
     b'revlog-compression',
-    default=b'zlib',
+    default=lambda: [b'zlib'],
     alias=[(b'experimental', b'format.compression')],
 )
 coreconfigitem(