Re: CR in notify.c

[Philip Martin]

Philip Martin  writes:

> the first will not be completely obliterated by the second.

I hacked the code rather than create a repository with millions of
revisions:

 case svn_wc_notify_tree_conflict_details_progress:
-  SVN_ERR(svn_cmdline_printf(pool, _("\rChecking r%ld..."), n->revision));
+  {
+  static long revision = 111;
+  SVN_ERR(svn_cmdline_printf(pool, _("\rChecking r%ld..."), revision));
+  revision = 99;
+  sleep(5);
+  }
   break;

and it does produce garbled output:

  Checking r111...

becomes:

  Checking r99...11...

-- 
Philip

CR in notify.c

[Philip Martin]

When rolling trunk tarballs I notice the error:

../svn/notify.c:483: warning: internationalized messages should not contain the 
'\r' escape sequence

The '\r' is intentional, the notification is of the form:

  Checking r32...

and the \r causes the line to get overwritten as the revision changes.
I haven't tested what happens if the revision number gets shorter by
lots of digits, but I suspect that if a long first notification:

  Checking r1234567...

is followed by a short second:

  Checking r99...

the first will not be completely obliterated by the second.

Perhaps we should move the \r out of the internationalized message and
use it in some longer message that is guaranteed to obliterate any
previous message.

-- 
Philip

Re: svn commit: r1803639 - in /subversion/trunk: build/run_tests.py subversion/libsvn_fs_fs/fs.h subversion/libsvn_fs_fs/fs_fs.c subversion/libsvn_fs_fs/transaction.c subversion/tests/cmdline/svntest/

[Evgeny Kotkov]

Daniel Shahaf  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, _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(>delta_compression_type,
>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, _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