[packchk] Add a validation step issuing a warning in case no license information is contained in a pack #1304 (#1149) (#1974)

* [packchk] Add a validation step issuing a warning in case no license
information is contained in a pack #1304

Co-authored-by: Thorsten de Buhr <[email protected]>

Author: spcaipers-arm

tools/packchk/include/ValidateSyntax.h

@@ -63,6 +63,7 @@ class ValidateSyntax : public Validate
   bool CheckRequirements(RtePackage* pKg);
   bool CheckRequirements_Packages(RteItem* requirement);
   bool CheckLicense(RtePackage* pKg, CheckFilesVisitor& fileVisitor);
+  bool CheckLicenseFile(RteItem* item, const std::string& path);
   bool CheckFeatureDevice(RteDeviceProperty* prop, const std::string& devName);
   bool CheckFeatureBoard(RteItem* prop, const std::string& boardName);
   bool CheckDeviceProperties(RtePackage* pKg);

tools/packchk/src/PackChk_Msgs.cpp

@@ -233,9 +233,9 @@ const MsgTable PackChk::msgTable = {
 
   // 600... further PackCHk Errors
   { "M600", { MsgLevel::LEVEL_WARNING,  CRLF_B, "Attribute '%ATTR%' on '%TAG%' is deprecated." } },
-  { "M601", { MsgLevel::LEVEL_WARNING,  CRLF_B, "" } },
-  { "M602", { MsgLevel::LEVEL_WARNING,  CRLF_B, "" } },
-  { "M603", { MsgLevel::LEVEL_WARNING,  CRLF_B, "" } },
+  { "M601", { MsgLevel::LEVEL_WARNING,  CRLF_B, "'%TAG%' missing on '%TAG2%'" } },
+  { "M602", { MsgLevel::LEVEL_WARNING,  CRLF_B, "Attribute '%ATTR%' already set, see Line %LINE%" } },
+  { "M603", { MsgLevel::LEVEL_WARNING,  CRLF_B, "No license information found" } },
   { "M604", { MsgLevel::LEVEL_WARNING,  CRLF_B, "" } },
   { "M605", { MsgLevel::LEVEL_WARNING,  CRLF_B, "" } },
   { "M606", { MsgLevel::LEVEL_WARNING,  CRLF_B, "" } },

tools/packchk/src/ValidateSyntax.cpp

@@ -152,6 +152,27 @@ bool ValidateSyntax::CheckAllFiles(RtePackage* pKg)
   return(true);
 }
 
