cmTarget: make HEADER_SETS and INTERFACE_HEADER_SETS read-only

There is no reason to allow these properties to be manipulated by user
code. Instead, use the stored visibility on the fileset objects to
derive what these properties should contain.

Help/prop_tgt/HEADER_SETS.rst

@@ -3,14 +3,10 @@ HEADER_SETS
 
 .. versionadded:: 3.23
 
-List of the target's ``PRIVATE`` and ``PUBLIC`` header sets (i.e. all
+Read-only list of the target's ``PRIVATE`` and ``PUBLIC`` header sets (i.e.
-file sets with the type ``HEADERS``). Files listed in these file sets
+all file sets with the type ``HEADERS``). Files listed in these file sets are
-are treated as source files for the purpose of IDE integration.
+treated as source files for the purpose of IDE integration. The files also
-The files also have their :prop_sf:`HEADER_FILE_ONLY` property set to
+have their :prop_sf:`HEADER_FILE_ONLY` property set to ``TRUE``.
-``TRUE``.
-
-This property is normally only set by :command:`target_sources(FILE_SET)`
-rather than being manipulated directly.
 
 See also :prop_tgt:`HEADER_SET_<NAME>`, :prop_tgt:`HEADER_SET` and
 :prop_tgt:`INTERFACE_HEADER_SETS`.

Help/prop_tgt/INTERFACE_HEADER_SETS.rst

@@ -3,12 +3,9 @@ INTERFACE_HEADER_SETS
 
 .. versionadded:: 3.23
 
-List of the target's ``INTERFACE`` and ``PUBLIC`` header sets (i.e. all
+Read-only list of the target's ``INTERFACE`` and ``PUBLIC`` header sets (i.e.
-file sets with the type ``HEADERS``). Files listed in these header sets
+all file sets with the type ``HEADERS``). Files listed in these header sets
 can be installed with :command:`install(TARGETS)` and exported with
 :command:`install(EXPORT)` and :command:`export`.
 
-This property is normally only set by :command:`target_sources(FILE_SET)`
-rather than being manipulated directly.
-
 See also :prop_tgt:`HEADER_SETS`.

Help/release/3.23.rst

@@ -114,8 +114,8 @@ Variables
 Properties
 ----------
 
-* The :prop_tgt:`HEADER_SETS` and :prop_tgt:`INTERFACE_HEADER_SETS` target
+* The :prop_tgt:`HEADER_SETS` and :prop_tgt:`INTERFACE_HEADER_SETS` read-only
-  properties were added to list header sets associated with a target.
+  target properties were added to list header sets associated with a target.
 
 * The :prop_tgt:`HEADER_SET` and :prop_tgt:`HEADER_SET_<NAME>` target
   properties were added to list files in the default header set

Source/cmTarget.cxx

@@ -1456,37 +1456,14 @@ void cmTarget::StoreProperty(const std::string& prop, ValueType value)
         BT<std::string>(value, this->impl->Makefile->GetBacktrace()));
     }
   } else if (prop == propHEADER_SETS) {
-    if (value) {
+    this->impl->Makefile->IssueMessage(MessageType::FATAL_ERROR,
-      for (auto const& name : cmExpandedList(value)) {
+                                       "HEADER_SETS property is read-only\n");
-        if (!this->GetFileSet(name)) {
+    return;
-          this->impl->Makefile->IssueMessage(
-            MessageType::FATAL_ERROR,
-            cmStrCat("Header set \"", name, "\" has not yet been created."));
-          return;
-        }
-      }
-    }
-    this->impl->HeaderSetsEntries.clear();
-    if (!StringIsEmpty(value)) {
-      this->impl->HeaderSetsEntries.emplace_back(
-        value, this->impl->Makefile->GetBacktrace());
-    }
   } else if (prop == propINTERFACE_HEADER_SETS) {
-    if (value) {
+    this->impl->Makefile->IssueMessage(
-      for (auto const& name : cmExpandedList(value)) {
+      MessageType::FATAL_ERROR,
-        if (!this->GetFileSet(name)) {
+      "INTERFACE_HEADER_SETS property is read-only\n");
-          this->impl->Makefile->IssueMessage(
+    return;
-            MessageType::FATAL_ERROR,
-            cmStrCat("Header set \"", name, "\" has not yet been created."));
-          return;
-        }
-      }
-    }
-    this->impl->InterfaceHeaderSetsEntries.clear();
-    if (!StringIsEmpty(value)) {
-      this->impl->InterfaceHeaderSetsEntries.emplace_back(
-        value, this->impl->Makefile->GetBacktrace());
-    }
   } else {
     this->impl->Properties.SetProperty(prop, value);
   }
