Merge topic 'source_file_both_props' into release-3.18

961ee62faa Help: Update get_property and get_source_file_property docs
1235f2d747 set_property: Allow both DIRECTORY and TARGET_DIRECTORY together
177052d6b8 set_property: Fix name of TARGET_DIRECTORY option in error messages

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

10 files changed, 75 insertions, 33 deletions

diff --git a/Help/command/get_property.rst b/Help/command/get_property.rst
index 80d97fd..123cd45 100644
--- a/Help/command/get_property.rst
+++ b/Help/command/get_property.rst
@@ -10,7 +10,7 @@ Get a property.
                 DIRECTORY [<dir>]  |
                 TARGET    <target> |
                 SOURCE    <source> |
-                          [<TARGET_DIRECTORY ... | DIRECTORY ...>]   |
+                          [TARGET_DIRECTORY <target> | DIRECTORY <dir>] |
                 INSTALL   <file>   |
                 TEST      <test>   |
                 CACHE     <entry>  |
diff --git a/Help/command/get_source_file_property.rst b/Help/command/get_source_file_property.rst
index 893a1b6..1060251 100644
--- a/Help/command/get_source_file_property.rst
+++ b/Help/command/get_source_file_property.rst
@@ -5,7 +5,7 @@ Get a property for a source file.
 
 .. code-block:: cmake
 
-  get_source_file_property(VAR file [<TARGET_DIRECTORY ... | DIRECTORY ...>] property)
+  get_source_file_property(VAR file [TARGET_DIRECTORY <target> | DIRECTORY <dir>] property)
 
 Gets a property from a source file.  The value of the property is
 stored in the variable ``VAR``.  If the source property is not found, the
diff --git a/Help/command/set_property.rst b/Help/command/set_property.rst
index de28be3..ccbd635 100644
--- a/Help/command/set_property.rst
+++ b/Help/command/set_property.rst
@@ -9,7 +9,8 @@ Set a named property in a given scope.
                 DIRECTORY [<dir>]           |
                 TARGET    [<target1> ...]   |
                 SOURCE    [<src1> ...]
-                          [<TARGET_DIRECTORY ... | DIRECTORY ...>]   |
+                          [TARGET_DIRECTORY <target1> ...]
+                          [DIRECTORY <dir1> ...] |
                 INSTALL   [<file1> ...]     |
                 TEST      [<test1> ...]     |
                 CACHE     [<entry1> ...]    >
@@ -38,7 +39,7 @@ It must be one of the following:
   file properties are by default visible only to targets added in the same
   directory (``CMakeLists.txt``).
   The file properties can be made visible in a different directory by specifying
-  one of the additional options: ``TARGET_DIRECTORY`` or ``DIRECTORY``.
+  one or both of the additional options: ``TARGET_DIRECTORY`` and ``DIRECTORY``.
 
   ``DIRECTORY`` takes a list of processed directories paths, and sets the file
   properties in those directory scopes.
diff --git a/Help/command/set_source_files_properties.rst b/Help/command/set_source_files_properties.rst
index 8adfb9e..59d5ba3 100644
--- a/Help/command/set_source_files_properties.rst
+++ b/Help/command/set_source_files_properties.rst
@@ -6,7 +6,8 @@ Source files can have properties that affect how they are built.
 .. code-block:: cmake
 
   set_source_files_properties([file1 [file2 [...]]]
-                              [<TARGET_DIRECTORY ... | DIRECTORY ...>]
+                              [TARGET_DIRECTORY <target1> ...]
+                              [DIRECTORY <dir1> ...]
                               PROPERTIES prop1 value1
                               [prop2 value2 [...]])
 
@@ -17,7 +18,7 @@ Note that source file properties are by default visible only to
 targets added in the same directory (``CMakeLists.txt``).
 
 The file properties can be made visible in a different directory by specifying
