[packchk] missing component (Cvariant) not detected when specified in…

… <accept> (M504:) #1052 (#650) (#1061) * [packchk] missing component (Cvariant) not detected when specified in <accept> (M504:) #1052 added missing checks added test cases Co-authored-by: Thorsten de Buhr <[email protected]>
Open-CMSIS-Pack · Jul 13, 2023 · 3e6154d · 3e6154d
1 parent 22be65c
commit 3e6154d
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 6 deletions.
diff --git a/test/packs/ARM/RteTest_DFP/0.2.0/ARM.RteTest_DFP.pdsc b/test/packs/ARM/RteTest_DFP/0.2.0/ARM.RteTest_DFP.pdsc
@@ -281,6 +281,15 @@
       <accept condition="BoardTest2"/>
       <accept condition="BoardTest3"/>
     </condition>
+
+    <condition id="Test_Components_exist">
+      <description>Test if components specified by expressions can be found</description>
+      <accept Cclass="RteTest" Cgroup="GlobalFile"/>
+      <accept Cclass="RteTest" Cgroup="LocalFile"/>
+      <require Cclass="RteTest" Cgroup="Dependency" Csub="Variant" Cvariant="Compatible"/>
+      <deny Cclass="RteTest" Cgroup="Dependency" Csub="Incompatible_component"/>
+    </condition>
+
   </conditions>
 
   <apis>
@@ -353,7 +362,7 @@
     </component>
 
     <!-- Component variants -->
-    <component Cclass="Device" Cgroup="Test variant" Cvariant="" Cversion="1.1.1" condition="ARMCM0 RteTest">
+    <component Cclass="Device" Cgroup="Test variant" Cversion="1.1.1" condition="ARMCM0 RteTest">
       <description>Test variant without variant name</description>
       <files>
         <file category="header"  name="Device/ARM/ARMCM0/Include/ARMCM0.h"/>
@@ -367,21 +376,21 @@
       </files>
     </component>
 
-    <component Cclass="Device" Cgroup="Test variant" Csub="Sub" Cvariant="" Cversion="1.1.1" condition="ARMCM0 RteTest">
+    <component Cclass="Device" Cgroup="Test variant" Csub="Sub" Cversion="1.1.1" condition="ARMCM0 RteTest">
       <description>Test variant with Csub</description>
       <files>
         <file category="header"  name="Device/ARM/ARMCM0/Include/ARMCM0.h"/>
       </files>
     </component>
 
-    <component Cclass="Device" Cgroup="Test variant 2" Cvariant="" Cversion="2.2.2" condition="ARMCM0 RteTest">
+    <component Cclass="Device" Cgroup="Test variant 2" Cversion="2.2.2" condition="ARMCM0 RteTest">
       <description>Test variant 2 without variant name, version 2.2.2</description>
       <files>
         <file category="header"  name="Device/ARM/ARMCM0/Include/ARMCM0.h"/>
       </files>
     </component>
 
-    <component Cclass="Device" Cgroup="Test variant 2" Cvariant="" Cversion="3.3.3" condition="ARMCM0 RteTest">
+    <component Cclass="Device" Cgroup="Test variant 2" Cversion="3.3.3" condition="ARMCM0 RteTest">
       <description>Test variant 2 without variant name, version 3.3.3</description>
       <files>
         <file category="header"  name="Device/ARM/ARMCM0/Include/ARMCM0.h"/>