@@ -1641,27 +1618,14 @@ void cmTarget::AppendProperty(const std::string& prop,
     fileSet->AddFileEntry(
       BT<std::string>(value, this->impl->Makefile->GetBacktrace()));
   } else if (prop == "HEADER_SETS") {
-    for (auto const& name : cmExpandedList(value)) {
+    this->impl->Makefile->IssueMessage(MessageType::FATAL_ERROR,
-      if (!this->GetFileSet(name)) {
+                                       "HEADER_SETS property is read-only\n");
-        this->impl->Makefile->IssueMessage(
+    return;
-          MessageType::FATAL_ERROR,
-          cmStrCat("Header set \"", name, "\" has not yet been created."));
-        return;
-      }
-    }
-    this->impl->HeaderSetsEntries.emplace_back(
-      value, this->impl->Makefile->GetBacktrace());
   } else if (prop == "INTERFACE_HEADER_SETS") {
-    for (auto const& name : cmExpandedList(value)) {
+    this->impl->Makefile->IssueMessage(
-      if (!this->GetFileSet(name)) {
+      MessageType::FATAL_ERROR,
-        this->impl->Makefile->IssueMessage(
+      "INTERFACE_HEADER_SETS property is read-only\n");
-          MessageType::FATAL_ERROR,
+    return;
-          cmStrCat("Header set \"", name, "\" has not yet been created."));
-        return;
-      }
-    }
-    this->impl->InterfaceHeaderSetsEntries.emplace_back(
-      value, this->impl->Makefile->GetBacktrace());
   } else {
     this->impl->Properties.AppendProperty(prop, value, asString);
   }
@@ -2038,13 +2002,26 @@ cmValue cmTarget::GetProperty(const std::string& prop) const
       return cmValue(output);
     }
     if (prop == propHEADER_SETS) {
+      std::vector<std::string> set_names;
+      for (auto const& file_set : this->impl->FileSets) {
+        if (cmFileSetVisibilityIsForSelf(file_set.second.GetVisibility())) {
+          set_names.push_back(file_set.second.GetName());
+        }
+      }
       static std::string output;
-      output = cmJoin(this->impl->HeaderSetsEntries, ";"_s);
+      output = cmJoin(set_names, ";"_s);
       return cmValue(output);
     }
     if (prop == propINTERFACE_HEADER_SETS) {
+      std::vector<std::string> set_names;
+      for (auto const& file_set : this->impl->FileSets) {
+        if (cmFileSetVisibilityIsForInterface(
+              file_set.second.GetVisibility())) {
+          set_names.push_back(file_set.second.GetName());
+        }
+      }
       static std::string output;
-      output = cmJoin(this->impl->InterfaceHeaderSetsEntries, ";"_s);
+      output = cmJoin(set_names, ";"_s);
       return cmValue(output);
     }
   }
