[packchk] no error message if ../etc/PACK.xsd not found #786 (#576) (#…

…946) added: - Schema Validation Test changed: - Xerxes continues checking after first error (do not treat everything as critical) - added test cases for schema check Co-authored-by: Thorsten de Buhr <[email protected]>
Open-CMSIS-Pack · May 30, 2023 · 200189a · 200189a
1 parent 73dda22
commit 200189a
Show file tree

Hide file tree

Showing 9 changed files with 227 additions and 20 deletions.
diff --git a/libs/xmlschemachecker/src/XmlErrorHandler.cpp b/libs/xmlschemachecker/src/XmlErrorHandler.cpp
@@ -8,18 +8,19 @@
 #include "XmlErrorHandler.h"
 #include "xercesc/util/XMLString.hpp"
 #include <xercesc/sax/SAXParseException.hpp>
+#include "ErrLog.h"
 
 using namespace std;
 using namespace XERCES_CPP_NAMESPACE;
 
+
 void XmlErrorHandler::error(const SAXParseException &exc)
 {
   char* file = xercesc::XMLString::transcode(exc.getSystemId());
   char* msg = xercesc::XMLString::transcode(exc.getMessage());
-  cerr << "Error at file " << file
-       << ", line " << exc.getLineNumber()
-       << ", column " << exc.getColumnNumber()
-       << ": " << msg << endl;
+  ErrLog::Get()->SetFileName(file);
+  LogMsg("M511", MSG(msg), exc.getLineNumber());
+  ErrLog::Get()->SetFileName("");
   xercesc::XMLString::release(&file);
   xercesc::XMLString::release(&msg);
 }
@@ -28,10 +29,9 @@ void XmlErrorHandler::warning(const SAXParseException &exc)
 {
   char* file = xercesc::XMLString::transcode(exc.getSystemId());
   char* msg = xercesc::XMLString::transcode(exc.getMessage());
-  cerr << "Warning at file " << file
-       << ", line " << exc.getLineNumber()
-       << ", column " << exc.getColumnNumber()
-       << ": " << msg << endl;
+  ErrLog::Get()->SetFileName(file);
+  LogMsg("M510", MSG(msg), exc.getLineNumber());
+  ErrLog::Get()->SetFileName("");
   xercesc::XMLString::release(&file);
   xercesc::XMLString::release(&msg);
 }
@@ -40,10 +40,9 @@ void XmlErrorHandler::fatalError(const SAXParseException &exc)
 {
   char* file = xercesc::XMLString::transcode(exc.getSystemId());
   char* msg = xercesc::XMLString::transcode(exc.getMessage());
-  cerr << "Fatal Error at file " << file
-       << ", line " << exc.getLineNumber()
-       << ", column " << exc.getColumnNumber()
-       << ": " << msg << endl;
+  ErrLog::Get()->SetFileName(file);
+  LogMsg("M511", MSG(msg), exc.getLineNumber());
+  ErrLog::Get()->SetFileName("");
   xercesc::XMLString::release(&file);
   xercesc::XMLString::release(&msg);
 }

diff --git a/libs/xmlschemachecker/src/XmlValidator.cpp b/libs/xmlschemachecker/src/XmlValidator.cpp
@@ -13,6 +13,9 @@
 #include "xercesc/sax/ErrorHandler.hpp"
 #include "xercesc/sax/SAXParseException.hpp"
 
+#include <sstream>
+#include "ErrLog.h"
+
 using namespace std;
 using namespace XERCES_CPP_NAMESPACE;
 
@@ -26,7 +29,7 @@ XmlValidator::XmlValidator()
   m_domParser->setValidationScheme(XercesDOMParser::Val_Always);
   m_domParser->setDoNamespaces(true);
   m_domParser->setDoSchema(true);
-  m_domParser->setValidationConstraintFatal(true);
+  m_domParser->setValidationConstraintFatal(false);   // report all errors
   m_domParser->setValidationSchemaFullChecking(true);
 }
 
