subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From danie...@apache.org
Subject svn commit: r1674301 - in /subversion/trunk: ./ build/generator/ build/generator/templates/ subversion/include/private/ subversion/libsvn_subr/ subversion/svn/ subversion/svnbench/ subversion/svnmucc/ subversion/svnrdump/ subversion/svnsync/ subversion...
Date Fri, 17 Apr 2015 13:30:25 GMT
Author: danielsh
Date: Fri Apr 17 13:30:25 2015
New Revision: 1674301

URL: http://svn.apache.org/r1674301
Log:
--config-option: Warn when the FILE:SECTION:OPTION combination may be invalid.

Currently there are false negatives: if each component is individually known,
then there won't be a warning, even if the tuple as a whole is invalid.

The implementation is as follows: first, parse svn_config.h to generate a list
of valid file/section/option names:

* subversion/libsvn_subr/config_keys.inc:
    Not a versioned file, but will be generated by gen-make.py.

* build/generator/gen_base.py
  (collections): Import.
  (GeneratorBase.FileSectionOptionEnum): New class.
  (GeneratorBase._client_configuration_defines): New method.
  (GeneratorBase.write_config_keys): New method.
  (IncludeDependencyInfo._scan_for_includes):
    Exempt config_keys.inc from dependency scanning.

* build/generator/templates/build-outputs.mk.ezt
  (EXTRACLEAN_FILES): Clean config_keys.inc.

* gen-make.py
  (main): Generate config_keys.inc.

Second, have the --config-option parser warn to stderr about unknown values:

* subversion/include/private/svn_cmdline_private.h
  (svn_cmdline__parse_config_option): Add PREFIX argument, document new
    functionality.

* subversion/libsvn_subr/cmdline.c
  (config_keys.inc): Include.
  (most_similar, string_in_array, validate_config_option): New functions.
  (svn_cmdline__parse_config_option): Validate the config option's coordinates
    and warn if they may be invalid.

* subversion/svn/similarity.c
  (svn_cl__similarity_check): Cross-reference most_similar().
    No functional change.

Third, add a unit test:

* subversion/tests/cmdline/getopt_tests.py
  (getopt_config_option): New test.

Finally, update callers for the trivial signature change:

* subversion/svn/svn.c           (sub_main),
* subversion/svnbench/svnbench.c (sub_main),
* subversion/svnmucc/svnmucc.c   (sub_main),
* subversion/svnrdump/svnrdump.c (sub_main):
      Pass new PREFIX argument.

* subversion/svnsync/svnsync.c
  (sub_main): Ditto, in two places.

Modified:
    subversion/trunk/build/generator/gen_base.py
    subversion/trunk/build/generator/templates/build-outputs.mk.ezt
    subversion/trunk/gen-make.py
    subversion/trunk/subversion/include/private/svn_cmdline_private.h
    subversion/trunk/subversion/libsvn_subr/cmdline.c
    subversion/trunk/subversion/svn/similarity.c
    subversion/trunk/subversion/svn/svn.c
    subversion/trunk/subversion/svnbench/svnbench.c
    subversion/trunk/subversion/svnmucc/svnmucc.c
    subversion/trunk/subversion/svnrdump/svnrdump.c
    subversion/trunk/subversion/svnsync/svnsync.c
    subversion/trunk/subversion/tests/cmdline/getopt_tests.py

Modified: subversion/trunk/build/generator/gen_base.py
URL: http://svn.apache.org/viewvc/subversion/trunk/build/generator/gen_base.py?rev=1674301&r1=1674300&r2=1674301&view=diff
==============================================================================
--- subversion/trunk/build/generator/gen_base.py (original)
+++ subversion/trunk/build/generator/gen_base.py Fri Apr 17 13:30:25 2015
@@ -22,6 +22,7 @@
 # gen_base.py -- infrastructure for generating makefiles, dependencies, etc.
 #
 
+import collections
 import os
 import sys
 import glob
@@ -319,6 +320,93 @@ class GeneratorBase:
   def errno_filter(self, codes):
     return codes
 
