idf.py: add exit_with_error for deprecation

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

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)

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
+                }
+            },
         },
     }
 

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()