[projmgr] Forbid version for pack selection with path specified

Open-CMSIS-Pack · Sep 6, 2023 · c5f3200 · c5f3200
1 parent 598bcf0
commit c5f3200
Show file tree

Hide file tree

Showing 10 changed files with 85 additions and 13 deletions.
diff --git a/tools/projmgr/schemas/common.schema.json b/tools/projmgr/schemas/common.schema.json
@@ -748,7 +748,7 @@
         "compiler":          { "$ref": "#/definitions/CompilerType" },
         "device":            { "$ref": "#/definitions/DeviceType" },
         "processor":         { "$ref": "#/definitions/BuildProcessorType" },
-        "packs":             { "$ref": "#/definitions/PacksType" },
+        "packs":             { "$ref": "#/definitions/BuildPacksType" },
         "misc":              { "$ref": "#/definitions/MiscType" },
         "define":            { "$ref": "#/definitions/DefinesType" },
         "add-path":          { "$ref": "#/definitions/AddpathsType" },
@@ -838,24 +838,44 @@
         }
       }
     },
+    "BuildPacksType": {
+      "type": "array",
+      "uniqueItems": true,
+      "items": { "$ref": "#/definitions/BuildPackType" }
+    },
+    "BuildPackType": {
+      "type": "object",
+      "properties": {
+        "pack": { "$ref": "#/definitions/PackID" },
+        "path": { "type": "string", "description": "Path to software pack" }
+      },
+      "required": [ "pack", "path" ],
+      "additionalProperties": false
+    },
     "PacksType": {
       "type": "array",
       "uniqueItems": true,
-      "items": { "$ref": "#/definitions/PackType" }
+      "items": { 
+        "oneOf": [
+          {"$ref": "#/definitions/PackType" },
+          {"$ref": "#/definitions/PackPathType" }
+        ]
+      }
     },
     "PackID": {
       "type": "string",
       "pattern": "^([\\w .-]+)((::[\\w .*-]+)(@(>=)?(\\d+\\.\\d+\\.\\d+((\\+|\\-)[\\w.+-]+)?))?)?$",
       "description": "Name of a required Software Pack in the format Vendor [:: Pack [@[>=] version]]"
     },
+    "VersionlessPackID": {
+      "type": "string",
+      "pattern": "^([\\w .-]+)(::[\\w .*-]+)?$",
+      "description": "Name of a required Software Pack in the format Vendor [:: Pack]"
+    },
     "PackType": {
       "type": "object",
       "properties": {
         "pack": { "$ref": "#/definitions/PackID" },
-        "path": {
-          "type": "string",
-          "description": "Path to software pack"
-        },
         "for-context":     { "$ref": "#/definitions/ForContext" },
         "not-for-context": { "$ref": "#/definitions/NotForContext" }
       },
@@ -865,6 +885,20 @@
       ],
       "additionalProperties": false
     },
