Browse Source

set_property: Allow both DIRECTORY and TARGET_DIRECTORY together

Allow to specify both DIRECTORY and TARGET_DIRECTORY at the same time in
`set_source_files_properties()` and `set_property(SOURCE)` commands.

Add test cases and update the documentation.

Fixes: #20932
Alexandru Croitor 5 years ago
parent
commit
1235f2d747

+ 3 - 2
Help/command/set_property.rst

@@ -9,7 +9,8 @@ Set a named property in a given scope.
                 DIRECTORY [<dir>]           |
                 DIRECTORY [<dir>]           |
                 TARGET    [<target1> ...]   |
                 TARGET    [<target1> ...]   |
                 SOURCE    [<src1> ...]
                 SOURCE    [<src1> ...]
-                          [<TARGET_DIRECTORY ... | DIRECTORY ...>]   |
+                          [TARGET_DIRECTORY <target1> ...]
+                          [DIRECTORY <dir1> ...] |
                 INSTALL   [<file1> ...]     |
                 INSTALL   [<file1> ...]     |
                 TEST      [<test1> ...]     |
                 TEST      [<test1> ...]     |
                 CACHE     [<entry1> ...]    >
                 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
   file properties are by default visible only to targets added in the same
   directory (``CMakeLists.txt``).
   directory (``CMakeLists.txt``).
   The file properties can be made visible in a different directory by specifying
   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
   ``DIRECTORY`` takes a list of processed directories paths, and sets the file
   properties in those directory scopes.
   properties in those directory scopes.

+ 3 - 2
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
 .. code-block:: cmake
 
 
   set_source_files_properties([file1 [file2 [...]]]
   set_source_files_properties([file1 [file2 [...]]]
-                              [<TARGET_DIRECTORY ... | DIRECTORY ...>]
+                              [TARGET_DIRECTORY <target1> ...]
+                              [DIRECTORY <dir1> ...]
                               PROPERTIES prop1 value1
                               PROPERTIES prop1 value1
                               [prop2 value2 [...]])
                               [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``).
 targets added in the same directory (``CMakeLists.txt``).
 
 
 The file properties can be made visible in a different directory by specifying
 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
 ``DIRECTORY`` takes a list of processed directories paths, and sets the file
 properties in those directory scopes.
 properties in those directory scopes.

+ 11 - 6
Source/cmSetPropertyCommand.cxx

@@ -96,7 +96,9 @@ bool HandleSourceFileDirectoryScopes(
       }
       }
       directory_makefiles.push_back(dir_mf);
       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) {
     for (const std::string& target_name : source_file_target_directories) {
       cmTarget* target = current_dir_mf->FindTargetToUse(target_name);
       cmTarget* target = current_dir_mf->FindTargetToUse(target_name);
       if (!target) {
       if (!target) {
@@ -110,7 +112,10 @@ bool HandleSourceFileDirectoryScopes(
           *target_source_dir);
           *target_source_dir);
       directory_makefiles.push_back(target_dir_mf);
       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);
     directory_makefiles.push_back(current_dir_mf);
   }
   }
   return true;
   return true;
@@ -271,12 +276,12 @@ bool cmSetPropertyCommand(std::vector<std::string> const& args,
       appendMode = true;
       appendMode = true;
       remove = false;
       remove = false;
       appendAsString = true;
       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;
       doing = DoingSourceDirectory;
       source_file_directory_option_enabled = true;
       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;
       doing = DoingSourceTargetDirectory;
       source_file_target_option_enabled = true;
       source_file_target_option_enabled = true;
     } else if (doing == DoingNames) {
     } else if (doing == DoingNames) {

+ 25 - 14
Source/cmSetSourceFilesPropertiesCommand.cxx

@@ -35,11 +35,11 @@ bool cmSetSourceFilesPropertiesCommand(std::vector<std::string> const& args,
     "OBJECT_DEPENDS", "PROPERTIES", "DIRECTORY",    "TARGET_DIRECTORY"
     "OBJECT_DEPENDS", "PROPERTIES", "DIRECTORY",    "TARGET_DIRECTORY"
   };
   };
 
 
-  auto isNotAPropertyKeyword =
+  auto isAPropertyKeyword =
     [](const std::vector<std::string>::const_iterator& arg_it) {
     [](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),
         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(
   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;
   bool source_file_target_option_enabled = false;
   std::vector<cmMakefile*> source_file_directory_makefiles;
   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);
       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);
       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;
   const auto props_begin = options_it;
 
 
   bool file_scopes_handled =
   bool file_scopes_handled =

+ 22 - 2
Tests/Properties/CMakeLists.txt

@@ -146,18 +146,25 @@ check_cache_props()
 
 
 function(generate_file_for_set_property_test i target_name)
 function(generate_file_for_set_property_test i target_name)
     set(src_path "${CMAKE_CURRENT_BINARY_DIR}/src${i}.cpp")
     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\
         "#ifndef def${i}\n\
         #error Expected def${i}\n\
         #error Expected def${i}\n\
         #endif\n\
         #endif\n\
         #ifdef _WIN32\n\
         #ifdef _WIN32\n\
         __declspec(dllexport)\n\
         __declspec(dllexport)\n\
         #endif\n\
         #endif\n\
-        void dummy_symbol${i}() {}\n")
+        void dummy_symbol${i}() {}\n"
+        NEWLINE_STYLE UNIX)
     target_sources(${target_name} PRIVATE "${src_path}")
     target_sources(${target_name} PRIVATE "${src_path}")
 endfunction()
 endfunction()
 
 
 add_library(maindirtest SHARED)
 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)
 add_subdirectory(SubDir2)
 
 
 set(src_prefix "${CMAKE_CURRENT_BINARY_DIR}/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"
 set_source_files_properties("${CMAKE_CURRENT_BINARY_DIR}/src30.cpp"
                             "${CMAKE_CURRENT_BINARY_DIR}/SubDir2/src31.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)
 function(check_get_property_value expected)
     if(NOT actual STREQUAL expected)
     if(NOT actual STREQUAL expected)
         message(SEND_ERROR "Error: get_property returned unexpected value\n"
         message(SEND_ERROR "Error: get_property returned unexpected value\n"

+ 4 - 0
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
 # For set_source_files_properties + multiple files in multiple directories
 generate_file_for_set_property_test(31 set_prop_lib_3)
 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")