The attached patch enables the following behaviour:
% svn --config-option=config:auth:password-stories= status iota
svn: warning: apr_err=SVN_ERR_CL_ARG_PARSING_ERROR
svn: warning: W205000: Ignoring unknown value 'password-stories'; did you
mean 'password-stores'?
M iota
% svn --config-option=config:foobar:password-stores= status iota
svn: warning: apr_err=SVN_ERR_CL_ARG_PARSING_ERROR
svn: warning: W205000: Ignoring unknown value 'foobar'
M iota
The combination itself is not validated: a correctly-spelled option in
the wrong section (such as config:helpers:http-library=...) won't
trigger a warning.
Unknown options will cause warnings even if they are used by the
substitution syntax, e.g.,
--config-option config:auth:foo=bar
will trigger a warning about 'foo', even if
--config-option 'config:auth:password-stores=%(foo)s'
is specified on the command-line or in the --config-dir. (If this is
a concern, we could move the spell-checking to a later point in the
argument parsing sequence, after the configuration from --config-dir has
been loaded.)
Log message skeleton:
[[[
* gen-make.py:
Generate a list of SVN_CONFIG_* file/section/option names for the
svn_cmdline_*() C code to use.
* subversion/libsvn_subr/cmdline.c
(svn_cmdline__parse_config_option):
When parsing the argument to --config-option, spell-check the
FILE:SECTION:OPTION coordinates, and warn if any component is
unknown.
]]]
Thoughts?
Daniel
(I'm going to be mostly offline for a few days; will reply when I'm back)
Index: build/generator/gen_base.py
===================================================================
--- build/generator/gen_base.py (revision 1670590)
+++ build/generator/gen_base.py (working copy)
@@ -22,6 +22,7 @@
# gen_base.py -- infrastructure for generating makefiles, dependencies, etc.
#
+import collections
import os
import sys
import glob
@@ -319,6 +320,91 @@ 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 (previous[1] is self.FileSectionOptionEnum.section
+ or previous[1] is self.FileSectionOptionEnum.option) \
+ and kind is self.FileSectionOptionEnum.section:
+ empty_sections.append(macro)
+ groupby[kind].append(macro)
+ previous = (macro, kind)
+
+ 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 +1298,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
Index: gen-make.py
===================================================================
--- gen-make.py (revision 1670590)
+++ gen-make.py (working copy)
@@ -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():
Index: subversion/include/private/svn_cmdline_private.h
===================================================================
--- subversion/include/private/svn_cmdline_private.h (revision 1670590)
+++ subversion/include/private/svn_cmdline_private.h (working copy)
@@ -89,6 +89,9 @@ typedef struct svn_cmdline__config_argument_t
* data in @a pool
*
* @since New in 1.7.
+ *
+ * @since 1.9: If the file/section/option combination is unknown, warn to @c
+ * stderr.
*/
svn_error_t *
svn_cmdline__parse_config_option(apr_array_header_t *config_options,
Index: subversion/libsvn_subr/cmdline.c
===================================================================
--- subversion/libsvn_subr/cmdline.c (revision 1670590)
+++ subversion/libsvn_subr/cmdline.c (working copy)
@@ -810,6 +810,116 @@ svn_cmdline__print_xml_prop(svn_stringbuf_t **outs
return;
}
+/* Return the most similar string to NEEDLE in HAYSTACK, which contains
+ * HAYSTACK_LEN elements. Return NULL if no string is sufficiently similar.
+ */
+static const char *
+most_similar(const char *needle_cstr,
+ const char **haystack,
+ apr_size_t haystack_len,
+ apr_pool_t *scratch_pool)
+{
+ /* TODO: use svn_cl__similarity_check()? */
+ 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 (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. */
+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 config:working-copy:diff-extensions and similar invalid
+ combinations. */
+
+#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++)
+ 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,
@@ -826,6 +936,8 @@ svn_cmdline__parse_config_option(apr_array_header_
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 +946,14 @@ svn_cmdline__parse_config_option(apr_array_header_
config_option->option = apr_pstrndup(pool, second_colon + 1,
equals_sign - second_colon - 1);
+ warning = validate_config_option(config_option, pool);
+ if (warning)
+ {
+ /* TODO: before committing, add 'prefix' argument */
+ svn_handle_warning2(stderr, warning, "svn: ");
+ svn_error_clear(warning);
+ }
+
if (! (strchr(config_option->option, ':')))
{
config_option->value = apr_pstrndup(pool, equals_sign + 1,
Index: subversion/tests/cmdline/getopt_tests.py
===================================================================
--- subversion/tests/cmdline/getopt_tests.py (revision 1670590)
+++ subversion/tests/cmdline/getopt_tests.py (working copy)
@@ -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__':
