From dcf146c34d887f2190298e6f8f014250bd1e82dc Mon Sep 17 00:00:00 2001 From: Martin Duffy Date: Fri, 29 May 2026 11:39:26 -0400 Subject: [PATCH] instrumentation: Revise Data Version format Give data version the new format ., so that the version can be incremented with the introduction of new features without bumping the major version. Add support for loading instrumentation JSON queries of unknown data versions without error. Issue: #27833 --- Help/command/cmake_instrumentation.rst | 16 ++-- Help/manual/cmake-instrumentation.7.rst | 43 ++++++++--- .../instrumentation/index-v1-schema.json | 20 ++++- .../instrumentation/query-v1-schema.json | 25 ++++++- Source/cmInstrumentation.cxx | 49 ++++++++----- Source/cmInstrumentation.h | 10 ++- Source/cmInstrumentationCommand.cxx | 49 ++++++++++++- Source/cmInstrumentationQuery.cxx | 73 +++++++++++++++++-- Source/cmInstrumentationQuery.h | 18 ++++- .../Instrumentation/RunCMakeTest.cmake | 13 ++++ .../bad-version-object-result.txt | 1 + .../bad-version-object-stderr.txt | 5 ++ .../check-custom-content.cmake | 5 ++ .../check-hooks-invalid-version-ignored.cmake | 4 + .../cmake-command-bad-data-version-stderr.txt | 3 +- .../cmake-command-missing-version-stderr.txt | 4 +- ...ommand-unsupported-data-version-result.txt | 1 + ...ommand-unsupported-data-version-stderr.txt | 6 ++ Tests/RunCMake/Instrumentation/hook.cmake | 16 ++-- .../query/bad-version-major.json.in | 3 + .../query/bad-version-minor.json.in | 6 ++ .../query/bad-version-object.json.in | 6 ++ .../Instrumentation/query/bad-version.json.in | 5 +- .../Instrumentation/query/both-query.json.in | 5 +- ...ake-command-unsupported-data-version.cmake | 4 + .../Instrumentation/query/cmake-command.cmake | 2 +- .../query/generated/query-0.json.in | 6 +- .../query/generated/query-1.json.in | 6 +- .../query/generated/query-2.json.in | 6 +- .../Instrumentation/query/hooks-2.json.in | 2 +- .../hooks-invalid-version-ignored.json.in | 8 ++ .../Instrumentation/verify-snippet.cmake | 7 +- 32 files changed, 348 insertions(+), 79 deletions(-) create mode 100644 Tests/RunCMake/Instrumentation/bad-version-object-result.txt create mode 100644 Tests/RunCMake/Instrumentation/bad-version-object-stderr.txt create mode 100644 Tests/RunCMake/Instrumentation/check-hooks-invalid-version-ignored.cmake create mode 100644 Tests/RunCMake/Instrumentation/cmake-command-unsupported-data-version-result.txt create mode 100644 Tests/RunCMake/Instrumentation/cmake-command-unsupported-data-version-stderr.txt create mode 100644 Tests/RunCMake/Instrumentation/query/bad-version-major.json.in create mode 100644 Tests/RunCMake/Instrumentation/query/bad-version-minor.json.in create mode 100644 Tests/RunCMake/Instrumentation/query/bad-version-object.json.in create mode 100644 Tests/RunCMake/Instrumentation/query/cmake-command-unsupported-data-version.cmake create mode 100644 Tests/RunCMake/Instrumentation/query/hooks-invalid-version-ignored.json.in diff --git a/Help/command/cmake_instrumentation.rst b/Help/command/cmake_instrumentation.rst index 3cd7febacc..9f555035ae 100644 --- a/Help/command/cmake_instrumentation.rst +++ b/Help/command/cmake_instrumentation.rst @@ -19,10 +19,14 @@ This allows for configuring instrumentation at the project-level. [CUSTOM_CONTENT ] ) -The ``API_VERSION`` and ``DATA_VERSION`` must always be given. Currently, the -only supported value for both fields is 1. See -:ref:`cmake-instrumentation API v1` for details of the ``API_VERSION`` and -:ref:`cmake-instrumentation Data Version` for details of the ``DATA_VERSION``. +The ``API_VERSION`` and ``DATA_VERSION`` must always be given. + +``API_VERSION`` is an integer. Currently, the only supported value is ``1``. +See :ref:`cmake-instrumentation API v1` for details. + +``DATA_VERSION`` is a version value of the form ``major`` or ``major.minor``. +Currently, the only supported version is ``1.0``. See +:ref:`cmake-instrumentation Data Version` for details. Each of the optional keywords ``HOOKS``, ``OPTIONS``, and ``CALLBACK`` correspond to one of the parameters to the :ref:`cmake-instrumentation v1 Query Files`. @@ -70,7 +74,7 @@ equivalent JSON query file. cmake_instrumentation( API_VERSION 1 - DATA_VERSION 1 + DATA_VERSION 1.0 HOOKS postGenerate preCMakeBuild postCMakeBuild OPTIONS staticSystemInformation dynamicSystemInformation trace CALLBACK ${CMAKE_COMMAND} -P /path/to/handle_data.cmake @@ -91,7 +95,7 @@ equivalent JSON query file. "staticSystemInformation", "dynamicSystemInformation", "trace" ], "callbacks": [ - "/path/to/cmake -P /path/to/handle_data.cmake" + "/path/to/cmake -P /path/to/handle_data.cmake", "/path/to/cmake -P /path/to/handle_data_2.cmake" ] } diff --git a/Help/manual/cmake-instrumentation.7.rst b/Help/manual/cmake-instrumentation.7.rst index 583eb710ec..d0612c3875 100644 --- a/Help/manual/cmake-instrumentation.7.rst +++ b/Help/manual/cmake-instrumentation.7.rst @@ -229,9 +229,11 @@ request a specific Data Version, and `v1 Data Files`_ of the corresponding version will be generated and sent to the user `Callbacks`_ defined in that query. -Currently, the only supported version is ``1``. A new version number will be -created whenever previously included data is removed or reformatted such that -scripts written to parse this data may become incompatible with the new format. +Currently, the only supported version is ``1.0``. A new major version number +will be created whenever previously included data is removed or reformatted such +that scripts written to parse this data may become incompatible with the new +format. A new minor version number will be created whenever new data becomes +available. .. _`cmake-instrumentation v1 Query Files`: @@ -245,8 +247,17 @@ These files must contain a JSON object with the following keys. The ``version`` key is required, but all other fields are optional. ``version`` - The `Data Version`_ of snippet file to generate, an integer. Currently the only - supported version is ``1``. + The `Data Version`_ of snippet files to generate. + + In query files, this may be specified either as an integer major version or + as an object with ``major`` and ``minor`` members. For example, ``1`` and + ``{ "major": 1, "minor": 0 }`` both request version ``1.0``. Specifying a + minor version is optional. CMake will always generate instrumentation data + for the most recent minor version, even if an earlier minor version is + requested. + + Currently, the only supported version is ``1.0``. Query files with an unknown + data version will be ignored. ``callbacks`` A list of command-line strings for `Callbacks`_ to handle collected @@ -402,8 +413,8 @@ Snippet files have a filename with the syntax ``--.json`` and contain the following data: ``version`` - The `Data Version`_ of the snippet file, an integer. Currently the version is - always ``1``. + The `Data Version`_ of the snippet file. Currently the version is + always ``{ "major": 1, "minor": 0 }``. ``command`` The full command executed. Excluded when ``role`` is ``build``. @@ -507,7 +518,10 @@ Example: .. code-block:: json { - "version": 1, + "version": { + "major": 1, + "minor": 0 + }, "command" : "\"/usr/bin/c++\" \"-MD\" \"-MT\" \"CMakeFiles/main.dir/main.cxx.o\" \"-MF\" \"CMakeFiles/main.dir/main.cxx.o.d\" \"-o\" \"CMakeFiles/main.dir/main.cxx.o\" \"-c\" \"/main.cxx\"", "role" : "compile", "result" : 1, @@ -538,8 +552,8 @@ generated whenever `Indexing`_ occurs and deleted after any user-specified `Callbacks`_ are executed. ``version`` - The `Data Version`_ of the index file, an integer. Currently the version is - always ``1``. + The `Data Version`_ of the index file. Currently this is always written as: + ``{ "major": 1, "minor": 0 }``. ``buildDir`` The build directory of the CMake project. @@ -604,7 +618,10 @@ Example: .. code-block:: json { - "version": 1, + "version": { + "major": 1, + "minor": 0 + }, "hook": "manual", "buildDir": "", "dataDir": "/.cmake/instrumentation/v1/data", @@ -637,6 +654,10 @@ corresponding to the CMake invocation responsible for generating its command. Each CMake content file contains the following: + ``version`` + The `Data Version`_ of the content file. Currently the version is + always ``{ "major": 1, "minor": 0 }``. + ``project`` The value of :variable:`CMAKE_PROJECT_NAME`. diff --git a/Help/manual/instrumentation/index-v1-schema.json b/Help/manual/instrumentation/index-v1-schema.json index 6b59bb62d7..5f8ec57d08 100644 --- a/Help/manual/instrumentation/index-v1-schema.json +++ b/Help/manual/instrumentation/index-v1-schema.json @@ -34,11 +34,23 @@ }, "properties": { "version": { - "type": "integer", "description": "The data version of the index file.", - "enum": [ - 1 - ] + "type": "object", + "required": [ + "major", + "minor" + ], + "properties": { + "major": { + "type": "integer", + "const": 1 + }, + "minor": { + "type": "integer", + "const": 0 + } + }, + "additionalProperties": false }, "buildDir": { "type": "string", diff --git a/Help/manual/instrumentation/query-v1-schema.json b/Help/manual/instrumentation/query-v1-schema.json index 23d4554fb9..9dba21599f 100644 --- a/Help/manual/instrumentation/query-v1-schema.json +++ b/Help/manual/instrumentation/query-v1-schema.json @@ -4,10 +4,29 @@ "required": ["version"], "properties": { "version": { - "type": "integer", "description": "The data version of snippet file to generate.", - "enum": [ - 1 + "oneOf": [ + { + "type": "integer", + "const": 1 + }, + { + "type": "object", + "required": [ + "major" + ], + "properties": { + "major": { + "type": "integer", + "const": 1 + }, + "minor": { + "type": "integer", + "const": 0 + } + }, + "additionalProperties": false + } ] }, "callbacks": { diff --git a/Source/cmInstrumentation.cxx b/Source/cmInstrumentation.cxx index ea32424598..fc5e99d7ed 100644 --- a/Source/cmInstrumentation.cxx +++ b/Source/cmInstrumentation.cxx @@ -44,6 +44,11 @@ using LoadQueriesAfter = cmInstrumentation::LoadQueriesAfter; +namespace { +cmInstrumentationQuery::Version latestDataVersion = + cmInstrumentationQuery::LatestDataVersion(); +} + std::map cmInstrumentation::cdashSnippetsMap = { { "configure", @@ -145,7 +150,7 @@ void cmInstrumentation::CheckCDashVariable() options_.insert(cmInstrumentationQuery::Option::CDashVerbose); } std::set hooks_; - this->WriteJSONQuery(options_, hooks_, {}); + this->WriteJSONQuery(latestDataVersion, options_, hooks_, {}); } } @@ -198,12 +203,12 @@ bool cmInstrumentation::HasErrors() const } void cmInstrumentation::WriteJSONQuery( + cmInstrumentationQuery::Version dataVersion, std::set const& options_, std::set const& hooks_, std::vector> const& callbacks_) { Json::Value root; - root["version"] = 1; root["options"] = Json::arrayValue; for (auto const& option : options_) { root["options"].append(cmInstrumentationQuery::OptionString[option]); @@ -217,7 +222,7 @@ void cmInstrumentation::WriteJSONQuery( root["callbacks"].append(cmInstrumentation::GetCommandStr(callback)); } this->WriteInstrumentationJson( - root, "query/generated", + dataVersion, root, "query/generated", cmStrCat("query-", this->writtenJsonQueries++, ".json")); } @@ -236,7 +241,7 @@ void cmInstrumentation::WriteCMakeContent( root["project"] = gg->GetCMakeInstance()->GetCacheDefinition("CMAKE_PROJECT_NAME").GetCStr(); this->WriteInstrumentationJson( - root, "data/content", + latestDataVersion, root, "data/content", cmStrCat("cmake-", this->ComputeSuffixTime(), ".json")); } @@ -362,6 +367,8 @@ int cmInstrumentation::CollectTimingData(cmInstrumentationQuery::Hook hook) cmsys::Directory d; std::string last_index_name = this->GetFileByTimestamp(LatestOrOldest::Latest, "index", index_name); + std::string last_index_path = + cmStrCat(this->dataDir, "/index/", last_index_name); if (d.Load(this->dataDir)) { for (unsigned int i = 0; i < d.GetNumberOfFiles(); i++) { std::string fpath = d.GetFilePath(i); @@ -379,18 +386,16 @@ int cmInstrumentation::CollectTimingData(cmInstrumentationQuery::Hook hook) index["hook"] = cmInstrumentationQuery::HookString[hook]; index["dataDir"] = this->dataDir; index["buildDir"] = this->binaryDir; - index["version"] = 1; if (this->HasOption( cmInstrumentationQuery::Option::StaticSystemInformation)) { this->InsertStaticSystemInformation(index); } + for (auto const& file : files) { if (last_index_name.empty()) { index["snippets"].append(file.first); } else { int compare; - std::string last_index_path = - cmStrCat(this->dataDir, "/index/", last_index_name); cmSystemTools::FileTimeCompare(file.second, last_index_path, &compare); if (compare == 1) { index["snippets"].append(file.first); @@ -406,13 +411,14 @@ int cmInstrumentation::CollectTimingData(cmInstrumentationQuery::Hook hook) } // Write index file - this->WriteInstrumentationJson(index, "data/index", index_name); + this->WriteInstrumentationJson(latestDataVersion, index, "data/index", + index_name); // Execute callbacks - for (auto& cb : this->callbacks) { - cmSystemTools::RunSingleCommand(cmStrCat(cb, " \"", index_path, '"'), - nullptr, nullptr, nullptr, nullptr, - cmSystemTools::OUTPUT_PASSTHROUGH); + for (auto const& cb : this->callbacks) { + cmSystemTools::RunSingleCommand( + cmStrCat(cb.Command, " \"", index_path, '"'), nullptr, nullptr, nullptr, + nullptr, cmSystemTools::OUTPUT_PASSTHROUGH); } // Special case for CDash collation @@ -545,10 +551,14 @@ Json::Value cmInstrumentation::ReadJsonSnippet(std::string const& file_name) return snippetData; } -void cmInstrumentation::WriteInstrumentationJson(Json::Value& root, - std::string const& subdir, - std::string const& file_name) +void cmInstrumentation::WriteInstrumentationJson( + cmInstrumentationQuery::Version version, Json::Value& root, + std::string const& subdir, std::string const& file_name) { + root["version"] = Json::objectValue; + root["version"]["major"] = version.Major; + root["version"]["minor"] = version.Minor; + Json::StreamWriterBuilder wbuilder; wbuilder["indentation"] = "\t"; std::unique_ptr JsonWriter = @@ -581,7 +591,6 @@ std::string cmInstrumentation::InstrumentTest( // Store command info Json::Value root(this->preTestStats); std::string command_str = cmStrCat(command, ' ', GetCommandStr(args)); - root["version"] = 1; root["command"] = command_str; root["role"] = "test"; root["testName"] = name; @@ -603,7 +612,7 @@ std::string cmInstrumentation::InstrumentTest( "test-", this->ComputeSuffixHash(cmStrCat(command_str, info.GetProcessId())), '-', this->ComputeSuffixTime(endTime), ".json"); - this->WriteInstrumentationJson(root, "data", file_name); + this->WriteInstrumentationJson(latestDataVersion, root, "data", file_name); return file_name; } @@ -637,7 +646,6 @@ int cmInstrumentation::InstrumentCommand( if (!command_str.empty()) { root["command"] = command_str; } - root["version"] = 1; // Pre-Command auto steady_start = std::chrono::steady_clock::now(); @@ -759,11 +767,12 @@ int cmInstrumentation::InstrumentCommand( } else { addCMakeContent(it->second); } - this->WriteInstrumentationJson(it->second, "data", it->first); + this->WriteInstrumentationJson(latestDataVersion, it->second, "data", + it->first); } this->configureSnippetData.clear(); } - this->WriteInstrumentationJson(root, "data", file_name); + this->WriteInstrumentationJson(latestDataVersion, root, "data", file_name); } return ret; } diff --git a/Source/cmInstrumentation.h b/Source/cmInstrumentation.h index c99a221274..6c67e20b9b 100644 --- a/Source/cmInstrumentation.h +++ b/Source/cmInstrumentation.h @@ -31,6 +31,8 @@ class cmGlobalGenerator; class cmInstrumentation { public: + using Callback = cmInstrumentationQuery::Callback; + enum class LoadQueriesAfter { Yes, @@ -60,7 +62,8 @@ public: bool HasHook(cmInstrumentationQuery::Hook hook) const; bool ReadJSONQueries(std::string const& directory); void ReadJSONQuery(std::string const& file); - void WriteJSONQuery(std::set const& options, + void WriteJSONQuery(cmInstrumentationQuery::Version dataVersion, + std::set const& options, std::set const& hooks, std::vector> const& callback); void AddCustomContent(std::string const& name, Json::Value const& contents); @@ -90,7 +93,8 @@ private: Json::Value ReadJsonSnippet(std::string const& file_name); bool AcquireLock(std::string const& lock_file, cmFileLock& lock, unsigned long timeout); - void WriteInstrumentationJson(Json::Value& index, + void WriteInstrumentationJson(cmInstrumentationQuery::Version version, + Json::Value& index, std::string const& directory, std::string const& file_name); void InsertStaticSystemInformation(Json::Value& index); @@ -122,7 +126,7 @@ private: std::string dataDir; std::set options; std::set hooks; - std::vector callbacks; + std::vector callbacks; std::vector queryFiles; static std::map cdashSnippetsMap; Json::Value preTestStats; diff --git a/Source/cmInstrumentationCommand.cxx b/Source/cmInstrumentationCommand.cxx index e48b3c4479..5b0bb2d16b 100644 --- a/Source/cmInstrumentationCommand.cxx +++ b/Source/cmInstrumentationCommand.cxx @@ -29,6 +29,8 @@ file LICENSE.rst or https://cmake.org/licensing for details. */ namespace { +using Version = cmInstrumentationQuery::Version; + bool validateVersion(std::string const& key, std::string const& versionString, int& version, cmExecutionStatus& status) { @@ -47,6 +49,45 @@ bool validateVersion(std::string const& key, std::string const& versionString, return true; } +bool validateDataVersion(std::string const& versionString, Version& version, + cmExecutionStatus& status) +{ + char const* vStart = versionString.c_str(); + if (!std::all_of(versionString.begin(), versionString.end(), [](char c) { + return cmsysString_isdigit(c) || c == '.'; + })) { + status.SetError( + cmStrCat("given a malformed DATA_VERSION \"", versionString, + "\". A numeric major or major.minor version is required.")); + + return false; + } + + version.Major = std::atoi(vStart); + version.Minor = 0; + std::string::size_type pos = versionString.find('.'); + if (pos != std::string::npos) { + vStart += pos + 1; + version.Minor = std::atoi(vStart); + } + + if (version.Major < 1 || version.Minor < 0) { + status.SetError( + cmStrCat("given a malformed DATA_VERSION \"", versionString, + "\". A numeric major or major.minor version is required.")); + return false; + } + + if (!cmInstrumentationQuery::ValidDataVersion(version)) { + status.SetError( + cmStrCat("given an unsupported DATA_VERSION \"", versionString, + "\" (the only currently supported version is 1.0).")); + return false; + } + + return true; +} + template std::function EnumParser( std::vector const toString) @@ -110,11 +151,10 @@ bool cmInstrumentationCommand(std::vector const& args, return false; } int apiVersion; - int dataVersion; + Version dataVersion; if (!validateVersion("API_VERSION", arguments.ApiVersion, apiVersion, status) || - !validateVersion("DATA_VERSION", arguments.DataVersion, dataVersion, - status)) { + !validateDataVersion(arguments.DataVersion, dataVersion, status)) { return false; } @@ -182,7 +222,8 @@ bool cmInstrumentationCommand(std::vector const& args, } // Write query file - instrumentation->WriteJSONQuery(options, hooks, arguments.Callbacks); + instrumentation->WriteJSONQuery(dataVersion, options, hooks, + arguments.Callbacks); return true; } diff --git a/Source/cmInstrumentationQuery.cxx b/Source/cmInstrumentationQuery.cxx index 6876870fed..fac5c2c0c9 100644 --- a/Source/cmInstrumentationQuery.cxx +++ b/Source/cmInstrumentationQuery.cxx @@ -3,7 +3,6 @@ #include #include #include -#include #include #include #include @@ -47,6 +46,7 @@ JsonErrors::ErrorGenerator InvalidRootQueryObject( }; using JSONHelperBuilder = cmJSONHelperBuilder; +using Version = cmInstrumentationQuery::Version; template static std::function EnumHelper( @@ -67,7 +67,7 @@ static std::function EnumHelper( } static auto const OptionHelper = EnumHelper( cmInstrumentationQuery::OptionString, "option"); -static auto const QueryListHelper = +static auto const OptionListHelper = JSONHelperBuilder::Vector( ErrorMessages::InvalidArray, OptionHelper); static auto const HookHelper = EnumHelper( @@ -78,7 +78,36 @@ static auto const HookListHelper = static auto const CallbackHelper = JSONHelperBuilder::String(); static auto const CallbackListHelper = JSONHelperBuilder::Vector( ErrorMessages::InvalidArray, CallbackHelper); -static auto const VersionHelper = JSONHelperBuilder::Int(); + +JsonErrors::ErrorGenerator InvalidVersionObject( + JsonErrors::ObjectError errorType, Json::Value::Members const& extraFields) +{ + return JsonErrors::INVALID_NAMED_OBJECT( + [](Json::Value const*, cmJSONState*) -> std::string { + return "version object"; + })(errorType, extraFields); +} + +static auto const VersionObjectHelper = + JSONHelperBuilder::Object(InvalidVersionObject, false) + .Bind("major"_s, &Version::Major, JSONHelperBuilder::Int(), true) + .Bind("minor"_s, &Version::Minor, JSONHelperBuilder::Int(), false); + +bool VersionHelper(Version& out, Json::Value const* value, cmJSONState* state) +{ + out.Minor = 0; + if (value->isInt()) { + out.Major = value->asInt(); + } else if (value->isObject()) { + if (!VersionObjectHelper(out, value, state)) { + return false; + } + } else { + state->AddErrorAtValue("Version must be an integer or object", value); + return false; + } + return true; +} using QueryRoot = cmInstrumentationQuery::QueryJSONRoot; @@ -86,22 +115,35 @@ static auto const QueryRootHelper = JSONHelperBuilder::Object(ErrorMessages::InvalidRootQueryObject, false) .Bind("version"_s, &QueryRoot::version, VersionHelper, true) - .Bind("options"_s, &QueryRoot::options, QueryListHelper, false) + .Bind("options"_s, &QueryRoot::options, OptionListHelper, false) .Bind("hooks"_s, &QueryRoot::hooks, HookListHelper, false) .Bind("callbacks"_s, &QueryRoot::callbacks, CallbackListHelper, false); +static auto const QueryRootVersionOnlyHelper = + JSONHelperBuilder::Object(ErrorMessages::InvalidRootQueryObject, + true) + .Bind("version"_s, &QueryRoot::version, VersionHelper, true); + bool cmInstrumentationQuery::ReadJSON(std::string const& filename, std::string& errorMessage, std::set