Restore "all" target in subdirectories marked EXCLUDE_FROM_ALL

The "all" target in each directory is supposed to have targets from that
directory even if the directory itself is marked `EXCLUDE_FROM_ALL` in
its parent.  This was broken by commit dc6888573d (Pass EXCLUDE_FROM_ALL
from directory to targets, 2019-01-15, v3.14.0-rc1~83^2) which made the
participation of a target in "all" independent of context.  Revert much
of the logic change from that commit to restore the old behavior.  Then
re-implement the behavior intended by the commit to keep its test
working.  Extend the test to cover the old behavior too.

Fixes: #19753

Help/prop_dir/EXCLUDE_FROM_ALL.rst

@@ -1,15 +1,13 @@
 EXCLUDE_FROM_ALL
 ----------------
 
-Exclude the directory from the all target of its parent.
+Set this directory property to a true value on a subdirectory to exclude
+its targets from the "all" target of its ancestors.  If excluded, running
+e.g. ``make`` in the parent directory will not build targets the
+subdirectory by default.  This does not affect the "all" target of the
+subdirectory itself.  Running e.g. ``make`` inside the subdirectory will
+still build its targets.
 
-A property on a directory that indicates if its targets are excluded
+If the :prop_tgt:`EXCLUDE_FROM_ALL` target property is set on a target
-from the default build target.  If it is not, then with a Makefile for
+then its value determines whether the target is included in the "all"
-example typing make will cause the targets to be built.  The same
+target of this directory and its ancestors.
-concept applies to the default build of other generators.
-
-Targets inherit the :prop_tgt:`EXCLUDE_FROM_ALL` property from the directory
-that they are created in. When a directory is excluded, all of its targets will
-have :prop_tgt:`EXCLUDE_FROM_ALL` set to ``TRUE``. After creating such a target
-you can change its :prop_tgt:`EXCLUDE_FROM_ALL` property to ``FALSE``. This
-will cause the target to be included in the default build target.

Help/prop_tgt/EXCLUDE_FROM_ALL.rst

@@ -1,12 +1,15 @@
 EXCLUDE_FROM_ALL
 ----------------
 
-Exclude the target from the all target.
+Set this target property to a true (or false) value to exclude (or include)
+the target from the "all" target of the containing directory and its
+ancestors.  If excluded, running e.g. ``make`` in the containing directory
+or its ancestors will not build the target by default.
 
-A property on a target that indicates if the target is excluded from
+If this target property is not set then the target will be included in
-the default build target.  If it is not, then with a Makefile for
+the "all" target of the containing directory.  Furthermore, it will be
-example typing make will cause this target to be built.  The same
+included in the "all" target of its ancestor directories unless the
-concept applies to the default build of other generators.
+:prop_dir:`EXCLUDE_FROM_ALL` directory property is set.
 
 With ``EXCLUDE_FROM_ALL`` set to false or not set at all, the target
 will be brought up to date as part of doing a ``make install`` or its
@@ -16,6 +19,3 @@ target has undefined behavior.  Note that such a target can still safely
 be listed in an :command:`install(TARGETS)` command as long as the install
 components the target belongs to are not part of the set of components
 that anything tries to install.
-
-This property is enabled by default for targets that are created in
-directories that have :prop_dir:`EXCLUDE_FROM_ALL` set to ``TRUE``.

Source/cmGlobalGenerator.cxx

@@ -2029,10 +2029,18 @@ bool cmGlobalGenerator::IsExcluded(cmLocalGenerator* root,
   return this->IsExcluded(rootSnp, snp);
 }
 