@@ -2346,6 +2323,16 @@ std::pair<cmFileSet*, bool> cmTarget::GetOrCreateFileSet(
 {
   auto result = this->impl->FileSets.emplace(
     std::make_pair(name, cmFileSet(name, type, vis)));
+  if (result.second) {
+    if (cmFileSetVisibilityIsForSelf(vis)) {
+      this->impl->HeaderSetsEntries.emplace_back(
+        name, this->impl->Makefile->GetBacktrace());
+    }
+    if (cmFileSetVisibilityIsForInterface(vis)) {
+      this->impl->InterfaceHeaderSetsEntries.emplace_back(
+        name, this->impl->Makefile->GetBacktrace());
+    }
+  }
   return std::make_pair(&result.first->second, result.second);
 }
 

Source/cmTargetSourcesCommand.cxx

@@ -264,15 +264,6 @@ bool TargetSourcesImpl::HandleOneFileSet(
     if (args.BaseDirs.empty()) {
       args.BaseDirs.emplace_back(this->Makefile->GetCurrentSourceDirectory());
     }
-
-    if (scope == "PRIVATE"_s || scope == "PUBLIC"_s) {
-      this->Target->AppendProperty(cmTarget::GetFileSetsPropertyName(type),
-                                   args.FileSet);
-    }
-    if (scope == "INTERFACE"_s || scope == "PUBLIC"_s) {
-      this->Target->AppendProperty(
-        cmTarget::GetInterfaceFileSetsPropertyName(type), args.FileSet);
-    }
   } else {
     type = fileSet.first->GetType();
     if (!args.Type.empty() && args.Type != type) {

Tests/RunCMake/target_sources/FileSetImport.cmake

@@ -17,7 +17,7 @@ include("${export_build_dir}/export.cmake")
 include("${export_build_dir}/install/lib/cmake/export.cmake")
 
 assert_prop_eq(export::lib1 HEADER_SETS "")
-assert_prop_eq(export::lib1 INTERFACE_HEADER_SETS "HEADERS;b;c;d;e;f;g;dir3")
+assert_prop_eq(export::lib1 INTERFACE_HEADER_SETS "HEADERS;b;c;d;dir3;e;f;g")
 assert_prop_eq(export::lib1 HEADER_SET "${CMAKE_CURRENT_SOURCE_DIR}/error.c")
 assert_prop_eq(export::lib1 HEADER_DIRS "${CMAKE_CURRENT_SOURCE_DIR}")
 assert_prop_eq(export::lib1 HEADER_SET_b "${CMAKE_CURRENT_SOURCE_DIR}/h2.h")
@@ -35,7 +35,7 @@ assert_prop_eq(export::lib1 HEADER_DIRS_g "${CMAKE_CURRENT_SOURCE_DIR}/dir1;${CM
 assert_prop_eq(export::lib1 INTERFACE_INCLUDE_DIRECTORIES "${CMAKE_CURRENT_SOURCE_DIR};${CMAKE_CURRENT_SOURCE_DIR};$<1:${CMAKE_CURRENT_SOURCE_DIR}/dir>;${CMAKE_CURRENT_SOURCE_DIR};${CMAKE_CURRENT_SOURCE_DIR}/$<IF:$<CONFIG:Debug>,debug,release>;${CMAKE_CURRENT_SOURCE_DIR};${CMAKE_CURRENT_SOURCE_DIR}/dir1;${CMAKE_CURRENT_SOURCE_DIR}/dir2;${CMAKE_CURRENT_SOURCE_DIR}/dir3;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>;$<BUILD_INTERFACE:$<1:${CMAKE_CURRENT_SOURCE_DIR}/dir>>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/$<IF:$<CONFIG:Debug>,debug,release>>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/dir1>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/dir2>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/dir3>")
 
 assert_prop_eq(install::lib1 HEADER_SETS "")
-assert_prop_eq(install::lib1 INTERFACE_HEADER_SETS "HEADERS;b;c;d;e;f;g;dir3")
+assert_prop_eq(install::lib1 INTERFACE_HEADER_SETS "HEADERS;b;c;d;dir3;e;f;g")
 assert_prop_eq(install::lib1 HEADER_SET "${export_build_dir}/install/include/error.c")
 assert_prop_eq(install::lib1 HEADER_DIRS "${export_build_dir}/install/include")
 assert_prop_eq(install::lib1 HEADER_SET_b "${export_build_dir}/install/include/h2.h")

Tests/RunCMake/target_sources/FileSetNoExistInterface-stderr.txt

@@ -1,4 +0,0 @@
-^CMake Error at FileSetNoExistInterface\.cmake:[0-9]+ \(set_property\):
-  Header set "a" has not yet been created\.
-Call Stack \(most recent call first\):
-  CMakeLists\.txt:[0-9]+ \(include\)$

Tests/RunCMake/target_sources/FileSetNoExistPrivate-stderr.txt

@@ -1,4 +0,0 @@
-^CMake Error at FileSetNoExistPrivate\.cmake:[0-9]+ \(set_property\):
-  Header set "a" has not yet been created\.
-Call Stack \(most recent call first\):
-  CMakeLists\.txt:[0-9]+ \(include\)$

Tests/RunCMake/target_sources/FileSetNoExistPrivate.cmake

@@ -1,7 +0,0 @@
-enable_language(C)
-
-add_library(lib1 STATIC empty.c)
-set_property(TARGET lib1 PROPERTY HEADER_SETS "a")
-
-# Error happens at configure-time, so this doesn't help.
-target_sources(lib1 PRIVATE FILE_SET a TYPE HEADERS)

Tests/RunCMake/target_sources/FileSetNoScope-result.txt

@@ -1 +0,0 @@
-1

Tests/RunCMake/target_sources/FileSetNoScope-stderr.txt

@@ -1,4 +0,0 @@
-^CMake Error at FileSetNoScope\.cmake:[0-9]+ \(target_sources\):
-  target_sources File set "a" is not in HEADER_SETS or INTERFACE_HEADER_SETS
-Call Stack \(most recent call first\):
-  CMakeLists\.txt:[0-9]+ \(include\)$

Tests/RunCMake/target_sources/FileSetNoScope.cmake

@@ -1,6 +0,0 @@
-enable_language(C)
-
-add_library(lib1 STATIC empty.c)
-target_sources(lib1 PRIVATE FILE_SET a TYPE HEADERS BASE_DIRS ${CMAKE_CURRENT_SOURCE_DIR} FILES h1.h)
-set_property(TARGET lib1 PROPERTY HEADER_SETS)
-target_sources(lib1 PRIVATE FILE_SET a TYPE HEADERS FILES h2.h)

Tests/RunCMake/target_sources/FileSetProperties.cmake

@@ -57,14 +57,14 @@ assert_prop_eq(lib1 INCLUDE_DIRECTORIES "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURC
 assert_prop_eq(lib1 INTERFACE_INCLUDE_DIRECTORIES "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/.>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>")
 
 target_sources(lib1 PUBLIC FILE_SET HEADERS BASE_DIRS "${CMAKE_CURRENT_SOURCE_DIR}" FILES h1.h)
-assert_prop_eq(lib1 INTERFACE_HEADER_SETS "a;c;d;HEADERS")
+assert_prop_eq(lib1 INTERFACE_HEADER_SETS "HEADERS;a;c;d")
 assert_prop_eq(lib1 HEADER_DIRS "${CMAKE_CURRENT_SOURCE_DIR}")
 assert_prop_eq(lib1 HEADER_SET "${CMAKE_CURRENT_SOURCE_DIR}/h1.h")
 assert_prop_eq(lib1 INCLUDE_DIRECTORIES "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/.>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/dir>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>")
 assert_prop_eq(lib1 INTERFACE_INCLUDE_DIRECTORIES "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/.>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>")
 
 target_sources(lib1 PUBLIC FILE_SET HEADERS FILES h2.h)
-assert_prop_eq(lib1 INTERFACE_HEADER_SETS "a;c;d;HEADERS")
+assert_prop_eq(lib1 INTERFACE_HEADER_SETS "HEADERS;a;c;d")
 assert_prop_eq(lib1 HEADER_DIRS "${CMAKE_CURRENT_SOURCE_DIR}")
 assert_prop_eq(lib1 HEADER_SET "${CMAKE_CURRENT_SOURCE_DIR}/h1.h;${CMAKE_CURRENT_SOURCE_DIR}/h2.h")
 assert_prop_eq(lib1 INCLUDE_DIRECTORIES "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/.>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/dir>;$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>")

Tests/RunCMake/target_sources/FileSetReadOnlyInterface-stderr.txt

@@ -0,0 +1,5 @@
+^CMake Error at FileSetReadOnlyInterface\.cmake:[0-9]+ \(set_property\):
+  INTERFACE_HEADER_SETS property is read-only
+
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:[0-9]+ \(include\)$

Tests/RunCMake/target_sources/FileSetReadOnlyInterface.cmake

@@ -2,6 +2,3 @@ enable_language(C)
 
 add_library(lib1 STATIC empty.c)
 set_property(TARGET lib1 PROPERTY INTERFACE_HEADER_SETS "a")
-
-# Error happens at configure-time, so this doesn't help.
-target_sources(lib1 INTERFACE FILE_SET a TYPE HEADERS)

Tests/RunCMake/target_sources/FileSetReadOnlyPrivate-stderr.txt

@@ -0,0 +1,5 @@
+^CMake Error at FileSetReadOnlyPrivate\.cmake:[0-9]+ \(set_property\):
+  HEADER_SETS property is read-only
+
+Call Stack \(most recent call first\):
+  CMakeLists\.txt:[0-9]+ \(include\)$

Tests/RunCMake/target_sources/FileSetReadOnlyPrivate.cmake

@@ -0,0 +1,4 @@
+enable_language(C)
+
+add_library(lib1 STATIC empty.c)
+set_property(TARGET lib1 PROPERTY HEADER_SETS "a")

Tests/RunCMake/target_sources/RunCMakeTest.cmake

@@ -33,9 +33,8 @@ run_cmake(FileSetWrongBaseDirsRelative)
 run_cmake(FileSetOverlappingBaseDirs)
 run_cmake(FileSetInstallMissingSetsPrivate)
 run_cmake(FileSetInstallMissingSetsInterface)
-run_cmake(FileSetNoScope)
+run_cmake(FileSetReadOnlyPrivate)
-run_cmake(FileSetNoExistPrivate)
+run_cmake(FileSetReadOnlyInterface)
-run_cmake(FileSetNoExistInterface)
 run_cmake(FileSetNoExistInstall)
 run_cmake(FileSetDirectories)
 run_cmake(FileSetCustomTarget)