Daniel Shahaf <d...@daniel.shahaf.name> writes:
> We have a mix of options that depend on SVN_VER_MINOR and on
> ffd->format.
>
> SVN_VER_MINOR:
> [memcached-servers]
> [caches]
> [debug]
>
> ffd->format:
> [rep-sharing]
> [packed-revprops]
> [io]
>
> both:
> [deltification] - requires ≥1.8 and ≥f4
I think that I have missed that the whole [deltification] section and even
the 'compression-level' option itself have been available starting from
certain minor versions, instead of just being new in some formats.
>From this perspective, there's nothing special about the new 'compression'
option, and I should agree that making it available starting from 1.10 and
also applicable to older fs formats would be better.
While here, I think that we could also error out on a configuration with
both 'compression' and 'compression-level' set to limit the amount of
possible configurations. (Currently, in such case the 'compression'
option silently overrides 'compression-level'.)
Perhaps, then, we could do all this as in the attached patch.
Thanks,
Evgeny Kotkov
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 1804416)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -861,7 +861,7 @@ read_config(fs_fs_data_t *ffd,
}
/* Initialize compression settings in ffd. */
- if (ffd->format >= SVN_FS_FS__MIN_SVNDIFF2_FORMAT)
+ if (ffd->format >= SVN_FS_FS__MIN_DELTIFICATION_FORMAT)
{
const char *compression_val;
const char *compression_level_val;
@@ -872,12 +872,23 @@ read_config(fs_fs_data_t *ffd,
svn_config_get(config, &compression_level_val,
CONFIG_SECTION_DELTIFICATION,
CONFIG_OPTION_COMPRESSION_LEVEL, NULL);
- if (compression_val)
+ if (compression_val && compression_level_val)
{
- /* 'compression' option overrides deprecated 'compression-level'. */
+ return svn_error_create(SVN_ERR_BAD_CONFIG_VALUE, NULL,
+ _("The 'compression' and 'compression-level'
"
+ "config options are mutually exclusive"));
+ }
+ else if (compression_val)
+ {
SVN_ERR(parse_compression_option(&ffd->delta_compression_type,
&ffd->delta_compression_level,
compression_val));
+ if (ffd->delta_compression_type == compression_type_lz4 &&
+ ffd->format < SVN_FS_FS__MIN_SVNDIFF2_FORMAT)
+ {
+ return svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
+ _("Unsupported compression type 'lz4'"));
+ }
}
else if (compression_level_val)
{
@@ -897,19 +908,6 @@ read_config(fs_fs_data_t *ffd,
ffd->delta_compression_level = SVN_DELTA_COMPRESSION_LEVEL_DEFAULT;
}
}
- else if (ffd->format >= SVN_FS_FS__MIN_DELTIFICATION_FORMAT)
- {
- apr_int64_t compression_level;
-
- SVN_ERR(svn_config_get_int64(config, &compression_level,
- CONFIG_SECTION_DELTIFICATION,
- CONFIG_OPTION_COMPRESSION_LEVEL,
- SVN_DELTA_COMPRESSION_LEVEL_DEFAULT));
- ffd->delta_compression_type = compression_type_zlib;
- ffd->delta_compression_level =
- (int)MIN(MAX(SVN_DELTA_COMPRESSION_LEVEL_NONE, compression_level),
- SVN_DELTA_COMPRESSION_LEVEL_MAX);
- }
else if (ffd->format >= SVN_FS_FS__MIN_SVNDIFF1_FORMAT)
{
ffd->delta_compression_type = compression_type_zlib;
@@ -1062,6 +1060,7 @@ write_config(svn_fs_t *fs,
"### significantly speed up commits as well as reading the data." NL
"### The syntax of this option is:" NL
"### " CONFIG_OPTION_COMPRESSION " = none | lz4 | zlib | zlib-1 ... zlib-9"
NL
+"### Versions prior to Subversion 1.10 will ignore this option." NL
"### The default value is 'zlib', which is currently equivalent to 'zlib-5'."
NL
"# " CONFIG_OPTION_COMPRESSION " = zlib" NL
"###" NL
Index: subversion/tests/cmdline/svntest/main.py
===================================================================
--- subversion/tests/cmdline/svntest/main.py (revision 1804416)
+++ subversion/tests/cmdline/svntest/main.py (working copy)
@@ -2202,10 +2202,6 @@ def parse_options(arglist=sys.argv[1:], usage=None
if options.fsfs_packing and not options.fsfs_sharding:
parser.error("--fsfs-packing requires --fsfs-sharding")
- if options.fsfs_compression is not None and \
- options.server_minor_version < 10:
- parser.error("--fsfs-compression requires --server-minor-version=10")
-
if options.server_minor_version not in range(3, SVN_VER_MINOR+1):
parser.error("test harness only supports server minor versions 3-%d"
% SVN_VER_MINOR)
