[projmgr] Improve pack version range resolution: re-use resolved packs

Author: brondani

tools/projmgr/include/ProjMgrUtils.h

@@ -22,6 +22,18 @@ typedef std::vector<const ConnectItem*> ConnectPtrVec;
 */
 typedef std::map<const ConnectItem*, bool> ActiveConnectMap;
 
+/**
+ * @brief version type
+*/
+enum class VersionType
+{
+  FIXED = 0,
+  EQUIVALENT,
+  COMPATIBLE,
+  MINIMUM,
+  ANY
+};
+
 /**
  * @brief connections collection item containing
  *        filename reference
@@ -212,6 +224,13 @@ class ProjMgrUtils {
   */
   static bool IsMatchingPackInfo(const PackInfo& exactPackInfo, const PackInfo& packInfoToMatch);
 
+  /**
+   * @brief get type of version string
+   * @param version string
+   * @return type according to VersionType enum
+  */
+  static VersionType GetVersionType(const std::string& version);
+
   /**
    * @brief convert version in YML format to CPRJ range format
    * @param version version in YML format

tools/projmgr/include/ProjMgrWorker.h

@@ -1222,6 +1222,7 @@ class ProjMgrWorker {
   bool IsEnvironmentCompatible(const std::string& environment, const StrVec& filter);
   bool HasCompatibleEnvironment(const Collection<RteItem*>& environments, const StrVec& filter);
   template<class T> bool CheckFilter(const std::string& filter, const T& item);
+  void ResolvePackRequirement(ContextItem& context, const PackItem& packEntry);
 };
 
 #endif  // PROJMGRWORKER_H

tools/projmgr/src/ProjMgrUtils.cpp

@@ -300,26 +300,47 @@ SemVer ProjMgrUtils::GetSemVer(const std::string version) {
   return semVer;
 }
 
+VersionType ProjMgrUtils::GetVersionType(const string& version) {
+  if (version.find(HIGHER_OR_EQUAL_OPERATOR) != string::npos) {
+    return VersionType::MINIMUM;
+  } else if (version.find(TILDE_OPERATOR) != string::npos) {
+    return VersionType::EQUIVALENT;
+  } else if (version.find(CARET_OPERATOR) != string::npos) {
+    return VersionType::COMPATIBLE;
+  } else if (version.empty()) {
+    return VersionType::ANY;
+  } else {
+    return VersionType::FIXED;
+  }
+}
+
 string ProjMgrUtils::ConvertToVersionRange(const string& version) {
-  string versionRange = version;
-  if (!versionRange.empty()) {
-    if (versionRange.find(HIGHER_OR_EQUAL_OPERATOR) != string::npos) {
-      // Minimum version
-      versionRange = RteUtils::StripPrefix(versionRange, HIGHER_OR_EQUAL_OPERATOR);
-    } else if (versionRange.find(TILDE_OPERATOR) != string::npos) {
-      // Equivalent version
-      versionRange = RteUtils::StripPrefix(versionRange, TILDE_OPERATOR);
-      SemVer semVer = GetSemVer(versionRange);
-      versionRange = versionRange + ":" + to_string(semVer.major) + "." + to_string(semVer.minor + 1) + ".0-0";
-    } else if (versionRange.find(CARET_OPERATOR) != string::npos) {
-      // Compatible version
-      versionRange = RteUtils::StripPrefix(versionRange, CARET_OPERATOR);
-      SemVer semVer = GetSemVer(versionRange);
-      versionRange = versionRange + ":" + to_string(semVer.major + 1) + ".0.0-0";
-    } else {
-      // Fixed version
-      versionRange = versionRange + ":" + versionRange;
-    }
+  string versionRange;
+  SemVer semVer;
+  switch (GetVersionType(version)) {
+  case VersionType::MINIMUM:
+    // Minimum version
+    versionRange = RteUtils::StripPrefix(version, HIGHER_OR_EQUAL_OPERATOR);
+    break;
+  case VersionType::EQUIVALENT:
+    // Equivalent version
+    versionRange = RteUtils::StripPrefix(version, TILDE_OPERATOR);
+    semVer = GetSemVer(versionRange);
+    versionRange = versionRange + ":" + to_string(semVer.major) + "." + to_string(semVer.minor + 1) + ".0-0";
+    break;
+  case VersionType::COMPATIBLE:
+    // Compatible version
+    versionRange = RteUtils::StripPrefix(version, CARET_OPERATOR);
+    semVer = GetSemVer(versionRange);
+    versionRange = versionRange + ":" + to_string(semVer.major + 1) + ".0.0-0";
+    break;
+  case VersionType::FIXED:
+    // Fixed version
+    versionRange = version + ":" + version;
+    break;
+  case VersionType::ANY:
+    // Any version
+    versionRange = RteUtils::EMPTY_STRING;
   }
   return versionRange;
 }

tools/projmgr/src/ProjMgrWorker.cpp

@@ -1675,6 +1675,7 @@ bool ProjMgrWorker::AddPackRequirements(ContextItem& context, const vector<PackI
   }
 
   // Process packages
+  map<VersionType, vector<PackItem>> packEntries;
   for (const auto& packageEntry : packages) {
     if (packageEntry.path.empty()) {
       // Store specified pack metadata
@@ -1696,28 +1697,11 @@ bool ProjMgrWorker::AddPackRequirements(ContextItem& context, const vector<PackI
       } else {
         // Not matching cbuild pack, add it unless a wildcard entry
         PackageItem package;
-        package.origin = packageEntry.origin;
         ProjMgrUtils::ConvertToPackInfo(packageEntry.pack, package.pack);
 
-        // Resolve version range using installed/local packs
+        // Store pack entries in separated vectors for later resolution in the right order
         if (!package.pack.name.empty() && !WildCards::IsWildcardPattern(package.pack.name)) {
-          XmlItem attributes({
-          {"name",    package.pack.name},
-          {"vendor",  package.pack.vendor},
-          {"version", ProjMgrUtils::ConvertToVersionRange(package.pack.version)},
-          });
-          auto pdsc = m_kernel->GetEffectivePdscFile(attributes);
-          // Only remember the version of the pack if we had it installed or local
-          // Will be used when serializing the cbuild-pack.yml file later
-          if (!pdsc.first.empty()) {
-            context.userInputToResolvedPackIdMap[packageEntry.pack].insert(pdsc.first);
-            string installedVersion = RtePackage::VersionFromId(pdsc.first);
-            package.pack.version = VersionCmp::RemoveVersionMeta(installedVersion);
-          } else {
-            // Remember that we had the user input, but it does not match any installed pack
-            context.userInputToResolvedPackIdMap[packageEntry.pack] = {};
-          }
-          context.packRequirements.push_back(package);
+          packEntries[ProjMgrUtils::GetVersionType(package.pack.version)].push_back(packageEntry);
         }
       }
     } else {
@@ -1742,6 +1726,14 @@ bool ProjMgrWorker::AddPackRequirements(ContextItem& context, const vector<PackI
     }
   }
 
+  // Resolve pack entries in the order according to the version type: fixed, equivalent, compatible, minimum, any
+  for (const auto& versionType : { VersionType::FIXED, VersionType::EQUIVALENT, VersionType::COMPATIBLE,
+    VersionType::MINIMUM, VersionType::ANY }) {
+    for (const auto& packEntry : packEntries[versionType]) {
+      ResolvePackRequirement(context, packEntry);
+    }
+  }
+
   // Add wildcard entries last so that they can be re-expanded if needed
   for (const auto& packageEntry : packages) {
     PackageItem package;
@@ -1766,6 +1758,44 @@ bool ProjMgrWorker::AddPackRequirements(ContextItem& context, const vector<PackI
   return true;
 }
 
+void ProjMgrWorker::ResolvePackRequirement(ContextItem& context, const PackItem& packageEntry) {
+  // Resolve version range using installed/local packs
+  // Reuse already resolved pack when possible
+  PackageItem package;
+  ProjMgrUtils::ConvertToPackInfo(packageEntry.pack, package.pack);
+  string versionRange = ProjMgrUtils::ConvertToVersionRange(package.pack.version);
+  string bestMatch = RteUtils::EMPTY_STRING;
+  for (const auto& resolved : context.packRequirements) {
+    if (resolved.pack.vendor == package.pack.vendor &&
+      resolved.pack.name == package.pack.name &&
+      VersionCmp::RangeCompare(resolved.pack.version, versionRange) == 0 &&
+      VersionCmp::Compare(resolved.pack.version, bestMatch) > 0) {
+      bestMatch = resolved.pack.version;
+    }
+  }
+  if (!bestMatch.empty()) {
+    versionRange = ProjMgrUtils::ConvertToVersionRange(bestMatch);
+  }
+  XmlItem attributes({
+      {"name",    package.pack.name},
+      {"vendor",  package.pack.vendor},
+      {"version", versionRange}
+    });
+  auto pdsc = m_kernel->GetEffectivePdscFile(attributes);
+  // Only remember the version of the pack if we had it installed or local
+  // Will be used when serializing the cbuild-pack.yml file later
+  if (!pdsc.first.empty()) {
+    context.userInputToResolvedPackIdMap[packageEntry.pack].insert(pdsc.first);
+    string installedVersion = RtePackage::VersionFromId(pdsc.first);
+    package.pack.version = VersionCmp::RemoveVersionMeta(installedVersion);
+  } else {
+    // Remember that we had the user input, but it does not match any installed pack
+    context.userInputToResolvedPackIdMap[packageEntry.pack] = {};
+  }
+  package.origin = packageEntry.origin;
+  context.packRequirements.push_back(package);
+}
+
 bool ProjMgrWorker::ProcessToolchain(ContextItem& context) {
   if (context.compiler.empty()) {
     // Use the default compiler if available

tools/projmgr/test/data/TestSolution/PackLocking/multiple_pack_entries.csolution.yml

@@ -0,0 +1,14 @@
+# 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::RteTest_DFP
+    - pack: ARM::RteTest_DFP@>=0.0.9
+    - pack: ARM::RteTest_DFP@^0.0.9
+    - pack: ARM::RteTest_DFP@~0.1.0
+    - pack: ARM::[email protected]
+  projects:
+    - project: ./project_with_dfp_components.cproject.yml

tools/projmgr/test/data/TestSolution/PackLocking/ref/multiple_pack_entries.cbuild-pack.yml

@@ -0,0 +1,9 @@
+cbuild-pack:
+  resolved-packs:
+    - resolved-pack: ARM::[email protected]
+      selected-by-pack:
+        - ARM::RteTest_DFP
+        - ARM::[email protected]
+        - ARM::RteTest_DFP@>=0.0.9
+        - ARM::RteTest_DFP@^0.0.9
+        - ARM::RteTest_DFP@~0.1.0

tools/projmgr/test/src/ProjMgrUnitTests.cpp

@@ -943,6 +943,18 @@ TEST_F(ProjMgrUnitTests, RunProjMgrSolution_LockPackVersion) {
   RteFsUtils::RemoveFile(cbuildPackBackup);
 }
 
+TEST_F(ProjMgrUnitTests, RunProjMgrSolution_MultiplePackEntries) {
+  char* argv[3];
+  const string csolution = testinput_folder + "/TestSolution/PackLocking/multiple_pack_entries.csolution.yml";
+  argv[1] = (char*)"convert";
+  argv[2] = (char*)csolution.c_str();
+  EXPECT_EQ(0, RunProjMgr(3, argv, m_envp));
+
+  // Check the generated cbuild-pack file
+  ProjMgrTestEnv::CompareFile(testinput_folder + "/TestSolution/PackLocking/multiple_pack_entries.cbuild-pack.yml",
+    testinput_folder + "/TestSolution/PackLocking/ref/multiple_pack_entries.cbuild-pack.yml");
+}
+
 TEST_F(ProjMgrUnitTests, RunProjMgrSolution_LockPackKeepExistingForContextSelections) {
   char* argv[9];
   string buf1, buf2, buf3;