From c38bb4809bf8b388424640bc7212ad4339c50e2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubom=C3=ADr=20Sedl=C3=A1=C5=99?= Date: Thu, 20 Oct 2016 10:49:28 +0200 Subject: [PATCH] config: Don't abort on deprecated options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These options are in fact removed and have no effect anymore. This patch changes the validation to print a warning that the option was removed and what should be done instead. It no longer stops the whole compose. The validation script still rejects configuration files with these removed keys. This change means we no longer check these removals with the JSON schema (as that makes it hard to determine where exactly the problem is). Fixes: #438 Signed-off-by: Lubomír Sedlář --- bin/pungi-config-validate | 5 +-- bin/pungi-koji | 2 ++ pungi/checks.py | 70 ++++++++++++++++++--------------------- tests/test_config.py | 6 ++-- 4 files changed, 41 insertions(+), 42 deletions(-) diff --git a/bin/pungi-config-validate b/bin/pungi-config-validate index d2142e17..851412b3 100755 --- a/bin/pungi-config-validate +++ b/bin/pungi-config-validate @@ -65,8 +65,9 @@ def run(config, topdir, has_old): conf.load_from_file(config) errors = pungi.checks.validate(conf) - if errors: - for error in errors: + warnings = list(pungi.checks.report_removed(conf)) + if errors or warnings: + for error in errors + warnings: print(error) sys.exit(1) diff --git a/bin/pungi-koji b/bin/pungi-koji index d256748d..0c3706a9 100755 --- a/bin/pungi-koji +++ b/bin/pungi-koji @@ -174,6 +174,8 @@ def main(): sys.exit(1) pungi.checks.check_umask(logger) errors = pungi.checks.validate(conf) + for warning in pungi.checks.report_removed(conf): + print >>sys.stderr, warning if errors: for error in errors: print >>sys.stderr, error diff --git a/pungi/checks.py b/pungi/checks.py index 1998ab75..5dd968dc 100644 --- a/pungi/checks.py +++ b/pungi/checks.py @@ -180,24 +180,29 @@ def validate(config): 'regex': (str, unicode)}) errors = [] for error in validator.iter_errors(config): - if isinstance(error, ConfigDeprecation): - errors.append(DEPRECATED.format('.'.join(error.path), error.message)) + if not error.path and error.validator == 'additionalProperties': + allowed_keys = set(error.schema['properties'].keys()) + used_keys = set(error.instance.keys()) + for key in used_keys - allowed_keys: + suggestion = _get_suggestion(key, allowed_keys) + if suggestion: + errors.append(UNKNOWN_SUGGEST.format(key, suggestion)) + else: + errors.append(UNKNOWN.format(key)) else: - if not error.path and error.validator == 'additionalProperties': - allowed_keys = set(error.schema['properties'].keys()) - used_keys = set(error.instance.keys()) - for key in used_keys - allowed_keys: - suggestion = _get_suggestion(key, allowed_keys) - if suggestion: - errors.append(UNKNOWN_SUGGEST.format(key, suggestion)) - else: - errors.append(UNKNOWN.format(key)) - else: - errors.append('Failed validation in %s: %s' % ( - '.'.join([str(x) for x in error.path]), error.message)) + errors.append('Failed validation in %s: %s' % ( + '.'.join([str(x) for x in error.path]), error.message)) return errors + _validate_requires(schema, config, CONFIG_DEPS) +def report_removed(config): + schema = _make_schema() + for key in config: + msg = schema['properties'].get(key, {}).get('deprecated') + if msg: + yield REMOVED.format(key, msg) + + def _get_suggestion(desired, names): """Find a value in ``names`` that is the closest match for ``desired``. @@ -214,11 +219,11 @@ def _get_suggestion(desired, names): return closest -CONFLICTS = 'Config option {0}={1} conflicts with option {2}.' -REQUIRES = 'Config option {0}={1} requires {2} which is not set.' -DEPRECATED = 'Deprecated config option: {0}; {1}.' -UNKNOWN = 'Unrecognized config option: {0}.' -UNKNOWN_SUGGEST = 'Unrecognized config option: {0}. Did you mean {1}?' +CONFLICTS = 'ERROR: Config option {0}={1} conflicts with option {2}.' +REQUIRES = 'ERROR: Config option {0}={1} requires {2} which is not set.' +REMOVED = 'WARNING: Config option {0} was removed and has no effect; {1}.' +UNKNOWN = 'ERROR: Unrecognized config option: {0}.' +UNKNOWN_SUGGEST = 'ERROR: Unrecognized config option: {0}. Did you mean {1}?' def _extend_with_default(validator_class): @@ -237,12 +242,6 @@ def _extend_with_default(validator_class): for error in validate_properties(validator, properties, instance, schema): yield error - def error_on_deprecated(validator, properties, instance, schema): - """Unconditionally raise deprecation error if encountered.""" - yield ConfigDeprecation( - 'use %s instead' % properties - ) - def validate_regex_type(validator, properties, instance, schema): """ Extend standard type validation to check correctness in regular @@ -262,15 +261,10 @@ def _extend_with_default(validator_class): return jsonschema.validators.extend( validator_class, {"properties": set_defaults, - "deprecated": error_on_deprecated, "type": validate_regex_type}, ) -class ConfigDeprecation(jsonschema.exceptions.ValidationError): - pass - - def _make_schema(): return { "$schema": "http://json-schema.org/draft-04/schema#", @@ -588,7 +582,7 @@ def _make_schema(): "default": True, }, "keep_original_comps": { - "deprecated": "no tag for respective variant in variants XML" + "deprecated": "remove tag from respective variant in variants XML" }, "link_type": { @@ -822,25 +816,25 @@ def _make_schema(): # Deprecated options "multilib_arches": { - "deprecated": "multilib" + "deprecated": "use multilib instead" }, "multilib_methods": { - "deprecated": "multilib" + "deprecated": "use multilib instead" }, "additional_packages_multiarch": { - "deprecated": "multilib_whitelist" + "deprecated": "use multilib_whitelist instead" }, "filter_packages_multiarch": { - "deprecated": "multilib_blacklist" + "deprecated": "use multilib_blacklist instead" }, "buildinstall_upgrade_image": { - "deprecated": "lorax_options" + "deprecated": "use lorax_options instead" }, "pkgset_koji_path_prefix": { - "deprecated": "koji_profile", + "deprecated": "use koji_profile instead", }, "pkgset_koji_url": { - "deprecated": "koji_profile", + "deprecated": "use koji_profile instead", }, }, diff --git a/tests/test_config.py b/tests/test_config.py index 75971046..b49b2c1f 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -194,9 +194,11 @@ class BuildinstallConfigTestCase(unittest.TestCase): buildinstall_upgrade_image=True, ) + self.assertItemsEqual(checks.validate(cfg), []) + self.assertItemsEqual( - checks.validate(cfg), - [checks.DEPRECATED.format('buildinstall_upgrade_image', 'use lorax_options instead')] + checks.report_removed(cfg), + [checks.REMOVED.format('buildinstall_upgrade_image', 'use lorax_options instead')] )