-bool cmGlobalGenerator::IsExcluded(cmGeneratorTarget* target) const
+bool cmGlobalGenerator::IsExcluded(cmLocalGenerator* root,
+                                   cmGeneratorTarget* target) const
 {
-  return target->GetType() == cmStateEnums::INTERFACE_LIBRARY ||
+  if (target->GetType() == cmStateEnums::INTERFACE_LIBRARY) {
-    target->GetPropertyAsBool("EXCLUDE_FROM_ALL");
+    return true;
+  }
+  if (const char* exclude = target->GetProperty("EXCLUDE_FROM_ALL")) {
+    return cmSystemTools::IsOn(exclude);
+  }
+  // This target is included in its directory.  Check whether the
+  // directory is excluded.
+  return this->IsExcluded(root, target->GetLocalGenerator());
 }
 
 void cmGlobalGenerator::GetEnabledLanguages(

Source/cmGlobalGenerator.h

@@ -522,7 +522,7 @@ protected:
   bool IsExcluded(cmStateSnapshot const& root,
                   cmStateSnapshot const& snp) const;
   bool IsExcluded(cmLocalGenerator* root, cmLocalGenerator* gen) const;
-  bool IsExcluded(cmGeneratorTarget* target) const;
+  bool IsExcluded(cmLocalGenerator* root, cmGeneratorTarget* target) const;
   virtual void InitializeProgressMarks() {}
 
   struct GlobalTargetInfo

Source/cmGlobalNinjaGenerator.h

@@ -329,9 +329,9 @@ public:
     return LocalGenerators;
   }
 
-  bool IsExcluded(cmGeneratorTarget* target)
+  bool IsExcluded(cmLocalGenerator* root, cmGeneratorTarget* target)
   {
-    return cmGlobalGenerator::IsExcluded(target);
+    return cmGlobalGenerator::IsExcluded(root, target);
   }
 
   int GetRuleCmdLength(const std::string& name) { return RuleCmdLength[name]; }

Source/cmGlobalUnixMakefileGenerator3.cxx

@@ -713,7 +713,7 @@ void cmGlobalUnixMakefileGenerator3::WriteConvenienceRules2(
                         localName, depends, commands, true);
 
       // add the all/all dependency
