From patchwork Fri Jun 20 06:37:41 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Moritz Haase X-Patchwork-Id: 65323 X-Patchwork-Delegate: steve@sakoman.com Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id 23767C71155 for ; Fri, 20 Jun 2025 06:38:25 +0000 (UTC) Received: from esa10.hc324-48.eu.iphmx.com (esa10.hc324-48.eu.iphmx.com [207.54.69.29]) by mx.groups.io with SMTP id smtpd.web11.1832.1750401498178911220 for ; Thu, 19 Jun 2025 23:38:19 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@bmw.de header.s=mailing1 header.b=kF6QU5ED; spf=pass (domain: bmw.de, ip: 207.54.69.29, mailfrom: prvs=2599e2df9=moritz.haase@bmw.de) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bmw.de; i=@bmw.de; q=dns/txt; s=mailing1; t=1750401498; x=1781937498; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=i/tzyOlvKKEZp5JDBTzypByN+RtohAAIbUzl+aiyJuQ=; b=kF6QU5EDb5E4KqEJCaEsqfmqnyTVP9qivjOVb/aGvCoLRiwSRfdecj2D grzQG/xxidfbo8/bRlH0/AHoJBQY6SbHZEoA3lU0if1Ac9uNL/ctNv4bp vqnIBL3QQ56KL6SZ49liJORRNvBph0i+jDpITUAM4FiX89TQP/NwgmZtp M=; X-CSE-ConnectionGUID: bV+T5Bk5SOSa9kTHPcN3dw== X-CSE-MsgGUID: U5sk7JCsSQ+cebBBDkcppQ== Received: from esagw3.bmwgroup.com (HELO esagw3.muc) ([160.46.252.35]) by esa10.hc324-48.eu.iphmx.com with ESMTP/TLS; 20 Jun 2025 08:38:16 +0200 Received: from esabb2.muc ([160.50.100.34]) by esagw3.muc with ESMTP/TLS; 20 Jun 2025 08:38:16 +0200 Received: from smucmp12a.bmwgroup.net (HELO smucmp12a.europe.bmw.corp) ([10.30.13.95]) by esabb2.muc with ESMTP/TLS; 20 Jun 2025 08:38:16 +0200 Received: from q1054628.de-cci.bmwgroup.net (10.30.85.205) by smucmp12a.europe.bmw.corp (2a03:1e80:a15:58f::1:9) with Microsoft SMTP Server (version=TLS; Fri, 20 Jun 2025 08:38:15 +0200 X-CSE-ConnectionGUID: YBUJ0LNZSCC6gXlKvs1AfA== X-CSE-MsgGUID: pAQ3n5urTk25s3/xQslmXg== X-CSE-ConnectionGUID: nW+V3R9+Rgmj8GOqawq6zw== X-CSE-MsgGUID: YkkDdL4WSFeonq77+yJr+w== From: Moritz Haase To: CC: Moritz Haase , John Drouhard , Mathieu Dubois-Briand , Richard Purdie Subject: [scarthgap][PATCH] cmake: Correctly handle cost data of tests with arbitrary chars in name Date: Fri, 20 Jun 2025 08:37:41 +0200 Message-ID: <20250620063741.3087457-1-Moritz.Haase@bmw.de> X-Mailer: git-send-email 2.49.0 MIME-Version: 1.0 X-ClientProxiedBy: smucmp17b.europe.bmw.corp (2a03:1e80:a15:58f::1:4c) To smucmp12a.europe.bmw.corp (2a03:1e80:a15:58f::1:9) List-Id: X-Webhook-Received: from li982-79.members.linode.com [45.33.32.79] by aws-us-west-2-korg-lkml-1.web.codeaurora.org with HTTPS for ; Fri, 20 Jun 2025 06:38:25 -0000 X-Groupsio-URL: https://lists.openembedded.org/g/openembedded-core/message/219102 ctest automatically optimizes the order of (parallel) test execution based on historic test case runtime via the COST property (see [0]), which can have a significant impact on overall test run times. Sadly this feature is broken in CMake < 4.0.0 for test cases that have spaces in their name (see [1]). This commit is a backport of f24178f3 (which itself backports the upstream fix). The patch was adapted slightly to apply cleanly to the older CMake version in scarthgap. As repeated test runs are expected to mainly take place inside the SDK, the patch is only applied to 'nativesdk' builds. [0]: https://cmake.org/cmake/help/latest/prop_test/COST.html [1]: https://gitlab.kitware.com/cmake/cmake/-/issues/26594 Reported-By: John Drouhard Signed-off-by: Moritz Haase Signed-off-by: Mathieu Dubois-Briand Signed-off-by: Richard Purdie --- .../cmake/cmake-native_3.28.3.bb | 2 +- ...trary-characters-in-test-names-of-CT.patch | 205 ++++++++++++++++++ meta/recipes-devtools/cmake/cmake_3.28.3.bb | 1 + 3 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 meta/recipes-devtools/cmake/cmake/0001-ctest-Allow-arbitrary-characters-in-test-names-of-CT.patch diff --git a/meta/recipes-devtools/cmake/cmake-native_3.28.3.bb b/meta/recipes-devtools/cmake/cmake-native_3.28.3.bb index 546d117156..376da3254b 100644 --- a/meta/recipes-devtools/cmake/cmake-native_3.28.3.bb +++ b/meta/recipes-devtools/cmake/cmake-native_3.28.3.bb @@ -51,7 +51,7 @@ do_compile() { do_install() { oe_runmake 'DESTDIR=${D}' install - # The following codes are here because eSDK needs to provide compatibilty + # The following codes are here because eSDK needs to provide compatibility # for SDK. That is, eSDK could also be used like traditional SDK. mkdir -p ${D}${datadir}/cmake install -m 644 ${WORKDIR}/OEToolchainConfig.cmake ${D}${datadir}/cmake/ diff --git a/meta/recipes-devtools/cmake/cmake/0001-ctest-Allow-arbitrary-characters-in-test-names-of-CT.patch b/meta/recipes-devtools/cmake/cmake/0001-ctest-Allow-arbitrary-characters-in-test-names-of-CT.patch new file mode 100644 index 0000000000..77c1d6378d --- /dev/null +++ b/meta/recipes-devtools/cmake/cmake/0001-ctest-Allow-arbitrary-characters-in-test-names-of-CT.patch @@ -0,0 +1,205 @@ +From 49576cf1df618609be4aa1000749ad087c143df0 Mon Sep 17 00:00:00 2001 +From: John Drouhard +Date: Thu, 9 Jan 2025 20:34:42 -0600 +Subject: [PATCH] ctest: Allow arbitrary characters in test names of + CTestCostData.txt + +This changes the way lines in CTestCostData.txt are parsed to allow for +spaces in the test name. + +It does so by looking for space characters from the end; and once two +have been found, assumes everything from the beginning up to that +second-to-last-space is the test name. + +Additionally, parsing the file should be much more efficient since there +is no string or vector heap allocation per line. The std::string used by +the parse function to convert the int and float should be within most +standard libraries' small string optimization. + +Fixes: #26594 + +Upstream-Status: Backport [4.0.0, 040da7d83216ace59710407e8ce35d5fd38e1340] +Signed-off-by: Moritz Haase +--- + Source/CTest/cmCTestMultiProcessHandler.cxx | 80 +++++++++++++++------ + Source/CTest/cmCTestMultiProcessHandler.h | 3 +- + Tests/CTestTestScheduler/CMakeLists.txt | 4 +- + 3 files changed, 64 insertions(+), 23 deletions(-) + +diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx +index ca07a081eafced40697d82b08c0e2a504939fc4d..59a101454b84367d219e79a01ff72702df0dfa7f 100644 +--- a/Source/CTest/cmCTestMultiProcessHandler.cxx ++++ b/Source/CTest/cmCTestMultiProcessHandler.cxx +@@ -20,6 +20,7 @@ + + #include + #include ++#include + #include + + #include +@@ -43,6 +44,51 @@ + #include "cmUVSignalHackRAII.h" // IWYU pragma: keep + #include "cmWorkingDirectory.h" + ++namespace { ++ ++struct CostEntry ++{ ++ cm::string_view name; ++ int prevRuns; ++ float cost; ++}; ++ ++cm::optional splitCostLine(cm::string_view line) ++{ ++ std::string part; ++ cm::string_view::size_type pos1 = line.size(); ++ cm::string_view::size_type pos2 = line.find_last_of(' ', pos1); ++ auto findNext = [line, &part, &pos1, &pos2]() -> bool { ++ if (pos2 != cm::string_view::npos) { ++ cm::string_view sub = line.substr(pos2 + 1, pos1 - pos2 - 1); ++ part.assign(sub.begin(), sub.end()); ++ pos1 = pos2; ++ if (pos1 > 0) { ++ pos2 = line.find_last_of(' ', pos1 - 1); ++ } ++ return true; ++ } ++ return false; ++ }; ++ ++ // parse the cost ++ if (!findNext()) { ++ return cm::nullopt; ++ } ++ float cost = static_cast(atof(part.c_str())); ++ ++ // parse the previous runs ++ if (!findNext()) { ++ return cm::nullopt; ++ } ++ int prev = atoi(part.c_str()); ++ ++ // from start to the last found space is the name ++ return CostEntry{ line.substr(0, pos1), prev, cost }; ++} ++ ++} ++ + namespace cmsys { + class RegularExpression; + } +@@ -697,24 +743,21 @@ void cmCTestMultiProcessHandler::UpdateCostData() + if (line == "---") { + break; + } +- std::vector parts = cmSystemTools::SplitString(line, ' '); + // Format: +- if (parts.size() < 3) { ++ cm::optional entry = splitCostLine(line); ++ if (!entry) { + break; + } + +- std::string name = parts[0]; +- int prev = atoi(parts[1].c_str()); +- float cost = static_cast(atof(parts[2].c_str())); +- +- int index = this->SearchByName(name); ++ int index = this->SearchByName(entry->name); + if (index == -1) { + // This test is not in memory. We just rewrite the entry +- fout << name << " " << prev << " " << cost << "\n"; ++ fout << entry->name << " " << entry->prevRuns << " " << entry->cost ++ << "\n"; + } else { + // Update with our new average cost +- fout << name << " " << this->Properties[index]->PreviousRuns << " " +- << this->Properties[index]->Cost << "\n"; ++ fout << entry->name << " " << this->Properties[index]->PreviousRuns ++ << " " << this->Properties[index]->Cost << "\n"; + temp.erase(index); + } + } +@@ -750,28 +793,25 @@ void cmCTestMultiProcessHandler::ReadCostData() + break; + } + +- std::vector parts = cmSystemTools::SplitString(line, ' '); ++ // Format: ++ cm::optional entry = splitCostLine(line); + + // Probably an older version of the file, will be fixed next run +- if (parts.size() < 3) { ++ if (!entry) { + fin.close(); + return; + } + +- std::string name = parts[0]; +- int prev = atoi(parts[1].c_str()); +- float cost = static_cast(atof(parts[2].c_str())); +- +- int index = this->SearchByName(name); ++ int index = this->SearchByName(entry->name); + if (index == -1) { + continue; + } + +- this->Properties[index]->PreviousRuns = prev; ++ this->Properties[index]->PreviousRuns = entry->prevRuns; + // When not running in parallel mode, don't use cost data + if (this->ParallelLevel > 1 && this->Properties[index] && + this->Properties[index]->Cost == 0) { +- this->Properties[index]->Cost = cost; ++ this->Properties[index]->Cost = entry->cost; + } + } + // Next part of the file is the failed tests +@@ -784,7 +824,7 @@ void cmCTestMultiProcessHandler::ReadCostData() + } + } + +-int cmCTestMultiProcessHandler::SearchByName(std::string const& name) ++int cmCTestMultiProcessHandler::SearchByName(cm::string_view name) + { + int index = -1; + +diff --git a/Source/CTest/cmCTestMultiProcessHandler.h b/Source/CTest/cmCTestMultiProcessHandler.h +index 3b4e9c59ad1871168d8528be0586831e2416ae36..8d33dabcf0d9fc6e11459105c65eadaa1de33e42 100644 +--- a/Source/CTest/cmCTestMultiProcessHandler.h ++++ b/Source/CTest/cmCTestMultiProcessHandler.h +@@ -12,6 +12,7 @@ + #include + + #include ++#include + + #include + +@@ -113,7 +114,7 @@ protected: + void UpdateCostData(); + void ReadCostData(); + // Return index of a test based on its name +- int SearchByName(std::string const& name); ++ int SearchByName(cm::string_view name); + + void CreateTestCostList(); + +diff --git a/Tests/CTestTestScheduler/CMakeLists.txt b/Tests/CTestTestScheduler/CMakeLists.txt +index 91d565d4020aafda6d49462cd8616d168d5844b6..daf6ce2b23d8c048334ae1047759130b246dccef 100644 +--- a/Tests/CTestTestScheduler/CMakeLists.txt ++++ b/Tests/CTestTestScheduler/CMakeLists.txt +@@ -1,9 +1,9 @@ +-cmake_minimum_required (VERSION 3.5) ++cmake_minimum_required(VERSION 3.19) + project (CTestTestScheduler) + include (CTest) + + add_executable (Sleep sleep.c) + + foreach (time RANGE 1 4) +- add_test (TestSleep${time} Sleep ${time}) ++ add_test ("TestSleep ${time}" Sleep ${time}) + endforeach () diff --git a/meta/recipes-devtools/cmake/cmake_3.28.3.bb b/meta/recipes-devtools/cmake/cmake_3.28.3.bb index 6a9a3266df..63d483801a 100644 --- a/meta/recipes-devtools/cmake/cmake_3.28.3.bb +++ b/meta/recipes-devtools/cmake/cmake_3.28.3.bb @@ -5,6 +5,7 @@ inherit cmake bash-completion DEPENDS += "curl expat zlib libarchive xz ncurses bzip2" SRC_URI:append:class-nativesdk = " \ + file://0001-ctest-Allow-arbitrary-characters-in-test-names-of-CT.patch \ file://OEToolchainConfig.cmake \ file://SDKToolchainConfig.cmake.template \ file://cmake-setup.py \