-one of the additional options: ``TARGET_DIRECTORY`` or ``DIRECTORY``.
+one or both of the additional options: ``TARGET_DIRECTORY`` and ``DIRECTORY``.
 
 ``DIRECTORY`` takes a list of processed directories paths, and sets the file
 properties in those directory scopes.
diff --git a/Source/cmSetPropertyCommand.cxx b/Source/cmSetPropertyCommand.cxx
index 07cb7c9..51509fd 100644
--- a/Source/cmSetPropertyCommand.cxx
+++ b/Source/cmSetPropertyCommand.cxx
@@ -96,12 +96,14 @@ bool HandleSourceFileDirectoryScopes(
       }
       directory_makefiles.push_back(dir_mf);
     }
-  } else if (!source_file_target_directories.empty()) {
+  }
+
+  if (!source_file_target_directories.empty()) {
     for (const std::string& target_name : source_file_target_directories) {
       cmTarget* target = current_dir_mf->FindTargetToUse(target_name);
       if (!target) {
         status.SetError(cmStrCat(
-          "given non-existent target for DIRECTORY_TARGET ", target_name));
+          "given non-existent target for TARGET_DIRECTORY ", target_name));
         return false;
       }
       cmProp target_source_dir = target->GetProperty("SOURCE_DIR");
@@ -110,7 +112,10 @@ bool HandleSourceFileDirectoryScopes(
           *target_source_dir);
       directory_makefiles.push_back(target_dir_mf);
     }
-  } else {
+  }
+
+  if (source_file_directories.empty() &&
+      source_file_target_directories.empty()) {
     directory_makefiles.push_back(current_dir_mf);
   }
   return true;