+    "PackPathType": {
+      "type": "object",
+      "properties": {
+        "pack": { "$ref": "#/definitions/VersionlessPackID" },
+        "path": { "type": "string", "description": "Path to software pack" },
+        "for-context":     { "$ref": "#/definitions/ForContext" },
+        "not-for-context": { "$ref": "#/definitions/NotForContext" }
+      },
+      "allOf": [
+        { "$ref": "#/definitions/TypeListMutualExclusion" },
+        { "required": [ "pack", "path" ] }
+      ],
+      "additionalProperties": false
+    },
     "ResolvedComponentsType": {
       "type": "array",
       "description": "List of software components in their resolved state.",

diff --git a/tools/projmgr/src/ProjMgrWorker.cpp b/tools/projmgr/src/ProjMgrWorker.cpp
@@ -282,6 +282,10 @@ bool ProjMgrWorker::GetRequiredPdscFiles(ContextItem& context, const std::string
         errMsgs.insert("no match found for pack filter: " + filterStr);
       }
     } else {
+      if (!reqVersion.empty()) {
+        errMsgs.insert("pack '" + (pack.vendor.empty() ? "" : pack.vendor + "::") + pack.name
+          + "' specified with 'path' must not have a version");
+      }
       string packPath = packItem.path;
       RteFsUtils::NormalizePath(packPath, context.csolution->directory + "/");
       if (!RteFsUtils::Exists(packPath)) {

diff --git a/tools/projmgr/test/data/TestSolution/pack_path.csolution.yml b/tools/projmgr/test/data/TestSolution/pack_path.csolution.yml
@@ -6,7 +6,7 @@ solution:
       device: RteTest_ARMCM0
 
   packs:
-    - pack: ARM::RteTest_DFP@0.2.0
+    - pack: ARM::RteTest_DFP
       path: ../SolutionSpecificPack
 
   projects:

diff --git a/tools/projmgr/test/data/TestSolution/pack_path_invalid.csolution.yml b/tools/projmgr/test/data/TestSolution/pack_path_invalid.csolution.yml
@@ -0,0 +1,13 @@
+# yaml-language-server: $schema=https://raw.githubusercontent.com/Open-CMSIS-Pack/devtools/main/tools/projmgr/schemas/csolution.schema.json
+
+solution:
+  target-types:
+    - type: CM0
+      device: RteTest_ARMCM0
+
+  packs:
+    - pack: ARM::[email protected]
+      path: ../SolutionSpecificPack
+
+  projects:
+    - project: pack_path.cproject.yml
diff --git a/tools/projmgr/test/data/TestSolution/test_local_pack_file_not_found.csolution.yml b/tools/projmgr/test/data/TestSolution/test_local_pack_file_not_found.csolution.yml
@@ -71,7 +71,7 @@ solution:
         - ./path/CM3/to-be-removed
 
   packs:
-    - pack: ARM::RteTest_DFP@0.2.0
+    - pack: ARM::RteTest_DFP
       path: ../SolutionSpecificPack/Device
 
   projects:

diff --git a/tools/projmgr/test/data/TestSolution/test_local_pack_path.csolution.yml b/tools/projmgr/test/data/TestSolution/test_local_pack_path.csolution.yml
@@ -71,7 +71,7 @@ solution:
         - ./path/CM3/to-be-removed
 
   packs:
-    - pack: ARM::RteTest_DFP@0.2.0
+    - pack: ARM::RteTest_DFP
       path: ../SolutionSpecificPack
 
   projects:

diff --git a/tools/projmgr/test/data/TestSolution/test_local_pack_path_not_found.csolution.yml b/tools/projmgr/test/data/TestSolution/test_local_pack_path_not_found.csolution.yml
@@ -71,7 +71,7 @@ solution:
         - ./path/CM3/to-be-removed
 
   packs:
-    - pack: ARM::RteTest_DFP@0.2.0
+    - pack: ARM::RteTest_DFP
       path: ./SolutionSpecificPack/ARM
 
   projects:

diff --git a/tools/projmgr/test/data/Validation/missing-condition.csolution.yml b/tools/projmgr/test/data/Validation/missing-condition.csolution.yml
@@ -6,7 +6,7 @@ solution:
 
   packs:
     - pack: ARM::[email protected]
-    - pack: ARM::RteTestMissingCondition@0.1.0
+    - pack: ARM::RteTestMissingCondition
       path: ../InvalidPacks/ARM/RteTestMissingCondition/0.1.0
 
   target-types:

diff --git a/tools/projmgr/test/data/Validation/recursive.csolution.yml b/tools/projmgr/test/data/Validation/recursive.csolution.yml
@@ -6,7 +6,7 @@ solution:
 
   packs:
     - pack: ARM::[email protected]
-    - pack: ARM::RteTestRecursive@0.1.0
+    - pack: ARM::RteTestRecursive
       path: ../InvalidPacks/ARM/RteTestRecursive/0.1.0
 
   target-types:

diff --git a/tools/projmgr/test/src/ProjMgrUnitTests.cpp b/tools/projmgr/test/src/ProjMgrUnitTests.cpp
@@ -2416,9 +2416,30 @@ TEST_F(ProjMgrUnitTests, RunProjMgrSolution_Local_Pack) {
   EXPECT_EQ(0, RunProjMgr(6, argv, 0));
 
   // Check generated CPRJ
- ProjMgrTestEnv:: CompareFile(testoutput_folder + "/pack_path+CM0.cprj",
+  ProjMgrTestEnv:: CompareFile(testoutput_folder + "/pack_path+CM0.cprj",
     testinput_folder + "/TestSolution/ref/pack_path+CM0.cprj");
+}
+
+TEST_F(ProjMgrUnitTests, RunProjMgrSolution_Local_Pack_Invalid) {
+  StdStreamRedirect streamRedirect;
+  char* argv[6];
+
+  // verify schema check
+  const string& csolution = testinput_folder + "/TestSolution/pack_path_invalid.csolution.yml";
+  argv[1] = (char*)"convert";
+  argv[2] = (char*)csolution.c_str();
+  argv[3] = (char*)"-o";
+  argv[4] = (char*)testoutput_folder.c_str();
+  EXPECT_EQ(1, RunProjMgr(5, argv, 0));
+  auto errStr = streamRedirect.GetErrorString();
+  EXPECT_NE(string::npos, errStr.find("error csolution: schema check failed, verify syntax"));
 
+  // run with schema check disabled
+  streamRedirect.ClearStringStreams();
+  argv[5] = (char*)"-n";
+  EXPECT_EQ(1, RunProjMgr(6, argv, 0));
+  errStr = streamRedirect.GetErrorString();
+  EXPECT_NE(string::npos, errStr.find("error csolution: pack 'ARM::RteTest_DFP' specified with 'path' must not have a version"));
 }
 
 TEST_F(ProjMgrUnitTests, RunProjMgrSolution_Local_Multiple_Pack_Files) {