From d8a53613f213298e50309f168c1608f461b8c31d Mon Sep 17 00:00:00 2001 From: Daniel Brondani Date: Mon, 27 Oct 2025 11:44:20 +0100 Subject: [PATCH] [devtools] Handle pack identifiers as case insensitive (#1338) * [devtools] Handle pack identifiers as case insensitive * Rework draft * Minor fixes * Fix test case --- libs/rtemodel/include/RteKernel.h | 17 +--- libs/rtemodel/include/RtePackage.h | 24 ++++- libs/rtemodel/src/RteKernel.cpp | 91 +++++++++++-------- libs/rtemodel/test/src/RteModelTest.cpp | 55 +---------- tools/projmgr/include/ProjMgrWorker.h | 1 + tools/projmgr/src/ProjMgrWorker.cpp | 28 +++++- ...ml => pack_case_insensitive.csolution.yml} | 0 tools/projmgr/test/src/ProjMgrTestEnv.cpp | 18 +++- tools/projmgr/test/src/ProjMgrUnitTests.cpp | 26 +++--- .../test/src/ProjMgrWorkerUnitTests.cpp | 15 ++- 10 files changed, 149 insertions(+), 126 deletions(-) rename tools/projmgr/test/data/TestSolution/{pack_case_sensitive.csolution.yml => pack_case_insensitive.csolution.yml} (100%) diff --git a/libs/rtemodel/include/RteKernel.h b/libs/rtemodel/include/RteKernel.h index eb608314a..0f6a4e375 100644 --- a/libs/rtemodel/include/RteKernel.h +++ b/libs/rtemodel/include/RteKernel.h @@ -8,7 +8,7 @@ */ /******************************************************************************/ /* - * Copyright (c) 2020-2021 Arm Limited. All rights reserved. + * Copyright (c) 2020-2025 Arm Limited. All rights reserved. * * SPDX-License-Identifier: Apache-2.0 */ @@ -241,21 +241,6 @@ class RteKernel */ void GetInstalledPdscFiles(std::map& pdscMap) const; - /** - * @brief getter for pdsc file determined by pack ID, pack path and pack attributes - * @param attributes pack attributes - * @param packId pack ID - * @return pair of pack ID to pdsc file - */ - std::pair GetInstalledPdscFile(const XmlItem& attributes) const; - - /** - * @brief getter for pdsc file pointed by the local repository index and determined by pack attributes, pack path and pack ID. - * @param attributes pack attributes - * @return pair of pack ID to pdsc file - */ - std::pair GetLocalPdscFile(const XmlItem& attributes) const; - /** * @brief get local or installed pdsc file corresponding to supplied pack ID, pack path and pack attributes * @param attributes pack attributes diff --git a/libs/rtemodel/include/RtePackage.h b/libs/rtemodel/include/RtePackage.h index 85e1c07d1..a97a77320 100644 --- a/libs/rtemodel/include/RtePackage.h +++ b/libs/rtemodel/include/RtePackage.h @@ -8,7 +8,7 @@ */ /******************************************************************************/ /* - * Copyright (c) 2020-2021 Arm Limited. All rights reserved. + * Copyright (c) 2020-2025 Arm Limited. All rights reserved. * * SPDX-License-Identifier: Apache-2.0 */ @@ -1333,11 +1333,33 @@ class RtePackRegistry */ const std::map& GetLoadedPacks() const { return m_loadedPacks;} + /** + * @brief get collection of effective pdscs + * @return const map of effective pdscs + */ + const std::map& GetPdscMap() const { return m_pdscMap; } + + /** + * @brief set collection of effective pdscs + * @param map of effective pdscs + */ + void SetPdscMap(const std::map& pdscMap) { m_pdscMap = pdscMap; } + + /** + * @brief clear collection of effective pdscs + */ + void ClearPdscMap() { m_pdscMap.clear(); } + protected: /** * @brief collection of loaded packs: absolute pdsc filename -> RtePackage* */ std::map m_loadedPacks; + + /** + * @brief collection of effective pdscs: lower-case pack id -> absolute pdsc filename + */ + std::map m_pdscMap; }; diff --git a/libs/rtemodel/src/RteKernel.cpp b/libs/rtemodel/src/RteKernel.cpp index d66e44632..ade28d3e1 100644 --- a/libs/rtemodel/src/RteKernel.cpp +++ b/libs/rtemodel/src/RteKernel.cpp @@ -6,7 +6,7 @@ */ /******************************************************************************/ /* - * Copyright (c) 2020-2021 Arm Limited. All rights reserved. + * Copyright (c) 2020-2025 Arm Limited. All rights reserved. * * SPDX-License-Identifier: Apache-2.0 */ @@ -80,6 +80,7 @@ bool RteKernel::SetCmsisPackRoot(const string& cmsisPackRoot) if (m_cmsisPackRoot == cmsisPackRoot) return false; m_cmsisPackRoot = cmsisPackRoot; + RteFsUtils::NormalizePath(m_cmsisPackRoot); return true; } @@ -413,12 +414,19 @@ bool RteKernel::GetEffectivePdscFilesAsMap(mapGetPdscMap(); + if (pdscMap.empty()) { + // Get all installed files + GetInstalledPdscFiles(pdscMap); + // Overwrite entries with local pdsc files if any + XmlItem emptyAttributes; + GetLocalPdscFiles(emptyAttributes, pdscMap); + // Store pdsc map + packRegistry->SetPdscMap(pdscMap); + } // purge entries if only latest are required if(latest) { @@ -482,51 +490,54 @@ void RteKernel::GetInstalledPdscFiles(std::map allFiles; RteFsUtils::GetPackageDescriptionFiles(allFiles, GetCmsisPackRoot(), 3); - for(auto& f : allFiles) { - string id = RtePackage::PackIdFromPath(f); + for (auto& f : allFiles) { + string id = RteUtils::ToLower(RtePackage::PackIdFromPath(f)); pdscMap[id] = f; } } -pair RteKernel::GetInstalledPdscFile(const XmlItem& attributes) const +pair RteKernel::GetEffectivePdscFile(const XmlItem& attributes) const { const string& name = attributes.GetAttribute("name"); const string& vendor = attributes.GetAttribute("vendor"); - if(!name.empty() && !vendor.empty()) { - string path = GetCmsisPackRoot() + '/' + vendor + '/' + name; - const string& versionRange = attributes.GetAttribute("version"); - string installedVersion = RteFsUtils::GetInstalledPackVersion(path, versionRange); - if(!installedVersion.empty()) { - string packId = RtePackage::ComposePackageID(vendor, name, installedVersion); - path += '/' + installedVersion + '/' + vendor + '.' + name + ".pdsc"; - return make_pair(packId, path); + if (!name.empty() && !vendor.empty()) { + const string& packId = RteUtils::ToLower(vendor + RteConstants::SUFFIX_PACK_VENDOR + name); + // get map of effective pdscs with lower-case ids + map pdscMap; + GetEffectivePdscFilesAsMap(pdscMap, false); + StrPairVec pdscs; + // get subset of pdscs for the searched packId + for (const auto& pdsc : pdscMap) { + if (RtePackage::CommonIdFromId(pdsc.first) == packId) { + pdscs.push_back(pdsc); + } + } + if (!pdscs.empty()) { + const string& versionRange = attributes.GetAttribute("version"); + StrPair effectivePdsc; + if (versionRange.empty()) { + // required version range is empty = get greatest effective version + effectivePdsc = *pdscs.begin(); + } else { + for (const auto& pdsc : pdscs) { + // find the greatest effective version in the required version range + if (VersionCmp::RangeCompare(RtePackage::VersionFromId(pdsc.first), versionRange) == 0) { + effectivePdsc = pdsc; + break; + } + } + } + auto& id = effectivePdsc.first; + if (!id.empty()) { + // preserve name::vendor as given in input attributes + id = RtePackage::ComposePackageID(vendor, name, RtePackage::VersionFromId(id)); + return effectivePdsc; + } } } return make_pair(RteUtils::EMPTY_STRING, RteUtils::EMPTY_STRING); } -pair RteKernel::GetLocalPdscFile(const XmlItem& attributes) const -{ - map pdscMap; - if(!attributes.IsEmpty() && GetLocalPdscFiles(attributes, pdscMap)) { - return *pdscMap.begin(); - } - return make_pair(RteUtils::EMPTY_STRING, RteUtils::EMPTY_STRING); -} - -pair RteKernel::GetEffectivePdscFile(const XmlItem& attributes) const -{ - auto localPdsc = GetLocalPdscFile(attributes); - auto installedPdsc = GetInstalledPdscFile(attributes); - - string localVersion = RtePackage::VersionFromId(localPdsc.first); - string installedVersion = RtePackage::VersionFromId(installedPdsc.first); - if(!localVersion.empty() && VersionCmp::Compare(localVersion, installedVersion) >= 0) { - return localPdsc; - } - return installedPdsc; -} - pair RteKernel::GetPdscFileFromPath(const XmlItem& attributes, const string& prjPath) const { @@ -579,7 +590,7 @@ bool RteKernel::GetLocalPdscFiles(const XmlItem& attr, std::mapGetVersionString(); if(versionRange.empty() || VersionCmp::RangeCompare(version, versionRange) == 0) { - pdscMap[pack->GetID()] = localPdscFile; + pdscMap[RteUtils::ToLower(pack->GetID())] = localPdscFile; found = true; } } diff --git a/libs/rtemodel/test/src/RteModelTest.cpp b/libs/rtemodel/test/src/RteModelTest.cpp index 7e5696027..c79d8a944 100644 --- a/libs/rtemodel/test/src/RteModelTest.cpp +++ b/libs/rtemodel/test/src/RteModelTest.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020-2021 Arm Limited. All rights reserved. + * Copyright (c) 2020-2025 Arm Limited. All rights reserved. * * SPDX-License-Identifier: Apache-2.0 */ @@ -725,60 +725,13 @@ TEST_F(RteModelPrjTest, LoadCprjConfigVer) { } -TEST_F(RteModelPrjTest, GetLocalPdscFile) { - RteKernelSlim rteKernel; - rteKernel.SetCmsisPackRoot(packsDir); - - XmlItem attributes; - auto pdsc = rteKernel.GetLocalPdscFile(attributes); - EXPECT_TRUE(pdsc.first.empty()); - EXPECT_TRUE(pdsc.second.empty()); - - attributes.AddAttribute("name", "LocalPack"); - attributes.AddAttribute("vendor", "LocalVendor"); - pdsc = rteKernel.GetLocalPdscFile(attributes); - - // check returned packId - EXPECT_EQ(pdsc.first, "LocalVendor::LocalPack@1.0.1"); - - // check returned pdsc - const string expectedPdsc = - RteFsUtils::MakePathCanonical(RteFsUtils::AbsolutePath(localPacks + "/L/LocalVendor.LocalPack.pdsc").generic_string()); - error_code ec; - EXPECT_TRUE(fs::equivalent(pdsc.second, expectedPdsc, ec)); -} - -TEST_F(RteModelPrjTest, GetInstalledPdscFile) { - RteKernelSlim rteKernel; - rteKernel.SetCmsisPackRoot(packsDir); - - XmlItem attributes; - auto pdsc = rteKernel.GetInstalledPdscFile(attributes); - EXPECT_TRUE(pdsc.first.empty()); - EXPECT_TRUE(pdsc.second.empty()); - - attributes.AddAttribute("name", "RteTestRequired"); - attributes.AddAttribute("vendor", "ARM"); - pdsc = rteKernel.GetInstalledPdscFile(attributes); - - // check returned packId - EXPECT_EQ(pdsc.first, "ARM::RteTestRequired@1.0.0"); - - // check returned pdsc - const string expectedPdsc = - RteFsUtils::MakePathCanonical(RteFsUtils::AbsolutePath(packsDir + - "/ARM/RteTestRequired/1.0.0/ARM.RteTestRequired.pdsc").generic_string()); - error_code ec; - EXPECT_TRUE(fs::equivalent(pdsc.second, expectedPdsc, ec)); -} - TEST_F(RteModelPrjTest, GetEffectivePdscFile) { RteKernelSlim rteKernel; rteKernel.SetCmsisPackRoot(packsDir); XmlItem attributes; // nothing has found for empty attributes - auto pdsc = rteKernel.GetInstalledPdscFile(attributes); + auto pdsc = rteKernel.GetEffectivePdscFile(attributes); EXPECT_TRUE(pdsc.first.empty()); EXPECT_TRUE(pdsc.second.empty()); @@ -821,14 +774,14 @@ TEST_F(RteModelPrjTest, GetEffectivePdscFile) { // outside range attributes.AddAttribute("version", "2.0.0"); - pdsc = rteKernel.GetInstalledPdscFile(attributes); + pdsc = rteKernel.GetEffectivePdscFile(attributes); EXPECT_TRUE(pdsc.first.empty()); EXPECT_TRUE(pdsc.second.empty()); // unknown name attributes.RemoveAttribute("version"); attributes.AddAttribute("name", "Unknown"); - pdsc = rteKernel.GetInstalledPdscFile(attributes); + pdsc = rteKernel.GetEffectivePdscFile(attributes); EXPECT_TRUE(pdsc.first.empty()); EXPECT_TRUE(pdsc.second.empty()); } diff --git a/tools/projmgr/include/ProjMgrWorker.h b/tools/projmgr/include/ProjMgrWorker.h index 20bc3d3c5..76a816cf2 100644 --- a/tools/projmgr/include/ProjMgrWorker.h +++ b/tools/projmgr/include/ProjMgrWorker.h @@ -1284,6 +1284,7 @@ class ProjMgrWorker { bool HasCompatibleEnvironment(const Collection& environments, const StrVec& filter); template bool CheckFilter(const std::string& filter, const T& item); void ResolvePackRequirement(ContextItem& context, const PackItem& packEntry); + void FormatResolvedPackIds(); }; #endif // PROJMGRWORKER_H diff --git a/tools/projmgr/src/ProjMgrWorker.cpp b/tools/projmgr/src/ProjMgrWorker.cpp index 288335e33..c9053606c 100644 --- a/tools/projmgr/src/ProjMgrWorker.cpp +++ b/tools/projmgr/src/ProjMgrWorker.cpp @@ -453,7 +453,9 @@ string ProjMgrWorker::GetPackRoot() { bool ProjMgrWorker::InitializeModel() { if(m_kernel) { - return true; // already initialized + // kernel is already initialized, clear pdsc map + m_kernel->GetPackRegistry()->ClearPdscMap(); + return true; } m_packRoot = GetPackRoot(); m_kernel = ProjMgrKernel::Get(); @@ -527,6 +529,8 @@ bool ProjMgrWorker::LoadAllRelevantPacks() { ProjMgrLogger::Get().Error("failed to load and insert packs"); return CheckRteErrors(); } + // Required packs are loaded: update pack-ids in 'userInputToResolvedPackIdMap' + FormatResolvedPackIds(); if (!m_model->Validate()) { RtePrintErrorVistior visitor(m_kernel->GetCallback()); m_model->AcceptVisitor(&visitor); @@ -5949,3 +5953,25 @@ bool ProjMgrWorker::ElaborateVariablesConfigurations() { } return configurationFound; } + +void ProjMgrWorker::FormatResolvedPackIds() { + StrMap realPackIds; + for (const auto& loadedPack : m_loadedPacks) { + const auto& realPackId = loadedPack->GetPackageID(); + realPackIds[RteUtils::ToLower(realPackId)] = realPackId; + } + for (auto& contextName : m_selectedContexts) { + auto& contextItem = m_contexts[contextName]; + for (auto& [_, resolvedPackIds] : contextItem.userInputToResolvedPackIdMap) { + StrSet formattedPackIds; + for (auto& resolvedPackId : resolvedPackIds) { + const auto& lowerCasePackId = RteUtils::ToLower(resolvedPackId); + const auto& packId = realPackIds.find(lowerCasePackId) != realPackIds.end() ? + realPackIds.at(lowerCasePackId) : resolvedPackId; + formattedPackIds.insert(packId); + } + resolvedPackIds = formattedPackIds; + } + } +} + diff --git a/tools/projmgr/test/data/TestSolution/pack_case_sensitive.csolution.yml b/tools/projmgr/test/data/TestSolution/pack_case_insensitive.csolution.yml similarity index 100% rename from tools/projmgr/test/data/TestSolution/pack_case_sensitive.csolution.yml rename to tools/projmgr/test/data/TestSolution/pack_case_insensitive.csolution.yml diff --git a/tools/projmgr/test/src/ProjMgrTestEnv.cpp b/tools/projmgr/test/src/ProjMgrTestEnv.cpp index 6c1d189e0..e32099f58 100644 --- a/tools/projmgr/test/src/ProjMgrTestEnv.cpp +++ b/tools/projmgr/test/src/ProjMgrTestEnv.cpp @@ -143,6 +143,16 @@ void ProjMgrTestEnv::SetUp() { RteFsUtils::CreateDirectories(destInvalidPacks); fs::copy(fs::path(srcInvalidPacks), fs::path(destInvalidPacks), fs::copy_options::recursive, ec); + // copy PDSCs for case insensitiveness check + string packsCaseInsensitive = testinput_folder + "/packs-case-insensitive"; + if (RteFsUtils::Exists(packsCaseInsensitive)) { + RteFsUtils::RemoveDir(packsCaseInsensitive); + } + RteFsUtils::CopyFileExAutoRetry(testcmsispack_folder + "/ARM/RteTest_DFP/0.2.0/ARM.RteTest_DFP.pdsc", + packsCaseInsensitive + "/Arm/RteTest_dfp/0.2.0/arm.rtetest_DFP.pdsc"); + RteFsUtils::CopyFileExAutoRetry(testcmsispack_folder + "/ARM/RteTest_DFP/0.1.1/ARM.RteTest_DFP.pdsc", + packsCaseInsensitive + "/ARM/RTETEST_DFP/0.1.1/Arm.RTETEST_DFP.pdsc"); + CrossPlatformUtils::SetEnv("CMSIS_PACK_ROOT", testcmsispack_folder); // create dummy cmsis compiler root @@ -231,7 +241,13 @@ std::map ProjMgrTestEnv::GetEffe std::map pdscMap; RteKernelSlim rteKernel; rteKernel.SetCmsisPackRoot(GetCmsisPackRoot()); - rteKernel.GetEffectivePdscFilesAsMap(pdscMap, bLatestsOnly); + std::list pdscFiles; + std::list packs; + rteKernel.GetEffectivePdscFiles(pdscFiles, bLatestsOnly); + rteKernel.LoadAndInsertPacks(packs, pdscFiles); + for (const auto& pack : packs) { + pdscMap[pack->GetID()] = pack->GetPackageFileName(); + } return pdscMap; } diff --git a/tools/projmgr/test/src/ProjMgrUnitTests.cpp b/tools/projmgr/test/src/ProjMgrUnitTests.cpp index 0d599cbaa..cc027cc81 100644 --- a/tools/projmgr/test/src/ProjMgrUnitTests.cpp +++ b/tools/projmgr/test/src/ProjMgrUnitTests.cpp @@ -6677,24 +6677,20 @@ TEST_F(ProjMgrUnitTests, GetToolboxVersion) { RteFsUtils::RemoveDir(testdir); } -TEST_F(ProjMgrUnitTests, PackCaseSensitive) { +TEST_F(ProjMgrUnitTests, PackCaseInsensitive) { + // pack identifiers are now case insensitive regardless of platform and file system char* argv[3]; - const string& csolution = testinput_folder + "/TestSolution/pack_case_sensitive.csolution.yml"; + const string& csolution = testinput_folder + "/TestSolution/pack_case_insensitive.csolution.yml"; argv[1] = (char*)"convert"; argv[2] = (char*)csolution.c_str(); - if (CrossPlatformUtils::GetHostType() == "linux") { - EXPECT_EQ(1, RunProjMgr(3, argv, 0)); - } else { - EXPECT_EQ(0, RunProjMgr(3, argv, 0)); - const YAML::Node& cbuild = YAML::LoadFile(testinput_folder + "/TestSolution/pack_case_sensitive.cbuild-idx.yml"); - const string& expected = "\ -RTE Model reports:\n\ -warning #500: pack 'ARM::RteTest_DFP@0.2.0' is duplicated, letter case is inconsistent:\n\ - .*/ARM/RteTest_DFP/0.2.0/ARM.RteTest_DFP.pdsc\n\ - .*/Arm/RteTest_DFP/0.2.0/Arm.RteTest_DFP.pdsc - ignored\ -"; - EXPECT_TRUE(regex_search(cbuild["build-idx"]["cbuilds"][0]["messages"]["warnings"][0].as(), regex(expected))); - } + EXPECT_EQ(0, RunProjMgr(3, argv, 0)); + + // check cbuild-pack.yml has a resolved pack selected by pack identifiers with mixed letter-cases + const YAML::Node& cbuild = YAML::LoadFile(testinput_folder + "/TestSolution/pack_case_insensitive.cbuild-pack.yml"); + const auto& resolvedPack = cbuild["cbuild-pack"]["resolved-packs"][0]; + EXPECT_EQ("ARM::RteTest_DFP@0.2.0", resolvedPack["resolved-pack"].as()); + EXPECT_EQ("ARM::RteTest_DFP", resolvedPack["selected-by-pack"][0].as()); + EXPECT_EQ("Arm::RteTest_DFP", resolvedPack["selected-by-pack"][1].as()); } TEST_F(ProjMgrUnitTests, InvalidContextSet) { diff --git a/tools/projmgr/test/src/ProjMgrWorkerUnitTests.cpp b/tools/projmgr/test/src/ProjMgrWorkerUnitTests.cpp index 371f9b914..92badcb13 100644 --- a/tools/projmgr/test/src/ProjMgrWorkerUnitTests.cpp +++ b/tools/projmgr/test/src/ProjMgrWorkerUnitTests.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020-2024 Arm Limited. All rights reserved. + * Copyright (c) 2020-2025 Arm Limited. All rights reserved. * * SPDX-License-Identifier: Apache-2.0 */ @@ -1846,3 +1846,16 @@ TEST_F(ProjMgrWorkerUnitTests, GetToolchainConfig) { "Failed to validate for name: " + name + ", version: " + version; } } + +TEST_F(ProjMgrWorkerUnitTests, LoadPacksCaseInsensitive) { + const string cmsisPackRoot = CrossPlatformUtils::GetEnv("CMSIS_PACK_ROOT"); + CrossPlatformUtils::SetEnv("CMSIS_PACK_ROOT", testinput_folder + "/packs-case-insensitive"); + + CsolutionItem csolution; + SetCsolutionPacks(&csolution, { "Arm::RTETest_Dfp@0.1.1" }, "Test"); + EXPECT_TRUE(LoadPacks(m_contexts["Test"])); + EXPECT_EQ(1, m_loadedPacks.size()); + EXPECT_EQ("ARM::RteTest_DFP@0.1.1", (*m_loadedPacks.begin())->GetPackageID()); + + CrossPlatformUtils::SetEnv("CMSIS_PACK_ROOT", cmsisPackRoot); +}