From a13be1301f58ac8679172168418729245ebe9172 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 6 Feb 2025 07:33:52 -0500 Subject: Tests/RunCMake/Instrumentation: Improve quoting and escaping in CMake code --- Tests/RunCMake/Instrumentation/RunCMakeTest.cmake | 2 +- .../RunCMake/Instrumentation/check-data-dir.cmake | 54 ++++----- .../Instrumentation/check-generated-queries.cmake | 15 +-- Tests/RunCMake/Instrumentation/hook.cmake | 70 ++++++----- Tests/RunCMake/Instrumentation/json.cmake | 12 +- .../RunCMake/Instrumentation/verify-snippet.cmake | 133 +++++++++++---------- 6 files changed, 147 insertions(+), 139 deletions(-) diff --git a/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake b/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake index 2b70972..d8e91f8 100644 --- a/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake +++ b/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake @@ -38,7 +38,7 @@ function(instrument test) if (ARGS_COPY_QUERIES) file(MAKE_DIRECTORY ${RunCMake_TEST_BINARY_DIR}/query) set(generated_queries "0;1;2") - foreach(n ${generated_queries}) + foreach(n IN LISTS generated_queries) configure_file( "${query_dir}/generated/query-${n}.json.in" "${RunCMake_TEST_BINARY_DIR}/query/query-${n}.json" diff --git a/Tests/RunCMake/Instrumentation/check-data-dir.cmake b/Tests/RunCMake/Instrumentation/check-data-dir.cmake index fb3ca8e..0c93d7a 100644 --- a/Tests/RunCMake/Instrumentation/check-data-dir.cmake +++ b/Tests/RunCMake/Instrumentation/check-data-dir.cmake @@ -7,13 +7,13 @@ if (NOT snippets) endif() set(FOUND_SNIPPETS "") -foreach(snippet ${snippets}) - get_filename_component(filename ${snippet} NAME) +foreach(snippet IN LISTS snippets) + get_filename_component(filename "${snippet}" NAME) - read_json(${snippet} contents) + read_json("${snippet}" contents) # Verify snippet file is valid - verify_snippet(${snippet} ${contents}) + verify_snippet("${snippet}" "${contents}") # Append to list of collected snippet roles if (NOT role IN_LIST FOUND_SNIPPETS) @@ -25,78 +25,78 @@ foreach(snippet ${snippets}) if (NOT target MATCHES NOTFOUND) set(targets "main;lib;customTarget;TARGET_NAME") if (NOT ${target} IN_LIST targets) - snippet_error(${snippet} "Unexpected target: ${target}") + snippet_error("${snippet}" "Unexpected target: ${target}") endif() endif() # Verify output string(JSON result GET "${contents}" result) if (NOT ${result} EQUAL 0) - snippet_error(${snippet} "Compile command had non-0 result") + snippet_error("${snippet}" "Compile command had non-0 result") endif() # Verify contents of compile-* Snippets - if (filename MATCHES ^compile-) + if (filename MATCHES "^compile-") string(JSON target GET "${contents}" target) string(JSON source GET "${contents}" source) string(JSON language GET "${contents}" language) if (NOT language MATCHES "C\\+\\+") - snippet_error(${snippet} "Expected C++ compile language") + snippet_error("${snippet}" "Expected C++ compile language") endif() if (NOT source MATCHES "${target}.cxx$") - snippet_error(${snippet} "Unexpected source file") + snippet_error("${snippet}" "Unexpected source file") endif() endif() # Verify contents of link-* Snippets - if (filename MATCHES ^link-) + if (filename MATCHES "^link-") string(JSON target GET "${contents}" target) string(JSON targetType GET "${contents}" targetType) string(JSON targetLabels GET "${contents}" targetLabels) - if (target MATCHES main) + if (target MATCHES "main") if (NOT targetType MATCHES "EXECUTABLE") - snippet_error(${snippet} "Expected EXECUTABLE, target type was ${targetType}") + snippet_error("${snippet}" "Expected EXECUTABLE, target type was ${targetType}") endif() string(JSON nlabels LENGTH "${targetLabels}") if (NOT nlabels STREQUAL 2) - snippet_error(${snippet} "Missing Target Labels for: ${target}") + snippet_error("${snippet}" "Missing Target Labels for: ${target}") else() string(JSON label1 GET "${contents}" targetLabels 0) string(JSON label2 GET "${contents}" targetLabels 1) if (NOT label1 MATCHES "label1" OR NOT label2 MATCHES "label2") - snippet_error(${snippet} "Missing Target Labels for: ${target}") + snippet_error("${snippet}" "Missing Target Labels for: ${target}") endif() endif() endif() - if (target MATCHES lib) + if (target MATCHES "lib") if (NOT targetType MATCHES "STATIC_LIBRARY") - snippet_error(${snippet} "Expected STATIC_LIBRARY, target type was ${targetType}") + snippet_error("${snippet}" "Expected STATIC_LIBRARY, target type was ${targetType}") endif() string(JSON nlabels LENGTH "${targetLabels}") if (NOT nlabels STREQUAL 1) - snippet_error(${snippet} "Missing Target Labels for: ${target}") + snippet_error("${snippet}" "Missing Target Labels for: ${target}") else() string(JSON label ERROR_VARIABLE noLabels GET "${contents}" targetLabels 0) if (NOT label MATCHES "label3") - snippet_error(${snippet} "Missing Target Labels for: ${target}") + snippet_error("${snippet}" "Missing Target Labels for: ${target}") endif() endif() endif() endif() # Verify contents of custom-* Snippets - if (filename MATCHES ^custom-) + if (filename MATCHES "^custom-") string(JSON outputs GET "${contents}" outputs) if (NOT output1 MATCHES "output1" OR NOT output2 MATCHES "output2") - snippet_error(${snippet} "Custom command missing outputs") + snippet_error("${snippet}" "Custom command missing outputs") endif() endif() # Verify contents of test-* Snippets - if (filename MATCHES ^test-) + if (filename MATCHES "^test-") string(JSON testName GET "${contents}" testName) if (NOT testName STREQUAL "test") - snippet_error(${snippet} "Unexpected testName: ${testName}") + snippet_error("${snippet}" "Unexpected testName: ${testName}") endif() endif() endforeach() @@ -115,14 +115,14 @@ if (ARGS_INSTALL) list(APPEND EXPECTED_SNIPPETS install) endif() endif() -foreach(role ${EXPECTED_SNIPPETS}) - list(FIND FOUND_SNIPPETS ${role} found) - if (${found} EQUAL -1) +foreach(role IN LISTS EXPECTED_SNIPPETS) + list(FIND FOUND_SNIPPETS "${role}" found) + if (found EQUAL -1) add_error("No snippet files of role \"${role}\" were found in ${v1}") endif() endforeach() -foreach(role ${FOUND_SNIPPETS}) - list(FIND EXPECTED_SNIPPETS ${role} found) +foreach(role IN LISTS FOUND_SNIPPETS) + list(FIND EXPECTED_SNIPPETS "${role}" found) if (${found} EQUAL -1) add_error("Found unexpected snippet file of role \"${role}\" in ${v1}") endif() diff --git a/Tests/RunCMake/Instrumentation/check-generated-queries.cmake b/Tests/RunCMake/Instrumentation/check-generated-queries.cmake index ba3fd6b..6ccdd28 100644 --- a/Tests/RunCMake/Instrumentation/check-generated-queries.cmake +++ b/Tests/RunCMake/Instrumentation/check-generated-queries.cmake @@ -1,17 +1,18 @@ include(${CMAKE_CURRENT_LIST_DIR}/json.cmake) -macro(check_generated_json n) - set(expected_file ${RunCMake_TEST_BINARY_DIR}/query/query-${n}.json) - set(generated_file ${v1}/query/generated/query-${n}.json) - read_json(${expected_file} expected) - read_json(${generated_file} generated) +function(check_generated_json n) + set(expected_file "${RunCMake_TEST_BINARY_DIR}/query/query-${n}.json") + set(generated_file "${v1}/query/generated/query-${n}.json") + read_json("${expected_file}" expected) + read_json("${generated_file}" generated) string(JSON equal EQUAL ${expected} ${generated}) if (NOT equal) set(RunCMake_TEST_FAILED "Generated JSON ${generated}\nNot equal to expected ${expected}" ) endif() -endmacro() + return(PROPAGATE RunCMake_TEST_FAILED) +endfunction() -foreach(n ${generated_queries}) +foreach(n IN LISTS generated_queries) check_generated_json(${n}) endforeach() diff --git a/Tests/RunCMake/Instrumentation/hook.cmake b/Tests/RunCMake/Instrumentation/hook.cmake index f84a3b9..cef088d 100644 --- a/Tests/RunCMake/Instrumentation/hook.cmake +++ b/Tests/RunCMake/Instrumentation/hook.cmake @@ -1,3 +1,5 @@ +cmake_minimum_required(VERSION 3.30) + include(${CMAKE_CURRENT_LIST_DIR}/json.cmake) # Test CALLBACK script. Prints output information and verifies index file # Called as: cmake -P hook.cmake [CheckForStaticQuery?] [index.json] @@ -5,39 +7,41 @@ set(index ${CMAKE_ARGV4}) if (NOT ${CMAKE_ARGV3}) set(hasStaticInfo "UNEXPECTED") endif() -read_json(${index} contents) +read_json("${index}" contents) string(JSON hook GET "${contents}" hook) # Output is verified by *-stdout.txt files that the HOOK is run message(STATUS ${hook}) # Not a check-*.cmake script, this is called as an instrumentation CALLBACK set(ERROR_MESSAGE "") -macro(add_error error) +function(add_error error) string(APPEND ERROR_MESSAGE "${error}\n") -endmacro() + return(PROPAGATE ERROR_MESSAGE) +endfunction() -macro(has_key key json) +function(has_key key json) cmake_parse_arguments(ARG "UNEXPECTED" "" "" ${ARGN}) unset(missingKey) string(JSON ${key} ERROR_VARIABLE missingKey GET "${json}" ${key}) - if (NOT ARG_UNEXPECTED AND NOT "${missingKey}" MATCHES NOTFOUND) + if (NOT ARG_UNEXPECTED AND NOT missingKey MATCHES NOTFOUND) add_error("\nKey \"${key}\" not in index:\n${json}") - elseif(ARG_UNEXPECTED AND "${missingKey}" MATCHES NOTFOUND) + elseif(ARG_UNEXPECTED AND missingKey MATCHES NOTFOUND) add_error("\nUnexpected key \"${key}\" in index:\n${json}") endif() -endmacro() + return(PROPAGATE RunCMake_TEST_FAILED ${key}) +endfunction() -has_key(version ${contents}) -has_key(buildDir ${contents}) -has_key(dataDir ${contents}) -has_key(snippets ${contents}) +has_key(version "${contents}") +has_key(buildDir "${contents}") +has_key(dataDir "${contents}") +has_key(snippets "${contents}") -if (NOT ${version} EQUAL 1) +if (NOT version EQUAL 1) add_error("Version must be 1, got: ${version}") endif() -string(JSON length LENGTH ${snippets}) -math(EXPR length ${length}-1) +string(JSON length LENGTH "${snippets}") +math(EXPR length "${length}-1") foreach(i RANGE ${length}) string(JSON filename GET "${snippets}" ${i}) if (NOT EXISTS ${dataDir}/${filename}) @@ -45,25 +49,25 @@ foreach(i RANGE ${length}) endif() endforeach() -has_key(staticSystemInformation ${contents} ${hasStaticInfo}) -has_key(OSName ${staticSystemInformation} ${hasStaticInfo}) -has_key(OSPlatform ${staticSystemInformation} ${hasStaticInfo}) -has_key(OSRelease ${staticSystemInformation} ${hasStaticInfo}) -has_key(OSVersion ${staticSystemInformation} ${hasStaticInfo}) -has_key(familyId ${staticSystemInformation} ${hasStaticInfo}) -has_key(hostname ${staticSystemInformation} ${hasStaticInfo}) -has_key(is64Bits ${staticSystemInformation} ${hasStaticInfo}) -has_key(modelId ${staticSystemInformation} ${hasStaticInfo}) -has_key(numberOfLogicalCPU ${staticSystemInformation} ${hasStaticInfo}) -has_key(numberOfPhysicalCPU ${staticSystemInformation} ${hasStaticInfo}) -has_key(processorAPICID ${staticSystemInformation} ${hasStaticInfo}) -has_key(processorCacheSize ${staticSystemInformation} ${hasStaticInfo}) -has_key(processorClockFrequency ${staticSystemInformation} ${hasStaticInfo}) -has_key(processorName ${staticSystemInformation} ${hasStaticInfo}) -has_key(totalPhysicalMemory ${staticSystemInformation} ${hasStaticInfo}) -has_key(totalVirtualMemory ${staticSystemInformation} ${hasStaticInfo}) -has_key(vendorID ${staticSystemInformation} ${hasStaticInfo}) -has_key(vendorString ${staticSystemInformation} ${hasStaticInfo}) +has_key(staticSystemInformation "${contents}" ${hasStaticInfo}) +has_key(OSName "${staticSystemInformation}" ${hasStaticInfo}) +has_key(OSPlatform "${staticSystemInformation}" ${hasStaticInfo}) +has_key(OSRelease "${staticSystemInformation}" ${hasStaticInfo}) +has_key(OSVersion "${staticSystemInformation}" ${hasStaticInfo}) +has_key(familyId "${staticSystemInformation}" ${hasStaticInfo}) +has_key(hostname "${staticSystemInformation}" ${hasStaticInfo}) +has_key(is64Bits "${staticSystemInformation}" ${hasStaticInfo}) +has_key(modelId "${staticSystemInformation}" ${hasStaticInfo}) +has_key(numberOfLogicalCPU "${staticSystemInformation}" ${hasStaticInfo}) +has_key(numberOfPhysicalCPU "${staticSystemInformation}" ${hasStaticInfo}) +has_key(processorAPICID "${staticSystemInformation}" ${hasStaticInfo}) +has_key(processorCacheSize "${staticSystemInformation}" ${hasStaticInfo}) +has_key(processorClockFrequency "${staticSystemInformation}" ${hasStaticInfo}) +has_key(processorName "${staticSystemInformation}" ${hasStaticInfo}) +has_key(totalPhysicalMemory "${staticSystemInformation}" ${hasStaticInfo}) +has_key(totalVirtualMemory "${staticSystemInformation}" ${hasStaticInfo}) +has_key(vendorID "${staticSystemInformation}" ${hasStaticInfo}) +has_key(vendorString "${staticSystemInformation}" ${hasStaticInfo}) if (NOT ERROR_MESSAGE MATCHES "^$") message(FATAL_ERROR ${ERROR_MESSAGE}) diff --git a/Tests/RunCMake/Instrumentation/json.cmake b/Tests/RunCMake/Instrumentation/json.cmake index 1b5b097..3cc88e0 100644 --- a/Tests/RunCMake/Instrumentation/json.cmake +++ b/Tests/RunCMake/Instrumentation/json.cmake @@ -1,8 +1,4 @@ -macro(read_json filename outvar) - file(READ ${filename} contents) - # string(JSON *) will fail if JSON file contains any forward-slash paths - string(REGEX REPLACE "[\\]([a-zA-Z0-9 ])" "/\\1" contents ${contents}) - # string(JSON *) will fail if JSON file contains any escaped quotes \" - string(REPLACE "\\\"" "'" contents ${contents}) - set(${outvar} ${contents}) -endmacro() +function(read_json filename outvar) + file(READ "${filename}" ${outvar}) + return(PROPAGATE ${outvar}) +endfunction() diff --git a/Tests/RunCMake/Instrumentation/verify-snippet.cmake b/Tests/RunCMake/Instrumentation/verify-snippet.cmake index 5c93c1e..dfc6332 100644 --- a/Tests/RunCMake/Instrumentation/verify-snippet.cmake +++ b/Tests/RunCMake/Instrumentation/verify-snippet.cmake @@ -1,94 +1,100 @@ # Performs generic (non-project specific) validation of v1 Snippet File Contents -macro(add_error error) - string(APPEND RunCMake_TEST_FAILED "${error}\n") -endmacro() +function(add_error error) + string(APPEND RunCMake_TEST_FAILED " ${error}\n") + return(PROPAGATE RunCMake_TEST_FAILED) +endfunction() -macro(snippet_error snippet error) +function(snippet_error snippet error) add_error("Error in snippet file ${snippet}:\n${error}") -endmacro() + return(PROPAGATE RunCMake_TEST_FAILED) +endfunction() -macro(has_key snippet json key) +function(has_key snippet json key) string(JSON data ERROR_VARIABLE missingKey GET "${json}" ${key}) - if (NOT ${missingKey} MATCHES NOTFOUND) - snippet_error(${snippet} "Missing ${key}") + if (NOT missingKey MATCHES NOTFOUND) + snippet_error("${snippet}" "Missing ${key}") endif() -endmacro() + return(PROPAGATE RunCMake_TEST_FAILED) +endfunction() -macro(has_not_key snippet json key) +function(has_not_key snippet json key) string(JSON data ERROR_VARIABLE missingKey GET "${json}" ${key}) - if (${missingKey} MATCHES NOTFOUND) - snippet_error(${snippet} "Has unexpected ${key}") + if (missingKey MATCHES NOTFOUND) + snippet_error("${snippet}" "Has unexpected ${key}") endif() -endmacro() + return(PROPAGATE RunCMake_TEST_FAILED) +endfunction() -macro(snippet_has_fields snippet contents) - get_filename_component(filename ${snippet} NAME) - has_key(${snippet} ${contents} command) - has_key(${snippet} ${contents} role) - has_key(${snippet} ${contents} result) - if (filename MATCHES ^link-*) - has_key(${snippet} ${contents} target) - has_key(${snippet} ${contents} outputs) - has_key(${snippet} ${contents} outputSizes) - has_key(${snippet} ${contents} targetType) - has_key(${snippet} ${contents} targetLabels) - elseif (filename MATCHES ^compile-*) - has_key(${snippet} ${contents} target) - has_key(${snippet} ${contents} outputs) - has_key(${snippet} ${contents} outputSizes) - has_key(${snippet} ${contents} source) - has_key(${snippet} ${contents} language) - elseif (filename MATCHES ^custom-*) - has_key(${snippet} ${contents} target) - has_key(${snippet} ${contents} outputs) - has_key(${snippet} ${contents} outputSizes) - elseif (filename MATCHES ^test-*) - has_key(${snippet} ${contents} testName) +function(snippet_has_fields snippet contents) + get_filename_component(filename "${snippet}" NAME) + has_key("${snippet}" "${contents}" command) + has_key("${snippet}" "${contents}" role) + has_key("${snippet}" "${contents}" result) + if (filename MATCHES "^link-*") + has_key("${snippet}" "${contents}" target) + has_key("${snippet}" "${contents}" outputs) + has_key("${snippet}" "${contents}" outputSizes) + has_key("${snippet}" "${contents}" targetType) + has_key("${snippet}" "${contents}" targetLabels) + elseif (filename MATCHES "^compile-*") + has_key("${snippet}" "${contents}" target) + has_key("${snippet}" "${contents}" outputs) + has_key("${snippet}" "${contents}" outputSizes) + has_key("${snippet}" "${contents}" source) + has_key("${snippet}" "${contents}" language) + elseif (filename MATCHES "^custom-*") + has_key("${snippet}" "${contents}" target) + has_key("${snippet}" "${contents}" outputs) + has_key("${snippet}" "${contents}" outputSizes) + elseif (filename MATCHES "^test-*") + has_key("${snippet}" "${contents}" testName) endif() if(ARGS_DYNAMIC_QUERY) - has_key(${snippet} ${contents} dynamicSystemInformation) + has_key("${snippet}" "${contents}" dynamicSystemInformation) string(JSON dynamicSystemInfo ERROR_VARIABLE noInfo GET "${contents}" dynamicSystemInformation) if (noInfo MATCHES NOTFOUND) - has_key(${snippet} ${dynamicSystemInfo} beforeCPULoadAverage) - has_key(${snippet} ${dynamicSystemInfo} beforeHostMemoryUsed) - has_key(${snippet} ${dynamicSystemInfo} beforeCPULoadAverage) - has_key(${snippet} ${dynamicSystemInfo} beforeHostMemoryUsed) + has_key("${snippet}" ${dynamicSystemInfo} beforeCPULoadAverage) + has_key("${snippet}" ${dynamicSystemInfo} beforeHostMemoryUsed) + has_key("${snippet}" ${dynamicSystemInfo} beforeCPULoadAverage) + has_key("${snippet}" ${dynamicSystemInfo} beforeHostMemoryUsed) endif() else() - has_not_key(${snippet} ${contents} dynamicSystemInformation) + has_not_key("${snippet}" "${contents}" dynamicSystemInformation) string(JSON dynamicSystemInfo ERROR_VARIABLE noInfo GET "${contents}" dynamicSystemInformation) if (noInfo MATCHES NOTFOUND) - has_not_key(${snippet} ${dynamicSystemInfo} beforeCPULoadAverage) - has_not_key(${snippet} ${dynamicSystemInfo} beforeHostMemoryUsed) - has_not_key(${snippet} ${dynamicSystemInfo} beforeCPULoadAverage) - has_not_key(${snippet} ${dynamicSystemInfo} beforeHostMemoryUsed) + has_not_key("${snippet}" ${dynamicSystemInfo} beforeCPULoadAverage) + has_not_key("${snippet}" ${dynamicSystemInfo} beforeHostMemoryUsed) + has_not_key("${snippet}" ${dynamicSystemInfo} beforeCPULoadAverage) + has_not_key("${snippet}" ${dynamicSystemInfo} beforeHostMemoryUsed) endif() endif() -endmacro() + return(PROPAGATE RunCMake_TEST_FAILED) +endfunction() -macro(snippet_valid_timing contents) +function(snippet_valid_timing contents) string(JSON start GET "${contents}" timeStart) string(JSON duration GET "${contents}" duration) - if (${start} LESS 0) - snippet_error(${snippet} "Negative time start: ${start}") + if (start LESS 0) + snippet_error("${snippet}" "Negative time start: ${start}") endif() - if (${duration} LESS 0) - snippet_error(${snippet} "Negative duration: ${end}") + if (duration LESS 0) + snippet_error("${snippet}" "Negative duration: ${end}") endif() -endmacro() + return(PROPAGATE RunCMake_TEST_FAILED) +endfunction() -macro(verify_snippet snippet contents) - snippet_has_fields(${snippet} ${contents}) - snippet_valid_timing(${contents}) +function(verify_snippet snippet contents) + snippet_has_fields("${snippet}" "${contents}") + snippet_valid_timing("${contents}") string(JSON version GET "${contents}" version) - if (NOT ${version} EQUAL 1) - snippet_error(${snippet} "Version must be 1, got: ${version}") + if (NOT version EQUAL 1) + snippet_error("${snippet}" "Version must be 1, got: ${version}") endif() string(JSON role GET "${contents}" role) - get_filename_component(filename ${snippet} NAME) - if (NOT ${filename} MATCHES ^${role}-) - snippet_error(${snippet} "Role \"${role}\" doesn't match snippet filename") + get_filename_component(filename "${snippet}" NAME) + if (NOT filename MATCHES "^${role}-") + snippet_error("${snippet}" "Role \"${role}\" doesn't match snippet filename") endif() string(JSON outputs ERROR_VARIABLE noOutputs GET "${contents}" outputs) if (NOT outputs MATCHES NOTFOUND) @@ -96,7 +102,8 @@ macro(verify_snippet snippet contents) list(LENGTH outputs outputsLen) list(LENGTH outputSizes outputSizesLen) if (outputSizes MATCHES NOTFOUND OR NOT outputsLen EQUAL outputSizesLen) - snippet_error(${snippet} "outputs and outputSizes do not match") + snippet_error("${snippet}" "outputs and outputSizes do not match") endif() endif() -endmacro() + return(PROPAGATE RunCMake_TEST_FAILED role) +endfunction() -- cgit v0.12 From 0fbb927bddb97a59f80d5549543d79a9d7a60614 Mon Sep 17 00:00:00 2001 From: Martin Duffy Date: Wed, 5 Feb 2025 13:43:30 -0500 Subject: instrumentation: Disable preBuild and postBuild hooks on Windows The implementation does not work on Windows. Issue: #26668 --- Help/manual/cmake-instrumentation.7.rst | 8 ++++---- Source/cmGlobalNinjaGenerator.cxx | 11 +++++++---- Source/cmGlobalNinjaGenerator.h | 3 +++ Tests/RunCMake/Instrumentation/RunCMakeTest.cmake | 2 ++ 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/Help/manual/cmake-instrumentation.7.rst b/Help/manual/cmake-instrumentation.7.rst index da9c0ff..d3a1619 100644 --- a/Help/manual/cmake-instrumentation.7.rst +++ b/Help/manual/cmake-instrumentation.7.rst @@ -117,10 +117,10 @@ optional. should be one of the following: * ``postGenerate`` - * ``preBuild`` (:ref:`Ninja Generators`. only, when ``ninja`` is invoked) - * ``postBuild`` (:ref:`Ninja Generators`. only, when ``ninja`` completes) - * ``preCMakeBuild`` (when ``cmake --build`` is invoked) - * ``postCMakeBuild`` (when ``cmake --build`` completes) + * ``preBuild`` (called when ``ninja`` is invoked; unavailable on Windows) + * ``postBuild`` (called when ``ninja`` completes; unavailable on Windows) + * ``preCMakeBuild`` (called when ``cmake --build`` is invoked) + * ``postCMakeBuild`` (called when ``cmake --build`` completes) * ``postInstall`` * ``postTest`` diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index e3eee6a..6b57669 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -1762,7 +1762,8 @@ void cmGlobalNinjaGenerator::WriteBuiltinTargets(std::ostream& os) this->WriteTargetRebuildManifest(os); this->WriteTargetClean(os); this->WriteTargetHelp(os); -#if !defined(CMAKE_BOOTSTRAP) +#if !defined(CMAKE_BOOTSTRAP) && !defined(_WIN32) + // FIXME(#26668) This does not work on Windows if (this->GetCMakeInstance() ->GetInstrumentation() ->HasPreOrPostBuildHook()) { @@ -1843,7 +1844,8 @@ void cmGlobalNinjaGenerator::WriteTargetRebuildManifest(std::ostream& os) } reBuild.ImplicitDeps.push_back(this->CMakeCacheFile); -#if !defined(CMAKE_BOOTSTRAP) +#if !defined(CMAKE_BOOTSTRAP) && !defined(_WIN32) + // FIXME(#26668) This does not work on Windows if (this->GetCMakeInstance() ->GetInstrumentation() ->HasPreOrPostBuildHook()) { @@ -2196,6 +2198,8 @@ void cmGlobalNinjaGenerator::WriteTargetHelp(std::ostream& os) } } +#if !defined(CMAKE_BOOTSTRAP) && !defined(_WIN32) +// FIXME(#26668) This does not work on Windows void cmGlobalNinjaGenerator::WriteTargetInstrument(std::ostream& os) { // Write rule @@ -2204,13 +2208,11 @@ void cmGlobalNinjaGenerator::WriteTargetInstrument(std::ostream& os) rule.Command = cmStrCat( "\"", cmSystemTools::GetCTestCommand(), "\" --start-instrumentation \"", this->GetCMakeInstance()->GetHomeOutputDirectory(), "\""); -#ifndef _WIN32 /* * On Unix systems, Ninja will prefix the command with `/bin/sh -c`. * Use exec so that Ninja is the parent process of the command. */ rule.Command = cmStrCat("exec ", rule.Command); -#endif rule.Description = "Collecting build metrics"; rule.Comment = "Rule to initialize instrumentation daemon."; rule.Restat = "1"; @@ -2231,6 +2233,7 @@ void cmGlobalNinjaGenerator::WriteTargetInstrument(std::ostream& os) WriteBuild(os, instrument); } } +#endif void cmGlobalNinjaGenerator::InitOutputPathPrefix() { diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h index acc9ee4..66e3bbd 100644 --- a/Source/cmGlobalNinjaGenerator.h +++ b/Source/cmGlobalNinjaGenerator.h @@ -536,7 +536,10 @@ private: void WriteTargetRebuildManifest(std::ostream& os); bool WriteTargetCleanAdditional(std::ostream& os); void WriteTargetClean(std::ostream& os); +#if !defined(CMAKE_BOOTSTRAP) && !defined(_WIN32) + // FIXME(#26668) This does not work on Windows void WriteTargetInstrument(std::ostream& os); +#endif void WriteTargetHelp(std::ostream& os); void ComputeTargetDependsClosure( diff --git a/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake b/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake index d8e91f8..f98bb32 100644 --- a/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake +++ b/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake @@ -115,6 +115,8 @@ instrument(cmake-command-bad-arg NO_WARN) instrument(cmake-command-parallel-install BUILD INSTALL TEST NO_WARN INSTALL_PARALLEL DYNAMIC_QUERY CHECK_SCRIPT check-data-dir.cmake) + +# FIXME(#26668) This does not work on Windows if (UNIX AND ${RunCMake_GENERATOR} MATCHES "^Ninja") instrument(cmake-command-ninja NO_WARN BUILD_MAKE_PROGRAM -- cgit v0.12 From f8339cb94460df0d789a9cf04402dd25cc532f3a Mon Sep 17 00:00:00 2001 From: Martin Duffy Date: Wed, 5 Feb 2025 13:43:30 -0500 Subject: instrumentation: Enable tests on more Makefile generators --- Tests/RunCMake/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt index b29ac57..4039cde 100644 --- a/Tests/RunCMake/CMakeLists.txt +++ b/Tests/RunCMake/CMakeLists.txt @@ -414,7 +414,7 @@ if(CMAKE_USE_SYSTEM_JSONCPP) endif() add_RunCMake_test(FileAPI -DPython_EXECUTABLE=${Python_EXECUTABLE} -DCMAKE_CXX_COMPILER_ID=${CMAKE_CXX_COMPILER_ID}) -if("${CMAKE_GENERATOR}" MATCHES "Unix Makefiles|Ninja") +if(CMAKE_GENERATOR MATCHES "Make|Ninja") add_RunCMake_test(Instrumentation) endif() add_RunCMake_test(ConfigDir) -- cgit v0.12 From 6598248da7553f79a3a1c45ed4d01131c0882c1c Mon Sep 17 00:00:00 2001 From: Martin Duffy Date: Wed, 5 Feb 2025 13:43:30 -0500 Subject: instrumentation: Avoid busy-wait on postBuild hook --- Source/cmInstrumentation.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/Source/cmInstrumentation.cxx b/Source/cmInstrumentation.cxx index 5877323..c3f4488 100644 --- a/Source/cmInstrumentation.cxx +++ b/Source/cmInstrumentation.cxx @@ -528,6 +528,7 @@ int cmInstrumentation::SpawnBuildDaemon() int cmInstrumentation::CollectTimingAfterBuild(int ppid) { while (0 == uv_kill(ppid, 0)) { + cmSystemTools::Delay(100); }; return this->CollectTimingData(cmInstrumentationQuery::Hook::PostBuild); } -- cgit v0.12 From 314440c32077dc2a902a494a99b96437d93af5b4 Mon Sep 17 00:00:00 2001 From: Martin Duffy Date: Wed, 5 Feb 2025 13:43:30 -0500 Subject: instrumentation: Run preBuild and postBuild hooks before and after make Updates the preBuild and postBuild instrumentation hooks to run before and after make is invoked. --- Help/manual/cmake-instrumentation.7.rst | 4 +- Source/cmLocalUnixMakefileGenerator3.cxx | 61 +++++++++++++++++++--- Tests/RunCMake/Instrumentation/RunCMakeTest.cmake | 6 +-- .../Instrumentation/check-make-program-hooks.cmake | 35 +++++++++++++ .../Instrumentation/check-ninja-hooks.cmake | 35 ------------- .../Instrumentation/project/CMakeLists.txt | 1 + .../query/cmake-command-make-program.cmake | 7 +++ .../query/cmake-command-ninja.cmake | 6 --- 8 files changed, 101 insertions(+), 54 deletions(-) create mode 100644 Tests/RunCMake/Instrumentation/check-make-program-hooks.cmake delete mode 100644 Tests/RunCMake/Instrumentation/check-ninja-hooks.cmake create mode 100644 Tests/RunCMake/Instrumentation/query/cmake-command-make-program.cmake delete mode 100644 Tests/RunCMake/Instrumentation/query/cmake-command-ninja.cmake diff --git a/Help/manual/cmake-instrumentation.7.rst b/Help/manual/cmake-instrumentation.7.rst index d3a1619..c3ebc75 100644 --- a/Help/manual/cmake-instrumentation.7.rst +++ b/Help/manual/cmake-instrumentation.7.rst @@ -117,8 +117,8 @@ optional. should be one of the following: * ``postGenerate`` - * ``preBuild`` (called when ``ninja`` is invoked; unavailable on Windows) - * ``postBuild`` (called when ``ninja`` completes; unavailable on Windows) + * ``preBuild`` (called when ``ninja`` or ``make`` is invoked; unavailable on Windows) + * ``postBuild`` (called when ``ninja`` or ``make`` completes; unavailable on Windows) * ``preCMakeBuild`` (called when ``cmake --build`` is invoked) * ``postCMakeBuild`` (called when ``cmake --build`` completes) * ``postInstall`` diff --git a/Source/cmLocalUnixMakefileGenerator3.cxx b/Source/cmLocalUnixMakefileGenerator3.cxx index 1256592..68c79ed 100644 --- a/Source/cmLocalUnixMakefileGenerator3.cxx +++ b/Source/cmLocalUnixMakefileGenerator3.cxx @@ -28,6 +28,7 @@ #include "cmGeneratorTarget.h" #include "cmGlobalGenerator.h" #include "cmGlobalUnixMakefileGenerator3.h" +#include "cmInstrumentation.h" #include "cmList.h" #include "cmListFileCache.h" #include "cmLocalGenerator.h" @@ -71,6 +72,26 @@ std::string cmSplitExtension(std::string const& in, std::string& base) return ext; } +#if !defined(CMAKE_BOOTSTRAP) && !defined(_WIN32) +// Helper function to add the Start Instrumentation command +void addInstrumentationCommand(cmInstrumentation* instrumentation, + std::vector& commands) +{ + // FIXME(#26668) This does not work on Windows + if (instrumentation->HasPreOrPostBuildHook()) { + std::string instrumentationCommand = + "$(CTEST_COMMAND) --start-instrumentation $(CMAKE_BINARY_DIR)"; + /* + * On Unix systems, Make will prefix the command with `/bin/sh -c`. + * Use exec so that Make is the parent process of the command. + * Add a `;` to convince BSD make to not optimize out the shell. + */ + instrumentationCommand = cmStrCat("exec ", instrumentationCommand, " ;"); + commands.push_back(instrumentationCommand); + } +} +#endif + // Helper predicate for removing absolute paths that don't point to the // source or binary directory. It is used when CMAKE_DEPENDS_IN_PROJECT_ONLY // is set ON, to only consider in-project dependencies during the build. @@ -651,19 +672,35 @@ void cmLocalUnixMakefileGenerator3::WriteMakeVariables( #endif } + auto getShellCommand = [this](std::string command) -> std::string { + std::string shellCommand = this->MaybeConvertWatcomShellCommand(command); + return shellCommand.empty() + ? this->ConvertToOutputFormat(command, cmOutputConverter::SHELL) + : shellCommand; + }; + std::string cmakeShellCommand = - this->MaybeConvertWatcomShellCommand(cmSystemTools::GetCMakeCommand()); - if (cmakeShellCommand.empty()) { - cmakeShellCommand = this->ConvertToOutputFormat( - cmSystemTools::GetCMakeCommand(), cmOutputConverter::SHELL); + getShellCommand(cmSystemTools::GetCMakeCommand()); + + makefileStream << "# The CMake executable.\n" + "CMAKE_COMMAND = " + << cmakeShellCommand << "\n"; + +#if !defined(CMAKE_BOOTSTRAP) && !defined(_WIN32) + // FIXME(#26668) This does not work on Windows + if (this->GetCMakeInstance() + ->GetInstrumentation() + ->HasPreOrPostBuildHook()) { + std::string ctestShellCommand = + getShellCommand(cmSystemTools::GetCTestCommand()); + makefileStream << "# The CTest executable.\n" + "CTEST_COMMAND = " + << ctestShellCommand << "\n"; } +#endif makefileStream - << "# The CMake executable.\n" - "CMAKE_COMMAND = " - << cmakeShellCommand << "\n" - "\n" "# The command to remove a file.\n" "RM = " << cmakeShellCommand @@ -811,6 +848,10 @@ void cmLocalUnixMakefileGenerator3::WriteSpecialTargetsBottom( std::vector no_depends; commands.push_back(std::move(runRule)); +#if !defined(CMAKE_BOOTSTRAP) && !defined(_WIN32) + addInstrumentationCommand(this->GetCMakeInstance()->GetInstrumentation(), + commands); +#endif if (!this->IsRootMakefile()) { this->CreateCDCommand(commands, this->GetBinaryDirectory(), this->GetCurrentBinaryDirectory()); @@ -1808,6 +1849,10 @@ void cmLocalUnixMakefileGenerator3::WriteLocalAllRules( this->ConvertToOutputFormat(cmakefileName, cmOutputConverter::SHELL), " 1"); commands.push_back(std::move(runRule)); +#if !defined(CMAKE_BOOTSTRAP) && !defined(_WIN32) + addInstrumentationCommand(this->GetCMakeInstance()->GetInstrumentation(), + commands); +#endif } this->CreateCDCommand(commands, this->GetBinaryDirectory(), this->GetCurrentBinaryDirectory()); diff --git a/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake b/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake index f98bb32..c270b71 100644 --- a/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake +++ b/Tests/RunCMake/Instrumentation/RunCMakeTest.cmake @@ -117,8 +117,8 @@ instrument(cmake-command-parallel-install CHECK_SCRIPT check-data-dir.cmake) # FIXME(#26668) This does not work on Windows -if (UNIX AND ${RunCMake_GENERATOR} MATCHES "^Ninja") - instrument(cmake-command-ninja NO_WARN +if (UNIX) + instrument(cmake-command-make-program NO_WARN BUILD_MAKE_PROGRAM - CHECK_SCRIPT check-ninja-hooks.cmake) + CHECK_SCRIPT check-make-program-hooks.cmake) endif() diff --git a/Tests/RunCMake/Instrumentation/check-make-program-hooks.cmake b/Tests/RunCMake/Instrumentation/check-make-program-hooks.cmake new file mode 100644 index 0000000..60b2f7b --- /dev/null +++ b/Tests/RunCMake/Instrumentation/check-make-program-hooks.cmake @@ -0,0 +1,35 @@ +set(NUM_TRIES 30) +set(DELAY 1) + +if (NOT EXISTS ${v1}/preBuild.hook) + set(RunCMake_TEST_FAILED "preBuild hook did not run\n") +endif() + +macro(hasPostBuildArtifacts) + if (NOT postBuildRan AND EXISTS ${v1}/postBuild.hook) + set(postBuildRan 1) + endif() + if (NOT dataDirClean) + file(GLOB snippets "${v1}/data/*") + if ("${snippets}" STREQUAL "") + set(dataDirClean 1) + endif() + endif() +endmacro() + +set(postBuildRan 0) +set(dataDirClean 0) +foreach(_ RANGE ${NUM_TRIES}) + hasPostBuildArtifacts() + if (postBuildRan AND dataDirClean) + break() + endif() + execute_process(COMMAND ${CMAKE_COMMAND} -E sleep ${DELAY}) +endforeach() + +if (NOT postBuildRan) + string(APPEND RunCMake_TEST_FAILED "postBuild hook did not run\n") +endif() +if (NOT dataDirClean) + string(APPEND RunCMake_TEST_FAILED "Snippet files not fully removed post build\n") +endif() diff --git a/Tests/RunCMake/Instrumentation/check-ninja-hooks.cmake b/Tests/RunCMake/Instrumentation/check-ninja-hooks.cmake deleted file mode 100644 index 60b2f7b..0000000 --- a/Tests/RunCMake/Instrumentation/check-ninja-hooks.cmake +++ /dev/null @@ -1,35 +0,0 @@ -set(NUM_TRIES 30) -set(DELAY 1) - -if (NOT EXISTS ${v1}/preBuild.hook) - set(RunCMake_TEST_FAILED "preBuild hook did not run\n") -endif() - -macro(hasPostBuildArtifacts) - if (NOT postBuildRan AND EXISTS ${v1}/postBuild.hook) - set(postBuildRan 1) - endif() - if (NOT dataDirClean) - file(GLOB snippets "${v1}/data/*") - if ("${snippets}" STREQUAL "") - set(dataDirClean 1) - endif() - endif() -endmacro() - -set(postBuildRan 0) -set(dataDirClean 0) -foreach(_ RANGE ${NUM_TRIES}) - hasPostBuildArtifacts() - if (postBuildRan AND dataDirClean) - break() - endif() - execute_process(COMMAND ${CMAKE_COMMAND} -E sleep ${DELAY}) -endforeach() - -if (NOT postBuildRan) - string(APPEND RunCMake_TEST_FAILED "postBuild hook did not run\n") -endif() -if (NOT dataDirClean) - string(APPEND RunCMake_TEST_FAILED "Snippet files not fully removed post build\n") -endif() diff --git a/Tests/RunCMake/Instrumentation/project/CMakeLists.txt b/Tests/RunCMake/Instrumentation/project/CMakeLists.txt index 30b28c6..0d90faf 100644 --- a/Tests/RunCMake/Instrumentation/project/CMakeLists.txt +++ b/Tests/RunCMake/Instrumentation/project/CMakeLists.txt @@ -13,6 +13,7 @@ add_custom_command( COMMAND ${CMAKE_COMMAND} -E true OUTPUT output1 output2 ) +set_property(SOURCE output1 output2 PROPERTY SYMBOLIC 1) add_custom_target(customTarget ALL DEPENDS output1 ) diff --git a/Tests/RunCMake/Instrumentation/query/cmake-command-make-program.cmake b/Tests/RunCMake/Instrumentation/query/cmake-command-make-program.cmake new file mode 100644 index 0000000..807d991 --- /dev/null +++ b/Tests/RunCMake/Instrumentation/query/cmake-command-make-program.cmake @@ -0,0 +1,7 @@ +file(TO_CMAKE_PATH "${CMAKE_SOURCE_DIR}/../hook.cmake" hook_path) +cmake_instrumentation( + API_VERSION 1 + DATA_VERSION 1 + HOOKS preBuild postBuild + CALLBACK "\"${CMAKE_COMMAND}\" -P \"${hook_path}\" 0" +) diff --git a/Tests/RunCMake/Instrumentation/query/cmake-command-ninja.cmake b/Tests/RunCMake/Instrumentation/query/cmake-command-ninja.cmake deleted file mode 100644 index 60acebd..0000000 --- a/Tests/RunCMake/Instrumentation/query/cmake-command-ninja.cmake +++ /dev/null @@ -1,6 +0,0 @@ -cmake_instrumentation( - API_VERSION 1 - DATA_VERSION 1 - HOOKS preBuild postBuild - CALLBACK "\"${CMAKE_COMMAND}\" -P \"${CMAKE_SOURCE_DIR}/../hook.cmake\" 0" -) -- cgit v0.12