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