@@ -45,22 +48,33 @@ XmlValidator::~XmlValidator()
  */
 bool XmlValidator::Validate(const string& xmlFile, const string& schemaFile)
 {
+  LogMsg("M084");
+
   try {
     m_domParser->setExternalNoNamespaceSchemaLocation(schemaFile.c_str());
     m_domParser->parse(xmlFile.c_str());
 
-    if (m_domParser->getErrorCount() == 0) {
+    auto errCnt = m_domParser->getErrorCount();
+
+    LogMsg("M016");
+    LogMsg("M024", ERR(errCnt));
+
+    if (errCnt == 0) {
       return true;
     } else {
       return false;
     }
   }
   catch (const XMLException& e) {
-    cerr << "Exception: " << e.getMessage() << endl;
+    stringstream ss;
+    ss << "Exception: " << e.getMessage();
+    LogMsg("M511", MSG(ss.str()));
     return false;
   }
   catch (const SAXException& e) {
-    cerr << "Exception: " << e.getMessage() << endl;
+    stringstream ss;
+    ss << "Exception: " << e.getMessage() << endl;
+    LogMsg("M511", MSG(ss.str()));
     return false;
   }
-}
+}
diff --git a/tools/packchk/src/CreateModel.cpp b/tools/packchk/src/CreateModel.cpp
@@ -118,7 +118,7 @@ bool CreateModel::AddPdsc(const string& pdscFile, bool bSkipCheckForOtherPdsc /*
 
   if(m_validatePdsc) {
     if(!XmlChecker::Validate(pdscFile, m_schemaFile)) {
-      return false;
+      ; // continue checking
     }
   }
 

diff --git a/tools/packchk/src/PackChk_Msgs.cpp b/tools/packchk/src/PackChk_Msgs.cpp
@@ -13,6 +13,7 @@ const MsgTable PackChk::msgTable = {
   { "M021", { MsgLevel::LEVEL_TEXT,     CRLF_NO,"" } },
   { "M022", { MsgLevel::LEVEL_TEXT,     CRLF_B, "Found %ERR% Error(s) and %WARN% Warning(s)." } },
   { "M023", { MsgLevel::LEVEL_TEXT,     CRLF_B, "\nPhase%CHECK%" } },
+  { "M024", { MsgLevel::LEVEL_TEXT,     CRLF_B, "Xerxes schema check: Found %ERR% Error(s)." } },
 
 // 50... Info Messages (INFO = verbose)
   { "M050", { MsgLevel::LEVEL_INFO,     CRLF_B, "" } },
@@ -49,7 +50,7 @@ const MsgTable PackChk::msgTable = {
   { "M081", { MsgLevel::LEVEL_PROGRESS, CRLF_B, "Checking File category '%CAT%' for '%COMP%' dependency: '%PATH%'" } },
   { "M082", { MsgLevel::LEVEL_PROGRESS, CRLF_B, "Checking defined Condition: '%COND%'" } },
   { "M083", { MsgLevel::LEVEL_PROGRESS, CRLF_B, "Checking used Condition: '%COND%'" } },
-  { "M084", { MsgLevel::LEVEL_INFO,     CRLF_B, "" } },
+  { "M084", { MsgLevel::LEVEL_INFO,     CRLF_B, "Xerxes schema check" } },
   { "M085", { MsgLevel::LEVEL_PROGRESS, CRLF_B, "Checking if Component has Condition: 'Cclass=%CCLASS%, Cgroup=%CGROUP%, Csub=%CSUB%, Cversion=%CVER%'" } },
   { "M086", { MsgLevel::LEVEL_PROGRESS, CRLF_B, "Checking if File has Version: '%PATH%'" } },
   { "M087", { MsgLevel::LEVEL_PROGRESS, CRLF_B, "Checking if Component declared as '%TYPE%' has reference to a device: 'Cclass=%CCLASS%, Cgroup=%CGROUP%, Cversion=%CVER%'" } },
@@ -223,6 +224,9 @@ const MsgTable PackChk::msgTable = {
   { "M504", { MsgLevel::LEVEL_TEXT,     CRLF_B, "RTE Model reports: MISSING:     -- %SPACE%%NAME%" } },
   { "M505", { MsgLevel::LEVEL_ERROR,    CRLF_B, "RTE Model reports: %MSG%" } },
   { "M506", { MsgLevel::LEVEL_WARNING,  CRLF_B, "RTE Model reports: %MSG%" } },
+
+  { "M510", { MsgLevel::LEVEL_WARNING,  CRLF_B, "Xerxes reports: %MSG%" } },
+  { "M511", { MsgLevel::LEVEL_ERROR,    CRLF_B, "Xerxes reports: %MSG%" } },
 };
 
 const MsgTableStrict PackChk::msgStrictTable = {

diff --git a/tools/packchk/src/PackOptions.cpp b/tools/packchk/src/PackOptions.cpp
@@ -19,7 +19,8 @@ using namespace std;
  * @brief class constructor
 */
 CPackOptions::CPackOptions() :
-  m_bIgnoreOtherPdscFiles(false)
+  m_bIgnoreOtherPdscFiles(false),
+  m_bDisableValidation(false)
 {
 }
 

diff --git a/tools/packchk/test/data/SchemaValidation/Files/header1.h b/tools/packchk/test/data/SchemaValidation/Files/header1.h
@@ -0,0 +1 @@
+//header 1
diff --git a/tools/packchk/test/data/SchemaValidation/Files/test1.c b/tools/packchk/test/data/SchemaValidation/Files/test1.c
@@ -0,0 +1 @@
+// tests1.c
diff --git a/tools/packchk/test/data/SchemaValidation/TestVendor.SchemaValidation.pdsc b/tools/packchk/test/data/SchemaValidation/TestVendor.SchemaValidation.pdsc
@@ -0,0 +1,40 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<package schemaVersion="1.4" xmlns:xs="http://www.w3.org/2001/XMLSchema-instance" xs:noNamespaceSchemaLocation="PACK.xsd">
+  <vendor>TestVendor</vendor>
+  <url>http://www.testurl.com/pack/</url>
+  <name>SchemaValidation</name>
+  <descripton>Schema Validation</descripton>
+
+  <releases>
+    <release version="0.0.1" date="2021-09-06">>
+      Initial release of SchemaValidation.
+    </release>
+  </releases>
+
+  <keywords>
+    <keyword>SchemaValidation</keyword>
+  </keywords>
+
+  <conditions>
+    <condition id="Test_Condition">
+      <description>Test Device</description>
+      <require Dvendor="ARM:82"/>
+    </condition>
+    <conditon id="False_Condition">
+      <description>Test Device</description>
+      <require Dvendor="ARM:82"/>
+    </conditon>
+  </conditions>
+
+  <components>
+    <component Cclass="TestClass" Cgroup="TestGlobal" Cversion="1.0.0" condition="Test_Condition">
+      <description>TestGlobal</description>
+      <files>
+        <file category="source" name="Files/test1.c"/>
+        <file category="TestGlobal" name="Files/header1.h"/>
+      </files>
+    </component>
+  </components>
+
+</package>
diff --git a/tools/packchk/test/integtests/src/PackChkIntegTests.cpp b/tools/packchk/test/integtests/src/PackChkIntegTests.cpp
@@ -16,15 +16,59 @@ class PackChkIntegTests : public ::testing::Test {
 public:
   void SetUp()    override;
   void TearDown() override;
+
+  string GetPackXsd();
+  void CheckCopyPackXsd();
+  void DeletePackXsd();
 };
 
 void PackChkIntegTests::SetUp() {
+  CheckCopyPackXsd();
 }
 
 void PackChkIntegTests::TearDown() {
   ErrLog::Get()->ClearLogMessages();
 }
 
+string PackChkIntegTests::GetPackXsd() {
+  const string schemaDestDir = string(PROJMGRUNITTESTS_BIN_PATH) + "/../etc";
+  const string schemaFileName = schemaDestDir + "/PACK.xsd";
+
+  return schemaFileName;
+}
+
+void PackChkIntegTests::CheckCopyPackXsd() {
+  error_code ec;
+
+  const string schemaDestDir = string(PROJMGRUNITTESTS_BIN_PATH) + "/../etc";
+  const string schemaFileName = GetPackXsd();
+
+  if (RteFsUtils::Exists(schemaFileName)) {
+    return;
+  }
+
+  // Copy Pack.xsd
+  const string packXsd = string(PACKXSD_FOLDER) + "/PACK.xsd";
+  if (RteFsUtils::Exists(schemaDestDir)) {
+    RteFsUtils::RemoveDir(schemaDestDir);
+  }
+  RteFsUtils::CreateDirectories(schemaDestDir);
+  fs::copy(fs::path(packXsd), fs::path(schemaFileName), ec);
+}
+
+void PackChkIntegTests::DeletePackXsd() {
+  const string schemaFileName = GetPackXsd();
+
+  if (!RteFsUtils::Exists(schemaFileName)) {
+    return;
+  }
+
+  // Delete Pack.xsd
+  RteFsUtils::RemoveFile(schemaFileName);
+  ASSERT_FALSE(RteFsUtils::Exists(schemaFileName));
+}
+
+
  // Validate packchk when no .pdsc file found
 TEST_F(PackChkIntegTests, FileNotAVailable) {
   const char* argv[2];
@@ -674,3 +718,106 @@ TEST_F(PackChkIntegTests, CheckDuplicateFlashAlgo) {
     FAIL() << "error: missing message M348 or M369";
   }
 }
+
+// Test schema validation
+TEST_F(PackChkIntegTests, CheckSchemaValidation) {
+  const char* argv[3];
+
+  const string& pdscFile = PackChkIntegTestEnv::localtestdata_dir +
+    "/SchemaValidation/TestVendor.SchemaValidation.pdsc";
+  ASSERT_TRUE(RteFsUtils::Exists(pdscFile));
+
+  argv[0] = (char*)"";
+  argv[1] = (char*)pdscFile.c_str();
+
+  PackChk packChk;
+  EXPECT_EQ(1, packChk.Check(2, argv, nullptr));
+
+  auto errMsgs = ErrLog::Get()->GetLogMessages();
+  int M510_foundCnt = 0;
+  int M511_foundCnt = 0;
+  int M306_foundCnt = 0;
+
+  for (const string& msg : errMsgs) {
+    size_t s;
+    if ((s = msg.find("M306")) != string::npos) {
+      M306_foundCnt++;    // follows one M511: <descripton>
+    }
+    else if ((s = msg.find("M510")) != string::npos) {
+      M510_foundCnt++;
+    }
+    else if ((s = msg.find("M511")) != string::npos) {
+      M511_foundCnt++;
+    }
+  }
+
+  if (M510_foundCnt != 0 || M511_foundCnt != 6 || M306_foundCnt != 1) {
+    FAIL() << "error: missing message M510, M511 or M306";
+  }
+}
+
+TEST_F(PackChkIntegTests, CheckPackXsdNotFound) {
+  const char* argv[2];
+
+  const string& pdscFile = PackChkIntegTestEnv::localtestdata_dir +
+    "/SchemaValidation/TestVendor.SchemaValidation.pdsc";
+  ASSERT_TRUE(RteFsUtils::Exists(pdscFile));
+
+  DeletePackXsd();
+
+  argv[0] = (char*)"";
+  argv[1] = (char*)pdscFile.c_str();
+
+  PackChk packChk;
+  EXPECT_EQ(1, packChk.Check(2, argv, nullptr));
+
+  auto errMsgs = ErrLog::Get()->GetLogMessages();
+  int M218_foundCnt = 0;
+
+  for (const string& msg : errMsgs) {
+    size_t s;
+    if ((s = msg.find("M218")) != string::npos) {
+      M218_foundCnt++;    // follows one M511: <descripton>
+    }
+  }
+
+  if (M218_foundCnt != 1) {
+    FAIL() << "error: missing message M218";
+  }
+}
+
+TEST_F(PackChkIntegTests, CheckPackNamedXsdNotFound) {
+  const char* argv[4];
+
+  const string& pdscFile = PackChkIntegTestEnv::localtestdata_dir +
+    "/SchemaValidation/TestVendor.SchemaValidation.pdsc";
+  ASSERT_TRUE(RteFsUtils::Exists(pdscFile));
+
+  DeletePackXsd();
+
+  const string schemaFileName = GetPackXsd();
+
+  argv[0] = (char*)"";
+  argv[1] = (char*)pdscFile.c_str();
+  argv[2] = (char*)"--xsd";
+  argv[3] = (char*)schemaFileName.c_str();
+
+  PackChk packChk;
+  EXPECT_EQ(1, packChk.Check(4, argv, nullptr));
+
+  auto errMsgs = ErrLog::Get()->GetLogMessages();
+  int M218_foundCnt = 0;
+
+  for (const string& msg : errMsgs) {
+    size_t s;
+    if ((s = msg.find("M218")) != string::npos) {
+      M218_foundCnt++;    // follows one M511: <descripton>
+    }
+  }
+
+  if (M218_foundCnt != 1) {
+    FAIL() << "error: missing message M218";
+  }
+}
+
+