set_property: Deduplicate source file directory scopes

alcroito · alcroito · commit f6969b917d5d · 2020-07-13T17:43:17.000+02:00
A user could specify the same directory scope to set_property()
multiple times, which in conjunction with APPEND would append the
property multiple times.

Make sure to deduplicate scopes across both DIRECTORY and
TARGET_DIRECTORY options, so that a property is only appended
once in such a scenario.

Fixes: #20941
diff --git a/Source/cmSetPropertyCommand.cxx b/Source/cmSetPropertyCommand.cxx
@@ -4,6 +4,7 @@
 
 #include <set>
 #include <sstream>
+#include <unordered_set>
 
 #include "cmExecutionStatus.h"
 #include "cmGlobalGenerator.h"
@@ -82,6 +83,8 @@ bool HandleSourceFileDirectoryScopes(
   std::vector<std::string>& source_file_target_directories,
   std::vector<cmMakefile*>& directory_makefiles)
 {
+  std::unordered_set<cmMakefile*> directory_makefiles_set;
+
   cmMakefile* current_dir_mf = &status.GetMakefile();
   if (!source_file_directories.empty()) {
     for (const std::string& dir_path : source_file_directories) {
@@ -94,7 +97,11 @@ bool HandleSourceFileDirectoryScopes(
         status.SetError(cmStrCat("given non-existent DIRECTORY ", dir_path));
         return false;
       }
-      directory_makefiles.push_back(dir_mf);
+      if (directory_makefiles_set.find(dir_mf) ==
+          directory_makefiles_set.end()) {
+        directory_makefiles.push_back(dir_mf);
+        directory_makefiles_set.insert(dir_mf);
+      }
     }
   }
 
@@ -110,7 +117,12 @@ bool HandleSourceFileDirectoryScopes(
       cmMakefile* target_dir_mf =
         status.GetMakefile().GetGlobalGenerator()->FindMakefile(
           *target_source_dir);
-      directory_makefiles.push_back(target_dir_mf);
+
+      if (directory_makefiles_set.find(target_dir_mf) ==
+          directory_makefiles_set.end()) {
+        directory_makefiles.push_back(target_dir_mf);
+        directory_makefiles_set.insert(target_dir_mf);
+      }
     }
   }
 
diff --git a/Tests/Properties/CMakeLists.txt b/Tests/Properties/CMakeLists.txt
@@ -261,6 +261,25 @@ function(check_get_property_value expected)
     endif()
 endfunction()
 
+# Check that source file directory scopes are deduplicated.
+set_property(SOURCE "${CMAKE_CURRENT_BINARY_DIR}/src32.cpp"
+    DIRECTORY SubDir2 SubDir2 SubDir2
+    TARGET_DIRECTORY set_prop_lib_3 set_prop_lib_3 set_prop_lib_3
+    APPEND
+    PROPERTY NON_DUPLICATED_CUSTOM_PROP 1
+)
+
+get_property(actual
+             SOURCE "${CMAKE_CURRENT_BINARY_DIR}/src32.cpp"
+             DIRECTORY SubDir2
+             PROPERTY NON_DUPLICATED_CUSTOM_PROP)
+check_get_property_value("1")
+
+get_source_file_property(actual "${CMAKE_CURRENT_BINARY_DIR}/src32.cpp"
+                         TARGET_DIRECTORY set_prop_lib_3
+                         NON_DUPLICATED_CUSTOM_PROP)
+check_get_property_value("1")
+
 # Get property + target directory
 get_property(actual
              SOURCE "${src_prefix}/src1.cpp"
