From 2ac835b9f9a7a4dec16c498a51c0dbc3d64844bc Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Tue, 4 Feb 2020 14:12:01 -0500 Subject: Refactor: Allow generators to decide default configuration for build And allow them to read any cache values they need. --- Source/cmGlobalGenerator.cxx | 8 ++++++-- Source/cmGlobalGenerator.h | 10 ++++++++++ Source/cmake.cxx | 4 ++++ Source/cmakemain.cxx | 2 +- 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx index 1f084f5..7ddeb99 100644 --- a/Source/cmGlobalGenerator.cxx +++ b/Source/cmGlobalGenerator.cxx @@ -1878,6 +1878,10 @@ int cmGlobalGenerator::Build( output += "\n"; return 1; } + std::string realConfig = config; + if (realConfig.empty()) { + realConfig = this->GetDefaultBuildConfig(); + } int retVal = 0; cmSystemTools::SetRunCommandHideConsole(true); @@ -1886,7 +1890,7 @@ int cmGlobalGenerator::Build( std::vector makeCommand = this->GenerateBuildCommand(makeCommandCSTR, projectName, bindir, targets, - config, fast, jobs, verbose, nativeOptions); + realConfig, fast, jobs, verbose, nativeOptions); // Workaround to convince some commands to produce output. if (outputflag == cmSystemTools::OUTPUT_PASSTHROUGH && @@ -1898,7 +1902,7 @@ int cmGlobalGenerator::Build( if (clean) { std::vector cleanCommand = this->GenerateBuildCommand(makeCommandCSTR, projectName, bindir, - { "clean" }, config, fast, jobs, verbose); + { "clean" }, realConfig, fast, jobs, verbose); output += "\nRun Clean Command:"; output += cleanCommand.front().Printable(); output += "\n"; diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h index f6ed10f..e6ab1dd 100644 --- a/Source/cmGlobalGenerator.h +++ b/Source/cmGlobalGenerator.h @@ -44,6 +44,7 @@ class cmLocalGenerator; class cmMakefile; class cmOutputConverter; class cmSourceFile; +class cmState; class cmStateDirectory; class cmake; @@ -135,6 +136,12 @@ public: virtual bool SetGeneratorToolset(std::string const& ts, bool build, cmMakefile* mf); + /** Read any other cache entries needed for cmake --build. */ + virtual bool ReadCacheEntriesForBuild(const cmState& /*state*/) + { + return true; + } + /** * Create LocalGenerators and process the CMakeLists files. This does not * actually produce any makefiles, DSPs, etc. @@ -382,6 +389,9 @@ public: // Lookup edit_cache target command preferred by this generator. virtual std::string GetEditCacheCommand() const { return ""; } + // Default config to use for cmake --build + virtual std::string GetDefaultBuildConfig() const { return "Debug"; } + // Class to track a set of dependencies. using TargetDependSet = cmTargetDependSet; diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 5fa40d5..f4b9f16 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -2767,6 +2767,10 @@ int cmake::Build(int jobs, const std::string& dir, } #endif + if (!this->GlobalGenerator->ReadCacheEntriesForBuild(*this->State)) { + return 1; + } + this->GlobalGenerator->PrintBuildCommandAdvice(std::cerr, jobs); return this->GlobalGenerator->Build( jobs, "", dir, projName, targets, output, "", config, clean, false, diff --git a/Source/cmakemain.cxx b/Source/cmakemain.cxx index 5579ae1..494d5d9 100644 --- a/Source/cmakemain.cxx +++ b/Source/cmakemain.cxx @@ -350,7 +350,7 @@ int do_build(int ac, char const* const* av) #else int jobs = cmake::NO_BUILD_PARALLEL_LEVEL; std::vector targets; - std::string config = "Debug"; + std::string config; std::string dir; std::vector nativeOptions; bool cleanFirst = false; -- cgit v0.12 From 16a4ba5b311669000d83c99b1b985597205d3d69 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Tue, 4 Feb 2020 11:06:46 -0500 Subject: Ninja Multi-Config: Use build.ninja if cmake --build has no --config If cmake --build is called with no --config argument, and a build.ninja file is available, use that instead of defaulting to the Debug config. --- Source/cmGlobalNinjaGenerator.cxx | 41 ++++++++++++++++------ Source/cmGlobalNinjaGenerator.h | 5 +++ Source/cmState.cxx | 10 ++++++ Source/cmState.h | 1 + Tests/RunCMake/NinjaMultiConfig/RunCMakeTest.cmake | 10 ++++-- ...ultBuildAliasList-all-configs-build-check.cmake | 39 ++++++++++++++++++++ ...ultBuildAliasList-all-configs-ninja-check.cmake | 39 -------------------- 7 files changed, 93 insertions(+), 52 deletions(-) create mode 100644 Tests/RunCMake/NinjaMultiConfig/SimpleDefaultBuildAliasList-all-configs-build-check.cmake delete mode 100644 Tests/RunCMake/NinjaMultiConfig/SimpleDefaultBuildAliasList-all-configs-ninja-check.cmake diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index f7694ff..ff1e5d6 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -800,8 +800,7 @@ cmGlobalNinjaGenerator::GenerateBuildCommand( makeCommand.Add("-j", std::to_string(jobs)); } - this->AppendNinjaFileArgument(makeCommand, - config.empty() ? "Debug" : config); + this->AppendNinjaFileArgument(makeCommand, config); makeCommand.Add(makeOptions.begin(), makeOptions.end()); for (const auto& tname : targetNames) { @@ -2559,8 +2558,10 @@ void cmGlobalNinjaMultiGenerator::CloseBuildFileStreams() void cmGlobalNinjaMultiGenerator::AppendNinjaFileArgument( GeneratedMakeCommand& command, const std::string& config) const { - command.Add("-f"); - command.Add(GetNinjaConfigFilename(config)); + if (!config.empty()) { + command.Add("-f"); + command.Add(GetNinjaConfigFilename(config)); + } } std::string cmGlobalNinjaMultiGenerator::GetNinjaImplFilename( @@ -2601,11 +2602,30 @@ void cmGlobalNinjaMultiGenerator::GetQtAutoGenConfigs( bool cmGlobalNinjaMultiGenerator::InspectConfigTypeVariables() { - auto configsVec = this->Makefiles.front()->GetGeneratorConfigs(); + return this->ReadCacheEntriesForBuild(*this->Makefiles.front()->GetState()); +} + +std::string cmGlobalNinjaMultiGenerator::GetDefaultBuildConfig() const +{ + if (this->DefaultFileConfig.empty()) { + return "Debug"; + } + return ""; +} + +bool cmGlobalNinjaMultiGenerator::ReadCacheEntriesForBuild( + const cmState& state) +{ + std::vector configsVec; + cmExpandList(state.GetSafeCacheEntryValue("CMAKE_CONFIGURATION_TYPES"), + configsVec); + if (configsVec.empty()) { + configsVec.emplace_back(); + } std::set configs(configsVec.cbegin(), configsVec.cend()); - this->DefaultFileConfig = this->Makefiles.front()->GetSafeDefinition( - "CMAKE_NMC_DEFAULT_BUILD_FILE_CONFIG"); + this->DefaultFileConfig = + state.GetSafeCacheEntryValue("CMAKE_NMC_DEFAULT_BUILD_FILE_CONFIG"); if (!this->DefaultFileConfig.empty() && !configs.count(this->DefaultFileConfig)) { std::ostringstream msg; @@ -2618,9 +2638,8 @@ bool cmGlobalNinjaMultiGenerator::InspectConfigTypeVariables() } std::vector crossConfigsVec; - cmExpandList( - this->Makefiles.front()->GetSafeDefinition("CMAKE_NMC_CROSS_CONFIGS"), - crossConfigsVec); + cmExpandList(state.GetSafeCacheEntryValue("CMAKE_NMC_CROSS_CONFIGS"), + crossConfigsVec); auto crossConfigs = ListSubsetWithAll(configs, crossConfigsVec); if (!crossConfigs) { std::ostringstream msg; @@ -2633,7 +2652,7 @@ bool cmGlobalNinjaMultiGenerator::InspectConfigTypeVariables() this->CrossConfigs = *crossConfigs; auto defaultConfigsString = - this->Makefiles.front()->GetSafeDefinition("CMAKE_NMC_DEFAULT_CONFIGS"); + state.GetSafeCacheEntryValue("CMAKE_NMC_DEFAULT_CONFIGS"); if (defaultConfigsString.empty()) { defaultConfigsString = this->DefaultFileConfig; } diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h index 3b45249..0e53c0e 100644 --- a/Source/cmGlobalNinjaGenerator.h +++ b/Source/cmGlobalNinjaGenerator.h @@ -32,6 +32,7 @@ class cmLinkLineComputer; class cmLocalGenerator; class cmMakefile; class cmOutputConverter; +class cmState; class cmStateDirectory; class cmake; struct cmDocumentationEntry; @@ -633,6 +634,10 @@ public: bool InspectConfigTypeVariables() override; + std::string GetDefaultBuildConfig() const override; + + bool ReadCacheEntriesForBuild(const cmState& state) override; + protected: bool OpenBuildFileStreams() override; void CloseBuildFileStreams() override; diff --git a/Source/cmState.cxx b/Source/cmState.cxx index 0ce8dd7..9fc7615 100644 --- a/Source/cmState.cxx +++ b/Source/cmState.cxx @@ -141,6 +141,16 @@ const char* cmState::GetCacheEntryValue(std::string const& key) const return e->Value.c_str(); } +std::string cmState::GetSafeCacheEntryValue(std::string const& key) const +{ + std::string retval; + auto val = this->GetCacheEntryValue(key); + if (val) { + retval = val; + } + return retval; +} + const std::string* cmState::GetInitializedCacheValue( std::string const& key) const { diff --git a/Source/cmState.h b/Source/cmState.h index 817046f..6ee2b0c 100644 --- a/Source/cmState.h +++ b/Source/cmState.h @@ -88,6 +88,7 @@ public: std::vector GetCacheEntryKeys() const; const char* GetCacheEntryValue(std::string const& key) const; + std::string GetSafeCacheEntryValue(std::string const& key) const; const std::string* GetInitializedCacheValue(std::string const& key) const; cmStateEnums::CacheEntryType GetCacheEntryType(std::string const& key) const; void SetCacheEntryValue(std::string const& key, std::string const& value); diff --git a/Tests/RunCMake/NinjaMultiConfig/RunCMakeTest.cmake b/Tests/RunCMake/NinjaMultiConfig/RunCMakeTest.cmake index 7518395..2344158 100644 --- a/Tests/RunCMake/NinjaMultiConfig/RunCMakeTest.cmake +++ b/Tests/RunCMake/NinjaMultiConfig/RunCMakeTest.cmake @@ -62,7 +62,12 @@ function(run_cmake_build case suffix config) foreach(tgt IN LISTS ARGN) list(APPEND tgts --target ${tgt}) endforeach() - run_cmake_command(${case}-${suffix}-build "${CMAKE_COMMAND}" --build . --config ${config} ${tgts}) + if(config) + set(config_arg --config ${config}) + else() + set(config_arg) + endif() + run_cmake_command(${case}-${suffix}-build "${CMAKE_COMMAND}" --build . ${config_arg} ${tgts}) endfunction() function(run_ninja case suffix file) @@ -122,7 +127,8 @@ run_cmake_configure(SimpleDefaultBuildAliasList) unset(RunCMake_TEST_OPTIONS) include(${RunCMake_TEST_BINARY_DIR}/target_files.cmake) run_ninja(SimpleDefaultBuildAliasList target-configs build.ninja simpleexe) -run_ninja(SimpleDefaultBuildAliasList all-configs build.ninja all) +# IMPORTANT: This tests cmake --build . with no config using build.ninja +run_cmake_build(SimpleDefaultBuildAliasList all-configs "" all) run_ninja(SimpleDefaultBuildAliasList all-relwithdebinfo build.ninja all:RelWithDebInfo) run_ninja(SimpleDefaultBuildAliasList clean-configs build.ninja clean) diff --git a/Tests/RunCMake/NinjaMultiConfig/SimpleDefaultBuildAliasList-all-configs-build-check.cmake b/Tests/RunCMake/NinjaMultiConfig/SimpleDefaultBuildAliasList-all-configs-build-check.cmake new file mode 100644 index 0000000..8ffdd20 --- /dev/null +++ b/Tests/RunCMake/NinjaMultiConfig/SimpleDefaultBuildAliasList-all-configs-build-check.cmake @@ -0,0 +1,39 @@ +check_files("${RunCMake_TEST_BINARY_DIR}" + INCLUDE + ${GENERATED_FILES} + + ${TARGET_FILE_simpleexe_Debug} + ${TARGET_OBJECT_FILES_simpleexe_Debug} + + ${TARGET_FILE_simpleshared_Debug} + ${TARGET_LINKER_FILE_simpleshared_Debug} + ${TARGET_OBJECT_FILES_simpleshared_Debug} + + ${TARGET_FILE_simplestatic_Debug} + ${TARGET_OBJECT_FILES_simplestatic_Debug} + + ${TARGET_OBJECT_FILES_simpleobj_Debug} + + ${TARGET_FILE_simpleexe_Release} + ${TARGET_OBJECT_FILES_simpleexe_Release} + + ${TARGET_FILE_simpleshared_Release} + ${TARGET_LINKER_FILE_simpleshared_Release} + ${TARGET_OBJECT_FILES_simpleshared_Release} + + ${TARGET_FILE_simplestatic_Release} + ${TARGET_OBJECT_FILES_simplestatic_Release} + + ${TARGET_OBJECT_FILES_simpleobj_Release} + + EXCLUDE + ${TARGET_OBJECT_FILES_simpleexe_MinSizeRel} + ${TARGET_OBJECT_FILES_simpleshared_MinSizeRel} + ${TARGET_OBJECT_FILES_simplestatic_MinSizeRel} + ${TARGET_OBJECT_FILES_simpleobj_MinSizeRel} + + ${TARGET_OBJECT_FILES_simpleexe_RelWithDebInfo} + ${TARGET_OBJECT_FILES_simpleshared_RelWithDebInfo} + ${TARGET_OBJECT_FILES_simplestatic_RelWithDebInfo} + ${TARGET_OBJECT_FILES_simpleobj_RelWithDebInfo} + ) diff --git a/Tests/RunCMake/NinjaMultiConfig/SimpleDefaultBuildAliasList-all-configs-ninja-check.cmake b/Tests/RunCMake/NinjaMultiConfig/SimpleDefaultBuildAliasList-all-configs-ninja-check.cmake deleted file mode 100644 index 8ffdd20..0000000 --- a/Tests/RunCMake/NinjaMultiConfig/SimpleDefaultBuildAliasList-all-configs-ninja-check.cmake +++ /dev/null @@ -1,39 +0,0 @@ -check_files("${RunCMake_TEST_BINARY_DIR}" - INCLUDE - ${GENERATED_FILES} - - ${TARGET_FILE_simpleexe_Debug} - ${TARGET_OBJECT_FILES_simpleexe_Debug} - - ${TARGET_FILE_simpleshared_Debug} - ${TARGET_LINKER_FILE_simpleshared_Debug} - ${TARGET_OBJECT_FILES_simpleshared_Debug} - - ${TARGET_FILE_simplestatic_Debug} - ${TARGET_OBJECT_FILES_simplestatic_Debug} - - ${TARGET_OBJECT_FILES_simpleobj_Debug} - - ${TARGET_FILE_simpleexe_Release} - ${TARGET_OBJECT_FILES_simpleexe_Release} - - ${TARGET_FILE_simpleshared_Release} - ${TARGET_LINKER_FILE_simpleshared_Release} - ${TARGET_OBJECT_FILES_simpleshared_Release} - - ${TARGET_FILE_simplestatic_Release} - ${TARGET_OBJECT_FILES_simplestatic_Release} - - ${TARGET_OBJECT_FILES_simpleobj_Release} - - EXCLUDE - ${TARGET_OBJECT_FILES_simpleexe_MinSizeRel} - ${TARGET_OBJECT_FILES_simpleshared_MinSizeRel} - ${TARGET_OBJECT_FILES_simplestatic_MinSizeRel} - ${TARGET_OBJECT_FILES_simpleobj_MinSizeRel} - - ${TARGET_OBJECT_FILES_simpleexe_RelWithDebInfo} - ${TARGET_OBJECT_FILES_simpleshared_RelWithDebInfo} - ${TARGET_OBJECT_FILES_simplestatic_RelWithDebInfo} - ${TARGET_OBJECT_FILES_simpleobj_RelWithDebInfo} - ) -- cgit v0.12 From 79e5b3c46ab0fbca16b0ce9ede6fbec9bdeb2814 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Tue, 4 Feb 2020 14:21:25 -0500 Subject: Help: Explain new behavior of cmake --build in Ninja Multi-Config --- Help/generator/Ninja Multi-Config.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Help/generator/Ninja Multi-Config.rst b/Help/generator/Ninja Multi-Config.rst index 31e8ea9..030142f 100644 --- a/Help/generator/Ninja Multi-Config.rst +++ b/Help/generator/Ninja Multi-Config.rst @@ -15,6 +15,11 @@ configurations (with ```` being the configuration name.) No ``build.ninja`` file is generated by default (see below for how to generate it.) +``cmake --build . --config `` will always use ``build-.ninja`` +to build. If no ``--config`` argument is specified, ``cmake --build .`` will +default to ``build-Debug.ninja``, unless a ``build.ninja`` is generated (see +below), in which case that will be used instead. + Each ``build-.ninja`` file contains ```` targets as well as ``:`` targets, where ```` is the same as the configuration specified in ``build-.ninja`` Additionally, if -- cgit v0.12 From bd4ae2af0fe00813919c8188da7bb07d57ef3734 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Tue, 4 Feb 2020 14:17:39 -0500 Subject: Help: Make note of ninja -f argument in Ninja Multi-Config docs --- Help/generator/Ninja Multi-Config.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Help/generator/Ninja Multi-Config.rst b/Help/generator/Ninja Multi-Config.rst index 030142f..11c59f2 100644 --- a/Help/generator/Ninja Multi-Config.rst +++ b/Help/generator/Ninja Multi-Config.rst @@ -11,7 +11,8 @@ Unlike the :generator:`Ninja` generator, ``Ninja Multi-Config`` generates multiple configurations at once with :variable:`CMAKE_CONFIGURATION_TYPES` instead of only one configuration with :variable:`CMAKE_BUILD_TYPE`. One ``build-.ninja`` file will be generated for each of these -configurations (with ```` being the configuration name.) No +configurations (with ```` being the configuration name.) These files +are intended to be run with ``ninja -f build-.ninja``. No ``build.ninja`` file is generated by default (see below for how to generate it.) -- cgit v0.12