Cannot pass more than one -D option to idf.py (IDFGH-1275)

[bpietsch]

I want to declare multiple cache entries via the -D option to idf.py, but each successive one overwrites the previous when they are being parsed. Ironically, the -T option in unit-test-app maps to -D, but explicitly overrides the action behavior to append rather than overwrite. I think the -D action just needs to do the same.

[bpietsch]

I figured out I had some slightly incorrect syntax. As defined, it needs to be "-D Foo=bar BAZ=zed", but that results in a sub-list under the define_cache_entry list. Then, when adding the -T option you end up with a mixture of the sub-list plus a separate individual string. With the change below, the syntax changes to specifying individual -D options for each cache entry you want to add, but each one ends up as a top-level string in the define_cache_entry list, which is what is needed for the options to actually take effect as expected.

diff --git a/tools/idf.py b/tools/idf.py
index c9a3e464e..fd765658f 100755
--- a/tools/idf.py
+++ b/tools/idf.py
@@ -557,7 +557,7 @@ def main():
     parser.add_argument('-G', '--generator', help="Cmake generator", choices=GENERATOR_CMDS.keys())
     parser.add_argument('-n', '--no-warnings', help="Disable Cmake warnings", action="store_true")
     parser.add_argument('-v', '--verbose', help="Verbose build output", action="store_true")
-    parser.add_argument('-D', '--define-cache-entry', help="Create a cmake cache entry", nargs='+')
+    parser.add_argument('-D', '--define-cache-entry', help="Create a cmake cache entry", action="append")
     parser.add_argument('--no-ccache', help="Disable ccache. Otherwise, if ccache is available on the PATH then it will be used for faster builds.",
                         action="store_true")
     parser.add_argument('actions', help="Actions (build targets or other operations)", nargs='+',

[bpietsch]

Additional recommended change... with -T having nargs=+, you can't put the "action" (i.e. build) immediately after -T (or -D, for that matter without the change in my previous comment), yet the usage output of idf.py implies you should put the action at the end, resulting in very cryptic errors (that I had to debug the python code in a debugger to figure out).

diff --git a/tools/unit-test-app/idf_ext.py b/tools/unit-test-app/idf_ext.py
index 06b843f01..8de2df569 100644
--- a/tools/unit-test-app/idf_ext.py
+++ b/tools/unit-test-app/idf_ext.py
@@ -45,6 +45,8 @@ class TestComponentAction(argparse.Action):
 
         # Brute force add reconfigure at the very beginning
         existing_actions = getattr(namespace, "actions", [])
+        if existing_actions is None:
+            existing_actions = []
         if "reconfigure" not in existing_actions:
             existing_actions = ["reconfigure"] + existing_actions
         setattr(namespace, "actions", existing_actions)
@@ -66,6 +68,8 @@ class TestExcludeComponentAction(argparse.Action):
 
         # Brute force add reconfigure at the very beginning
         existing_actions = getattr(namespace, "actions", [])
+        if existing_actions is None:
+            existing_actions = []
         if "reconfigure" not in existing_actions:
             existing_actions = ["reconfigure"] + existing_actions
         setattr(namespace, "actions", existing_actions)
@@ -73,9 +77,9 @@ class TestExcludeComponentAction(argparse.Action):
 
 def add_argument_extensions(parser):
     # For convenience, define a -T argument that gets converted to -D arguments
-    parser.add_argument('-T', '--test-component', help="Specify the components to test", nargs='+', action=TestComponentAction)
+    parser.add_argument('-T', '--test-component', help="Specify the components to test", action=TestComponentAction)
     # For convenience, define a -T argument that gets converted to -D arguments
-    parser.add_argument('-E', '--test-exclude-components', help="Specify the components to exclude from testing", nargs='+', action=TestExcludeComponentAction)
+    parser.add_argument('-E', '--test-exclude-components', help="Specify the components to exclude from testing", action=TestExcludeComponentAction)

[bpietsch]

Hmmm... OK, scratch the previous idf_ext.py patch... that introduced a new problem (it needs to translate to a single -D TEST_COMPONENTS='...' For now, I just moved the "build" positional arg to the beginning of my argument list, but that isn't obvious (per my previous comment)

[renzbagaporo]

Hi @bpietsch, like #3571, this is also in the scope of the changes under review.

[bpietsch]

Thanks. I also am struggling with how quote -D options that have spaces in them... e.g. if I want to pass multiple paths to EXTRA_COMPONENT_DIRS (i.e. -D EXTRA_COMPONENT_DIRS="/foo /bar"). I get a cmake error: "define_property command is not scriptable"

[renzbagaporo]

Is a component in your EXTRA_COMPONENT_DIRS calling define_property before register_component call? We do an early expansion of components, that is, there is a step where component CMakeLists.txt is processed in script mode up until register_component. And script mode forbids certain commands, define_property being one of them.

[bpietsch]

Nope, only a few "set" calls, but just to be sure I moved register_component to the very top and am still getting the error.

[bpietsch]

Ugh... user error... the call stack from the make error clued me in that I was specifying my project directory and not the components sub-directory for EXTRA_COMPONENTS_DIR. Sorry for the noise, but appreciate your help.

[renzbagaporo]

Glad to see it work out @bpietsch. Regarding MR, this issue (and your other one) should be closed once those changes I mentioned are synced to this repo.