diff --git a/tools/packchk/src/CheckConditions.cpp b/tools/packchk/src/CheckConditions.cpp
@@ -20,7 +20,9 @@ DefinedConditionsVisitor::DefinedConditionsVisitor(CheckConditions* conditions)
 {
   static const map<string, string> EMPTY_MAP;
   m_filteredModel = new RteModel();
-  m_target = new RteTarget(nullptr, m_filteredModel, "CondTest", EMPTY_MAP);
+  m_target = new RteTarget(&conditions->GetModel(), m_filteredModel, "CondTest", EMPTY_MAP);
+  m_target->SetTargetSupported(true);
+  m_target->UpdateFilterModel();
 };
 
 /**
@@ -53,6 +55,18 @@ VISIT_RESULT DefinedConditionsVisitor::Visit(RteItem* item)
     }
 
     m_Conditions->AddDefinedCondition(cond);
+
+    return VISIT_RESULT::CONTINUE_VISIT;
+  }
+
+
+  RteConditionExpression* expr = dynamic_cast<RteConditionExpression*>(item);
+  if(expr && expr->IsDependencyExpression()) {
+    set<RteComponentAggregate*> components;
+    m_target->GetComponentAggregates(*expr, components);
+    if (components.empty()) {
+      LogMsg("M317", NAME(expr->GetParent()->GetID()), NAME2(expr->GetID()), expr->GetLineNumber());
+    }
   }
 
   return VISIT_RESULT::CONTINUE_VISIT;

diff --git a/tools/packchk/src/PackChk_Msgs.cpp b/tools/packchk/src/PackChk_Msgs.cpp
@@ -130,7 +130,7 @@ const MsgTable PackChk::msgTable = {
   { "M314", { MsgLevel::LEVEL_WARNING,  CRLF_B, "Filename '%NAME%' contains whitespaces, this is not recommended" } },
   { "M315", { MsgLevel::LEVEL_ERROR,    CRLF_B, "Invalid URL / Paths to Drives are not allowed in Package URL: '%URL%'" } },
   { "M316", { MsgLevel::LEVEL_WARNING,  CRLF_B, "URL must end with slash '/': '%URL%'" } },
-  { "M317", { MsgLevel::LEVEL_ERROR,    CRLF_B, "" } },
+  { "M317", { MsgLevel::LEVEL_WARNING,  CRLF_B, "Condition '%NAME%': No component found for '%NAME2%'" } },
   { "M318", { MsgLevel::LEVEL_ERROR,    CRLF_B, "" } },
   { "M319", { MsgLevel::LEVEL_ERROR,    CRLF_B, "" } },
   { "M320", { MsgLevel::LEVEL_ERROR,    CRLF_B, "" } },

diff --git a/tools/packchk/test/integtests/src/PackChkIntegTests.cpp b/tools/packchk/test/integtests/src/PackChkIntegTests.cpp
@@ -791,5 +791,76 @@ TEST_F(PackChkIntegTests, CheckSchemaValidation) {
 }
 
 
+TEST_F(PackChkIntegTests, CheckConditionComponentDependency_Neg) {
+  const char* argv[5];
+
+  const string& pdscFile = PackChkIntegTestEnv::globaltestdata_dir +
+    "/packs/ARM/RteTest_DFP/0.2.0/ARM.RteTest_DFP.pdsc";
+  const string& refFile = PackChkIntegTestEnv::globaltestdata_dir +
+    "/packs/ARM/RteTest/0.1.0/ARM.RteTest.pdsc";
+  ASSERT_TRUE(RteFsUtils::Exists(pdscFile));
+  ASSERT_TRUE(RteFsUtils::Exists(refFile));
+
+  argv[0] = (char*)"";
+  argv[1] = (char*)pdscFile.c_str();
+  argv[2] = (char*)"-x";
+  argv[3] = (char*)"!M317";
+  argv[4] = (char*)"--disable-validation";
+
+  PackChk packChk;
+  EXPECT_EQ(0, packChk.Check(5, argv, nullptr));
+
+  auto errMsgs = ErrLog::Get()->GetLogMessages();
+  int M317_foundCnt = 0;
+  for (const string& msg : errMsgs) {
+    size_t s;
+    if ((s = msg.find("M317", 0)) != string::npos) {
+      M317_foundCnt++;
+    }
+  }
+
+  if(M317_foundCnt != 4) {
+    FAIL() << "error: warning M317 count != 4";
+  }
+}
+
+TEST_F(PackChkIntegTests, CheckConditionComponentDependency_Pos) {
+  const char* argv[7];
+
+  const string& pdscFile = PackChkIntegTestEnv::globaltestdata_dir +
+    "/packs/ARM/RteTest_DFP/0.2.0/ARM.RteTest_DFP.pdsc";
+  const string& refFile = PackChkIntegTestEnv::globaltestdata_dir +
+    "/packs/ARM/RteTest/0.1.0/ARM.RteTest.pdsc";
+  ASSERT_TRUE(RteFsUtils::Exists(pdscFile));
+  ASSERT_TRUE(RteFsUtils::Exists(refFile));
+
+  argv[0] = (char*)"";
+  argv[1] = (char*)pdscFile.c_str();
+  argv[2] = (char*)"-i";
+  argv[3] = (char*)refFile.c_str();
+  argv[4] = (char*)"-x";
+  argv[5] = (char*)"!M317";
+  argv[6] = (char*)"--disable-validation";
+
+  PackChk packChk;
+  EXPECT_EQ(0, packChk.Check(7, argv, nullptr));
+
+  auto errMsgs = ErrLog::Get()->GetLogMessages();
+  int M317_foundCnt = 0;
+  for (const string& msg : errMsgs) {
+    size_t s;
+    if ((s = msg.find("M317", 0)) != string::npos) {
+      M317_foundCnt++;
+    }
+  }
+
+  if(M317_foundCnt) {
+    FAIL() << "error: warning M317 found";
+  }
+}
+
+
+
+