From 506c46fddcc7ded7f25a073fdcaf3343cd4d4735 Mon Sep 17 00:00:00 2001 From: Sergei Silnov Date: Thu, 14 Nov 2019 13:48:24 +0100 Subject: [PATCH] idf.py: add exit_with_error for deprecation --- tools/ci/test_build_system_cmake.sh | 12 -------- tools/idf.py | 46 +++++++++++++++++++++------- tools/test_idf_py/idf_ext.py | 18 ++++++++++- tools/test_idf_py/test_idf_py.py | 47 +++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 24 deletions(-) diff --git a/tools/ci/test_build_system_cmake.sh b/tools/ci/test_build_system_cmake.sh index 88c2838c1..e777c6c78 100755 --- a/tools/ci/test_build_system_cmake.sh +++ b/tools/ci/test_build_system_cmake.sh @@ -555,18 +555,6 @@ endmenu\n" >> ${IDF_PATH}/Kconfig mv CMakeLists.txt.bak CMakeLists.txt rm -rf CMakeLists.txt.bak - print_status "Print all required argument deprecation warnings" - idf.py -C${IDF_PATH}/tools/test_idf_py --test-0=a --test-1=b --test-2=c --test-3=d test-0 --test-sub-0=sa --test-sub-1=sb ta test-1 > out.txt - ! grep -e '"test-0" is deprecated' -e '"test_0" is deprecated' out.txt || failure "Deprecation warnings are displayed for non-deprecated option/command" - grep -e 'Warning: Option "test_sub_1" is deprecated and will be removed in future versions.' \ - -e 'Warning: Command "test-1" is deprecated and will be removed in future versions. Please use alternative command.' \ - -e 'Warning: Option "test_1" is deprecated and will be removed in future versions.' \ - -e 'Warning: Option "test_2" is deprecated and will be removed in future versions. Please update your parameters.' \ - -e 'Warning: Option "test_3" is deprecated and will be removed in future versions.' \ - out.txt \ - || failure "Deprecation warnings are not displayed" - rm out.txt - print_status "should be able to specify multiple sdkconfig default files" idf.py clean > /dev/null idf.py fullclean > /dev/null diff --git a/tools/idf.py b/tools/idf.py index b4077a194..e6573f149 100755 --- a/tools/idf.py +++ b/tools/idf.py @@ -138,26 +138,39 @@ def init_cli(verbose_output=None): # Click is imported here to run it after check_environment() import click - class DeprecationMessage(object): + class Deprecation(object): """Construct deprecation notice for help messages""" def __init__(self, deprecated=False): self.deprecated = deprecated self.since = None self.removed = None + self.exit_with_error = None self.custom_message = "" if isinstance(deprecated, dict): self.custom_message = deprecated.get("message", "") self.since = deprecated.get("since", None) self.removed = deprecated.get("removed", None) + self.exit_with_error = deprecated.get("exit_with_error", None) elif isinstance(deprecated, str): self.custom_message = deprecated def full_message(self, type="Option"): - return "%s is deprecated %sand will be removed in%s.%s" % ( - type, "since %s " % self.since if self.since else "", " %s" % self.removed - if self.removed else " future versions", " %s" % self.custom_message if self.custom_message else "") + if self.exit_with_error: + return "%s is deprecated %sand was removed%s.%s" % ( + type, + "since %s " % self.since if self.since else "", + " in %s" % self.removed if self.removed else "", + " %s" % self.custom_message if self.custom_message else "", + ) + else: + return "%s is deprecated %sand will be removed in%s.%s" % ( + type, + "since %s " % self.since if self.since else "", + " %s" % self.removed if self.removed else " future versions", + " %s" % self.custom_message if self.custom_message else "", + ) def help(self, text, type="Option", separator=" "): text = text or "" @@ -167,12 +180,16 @@ def init_cli(verbose_output=None): text = text or "" return ("Deprecated! " + text) if self.deprecated else text - def print_deprecation_warning(ctx): + def check_deprecation(ctx): """Prints deprectation warnings for arguments in given context""" for option in ctx.command.params: default = () if option.multiple else option.default if isinstance(option, Option) and option.deprecated and ctx.params[option.name] != default: - print("Warning: %s" % DeprecationMessage(option.deprecated).full_message('Option "%s"' % option.name)) + deprecation = Deprecation(option.deprecated) + if deprecation.exit_with_error: + raise FatalError("Error: %s" % deprecation.full_message('Option "%s"' % option.name)) + else: + print("Warning: %s" % deprecation.full_message('Option "%s"' % option.name)) class Task(object): def __init__(self, callback, name, aliases, dependencies, order_dependencies, action_args): @@ -223,7 +240,7 @@ def init_cli(verbose_output=None): self.short_help = self.short_help or self.help.split("\n")[0] if deprecated: - deprecation = DeprecationMessage(deprecated) + deprecation = Deprecation(deprecated) self.short_help = deprecation.short_help(self.short_help) self.help = deprecation.help(self.help, type="Command", separator="\n") @@ -251,11 +268,18 @@ def init_cli(verbose_output=None): def invoke(self, ctx): if self.deprecated: - print("Warning: %s" % DeprecationMessage(self.deprecated).full_message('Command "%s"' % self.name)) + deprecation = Deprecation(self.deprecated) + message = deprecation.full_message('Command "%s"' % self.name) + + if deprecation.exit_with_error: + raise FatalError("Error: %s" % message) + else: + print("Warning: %s" % message) + self.deprecated = False # disable Click's built-in deprecation handling # Print warnings for options - print_deprecation_warning(ctx) + check_deprecation(ctx) return super(Action, self).invoke(ctx) class Argument(click.Argument): @@ -324,7 +348,7 @@ def init_cli(verbose_output=None): self.hidden = hidden if deprecated: - deprecation = DeprecationMessage(deprecated) + deprecation = Deprecation(deprecated) self.help = deprecation.help(self.help) if self.scope.is_global: @@ -517,7 +541,7 @@ def init_cli(verbose_output=None): global_args[key] = local_value # Show warnings about global arguments - print_deprecation_warning(ctx) + check_deprecation(ctx) # Make sure that define_cache_entry is mutable list and can be modified in callbacks global_args.define_cache_entry = list(global_args.define_cache_entry) diff --git a/tools/test_idf_py/idf_ext.py b/tools/test_idf_py/idf_ext.py index d35ea50f3..ae0a65a46 100644 --- a/tools/test_idf_py/idf_ext.py +++ b/tools/test_idf_py/idf_ext.py @@ -34,12 +34,21 @@ def action_extensions(base_actions, project_path=None): }, { "names": ["--test-4"], - "help": "Deprecated option 3.", + "help": "Deprecated option 4.", "deprecated": { "since": "v4.0", "removed": "v5.0" } }, + { + "names": ["--test-5"], + "help": "Deprecated option 5.", + "deprecated": { + "since": "v2.0", + "removed": "v3.0", + "exit_with_error": True + } + }, ], "actions": { "test-verbose": { @@ -71,6 +80,13 @@ def action_extensions(base_actions, project_path=None): "help": "Deprecated command 1", "deprecated": "Please use alternative command." }, + "test-2": { + "callback": echo, + "help": "Deprecated command 2", + "deprecated": { + "exit_with_error": True + } + }, }, } diff --git a/tools/test_idf_py/test_idf_py.py b/tools/test_idf_py/test_idf_py.py index 9c7cb7a1a..5b9216a99 100755 --- a/tools/test_idf_py/test_idf_py.py +++ b/tools/test_idf_py/test_idf_py.py @@ -174,5 +174,52 @@ class TestGlobalAndSubcommandParameters(unittest.TestCase): ) +class TestDeprecations(unittest.TestCase): + def test_exit_with_error_for_subcommand(self): + try: + subprocess.check_output([sys.executable, idf_py_path, "-C%s" % current_dir, "test-2"], env=os.environ) + except subprocess.CalledProcessError as e: + self.assertIn('Error: Command "test-2" is deprecated and was removed.', e.output.decode('utf-8', 'ignore')) + + def test_exit_with_error_for_option(self): + try: + subprocess.check_output( + [sys.executable, idf_py_path, "-C%s" % current_dir, "--test-5=asdf"], env=os.environ) + except subprocess.CalledProcessError as e: + self.assertIn( + 'Error: Option "test_5" is deprecated since v2.0 and was removed in v3.0.', + e.output.decode('utf-8', 'ignore')) + + def test_deprecation_messages(self): + output = subprocess.check_output( + [ + sys.executable, + idf_py_path, + "-C%s" % current_dir, + "--test-0=a", + "--test-1=b", + "--test-2=c", + "--test-3=d", + "test-0", + "--test-sub-0=sa", + "--test-sub-1=sb", + "ta", + "test-1", + ], + env=os.environ).decode('utf-8', 'ignore') + + self.assertIn('Warning: Option "test_sub_1" is deprecated and will be removed in future versions.', output) + self.assertIn( + 'Warning: Command "test-1" is deprecated and will be removed in future versions. ' + 'Please use alternative command.', output) + self.assertIn('Warning: Option "test_1" is deprecated and will be removed in future versions.', output) + self.assertIn( + 'Warning: Option "test_2" is deprecated and will be removed in future versions. ' + 'Please update your parameters.', output) + self.assertIn('Warning: Option "test_3" is deprecated and will be removed in future versions.', output) + self.assertNotIn('"test-0" is deprecated', output) + self.assertNotIn('"test_0" is deprecated', output) + + if __name__ == '__main__': unittest.main()