diff --git a/tools/ci/test_build_system_cmake.sh b/tools/ci/test_build_system_cmake.sh index 80686322c7..9d033cd013 100755 --- a/tools/ci/test_build_system_cmake.sh +++ b/tools/ci/test_build_system_cmake.sh @@ -299,6 +299,13 @@ function run_tests() grep "CONFIG_IDF_TARGET=\"${fake_target}\"" sdkconfig || failure "Project not configured correctly using idf.py -D" grep "IDF_TARGET:STRING=${fake_target}" build/CMakeCache.txt || failure "IDF_TARGET not set in CMakeCache.txt using idf.py -D" + print_status "Can set target using -D as subcommand parameter for idf.py" + clean_build_dir + rm sdkconfig + idf.py reconfigure -DIDF_TARGET=$fake_target || failure "Failed to set target via idf.py subcommand -D parameter" + grep "CONFIG_IDF_TARGET=\"${fake_target}\"" sdkconfig || failure "Project not configured correctly using idf.py reconfigure -D" + grep "IDF_TARGET:STRING=${fake_target}" build/CMakeCache.txt || failure "IDF_TARGET not set in CMakeCache.txt using idf.py reconfigure -D" + # Clean up modifications for the fake target mv CMakeLists.txt.bak CMakeLists.txt rm -rf components @@ -471,6 +478,16 @@ endmenu\n" >> ${IDF_PATH}/Kconfig; rm -rf esp32 rm -rf mycomponents + # idf.py global and subcommand parameters + print_status "Cannot set -D twice: for command and subcommand of idf.py (with different values)" + idf.py -DAAA=BBB build -DAAA=BBB -DCCC=EEE + if [ $? -eq 0 ]; then + failure "It shouldn't be allowed to set -D twice (globally and for subcommand) with different set of options" + fi + + print_status "Can set -D twice: globally and for subcommand, only if values are the same" + idf.py -DAAA=BBB -DCCC=EEE build -DAAA=BBB -DCCC=EEE || failure "It should be allowed to set -D twice (globally and for subcommand) if values are the same" + print_status "All tests completed" if [ -n "${FAILURES}" ]; then echo "Some failures were detected:" diff --git a/tools/idf.py b/tools/idf.py index bbf9f609dd..e0037c96d1 100755 --- a/tools/idf.py +++ b/tools/idf.py @@ -544,8 +544,11 @@ def init_cli(): self.action_args = action_args self.aliases = aliases - def run(self, context, global_args): - self.callback(self.name, context, global_args, **self.action_args) + def run(self, context, global_args, action_args=None): + if action_args is None: + action_args = self.action_args + + self.callback(self.name, context, global_args, **action_args) class Action(click.Command): def __init__( @@ -599,6 +602,57 @@ def init_cli(): self.callback = wrapped_callback + class Argument(click.Argument): + """Positional argument""" + + def __init__(self, **kwargs): + names = kwargs.pop("names") + super(Argument, self).__init__(names, **kwargs) + + class Scope(object): + """ + Scope for sub-command option. + possible values: + - default - only available on defined level (global/action) + - global - When defined for action, also available as global + - shared - Opposite to 'global': when defined in global scope, also available for all actions + """ + + SCOPES = ("default", "global", "shared") + + def __init__(self, scope=None): + if scope is None: + self._scope = "default" + elif isinstance(scope, str) and scope in self.SCOPES: + self._scope = scope + elif isinstance(scope, Scope): + self._scope = str(scope) + else: + raise FatalError("Unknown scope for option: %s" % scope) + + @property + def is_global(self): + return self._scope == "global" + + @property + def is_shared(self): + return self._scope == "shared" + + def __str__(self): + return self._scope + + class Option(click.Option): + """Option that knows whether it should be global""" + + def __init__(self, scope=None, **kwargs): + kwargs["param_decls"] = kwargs.pop("names") + super(Option, self).__init__(**kwargs) + + self.scope = Scope(scope) + + if self.scope.is_global: + self.help += " This option can be used at most once either globally, or for one subcommand." + class CLI(click.MultiCommand): """Action list contains all actions with options available for CLI""" @@ -617,21 +671,32 @@ def init_cli(): if action_lists is None: action_lists = [] + shared_options = [] + for action_list in action_lists: # Global options for option_args in action_list.get("global_options", []): - option_args["param_decls"] = option_args.pop("names") - self.params.append(click.Option(**option_args)) + option = Option(**option_args) + self.params.append(option) + if option.scope.is_shared: + shared_options.append(option) + + for action_list in action_lists: # Global options validators self.global_action_callbacks.extend( action_list.get("global_action_callbacks", []) ) + for action_list in action_lists: # Actions for name, action in action_list.get("actions", {}).items(): + arguments = action.pop("arguments", []) options = action.pop("options", []) + if arguments is None: + arguments = [] + if options is None: options = [] @@ -639,9 +704,27 @@ def init_cli(): for alias in [name] + action.get("aliases", []): self.commands_with_aliases[alias] = name + for argument_args in arguments: + self._actions[name].params.append(Argument(**argument_args)) + + # Add all shared options + for option in shared_options: + self._actions[name].params.append(option) + for option_args in options: - option_args["param_decls"] = option_args.pop("names") - self._actions[name].params.append(click.Option(**option_args)) + option = Option(**option_args) + + if option.scope.is_shared: + raise FatalError( + '"%s" is defined for action "%s". ' + ' "shared" options can be declared only on global level' % (option.name, name) + ) + + # Promote options to global if see for the first time + if option.scope.is_global and option.name not in [o.name for o in self.params]: + self.params.append(option) + + self._actions[name].params.append(option) def list_commands(self, ctx): return sorted(self._actions) @@ -722,10 +805,26 @@ def init_cli(): def execute_tasks(self, tasks, **kwargs): ctx = click.get_current_context() - - # Validate global arguments global_args = PropertyDict(ctx.params) + # Set propagated global options + for task in tasks: + for key in list(task.action_args): + option = next((o for o in ctx.command.params if o.name == key), None) + if option and (option.scope.is_global or option.scope.is_shared): + local_value = task.action_args.pop(key) + global_value = global_args[key] + default = () if option.multiple else option.default + + if global_value != default and local_value != default and global_value != local_value: + raise FatalError( + 'Option "%s" provided for "%s" is already defined to a different value. ' + "This option can appear at most once in the command line." % (key, task.name) + ) + if local_value != default: + global_args[key] = local_value + + # Validate global arguments for action_callback in ctx.command.global_action_callbacks: action_callback(ctx, global_args, tasks) @@ -752,6 +851,13 @@ def init_cli(): % (task.name, dep) ) dep_task = ctx.invoke(ctx.command.get_command(ctx, dep)) + + # Remove global options from dependent tasks + for key in list(dep_task.action_args): + option = next((o for o in ctx.command.params if o.name == key), None) + if option and (option.scope.is_global or option.scope.is_shared): + dep_task.action_args.pop(key) + tasks.insert(0, dep_task) ready_to_run = False @@ -770,7 +876,7 @@ def init_cli(): ) else: print("Executing action: %s" % name_with_aliases) - task.run(ctx, global_args) + task.run(ctx, global_args, **task.action_args) completed_tasks.add(task.name) @@ -818,37 +924,41 @@ def init_cli(): args.build_dir = _realpath(args.build_dir) # Possible keys for action dict are: global_options, actions and global_action_callbacks + global_options = [ + { + "names": ["-D", "--define-cache-entry"], + "help": "Create a cmake cache entry.", + "scope": "global", + "multiple": True, + } + ] + root_options = { "global_options": [ { "names": ["-C", "--project-dir"], - "help": "Project directory", + "help": "Project directory.", "type": click.Path(), "default": os.getcwd(), }, { "names": ["-B", "--build-dir"], - "help": "Build directory", + "help": "Build directory.", "type": click.Path(), "default": None, }, { "names": ["-n", "--no-warnings"], - "help": "Disable Cmake warnings", + "help": "Disable Cmake warnings.", "is_flag": True, "default": False, }, { "names": ["-v", "--verbose"], - "help": "Verbose build output", + "help": "Verbose build output.", "is_flag": True, "default": False, }, - { - "names": ["-D", "--define-cache-entry"], - "help": "Create a cmake cache entry", - "multiple": True, - }, { "names": ["--no-ccache"], "help": "Disable ccache. Otherwise, if ccache is available on the PATH then it will be used for faster builds.", @@ -857,7 +967,7 @@ def init_cli(): }, { "names": ["-G", "--generator"], - "help": "CMake generator", + "help": "CMake generator.", "type": click.Choice(GENERATOR_CMDS.keys()), }, ], @@ -876,6 +986,7 @@ def init_cli(): + "2. Run CMake as necessary to configure the project and generate build files for the main build tool.\n\n" + "3. Run the main build tool (Ninja or GNU Make). By default, the build tool is automatically detected " + "but it can be explicitly set by passing the -G option to idf.py.\n\n", + "options": global_options, "order_dependencies": [ "reconfigure", "menuconfig", @@ -886,59 +997,75 @@ def init_cli(): "menuconfig": { "callback": build_target, "help": 'Run "menuconfig" project configuration tool.', + "options": global_options, }, "confserver": { "callback": build_target, "help": "Run JSON configuration server.", + "options": global_options, }, "size": { "callback": build_target, "help": "Print basic size information about the app.", + "options": global_options, "dependencies": ["app"], }, "size-components": { "callback": build_target, "help": "Print per-component size information.", + "options": global_options, "dependencies": ["app"], }, "size-files": { "callback": build_target, "help": "Print per-source-file size information.", + "options": global_options, "dependencies": ["app"], }, - "bootloader": {"callback": build_target, "help": "Build only bootloader."}, + "bootloader": { + "callback": build_target, + "help": "Build only bootloader.", + "options": global_options, + }, "app": { "callback": build_target, "help": "Build only the app.", "order_dependencies": ["clean", "fullclean", "reconfigure"], + "options": global_options, }, "efuse_common_table": { "callback": build_target, "help": "Genereate C-source for IDF's eFuse fields.", "order_dependencies": ["reconfigure"], + "options": global_options, }, "efuse_custom_table": { "callback": build_target, "help": "Genereate C-source for user's eFuse fields.", "order_dependencies": ["reconfigure"], + "options": global_options, }, "show_efuse_table": { "callback": build_target, "help": "Print eFuse table.", "order_dependencies": ["reconfigure"], + "options": global_options, }, "partition_table": { "callback": build_target, "help": "Build only partition table.", "order_dependencies": ["reconfigure"], + "options": global_options, }, "erase_otadata": { "callback": build_target, "help": "Erase otadata partition.", + "options": global_options, }, "read_otadata": { "callback": build_target, "help": "Read otadata partition.", + "options": global_options, }, } } @@ -952,6 +1079,7 @@ def init_cli(): + "but can be useful after adding/removing files from the source tree, or when modifying CMake cache variables. " + "For example, \"idf.py -DNAME='VALUE' reconfigure\" " + 'can be used to set variable "NAME" in CMake cache to value "VALUE".', + "options": global_options, "order_dependencies": ["menuconfig"], }, "clean": { @@ -972,36 +1100,41 @@ def init_cli(): } } + baud_rate = { + "names": ["-b", "--baud"], + "help": "Baud rate.", + "scope": "global", + "envvar": "ESPBAUD", + "default": 460800, + } + + port = { + "names": ["-p", "--port"], + "help": "Serial port.", + "scope": "global", + "envvar": "ESPPORT", + "default": None, + } + serial_actions = { - "global_options": [ - { - "names": ["-p", "--port"], - "help": "Serial port", - "envvar": "ESPPORT", - "default": None, - }, - { - "names": ["-b", "--baud"], - "help": "Baud rate", - "envvar": "ESPBAUD", - "default": 460800, - }, - ], "actions": { "flash": { "callback": flash, "help": "Flash the project.", + "options": global_options + [baud_rate, port], "dependencies": ["all"], "order_dependencies": ["erase_flash"], }, "erase_flash": { "callback": erase_flash, "help": "Erase entire flash chip.", + "options": [baud_rate, port], }, "monitor": { "callback": monitor, "help": "Display serial output.", "options": [ + port, { "names": ["--print-filter", "--print_filter"], "help": ( @@ -1027,18 +1160,21 @@ def init_cli(): "partition_table-flash": { "callback": flash, "help": "Flash partition table only.", + "options": [baud_rate, port], "dependencies": ["partition_table"], "order_dependencies": ["erase_flash"], }, "bootloader-flash": { "callback": flash, "help": "Flash bootloader only.", + "options": [baud_rate, port], "dependencies": ["bootloader"], "order_dependencies": ["erase_flash"], }, "app-flash": { "callback": flash, "help": "Flash the app only.", + "options": [baud_rate, port], "dependencies": ["app"], "order_dependencies": ["erase_flash"], }, @@ -1127,7 +1263,7 @@ def _find_usable_locale(): usable_locales.append(locale) if not usable_locales: - FatalError( + raise FatalError( "Support for Unicode filenames is required, but no suitable UTF-8 locale was found on your system." " Please refer to the manual for your operating system for details on locale reconfiguration." ) diff --git a/tools/unit-test-app/idf_ext.py b/tools/unit-test-app/idf_ext.py index d2e11d88a0..39973dc1c5 100644 --- a/tools/unit-test-app/idf_ext.py +++ b/tools/unit-test-app/idf_ext.py @@ -26,9 +26,7 @@ def action_extensions(base_actions, project_path=os.getcwd()): config_name = re.match(r"ut-apply-config-(.*)", ut_apply_config_name).group(1) def set_config_build_variables(prop, defval=None): - property_value = re.findall( - r"^%s=(.+)" % prop, config_file_content, re.MULTILINE - ) + property_value = re.findall(r"^%s=(.+)" % prop, config_file_content, re.MULTILINE) if property_value: property_value = property_value[0] else: @@ -98,22 +96,15 @@ def action_extensions(base_actions, project_path=os.getcwd()): sdkconfig_temp.flush() try: - args.define_cache_entry.append( - "SDKCONFIG_DEFAULTS=" + sdkconfig_temp.name - ) + args.define_cache_entry.append("SDKCONFIG_DEFAULTS=" + sdkconfig_temp.name) except AttributeError: - args.define_cache_entry = [ - "SDKCONFIG_DEFAULTS=" + sdkconfig_temp.name - ] + args.define_cache_entry = ["SDKCONFIG_DEFAULTS=" + sdkconfig_temp.name] reconfigure = base_actions["actions"]["reconfigure"]["callback"] reconfigure(None, ctx, args) else: if not config_name == "all-configs": - print( - "unknown unit test app config for action '%s'" - % ut_apply_config_name - ) + print("unknown unit test app config for action '%s'" % ut_apply_config_name) # This target builds the configuration. It does not currently track dependencies, # but is good enough for CI builds if used together with clean-all-configs. @@ -165,18 +156,14 @@ def action_extensions(base_actions, project_path=os.getcwd()): os.path.join(dest, "bootloader", "bootloader.bin"), ) - for partition_table in glob.glob( - os.path.join(src, "partition_table", "partition-table*.bin") - ): + for partition_table in glob.glob(os.path.join(src, "partition_table", "partition-table*.bin")): try: os.mkdir(os.path.join(dest, "partition_table")) except OSError: pass shutil.copyfile( partition_table, - os.path.join( - dest, "partition_table", os.path.basename(partition_table) - ), + os.path.join(dest, "partition_table", os.path.basename(partition_table)), ) shutil.copyfile( @@ -219,9 +206,7 @@ def action_extensions(base_actions, project_path=os.getcwd()): cache_entries.append("TEST_COMPONENTS='%s'" % " ".join(test_components)) if test_exclude_components: - cache_entries.append( - "TEST_EXCLUDE_COMPONENTS='%s'" % " ".join(test_exclude_components) - ) + cache_entries.append("TEST_EXCLUDE_COMPONENTS='%s'" % " ".join(test_exclude_components)) if cache_entries: global_args.define_cache_entry = list(global_args.define_cache_entry) @@ -229,23 +214,23 @@ def action_extensions(base_actions, project_path=os.getcwd()): # Brute force add reconfigure at the very beginning reconfigure_task = ctx.invoke(ctx.command.get_command(ctx, "reconfigure")) + # Strip arguments from the task + reconfigure_task.action_args = {} tasks.insert(0, reconfigure_task) # Add global options extensions = { - "global_options": [ - # For convenience, define a -T and -E argument that gets converted to -D arguments - { - "names": ["-T", "--test-components"], - "help": "Specify the components to test", - "multiple": True, - }, - { - "names": ["-E", "--test-exclude-components"], - "help": "Specify the components to exclude from testing", - "multiple": True, - }, - ], + "global_options": [{ + "names": ["-T", "--test-components"], + "help": "Specify the components to test.", + "scope": "shared", + "multiple": True, + }, { + "names": ["-E", "--test-exclude-components"], + "help": "Specify the components to exclude from testing.", + "scope": "shared", + "multiple": True, + }], "global_action_callbacks": [test_component_callback], "actions": {}, } @@ -260,23 +245,24 @@ def action_extensions(base_actions, project_path=os.getcwd()): config_apply_config_action_name = "ut-apply-config-" + config extensions["actions"][config_build_action_name] = { - "callback": ut_build, - "help": "Build unit-test-app with configuration provided in configs/NAME. " - + "Build directory will be builds/%s/, " % config_build_action_name - + "output binaries will be under output/%s/" % config_build_action_name, + "callback": + ut_build, + "help": + ("Build unit-test-app with configuration provided in configs/%s. " + "Build directory will be builds/%s/, output binaries will be under output/%s/" % (config, config, config)), } extensions["actions"][config_clean_action_name] = { "callback": ut_clean, - "help": "Remove build and output directories for configuration %s." - % config_clean_action_name, + "help": "Remove build and output directories for configuration %s." % config_clean_action_name, } extensions["actions"][config_apply_config_action_name] = { - "callback": ut_apply_config, - "help": "Generates configuration based on configs/%s in sdkconfig file." - % config_apply_config_action_name - + "After this, normal all/flash targets can be used. Useful for development/debugging.", + "callback": + ut_apply_config, + "help": + "Generates configuration based on configs/%s in sdkconfig file. " % config_apply_config_action_name + + "After this, normal all/flash targets can be used. Useful for development/debugging.", } build_all_config_deps.append(config_build_action_name)