+  class FileSectionOptionEnum(object):
+    # These are accessed via getattr() later on
+    file = object()
+    section = object()
+    option = object()
+
+  def _client_configuration_defines(self):
+    """Return an iterator over SVN_CONFIG_* #define's in the "Client
+    configuration files strings" section of svn_config.h."""
+
+    pattern = re.compile(
+      r'^\s*#\s*define\s+'
+      r'(?P<macro>SVN_CONFIG_(?P<kind>CATEGORY|SECTION|OPTION)_[A-Z0-9a-z_]+)'
+    )
+    kind = {
+      'CATEGORY': self.FileSectionOptionEnum.file,
+      'SECTION': self.FileSectionOptionEnum.section,
+      'OPTION': self.FileSectionOptionEnum.option,
+    }
+
+    fname = os.path.join(os.path.abspath(os.path.dirname(sys.argv[0])),
+                         'subversion', 'include', 'svn_config.h')
+    lines = iter(open(fname))
+    for line in lines:
+      if "@name Client configuration files strings" in line:
+        break
+    else:
+      raise Exception("Unable to parse svn_config.h")
+
+    for line in lines:
+      if "@{" in line:
+        break
+    else:
+      raise Exception("Unable to parse svn_config.h")
+
+    for line in lines:
+      if "@}" in line:
+        break
+      match = pattern.match(line)
+      if match:
+        yield (
+          match.group('macro'),
+          kind[match.group('kind')],
+        )
+    else:
+      raise Exception("Unable to parse svn_config.h")
+
+  def write_config_keys(self):
+    groupby = collections.defaultdict(list)
+    empty_sections = []
+    previous = (None, None)
+    for macro, kind in self._client_configuration_defines():
+      if kind is previous[1] is self.FileSectionOptionEnum.section:
+        empty_sections.append(previous[0])
+      groupby[kind].append(macro)
+      previous = (macro, kind)
+    else:
+      # If the last (macro, kind) is a section, then it's an empty section.
+      if kind is self.FileSectionOptionEnum.section:
+        empty_sections.append(macro)
+
+    lines = []
+    lines.append('/* Automatically generated by %s:write_config_keys() */'
+                 % (__file__,))
+    lines.append('')
+
+    for kind in ('file', 'section', 'option'):
+      macros = groupby[getattr(self.FileSectionOptionEnum, kind)]
+      lines.append('const char *svn__valid_config_%ss[] = {' % (kind,))
+      for macro in macros:
+        lines.append('  %s,' % (macro,))
+      # Remove ',' for c89 compatibility
+      lines[-1] = lines[-1][0:-1]
+      lines.append('};')
+      lines.append('')
+
+    lines.append('const char *svn__empty_config_sections[] = {');
+    for section in empty_sections:
+      lines.append('  %s,' % (section,))
+    # Remove ',' for c89 compatibility
+    lines[-1] = lines[-1][0:-1]
+    lines.append('};')
+    lines.append('')
+
+    self.write_file_if_changed('subversion/libsvn_subr/config_keys.inc',
+                               '\n'.join(lines))
+
 class DependencyGraph:
   """Record dependencies between build items.
 
@@ -1212,6 +1300,9 @@ class IncludeDependencyInfo:
       if os.sep.join(['libsvn_subr', 'error.c']) in fname \
            and 'errorcode.inc' == include_param:
         continue # generated by GeneratorBase.write_errno_table
+      if os.sep.join(['libsvn_subr', 'cmdline.c']) in fname \
+           and 'config_keys.inc' == include_param:
+        continue # generated by GeneratorBase.write_config_keys
       elif direct_possibility_fname in domain_fnames:
         self._upd_dep_hash(hdrs, direct_possibility_fname, type_code)
       elif (len(domain_fnames) == 1

Modified: subversion/trunk/build/generator/templates/build-outputs.mk.ezt
URL: http://svn.apache.org/viewvc/subversion/trunk/build/generator/templates/build-outputs.mk.ezt?rev=1674301&r1=1674300&r2=1674301&view=diff
==============================================================================
--- subversion/trunk/build/generator/templates/build-outputs.mk.ezt (original)
+++ subversion/trunk/build/generator/templates/build-outputs.mk.ezt Fri Apr 17 13:30:25 2015
@@ -46,6 +46,7 @@ MANPAGES =[for manpages] [manpages][end]
 CLEAN_FILES =[for cfiles] [cfiles][end]
 EXTRACLEAN_FILES =[for sql] [sql.header][end] \
   $(abs_builddir)/subversion/libsvn_subr/errorcode.inc \
+  $(abs_builddir)/subversion/libsvn_subr/config_keys.inc \
   $(abs_srcdir)/compile_commands.json
 
 SWIG_INCLUDES = -I$(abs_builddir)/subversion \

Modified: subversion/trunk/gen-make.py
URL: http://svn.apache.org/viewvc/subversion/trunk/gen-make.py?rev=1674301&r1=1674300&r2=1674301&view=diff
==============================================================================
--- subversion/trunk/gen-make.py (original)
+++ subversion/trunk/gen-make.py Fri Apr 17 13:30:25 2015
@@ -67,6 +67,7 @@ def main(fname, gentype, verfname=None,
   generator.write()
   generator.write_sqlite_headers()
   generator.write_errno_table()
+  generator.write_config_keys()
 
   if ('--debug', '') in other_options:
     for dep_type, target_dict in generator.graph.deps.items():

Modified: subversion/trunk/subversion/include/private/svn_cmdline_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_cmdline_private.h?rev=1674301&r1=1674300&r2=1674301&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_cmdline_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_cmdline_private.h Fri Apr 17 13:30:25
2015
@@ -88,11 +88,15 @@ typedef struct svn_cmdline__config_argum
  * containing svn_cmdline__config_argument_t* elements, allocating the option
  * data in @a pool
  *
+ * [Since 1.9/1.10:] If the file, section, or option value is not recognized,
+ * warn to @c stderr, using @a prefix as in svn_handle_warning2().
+ *
  * @since New in 1.7.
  */
 svn_error_t *
 svn_cmdline__parse_config_option(apr_array_header_t *config_options,
                                  const char *opt_arg,
+                                 const char *prefix,
                                  apr_pool_t *pool);
 
 /** Sets the config options in @a config_options, an apr array containing

Modified: subversion/trunk/subversion/libsvn_subr/cmdline.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cmdline.c?rev=1674301&r1=1674300&r2=1674301&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/cmdline.c (original)
+++ subversion/trunk/subversion/libsvn_subr/cmdline.c Fri Apr 17 13:30:25 2015
@@ -810,9 +810,125 @@ svn_cmdline__print_xml_prop(svn_stringbu
   return;
 }
 
+/* Return the most similar string to NEEDLE in HAYSTACK, which contains
+ * HAYSTACK_LEN elements.  Return NULL if no string is sufficiently similar.
+ */
+/* See svn_cl__similarity_check() for a more general solution. */
+static const char *
+most_similar(const char *needle_cstr,
+             const char **haystack,
+             apr_size_t haystack_len,
+             apr_pool_t *scratch_pool)
+{
+  const char *max_similar;
+  apr_size_t max_score = 0;
+  apr_size_t i;
+  svn_membuf_t membuf;
+  svn_string_t *needle_str = svn_string_create(needle_cstr, scratch_pool);
+
+  svn_membuf__create(&membuf, 64, scratch_pool);
+
+  for (i = 0; i < haystack_len; i++)
+    {
+      apr_size_t score;
+      svn_string_t *hay = svn_string_create(haystack[i], scratch_pool);
+
+      score = svn_string__similarity(needle_str, hay, &membuf, NULL);
+
+      /* If you update this factor, consider updating
+       * svn_cl__similarity_check(). */
+      if (score >= (2 * SVN_STRING__SIM_RANGE_MAX + 1) / 3
+          && score > max_score)
+        {
+          max_score = score;
+          max_similar = haystack[i];
+        }
+    }
+
+  if (max_score)
+    return max_similar;
+  else
+    return NULL;
+}
+
+/* Verify that NEEDLE is in HAYSTACK, which contains HAYSTACK_LEN elements. */
+static svn_error_t *
+string_in_array(const char *needle,
+                const char **haystack,
+                apr_size_t haystack_len,
+                apr_pool_t *scratch_pool)
+{
+  const char *next_of_kin;
+  apr_size_t i;
+  for (i = 0; i < haystack_len; i++)
+    {
+      if (!strcmp(needle, haystack[i]))
+        return SVN_NO_ERROR;
+    }
+
+  /* Error. */
+  next_of_kin = most_similar(needle, haystack, haystack_len, scratch_pool);
+  if (next_of_kin)
+    return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+                             _("Ignoring unknown value '%s'; "
+                               "did you mean '%s'?"),
+                             needle, next_of_kin);
+  else
+    return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+                             _("Ignoring unknown value '%s'"),
+                             needle);
+}
+
+#include "config_keys.inc"
+
+/* Validate the FILE, SECTION, and OPTION components of CONFIG_OPTION are
+ * known.  Warn to stderr if not.  (An unknown value may be either a typo
+ * or added in a newer minor version of Subversion.) */
+static svn_error_t *
+validate_config_option(svn_cmdline__config_argument_t *config_option,
+                       apr_pool_t *scratch_pool)
+{
+  svn_boolean_t arbitrary_keys = FALSE;
+
+  /* TODO: some day, we could also verify that OPTION is valid for SECTION;
+     i.e., forbid invalid combinations such as config:auth:diff-extensions. */
+
+#define ARRAYLEN(x) ( sizeof((x)) / sizeof((x)[0]) )
+
+  SVN_ERR(string_in_array(config_option->file, svn__valid_config_files,
+                          ARRAYLEN(svn__valid_config_files),
+                          scratch_pool));
+  SVN_ERR(string_in_array(config_option->section, svn__valid_config_sections,
+                          ARRAYLEN(svn__valid_config_sections),
+                          scratch_pool));
+
+  /* Don't validate option names for sections such as servers[group],
+   * config[tunnels], and config[auto-props] that permit arbitrary options. */
+    {
+      int i;
+
+      for (i = 0; i < ARRAYLEN(svn__empty_config_sections); i++)
+        {
+        SVN_DBG(("Checking '%s'", svn__empty_config_sections[i]));
+        if (!strcmp(config_option->section, svn__empty_config_sections[i]))
+          arbitrary_keys = TRUE;
+        }
+    }
+
+  if (! arbitrary_keys)
+    SVN_ERR(string_in_array(config_option->option, svn__valid_config_options,
+                            ARRAYLEN(svn__valid_config_options),
+                            scratch_pool));
+
+#undef ARRAYLEN
+
+  return SVN_NO_ERROR;
+}
+
 svn_error_t *
 svn_cmdline__parse_config_option(apr_array_header_t *config_options,
                                  const char *opt_arg,
+                                 const char *prefix,
                                  apr_pool_t *pool)
 {
   svn_cmdline__config_argument_t *config_option;
@@ -826,6 +942,8 @@ svn_cmdline__parse_config_option(apr_arr
           if ((equals_sign = strchr(second_colon + 1, '=')) &&
               (equals_sign != second_colon + 1))
             {
+              svn_error_t *warning;
+
               config_option = apr_pcalloc(pool, sizeof(*config_option));
               config_option->file = apr_pstrndup(pool, opt_arg,
                                                  first_colon - opt_arg);
@@ -834,6 +952,13 @@ svn_cmdline__parse_config_option(apr_arr
               config_option->option = apr_pstrndup(pool, second_colon + 1,
                                                    equals_sign - second_colon - 1);
 
+              warning = validate_config_option(config_option, pool);
+              if (warning)
+                {
+                  svn_handle_warning2(stderr, warning, prefix);
+                  svn_error_clear(warning);
+                }
+
               if (! (strchr(config_option->option, ':')))
                 {
                   config_option->value = apr_pstrndup(pool, equals_sign + 1,

Modified: subversion/trunk/subversion/svn/similarity.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/similarity.c?rev=1674301&r1=1674300&r2=1674301&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/similarity.c (original)
+++ subversion/trunk/subversion/svn/similarity.c Fri Apr 17 13:30:25 2015
@@ -114,6 +114,8 @@ svn_cl__similarity_check(const char *key
     {
       svn_cl__simcheck_t *const token = tokens[i];
       token->context = NULL;
+      /* If you update this factor, consider updating
+       * ../libsvn_subr/cmdline.c:most_similar(). */
       if (token->score >= (2 * SVN_STRING__SIM_RANGE_MAX + 1) / 3)
         ++result;
     }

Modified: subversion/trunk/subversion/svn/svn.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/svn.c?rev=1674301&r1=1674300&r2=1674301&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/svn.c (original)
+++ subversion/trunk/subversion/svn/svn.c Fri Apr 17 13:30:25 2015
@@ -2265,7 +2265,7 @@ sub_main(int *exit_code, int argc, const
 
         SVN_ERR(svn_utf_cstring_to_utf8(&utf8_opt_arg, opt_arg, pool));
         SVN_ERR(svn_cmdline__parse_config_option(opt_state.config_options,
-                                                 utf8_opt_arg, pool));
+                                                 utf8_opt_arg, "svn: ", pool));
         break;
       case opt_autoprops:
         opt_state.autoprops = TRUE;

Modified: subversion/trunk/subversion/svnbench/svnbench.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnbench/svnbench.c?rev=1674301&r1=1674300&r2=1674301&view=diff
==============================================================================
--- subversion/trunk/subversion/svnbench/svnbench.c (original)
+++ subversion/trunk/subversion/svnbench/svnbench.c Fri Apr 17 13:30:25 2015
@@ -654,7 +654,7 @@ sub_main(int *exit_code, int argc, const
 
         SVN_ERR(svn_utf_cstring_to_utf8(&opt_arg, opt_arg, pool));
         SVN_ERR(svn_cmdline__parse_config_option(opt_state.config_options,
-                                                     opt_arg, pool));
+                                                 opt_arg, "svnbench", pool));
         break;
       case opt_with_all_revprops:
         /* If --with-all-revprops is specified along with one or more

Modified: subversion/trunk/subversion/svnmucc/svnmucc.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnmucc/svnmucc.c?rev=1674301&r1=1674300&r2=1674301&view=diff
==============================================================================
--- subversion/trunk/subversion/svnmucc/svnmucc.c (original)
+++ subversion/trunk/subversion/svnmucc/svnmucc.c Fri Apr 17 13:30:25 2015
@@ -625,6 +625,7 @@ sub_main(int *exit_code, int argc, const
         case config_inline_opt:
           SVN_ERR(svn_utf_cstring_to_utf8(&opt_arg, arg, pool));
           SVN_ERR(svn_cmdline__parse_config_option(config_options, opt_arg,
+                                                   "svnmucc: ", 
                                                    pool));
           break;
         case no_auth_cache_opt:

Modified: subversion/trunk/subversion/svnrdump/svnrdump.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnrdump/svnrdump.c?rev=1674301&r1=1674300&r2=1674301&view=diff
==============================================================================
--- subversion/trunk/subversion/svnrdump/svnrdump.c (original)
+++ subversion/trunk/subversion/svnrdump/svnrdump.c Fri Apr 17 13:30:25 2015
@@ -950,7 +950,9 @@ sub_main(int *exit_code, int argc, const
 
             SVN_ERR(svn_utf_cstring_to_utf8(&opt_arg, opt_arg, pool));
             SVN_ERR(svn_cmdline__parse_config_option(config_options,
-                                                     opt_arg, pool));
+                                                     opt_arg, 
+                                                     "svnrdump: ",
+                                                     pool));
         }
     }
 

Modified: subversion/trunk/subversion/svnsync/svnsync.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnsync/svnsync.c?rev=1674301&r1=1674300&r2=1674301&view=diff
==============================================================================
--- subversion/trunk/subversion/svnsync/svnsync.c (original)
+++ subversion/trunk/subversion/svnsync/svnsync.c Fri Apr 17 13:30:25 2015
@@ -2073,7 +2073,7 @@ sub_main(int *exit_code, int argc, const
 
             SVN_ERR(svn_utf_cstring_to_utf8(&opt_arg, opt_arg, pool));
             SVN_ERR(svn_cmdline__parse_config_option(config_options,
-                                                     opt_arg, pool));
+                                                     opt_arg, "svnsync", pool));
             break;
 
           case svnsync_opt_source_prop_encoding:
@@ -2139,6 +2139,7 @@ sub_main(int *exit_code, int argc, const
                       apr_psprintf(pool,
                                    "config:miscellany:memory-cache-size=%s",
                                    opt_arg),
+                      NULL /* won't be used */,
                       pool));
             break;
 

Modified: subversion/trunk/subversion/tests/cmdline/getopt_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/getopt_tests.py?rev=1674301&r1=1674300&r2=1674301&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/getopt_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/getopt_tests.py Fri Apr 17 13:30:25 2015
@@ -223,6 +223,18 @@ def getopt_help_bogus_cmd(sbox):
   "run svn help bogus-cmd"
   run_one_test(sbox, 'svn_help_bogus-cmd', 'help', 'bogus-cmd')
 
+def getopt_config_option(sbox):
+  "--config-option's spell checking"
+  sbox.build(create_wc=False, read_only=True)
+  expected_stderr = '.*W205000.*did you mean.*'
+  expected_stdout = svntest.verify.AnyOutput
+  svntest.actions.run_and_verify_svn(expected_stdout, expected_stderr,
+                                     'info', 
+                                     '--config-option',
+                                     'config:miscellanous:diff-extensions=' +
+                                       '-u -p',
+                                     '--', sbox.repo_url)
+
 ########################################################################
 # Run the tests
 
@@ -237,6 +249,7 @@ test_list = [ None,
               getopt_help,
               getopt_help_bogus_cmd,
               getopt_help_log_switch,
+              getopt_config_option,
             ]
 
 if __name__ == '__main__':



Mime
View raw message