Fix for issue #1356 (#1364)

* Fix for issue #1356 unclear warning csolution: required pack 'ARM::[email protected]' is not loaded #1356 Co-authored-by: Evgueni Driouk <[email protected]> Co-authored-by: Daniel Brondani <[email protected]>
Open-CMSIS-Pack · Mar 8, 2024 · da790a4 · da790a4
1 parent 45ecce5
commit da790a4
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 29 deletions.
diff --git a/tools/projmgr/include/ProjMgrWorker.h b/tools/projmgr/include/ProjMgrWorker.h
@@ -639,6 +639,7 @@ class ProjMgrWorker {
   bool m_relativePaths;
 
   bool LoadPacks(ContextItem& context);
+  bool CheckMissingPackRequirements(const std::string& contextName);
   bool CollectRequiredPdscFiles(ContextItem& context, const std::string& packRoot);
   bool CheckRteErrors(void);
   bool CheckBoardDeviceInLayer(const ContextItem& context, const ClayerItem& clayer);

diff --git a/tools/projmgr/src/ProjMgrWorker.cpp b/tools/projmgr/src/ProjMgrWorker.cpp
@@ -425,22 +425,33 @@ bool ProjMgrWorker::LoadPacks(ContextItem& context) {
   filter.SetSelectedPackages(selectedPacks);
   context.rteActiveTarget->SetPackageFilter(filter);
   context.rteActiveTarget->UpdateFilterModel();
+  return CheckRteErrors();
+}
 
-  RtePackageMap allRequiredPacks;
+bool ProjMgrWorker::CheckMissingPackRequirements(const std::string& contextName)
+{
+  bool bRequiredPacksLoaded = true;
   // check if all pack requirements are fulfilled
-  for (auto pack : m_loadedPacks) {
+  for(auto pack : m_loadedPacks) {
+    RtePackageMap allRequiredPacks;
     pack->GetRequiredPacks(allRequiredPacks, m_model);
-  }
-  for (auto [id, pack] : allRequiredPacks) {
-    if (!pack) {
-      string msg("context '");
-      msg += context.name;
-      msg += "': required pack '";
-      msg += id + "' is not loaded";
-      ProjMgrLogger::Warn(msg);
+    for(auto [id, p] : allRequiredPacks) {
+      if(!p) {
+        bRequiredPacksLoaded = false;
+        string msg;
+        if(!contextName.empty()) {
+          msg += "context '";
+          msg += contextName;
+          msg += "': ";
+        }
+        msg += "pack '";
+        msg += id + "' required by pack '";
+        msg += pack->GetID() + "' is not specified";
+        ProjMgrLogger::Warn(msg);
+      }
     }
   }
-  return CheckRteErrors();
+  return bRequiredPacksLoaded;
 }
 
 std::vector<PackageItem> ProjMgrWorker::GetFilteredPacks(const PackageItem& packItem, const string& rtePath) const
@@ -3120,6 +3131,7 @@ bool ProjMgrWorker::ProcessContext(ContextItem& context, bool loadGenFiles, bool
     return false;
   }
   if (!ProcessDevice(context)) {
+    CheckMissingPackRequirements(context.name);
     return false;
   }
   if (!SetTargetAttributes(context, context.targetAttributes)) {
@@ -3134,11 +3146,13 @@ bool ProjMgrWorker::ProcessContext(ContextItem& context, bool loadGenFiles, bool
   }
   ret &= ProcessConfigFiles(context);
   ret &= ProcessComponentFiles(context);
+  bool bUnresolvedDependencies = false;
   if (resolveDependencies) {
     // TODO: Add uniquely identified missing dependencies to RTE Model
 
     // Get dependency validation results
     if (!ValidateContext(context)) {
+      bUnresolvedDependencies = true;
       string msg = "dependency validation for context '" + context.name + "' failed:";
       set<string> results;
       FormatValidationResults(results, context);
@@ -3152,6 +3166,9 @@ bool ProjMgrWorker::ProcessContext(ContextItem& context, bool loadGenFiles, bool
       }
     }
   }
+  if(!ret || bUnresolvedDependencies) {
+    CheckMissingPackRequirements(context.name);
+  }
   return ret;
 }
 
@@ -3214,6 +3231,10 @@ bool ProjMgrWorker::ListPacks(vector<string>&packs, bool bListMissingPacksOnly,
       for (const auto& pack : m_loadedPacks) {
         packsMap[pack->GetID()] = pack->GetPackageFileName();
       }
+      if(m_loadPacksPolicy == LoadPacksPolicy::REQUIRED || m_loadPacksPolicy == LoadPacksPolicy::DEFAULT) {
+        // check if additional dependencies must be added
+        CheckMissingPackRequirements(RteUtils::EMPTY_STRING);
+      }
     }
 
     if (!m_contextErrMap.empty()) {

diff --git a/tools/projmgr/test/src/ProjMgrUnitTests.cpp b/tools/projmgr/test/src/ProjMgrUnitTests.cpp
@@ -88,37 +88,47 @@ TEST_F(ProjMgrUnitTests, RunProjMgr_Version) {
 
 TEST_F(ProjMgrUnitTests, RunProjMgr_Packs_Required_Warning) {
   StdStreamRedirect streamRedirect;
+  const vector<string> warnings = {
+    "pack 'ARM::[email protected]:0.2.0' required by pack 'ARM::[email protected]' is not specified",
+    "pack 'ARM::[email protected]:2.0.0' required by pack 'ARM::[email protected]' is not specified",
+    "pack 'ARM::[email protected]:0.2.0' required by pack 'ARM::[email protected]' is not specified"
+  };
+
   const string csolution = testinput_folder + "/TestSolution/test_pack_requirements.csolution.yml";
   char* argv[9];
+
+  //  list packs
+  argv[1] = (char*)"list";
+  argv[2] = (char*)"packs";
+  argv[3] = (char*)"--solution";
+  argv[4] = (char*)csolution.c_str();
+  EXPECT_EQ(0, RunProjMgr(5, argv, 0));
+
+  auto errStr = streamRedirect.GetErrorString();
+  EXPECT_TRUE(errStr.find(warnings[1]) != string::npos);
+
+// convert
   argv[1] = (char*)"convert";
   argv[2] = (char*)"--solution";
   argv[3] = (char*)csolution.c_str();
   argv[4] = (char*)"-o";
   argv[5] = (char*)testoutput_folder.c_str();
   argv[6] = (char*)"-c";
   argv[7] = (char*)"test1.Debug+CM0";
-  EXPECT_EQ(1, RunProjMgr(8, argv, 0)); //fails because DFP is not loaded
-
-  auto errStr = streamRedirect.GetErrorString();
-
-  auto pos = errStr.find("required pack 'ARM::[email protected]:2.0.0' is not loaded");
-  EXPECT_TRUE(pos != string::npos);
-  pos = errStr.find("required pack 'ARM::[email protected]:0.2.0' is not loaded");
-  EXPECT_TRUE(pos != string::npos);
-  pos = errStr.find("required pack 'ARM::[email protected]:0.2.0' is not loaded");
-  EXPECT_FALSE(pos != string::npos);
+  EXPECT_EQ(1, RunProjMgr(8, argv, 0)); //fails because DFP is not loaded => pack warnings
+  errStr = streamRedirect.GetErrorString();
+  // pack warnings are printed
+  for(auto& w : warnings) {
+    EXPECT_TRUE(errStr.find(w) != string::npos);
+  }
 
   streamRedirect.ClearStringStreams();
   argv[7] = (char*)"test1.Release+CM0";
-  EXPECT_EQ(0, RunProjMgr(8, argv, 0)); // succeeds regardless of warnings
-
+  EXPECT_EQ(0, RunProjMgr(8, argv, 0)); // succeeds regardless missing pack requirement  => no pack warnings
   errStr = streamRedirect.GetErrorString();
-  pos = errStr.find("required pack 'ARM::[email protected]:0.2.0' is not loaded");
-  EXPECT_TRUE(pos != string::npos);
-  pos = errStr.find("required pack 'ARM::[email protected]:2.0.0' is not loaded");
-  EXPECT_TRUE(pos != string::npos);
-  pos = errStr.find("required pack 'ARM::[email protected]:0.2.0' is not loaded");
-  EXPECT_FALSE(pos != string::npos);
+  for(auto& w : warnings) {
+    EXPECT_FALSE(errStr.find(w) != string::npos);
+  }
 }
 
 TEST_F(ProjMgrUnitTests, RunProjMgr_ListPacks) {