+bool ValidateSyntax::CheckLicenseFile(RteItem* item, const string& path) {
+  bool bOk = false;
+  const auto lineNo = item->GetLineNumber();
+
+  if(!path.empty()) {
+    if(XmlValueAdjuster::IsAbsolute(path)) {
+      LogMsg("M326", PATH(path), lineNo);   // error : absolute paths are not permitted
+    }
+    else if(path.find('\\') != string::npos) {
+      if(XmlValueAdjuster::IsURL(path)) {
+        LogMsg("M370", URL(path), lineNo);  // error : backslash are non permitted in URL
+      }
+      else {
+        LogMsg("M327", PATH(path), lineNo); // error : backslash are not recommended
+      }
+    }
+  }
+
+  return bOk;
+}
+
 /**
  * @brief check (for) license file
  * @param pKg package under test
@@ -160,23 +181,82 @@ bool ValidateSyntax::CheckAllFiles(RtePackage* pKg)
  */
 bool ValidateSyntax::CheckLicense(RtePackage* pKg, CheckFilesVisitor& fileVisitor)
 {
+  bool bLicFound = false;
   const string& licPath = pKg->GetItemValue("license");
-  if(licPath.empty()) {
-    return true;
+  if(!licPath.empty()) {
+    bLicFound = true;
+    CheckLicenseFile(pKg, licPath);
   }
 
-  if(XmlValueAdjuster::IsAbsolute(licPath)) {
-    LogMsg("M326", PATH(licPath));   // error : absolute paths are not permitted
-  }
-  else if(licPath.find('\\') != string::npos) {
-    if(XmlValueAdjuster::IsURL(licPath)) {
-      LogMsg("M370", URL(licPath));  // error : backslash are non permitted in URL
+  const auto licenseSets = pKg->GetLicenseSets();
+  if(licenseSets) {
+    const auto& children = licenseSets->GetChildren();
+    if(children.empty()) {
+      LogMsg("M601", TAG("licenseSet"), TAG2("licenseSets")); // license set must be defined if <licenseSets> 
     }
-    else {
-      LogMsg("M327", PATH(licPath)); // error : backslash are not recommended
+
+    map<string, RteItem*> licenseSetIds;
+    RteItem* defaultLicenseSet = nullptr;
+
+    for(const auto& child : children) {
+      bLicFound = true;
+      const auto& id = child->GetID();
+      if(id.empty()) {
+        LogMsg("M308", TAG("id"), TAG2("licenseSet")); // Attribute '%TAG%' missing on '%TAG2%'
+        continue;
+      }
+
+      const auto& found = licenseSetIds.find(id);
+      if(found != licenseSetIds.end()) {
+        const auto& foundId = found->first;
+        const auto foundChild = found->second;
+        LogMsg("M367", TYP("id"), NAME(foundId), LINE(foundChild->GetLineNumber()), child->GetLineNumber()); // Redefined %TYPE% '%NAME%' found, see Line %LINE%
+      }
+      else {
+        licenseSetIds[id] = child;
+      }
+
+      if(child->GetAttribute("default") == "1") {
+        if(defaultLicenseSet) {
+          LogMsg("M602", VAL("ATTR", "default")); // Attribute '%ATTR%' already set, see Line %LINE%
+        }
+        else {
+          defaultLicenseSet = child;
+        }
+      }
+
+      const auto& lincenseChilds = child->GetChildren();
+      if(lincenseChilds.empty()) {
+        LogMsg("M601", TAG("license"), TAG2("licenseSet")); // '%TAG%' missing on '%TAG2%'
+      }
+      else {
+        for(const auto lincenseChild : lincenseChilds) {
+          const auto& licensePath = lincenseChild->GetAttribute("name");
+          if(licensePath.empty()) {
+            LogMsg("M601", TAG("name"), TAG2("license")); // '%TAG%' missing on '%TAG2%'
+          }
+          else {
+            CheckLicenseFile(lincenseChild, licensePath);
+          }
+
+          const auto& title = lincenseChild->GetAttribute("title");
+          if(!title.length()) {
+            LogMsg("M601", TAG("title"), TAG2("license")); // '%TAG%' missing on '%TAG2%'
+          }
+
+          const auto& url = lincenseChild->GetAttribute("url");
+          if(!url.empty()) {
+            CheckLicenseFile(lincenseChild, url);
+          }
+        }
+      }
     }
   }
 
+  if(!bLicFound) {
+    LogMsg("M603"); // no license information found
+  }
+
   CheckFiles& checkFiles = fileVisitor.GetCheckFiles();
   if(!checkFiles.CheckFileExists(licPath, -1)) {
     return false;

tools/packchk/test/data/TestLicense/TestVendor.TestPackLicense.pdsc

@@ -13,6 +13,22 @@
     </release>
   </releases>
 
+  <licenseSets>
+    <licenseSet id="all" default="true" gating="true">
+      <license name="./licenses/license1.txt" title="BSD-3 Clause License for components" spdx="BSD-3-Clause"/>
+      <license name="./licenses/license2.txt" title="MIT License for device support" spdx="MIT"/>
+    </licenseSet>
+    <licenseSet id="Implementation">
+    <license name="./licenses/myProprietaryLicense.txt" title="ProprietaryLicense" url="https://myvendor.com/licenses/myProprietaryLicense.html"/>
+    </licenseSet>
+    <licenseSet id="Implementation" default="true">
+    <license name="./licenses/myProprietaryLicense.txt" title="ProprietaryLicense" url="https://myvendor.com/licenses/myProprietaryLicense.html"/>
+    </licenseSet>
+    <licenseSet>
+    <license title="ProprietaryLicense" url="https://myvendor.com/licenses/myProprietaryLicense.html"/>
+    </licenseSet>
+  </licenseSets>
+
   <keywords>
     <keyword>TestPackLicense</keyword>
   </keywords>
@@ -25,7 +41,7 @@
   </conditions>
 
   <components>
-    <component Cclass="TestClass" Cgroup="TestGlobal" Cversion="1.0.0" condition="Test_Condition">
+    <component Cclass="TestClass" Cgroup="TestGlobal" Cversion="1.0.0" condition="Test_Condition" licenseSet="Implementation">
       <description>TestGlobal</description>
     </component>
   </components>

tools/packchk/test/data/TestLicense/licenses/license1.txt

@@ -0,0 +1,2 @@
+BSD-3 Clause License for components
+

tools/packchk/test/data/TestLicense/licenses/license2.txt

@@ -0,0 +1 @@
+MIT License for device support

tools/packchk/test/data/TestLicense/licenses/myProprietaryLicense.txt

@@ -0,0 +1,2 @@
+My Proprietary License
+

tools/packchk/test/integtests/src/PackChkIntegTests.cpp

@@ -419,24 +419,39 @@ TEST_F(PackChkIntegTests, CheckPackLicense) {
   argv[2] = (char*)"--disable-validation";
 
   PackChk packChk;
-  EXPECT_EQ(0, packChk.Check(3, argv, nullptr));
+  EXPECT_EQ(1, packChk.Check(3, argv, nullptr));
 
   auto errMsgs = ErrLog::Get()->GetLogMessages();
-  bool bFound = false;
+  int M300_foundCnt = 0;
+  int M327_foundCnt = 0;
+  int M367_foundCnt = 0;
+  int M601_foundCnt = 0;
+  int M602_foundCnt = 0;
   for (const string& msg : errMsgs) {
     size_t s;
+    if ((s = msg.find("M300")) != string::npos) {
+      M300_foundCnt++;
+    }
     if ((s = msg.find("M327")) != string::npos) {
-      bFound = true;
-      break;
+      M327_foundCnt++;
+    }
+    if ((s = msg.find("M367")) != string::npos) {
+      M367_foundCnt++;
+    }
+    if ((s = msg.find("M601")) != string::npos) {
+      M601_foundCnt++;
+    }
+    if ((s = msg.find("M602")) != string::npos) {
+      M602_foundCnt++;
     }
   }
 
-  if (!bFound) {
-    FAIL() << "error: missing warning M327";
+  if (M300_foundCnt != 1 || M327_foundCnt != 1 || M367_foundCnt != 1 || M601_foundCnt != 1 || M602_foundCnt != 1) {
+    FAIL() << "error: missing license check warnings";
   }
 }
 
-// Validate license path
+// Validate CPU feature SON
 TEST_F(PackChkIntegTests, CheckFeatureSON) {
   const char* argv[3];