Merge topic 'ninja-multi-cmake-build-no-config'

bd4ae2af0f Help: Make note of ninja -f argument in Ninja Multi-Config docs
79e5b3c46a Help: Explain new behavior of cmake --build in Ninja Multi-Config
16a4ba5b31 Ninja Multi-Config: Use build.ninja if cmake --build has no --config
2ac835b9f9 Refactor: Allow generators to decide default configuration for build

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !4323

Help/generator/Ninja Multi-Config.rst

@@ -11,10 +11,16 @@ 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-<Config>.ninja`` file will be generated for each of these
-configurations (with ``<Config>`` being the configuration name.) No
+configurations (with ``<Config>`` being the configuration name.) These files
+are intended to be run with ``ninja -f build-<Config>.ninja``. No
 ``build.ninja`` file is generated by default (see below for how to generate
 it.)
 
+``cmake --build . --config <Config>`` will always use ``build-<Config>.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-<Config>.ninja`` file contains ``<target>`` targets as well as
 ``<target>:<Config>`` targets, where ``<Config>`` is the same as the
 configuration specified in ``build-<Config>.ninja`` Additionally, if

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<GeneratedMakeCommand> 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<GeneratedMakeCommand> 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";

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;
 

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<std::string> configsVec;
+  cmExpandList(state.GetSafeCacheEntryValue("CMAKE_CONFIGURATION_TYPES"),
+               configsVec);
+  if (configsVec.empty()) {
+    configsVec.emplace_back();
+  }
   std::set<std::string> 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<std::string> 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;
   }

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;

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
 {

Source/cmState.h

@@ -88,6 +88,7 @@ public:
 
   std::vector<std::string> 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);

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,

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<std::string> targets;
-  std::string config = "Debug";
+  std::string config;
   std::string dir;
   std::vector<std::string> nativeOptions;
   bool cleanFirst = false;

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)