@@ -271,12 +276,12 @@ bool cmSetPropertyCommand(std::vector<std::string> const& args,
       appendMode = true;
       remove = false;
       appendAsString = true;
-    } else if (doing == DoingNames && scope == cmProperty::SOURCE_FILE &&
-               arg == "DIRECTORY") {
+    } else if (doing != DoingProperty && doing != DoingValues &&
+               scope == cmProperty::SOURCE_FILE && arg == "DIRECTORY") {
       doing = DoingSourceDirectory;
       source_file_directory_option_enabled = true;
-    } else if (doing == DoingNames && scope == cmProperty::SOURCE_FILE &&
-               arg == "TARGET_DIRECTORY") {
+    } else if (doing != DoingProperty && doing != DoingValues &&
+               scope == cmProperty::SOURCE_FILE && arg == "TARGET_DIRECTORY") {
       doing = DoingSourceTargetDirectory;
       source_file_target_option_enabled = true;
     } else if (doing == DoingNames) {
diff --git a/Source/cmSetSourceFilesPropertiesCommand.cxx b/Source/cmSetSourceFilesPropertiesCommand.cxx
index 3135a06..c1b0c28 100644
--- a/Source/cmSetSourceFilesPropertiesCommand.cxx
+++ b/Source/cmSetSourceFilesPropertiesCommand.cxx
@@ -35,11 +35,11 @@ bool cmSetSourceFilesPropertiesCommand(std::vector<std::string> const& args,
     "OBJECT_DEPENDS", "PROPERTIES", "DIRECTORY",    "TARGET_DIRECTORY"
   };
 
-  auto isNotAPropertyKeyword =
+  auto isAPropertyKeyword =
     [](const std::vector<std::string>::const_iterator& arg_it) {
-      return std::all_of(
+      return std::any_of(
         std::begin(prop_names), std::end(prop_names),
-        [&arg_it](cm::string_view prop_name) { return *arg_it != prop_name; });
+        [&arg_it](cm::string_view prop_name) { return *arg_it == prop_name; });
     };
 
   auto options_begin = std::find_first_of(
@@ -53,21 +53,32 @@ bool cmSetSourceFilesPropertiesCommand(std::vector<std::string> const& args,
   bool source_file_target_option_enabled = false;
   std::vector<cmMakefile*> source_file_directory_makefiles;
 
-  if (options_it != args.end() && *options_it == "DIRECTORY") {
-    source_file_directory_option_enabled = true;
-    ++options_it;
-    while (options_it != args.end() && isNotAPropertyKeyword(options_it)) {
+  enum Doing
+  {
+    DoingNone,
+    DoingSourceDirectory,
+    DoingSourceTargetDirectory
+  };
+  Doing doing = DoingNone;
+  for (; options_it != args.end(); ++options_it) {
+    if (*options_it == "DIRECTORY") {
+      doing = DoingSourceDirectory;
+      source_file_directory_option_enabled = true;
+    } else if (*options_it == "TARGET_DIRECTORY") {
+      doing = DoingSourceTargetDirectory;
+      source_file_target_option_enabled = true;
+    } else if (isAPropertyKeyword(options_it)) {
+      break;
+    } else if (doing == DoingSourceDirectory) {
       source_file_directories.push_back(*options_it);
-      ++options_it;
-    }
-  } else if (options_it != args.end() && *options_it == "TARGET_DIRECTORY") {
-    source_file_target_option_enabled = true;
-    ++options_it;
-    while (options_it != args.end() && isNotAPropertyKeyword(options_it)) {
+    } else if (doing == DoingSourceTargetDirectory) {
       source_file_target_directories.push_back(*options_it);
-      ++options_it;
+    } else {
+      status.SetError(
+        cmStrCat("given invalid argument \"", *options_it, "\"."));
     }
   }
+
   const auto props_begin = options_it;
 
   bool file_scopes_handled =
diff --git a/Tests/Properties/CMakeLists.txt b/Tests/Properties/CMakeLists.txt
index f93f553..74d99fa 100644
--- a/Tests/Properties/CMakeLists.txt
+++ b/Tests/Properties/CMakeLists.txt
@@ -146,18 +146,25 @@ check_cache_props()
 
 function(generate_file_for_set_property_test i target_name)
     set(src_path "${CMAKE_CURRENT_BINARY_DIR}/src${i}.cpp")
-    file(GENERATE OUTPUT "${src_path}" CONTENT
+    file(CONFIGURE OUTPUT "${src_path}" CONTENT
         "#ifndef def${i}\n\
         #error Expected def${i}\n\
         #endif\n\
         #ifdef _WIN32\n\
         __declspec(dllexport)\n\
         #endif\n\
-        void dummy_symbol${i}() {}\n")
+        void dummy_symbol${i}() {}\n"
+        NEWLINE_STYLE UNIX)
     target_sources(${target_name} PRIVATE "${src_path}")
 endfunction()
 
 add_library(maindirtest SHARED)
+
+# Generate file to be used with both DIRECTORY and TARGET_DIRECTORY options in
+# set_source_files_properties and set_property().
+generate_file_for_set_property_test(32 maindirtest)
+generate_file_for_set_property_test(33 maindirtest)
+
 add_subdirectory(SubDir2)
 
 set(src_prefix "${CMAKE_CURRENT_BINARY_DIR}/SubDir2/")
@@ -233,6 +240,19 @@ set_source_files_properties("${CMAKE_CURRENT_BINARY_DIR}/src30.cpp"
 set_source_files_properties("${CMAKE_CURRENT_BINARY_DIR}/src30.cpp"
                             "${CMAKE_CURRENT_BINARY_DIR}/SubDir2/src31.cpp")
 
+
+# Check that specifying both DIRECTORY and TARGET_DIRECTORY works.
+set_source_files_properties("${CMAKE_CURRENT_BINARY_DIR}/src32.cpp"
+                            DIRECTORY .
+                            TARGET_DIRECTORY set_prop_lib_3
+                            PROPERTIES COMPILE_DEFINITIONS "def32")
+
+set_property(SOURCE "${CMAKE_CURRENT_BINARY_DIR}/src33.cpp"
+             DIRECTORY SubDir2
+             TARGET_DIRECTORY maindirtest
+             PROPERTY COMPILE_DEFINITIONS "def33")
+
+
 function(check_get_property_value expected)
     if(NOT actual STREQUAL expected)
         message(SEND_ERROR "Error: get_property returned unexpected value\n"
diff --git a/Tests/Properties/SubDir2/CMakeLists.txt b/Tests/Properties/SubDir2/CMakeLists.txt
index 9b2c79e..88e5531 100644
--- a/Tests/Properties/SubDir2/CMakeLists.txt
+++ b/Tests/Properties/SubDir2/CMakeLists.txt
@@ -28,3 +28,7 @@ generate_file_for_set_property_test(23 set_prop_lib_3)
 
 # For set_source_files_properties + multiple files in multiple directories
 generate_file_for_set_property_test(31 set_prop_lib_3)
+
+# For specifying both DIRECTORY and TARGET_DIRECTORY
+target_sources(set_prop_lib_3 PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/../src32.cpp")
+target_sources(set_prop_lib_3 PRIVATE "${CMAKE_CURRENT_BINARY_DIR}/../src33.cpp")
diff --git a/Tests/RunCMake/get_property/source_properties_failures-stderr.txt b/Tests/RunCMake/get_property/source_properties_failures-stderr.txt
index a500e2e..0accb8f 100644
--- a/Tests/RunCMake/get_property/source_properties_failures-stderr.txt
+++ b/Tests/RunCMake/get_property/source_properties_failures-stderr.txt
@@ -16,7 +16,7 @@ Call Stack \(most recent call first\):
   CMakeLists\.txt:[0-9]+ \(include\)
 +
 CMake Error at source_properties_failures.cmake:4 \(set_source_files_properties\):
-  set_source_files_properties given non-existent target for DIRECTORY_TARGET
+  set_source_files_properties given non-existent target for TARGET_DIRECTORY
   non_existing_target
 Call Stack \(most recent call first\):
   CMakeLists\.txt:[0-9]+ \(include\)
@@ -39,7 +39,7 @@ Call Stack \(most recent call first\):
   CMakeLists\.txt:[0-9]+ \(include\)
 +
 CMake Error at source_properties_failures.cmake:9 \(get_property\):
-  get_property given non-existent target for DIRECTORY_TARGET
+  get_property given non-existent target for TARGET_DIRECTORY
   non_existing_dir
 Call Stack \(most recent call first\):
   CMakeLists\.txt:[0-9]+ \(include\)
@@ -55,7 +55,7 @@ Call Stack \(most recent call first\):
   CMakeLists\.txt:[0-9]+ \(include\)
 +
 CMake Error at source_properties_failures.cmake:13 \(get_source_file_property\):
-  get_source_file_property given non-existent target for DIRECTORY_TARGET
+  get_source_file_property given non-existent target for TARGET_DIRECTORY
   PROPERTY
 Call Stack \(most recent call first\):
   CMakeLists\.txt:[0-9]+ \(include\)
diff --git a/Tests/RunCMake/set_property/SOURCE_FILE-stderr.txt b/Tests/RunCMake/set_property/SOURCE_FILE-stderr.txt
index 2e0b238..a2b6cf4 100644
--- a/Tests/RunCMake/set_property/SOURCE_FILE-stderr.txt
+++ b/Tests/RunCMake/set_property/SOURCE_FILE-stderr.txt
@@ -16,7 +16,7 @@ Call Stack \(most recent call first\):
   CMakeLists\.txt:[0-9]+ \(include\)
 +
 CMake Error at SOURCE_FILE.cmake:4 \(set_property\):
-  set_property given non-existent target for DIRECTORY_TARGET
+  set_property given non-existent target for TARGET_DIRECTORY
   non_existing_target
 Call Stack \(most recent call first\):
   CMakeLists\.txt:[0-9]+ \(include\)$