-      if (!this->IsExcluded(gtarget)) {
+      if (!this->IsExcluded(this->LocalGenerators[0], gtarget)) {
         depends.clear();
         depends.push_back(localName);
         commands.clear();
@@ -777,7 +777,7 @@ void cmGlobalUnixMakefileGenerator3::WriteConvenienceRules2(
                           "Pre-install relink rule for target.", localName,
                           depends, commands, true);
 
-        if (!this->IsExcluded(gtarget)) {
+        if (!this->IsExcluded(this->LocalGenerators[0], gtarget)) {
           depends.clear();
           depends.push_back(localName);
           commands.clear();

Source/cmGlobalVisualStudioGenerator.cxx

@@ -209,7 +209,7 @@ void cmGlobalVisualStudioGenerator::AddExtraIDETargets()
               tgt->IsImported()) {
             continue;
           }
-          if (!this->IsExcluded(tgt)) {
+          if (!this->IsExcluded(gen[0], tgt)) {
             allBuild->AddUtility(tgt->GetName());
           }
         }

Source/cmGlobalXCodeGenerator.cxx

@@ -567,7 +567,7 @@ void cmGlobalXCodeGenerator::AddExtraTargets(
           false, "", false, cmMakefile::AcceptObjectLibraryCommands);
       }
 
-      if (!this->IsExcluded(target)) {
+      if (!this->IsExcluded(gens[0], target)) {
         allbuild->AddUtility(target->GetName());
       }
     }

Source/cmLocalNinjaGenerator.cxx

@@ -88,7 +88,9 @@ void cmLocalNinjaGenerator::Generate()
     if (tg) {
       tg->Generate();
       // Add the target to "all" if required.
-      if (!this->GetGlobalNinjaGenerator()->IsExcluded(target)) {
+      if (!this->GetGlobalNinjaGenerator()->IsExcluded(
+            this->GetGlobalNinjaGenerator()->GetLocalGenerators()[0],
+            target)) {
         this->GetGlobalNinjaGenerator()->AddDependencyToAll(target);
       }
       delete tg;

Source/cmMakefile.cxx

@@ -1151,7 +1151,7 @@ cmTarget* cmMakefile::AddUtilityCommand(
   // Create a target instance for this utility.
   cmTarget* target = this->AddNewTarget(cmStateEnums::UTILITY, utilityName);
   target->SetIsGeneratorProvided(origin == TargetOrigin::Generator);
-  if (excludeFromAll || this->GetPropertyAsBool("EXCLUDE_FROM_ALL")) {
+  if (excludeFromAll) {
     target->SetProperty("EXCLUDE_FROM_ALL", "TRUE");
   }
   if (!comment) {
@@ -1689,7 +1689,7 @@ void cmMakefile::AddSubDirectory(const std::string& srcPath,
   cmMakefile* subMf = new cmMakefile(this->GlobalGenerator, newSnapshot);
   this->GetGlobalGenerator()->AddMakefile(subMf);
 
-  if (excludeFromAll || this->GetPropertyAsBool("EXCLUDE_FROM_ALL")) {
+  if (excludeFromAll) {
     subMf->SetProperty("EXCLUDE_FROM_ALL", "TRUE");
   }
 
@@ -1985,9 +1985,7 @@ cmTarget* cmMakefile::AddLibrary(const std::string& lname,
   // over changes in CMakeLists.txt, making the information stale and
   // hence useless.
   target->ClearDependencyInformation(*this);
-  if (excludeFromAll ||
+  if (excludeFromAll) {
-      (type != cmStateEnums::INTERFACE_LIBRARY &&
-       this->GetPropertyAsBool("EXCLUDE_FROM_ALL"))) {
     target->SetProperty("EXCLUDE_FROM_ALL", "TRUE");
   }
   target->AddSources(srcs);
@@ -2000,7 +1998,7 @@ cmTarget* cmMakefile::AddExecutable(const std::string& exeName,
                                     bool excludeFromAll)
 {
   cmTarget* target = this->AddNewTarget(cmStateEnums::EXECUTABLE, exeName);
-  if (excludeFromAll || this->GetPropertyAsBool("EXCLUDE_FROM_ALL")) {
+  if (excludeFromAll) {
     target->SetProperty("EXCLUDE_FROM_ALL", "TRUE");
   }
   target->AddSources(srcs);

Tests/RunCMake/add_subdirectory/ExcludeFromAll-build-sub-stderr.txt

@@ -0,0 +1 @@
+.*

Tests/RunCMake/add_subdirectory/ExcludeFromAll.cmake

@@ -9,5 +9,6 @@ file(GENERATE OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/check-$<LOWER_CASE:$<CONFIG>>.c
 set(main_exe \"$<TARGET_FILE:main>\")
 set(foo_lib \"$<TARGET_FILE:foo>\")
 set(bar_lib \"$<TARGET_FILE:bar>\")
+set(zot_lib \"$<TARGET_FILE:zot>\")
 set(subinc_lib \"$<TARGET_FILE:subinc>\")
 ")

Tests/RunCMake/add_subdirectory/ExcludeFromAll/CMakeLists.txt

@@ -1,4 +1,8 @@
-add_library(bar STATIC bar.cpp)
+project(ExcludeFromAllSub NONE)
+
+add_library(bar STATIC EXCLUDE_FROM_ALL bar.cpp)
+
+add_library(zot STATIC zot.cpp)
 
 add_library(foo STATIC foo.cpp)
 target_include_directories(foo PUBLIC .)

Tests/RunCMake/add_subdirectory/ExcludeFromAll/check-sub.cmake

@@ -0,0 +1,32 @@
+if(EXISTS ${RunCMake_TEST_BINARY_DIR}/check-debug.cmake)
+  include(${RunCMake_TEST_BINARY_DIR}/check-debug.cmake)
+  if(RunCMake_TEST_FAILED)
+    return()
+  endif()
+
+  foreach(file
+      "${foo_lib}"
+      "${subinc_lib}"
+      "${zot_lib}"
+      )
+    if(NOT EXISTS "${file}")
+      set(RunCMake_TEST_FAILED
+        "Artifact should exist but is missing:\n  ${file}")
+      return()
+    endif()
+  endforeach()
+  foreach(file
+      "${main_exe}"
+      "${bar_lib}"
+      )
+    if(EXISTS "${file}")
+      set(RunCMake_TEST_FAILED
+        "Artifact should be missing but exists:\n  ${file}")
+      return()
+    endif()
+  endforeach()
+else()
+  set(RunCMake_TEST_FAILED "
+ '${RunCMake_TEST_BINARY_DIR}/check-debug.cmake' missing
+")
+endif()

Tests/RunCMake/add_subdirectory/ExcludeFromAll/check.cmake

@@ -9,13 +9,17 @@ if(EXISTS ${RunCMake_TEST_BINARY_DIR}/check-debug.cmake)
       "${subinc_lib}"
       "${main_exe}"
       )
-    if(NOT EXISTS "${file}")
+    if(EXISTS "${file}")
+      # Remove for next step of test.
+      file(REMOVE "${file}")
+    else()
       set(RunCMake_TEST_FAILED
         "Artifact should exist but is missing:\n  ${file}")
       return()
     endif()
   endforeach()
   foreach(file
+      "${zot_lib}"
       "${bar_lib}"
       )
     if(EXISTS "${file}")

Tests/RunCMake/add_subdirectory/ExcludeFromAll/zot.cpp

@@ -0,0 +1,4 @@
+int zot()
+{
+  return 0;
+}

Tests/RunCMake/add_subdirectory/RunCMakeTest.cmake

@@ -34,6 +34,26 @@ run_cmake(ExcludeFromAll)
 set(RunCMake_TEST_NO_CLEAN 1)
 set(RunCMake-check-file ExcludeFromAll/check.cmake)
 run_cmake_command(ExcludeFromAll-build ${CMAKE_COMMAND} --build . --config Debug)
+if(RunCMake_GENERATOR STREQUAL "Ninja")
+  if(WIN32)
+    set(slash [[\]])
+  else()
+    set(slash [[/]])
+  endif()
+  set(RunCMake-check-file ExcludeFromAll/check-sub.cmake)
+  run_cmake_command(ExcludeFromAll-build-sub ${CMAKE_COMMAND} --build . --target "ExcludeFromAll${slash}all")
+elseif(RunCMake_GENERATOR MATCHES "Make")
+  set(RunCMake-check-file ExcludeFromAll/check-sub.cmake)
+  set(RunCMake_TEST_COMMAND_WORKING_DIRECTORY ${RunCMake_BINARY_DIR}/ExcludeFromAll-build/ExcludeFromAll)
+  run_cmake_command(ExcludeFromAll-build-sub "${RunCMake_MAKE_PROGRAM}")
+elseif(RunCMake_GENERATOR MATCHES "^Visual Studio [1-9][0-9]")
+  set(RunCMake-check-file ExcludeFromAll/check-sub.cmake)
+  run_cmake_command(ExcludeFromAll-build-sub ${CMAKE_COMMAND} --build ExcludeFromAll --config Debug)
+elseif(RunCMake_GENERATOR STREQUAL "Xcode")
+  set(RunCMake-check-file ExcludeFromAll/check-sub.cmake)
+  set(RunCMake_TEST_COMMAND_WORKING_DIRECTORY ${RunCMake_BINARY_DIR}/ExcludeFromAll-build/ExcludeFromAll)
+  run_cmake_command(ExcludeFromAll-build-sub xcodebuild -configuration Debug)
+endif()
 unset(RunCMake-check-file)
 unset(RunCMake_TEST_NO_CLEAN)
 unset(RunCMake_TEST_OPTIONS)