Merge topic 'custom-command-output-hash-character'

8d2a503c1e add_custom_command: Allow OUTPUT filenames containing a hash '#' character
b38000d774 cmGlobalXCodeGenerator: Re-implement legacy makefile path escaping
d929089687 cmGlobalXCodeGenerator: Do not use legacy makefile escaping in shell commands
d61fc2c52e cmGlobalXCodeGenerator: Migrate legacy makefile path escaping to local helper
6010e007c7 cmState: Add method to check for the Borland Makefiles generator

Acked-by: Kitware Robot <[email protected]>
Merge-request: !9514

Help/command/add_custom_command.rst

@@ -269,6 +269,8 @@ The options are:
   source tree is mentioned as an absolute source file path elsewhere
   in the current directory.
 
+  The output file path may not contain ``<`` or ``>`` characters.
+
   .. versionadded:: 3.20
     Arguments to ``OUTPUT`` may use a restricted set of
     :manual:`generator expressions <cmake-generator-expressions(7)>`.
@@ -280,6 +282,10 @@ The options are:
     considered private unless they are listed in a non-private file set.
     See policy :policy:`CMP0154`.
 
+  .. versionchanged:: 3.30
+    The output file path may now use ``#`` characters, except
+    when using the :generator:`Borland Makefiles` generator.
+
 ``USES_TERMINAL``
   .. versionadded:: 3.2
 

Source/cmGlobalBorlandMakefileGenerator.cxx

@@ -24,6 +24,7 @@ cmGlobalBorlandMakefileGenerator::cmGlobalBorlandMakefileGenerator(cmake* cm)
   this->ToolSupportsColor = true;
   this->UseLinkScript = false;
   cm->GetState()->SetWindowsShell(true);
+  cm->GetState()->SetBorlandMake(true);
   this->IncludeDirective = "!include";
   this->DefineWindowsNULL = true;
   this->PassMakeflags = true;

Source/cmGlobalXCodeGenerator.cxx

@@ -264,6 +264,27 @@ cmGlobalXCodeGenerator::Factory::CreateGlobalGenerator(const std::string& name,
 #endif
 }
 
+namespace {
+std::string ConvertToMakefilePath(std::string const& path)
+{
+  std::string result;
+  result.reserve(path.size());
+  for (char c : path) {
+    switch (c) {
+      case '\\':
+      case ' ':
+      case '#':
+        result.push_back('\\');
+        CM_FALLTHROUGH;
+      default:
+        result.push_back(c);
+        break;
+    }
+  }
+  return result;
+}
+}
+
 bool cmGlobalXCodeGenerator::FindMakeProgram(cmMakefile* mf)
 {
   // The Xcode generator knows how to lookup its build tool
@@ -653,7 +674,7 @@ void cmGlobalXCodeGenerator::AddExtraTargets(
   if (regenerate && isGenerateProject) {
     this->CreateReRunCMakeFile(root, gens);
     std::string file =
-      this->ConvertToRelativeForMake(this->CurrentReRunCMakeMakefile);
+      cmSystemTools::ConvertToOutputPath(this->CurrentReRunCMakeMakefile);
     cmSystemTools::ReplaceString(file, "\\ ", " ");
     cc = cm::make_unique<cmCustomCommand>();
     cc->SetCommandLines(cmMakeSingleCommandLine({ "make", "-f", file }));
@@ -732,7 +753,7 @@ void cmGlobalXCodeGenerator::CreateReRunCMakeFile(
 
   for (const auto& lfile : lfiles) {
     makefileStream << "TARGETS += $(subst $(space),$(spaceplus),$(wildcard "
-                   << this->ConvertToRelativeForMake(lfile) << "))\n";
+                   << ConvertToMakefilePath(lfile) << "))\n";
   }
   makefileStream << '\n';
 
@@ -743,25 +764,25 @@ void cmGlobalXCodeGenerator::CreateReRunCMakeFile(
     makefileStream << ".NOTPARALLEL:\n\n"
                       ".PHONY: all VERIFY_GLOBS\n\n"
                       "all: VERIFY_GLOBS "
-                   << this->ConvertToRelativeForMake(checkCache)
+                   << ConvertToMakefilePath(checkCache)
                    << "\n\n"
                       "VERIFY_GLOBS:\n"
                       "\t"
-                   << this->ConvertToRelativeForMake(
-                        cmSystemTools::GetCMakeCommand())
+                   << ConvertToMakefilePath(cmSystemTools::GetCMakeCommand())
                    << " -P "
-                   << this->ConvertToRelativeForMake(cm->GetGlobVerifyScript())
+                   << ConvertToMakefilePath(cm->GetGlobVerifyScript())
                    << "\n\n";
   }
 
-  makefileStream << this->ConvertToRelativeForMake(checkCache)
-                 << ": $(TARGETS)\n";
-  makefileStream
-    << '\t' << this->ConvertToRelativeForMake(cmSystemTools::GetCMakeCommand())
-    << " -S" << this->ConvertToRelativeForMake(root->GetSourceDirectory())
-    << " -B" << this->ConvertToRelativeForMake(root->GetBinaryDirectory())
-    << (cm->GetIgnoreWarningAsError() ? " --compile-no-warning-as-error" : "")
-    << '\n';
+  makefileStream << ConvertToMakefilePath(checkCache) << ": $(TARGETS)\n";
+  makefileStream << '\t'
+                 << ConvertToMakefilePath(cmSystemTools::GetCMakeCommand())
+                 << " -S" << ConvertToMakefilePath(root->GetSourceDirectory())
+                 << " -B" << ConvertToMakefilePath(root->GetBinaryDirectory())
+                 << (cm->GetIgnoreWarningAsError()
+                       ? " --compile-no-warning-as-error"
+                       : "")
+                 << '\n';
 }
 
 static bool objectIdLessThan(const std::unique_ptr<cmXCodeObject>& l,
@@ -2199,11 +2220,10 @@ void cmGlobalXCodeGenerator::AddCommandsToBuildPhase(
   }
 
   std::string cdir = this->CurrentLocalGenerator->GetCurrentBinaryDirectory();
-  cdir = this->ConvertToRelativeForMake(cdir);
+  cdir = cmSystemTools::ConvertToOutputPath(cdir);
   std::string makecmd = cmStrCat(
-    "make -C ", cdir, " -f ",
-    this->ConvertToRelativeForMake(cmStrCat(makefile, "$CONFIGURATION")),
-    " OBJDIR=$(basename \"$OBJECT_FILE_DIR_normal\") all");
+    "make -C ", cdir, " -f ", cmSystemTools::ConvertToOutputPath(makefile),
+    "$CONFIGURATION", " OBJDIR=$(basename \"$OBJECT_FILE_DIR_normal\") all");
   buildphase->AddAttribute("shellScript", this->CreateString(makecmd));
   buildphase->AddAttribute("showEnvVarsInLog", this->CreateString("0"));
 }
@@ -2237,7 +2257,7 @@ void cmGlobalXCodeGenerator::CreateCustomRulesMakefile(
       const std::vector<std::string>& outputs = ccg.GetOutputs();
       if (!outputs.empty()) {
         for (auto const& output : outputs) {
-          makefileStream << "\\\n\t" << this->ConvertToRelativeForMake(output);
+          makefileStream << "\\\n\t" << ConvertToMakefilePath(output);
         }
       } else {
         std::ostringstream str;
@@ -2291,7 +2311,7 @@ void cmGlobalXCodeGenerator::CreateCustomRulesMakefile(
         // There is at least one output, start the rule for it
         const char* sep = "";
         for (auto const& output : outputs) {
-          makefileStream << sep << this->ConvertToRelativeForMake(output);
+          makefileStream << sep << ConvertToMakefilePath(output);
           sep = " ";
         }
         makefileStream << ": ";
@@ -2300,7 +2320,7 @@ void cmGlobalXCodeGenerator::CreateCustomRulesMakefile(
         makefileStream << tname[&ccg.GetCC()] << ": ";
       }
       for (auto const& dep : realDepends) {
-        makefileStream << "\\\n" << this->ConvertToRelativeForMake(dep);
+        makefileStream << "\\\n" << ConvertToMakefilePath(dep);
       }
       makefileStream << '\n';
 
@@ -2317,12 +2337,12 @@ void cmGlobalXCodeGenerator::CreateCustomRulesMakefile(
         // Build the command line in a single string.
         std::string cmd2 = ccg.GetCommand(c);
         cmSystemTools::ReplaceString(cmd2, "/./", "/");
-        cmd2 = this->ConvertToRelativeForMake(cmd2);
+        cmd2 = ConvertToMakefilePath(cmd2);
         std::string cmd;
         std::string wd = ccg.GetWorkingDirectory();
         if (!wd.empty()) {
           cmd += "cd ";
-          cmd += this->ConvertToRelativeForMake(wd);
+          cmd += ConvertToMakefilePath(wd);
           cmd += " && ";
         }
         cmd += cmd2;
@@ -2336,7 +2356,7 @@ void cmGlobalXCodeGenerator::CreateCustomRulesMakefile(
               target->GetLocalGenerator()->GetMakefile()->GetSource(
                 dep, cmSourceFileLocationKind::Known)) {
           if (dsf->GetPropertyAsBool("SYMBOLIC")) {
-            makefileStream << this->ConvertToRelativeForMake(dep) << ":\n";
+            makefileStream << ConvertToMakefilePath(dep) << ":\n";
           }
         }
       }
@@ -4865,7 +4885,7 @@ void cmGlobalXCodeGenerator::CreateXCodeDependHackMakefile(
           gt->GetType() == cmStateEnums::SHARED_LIBRARY ||
           gt->GetType() == cmStateEnums::MODULE_LIBRARY) {
         std::string tfull = gt->GetFullPath(configName);
-        std::string trel = this->ConvertToRelativeForMake(tfull);
+        std::string trel = ConvertToMakefilePath(tfull);
 
         // Add this target to the post-build phases of its dependencies.
         auto const y = target->GetDependTargets().find(configName);
@@ -4891,7 +4911,7 @@ void cmGlobalXCodeGenerator::CreateXCodeDependHackMakefile(
         auto const x = target->GetDependLibraries().find(configName);
         if (x != target->GetDependLibraries().end()) {
           for (auto const& deplib : x->second) {
-            std::string file = this->ConvertToRelativeForMake(deplib);
+            std::string file = ConvertToMakefilePath(deplib);
             makefileStream << "\\\n\t" << file;
             dummyRules.insert(file);
           }
@@ -4903,7 +4923,7 @@ void cmGlobalXCodeGenerator::CreateXCodeDependHackMakefile(
           std::string d = cmStrCat(this->GetTargetTempDir(gt, configName),
                                    "/lib", objLibName, ".a");
 
-          std::string dependency = this->ConvertToRelativeForMake(d);
+          std::string dependency = ConvertToMakefilePath(d);
           makefileStream << "\\\n\t" << dependency;
           dummyRules.insert(dependency);
         }
@@ -4911,7 +4931,7 @@ void cmGlobalXCodeGenerator::CreateXCodeDependHackMakefile(
         // Write the action to remove the target if it is out of date.
         makefileStream << "\n"
                           "\t/bin/rm -f "
-                       << this->ConvertToRelativeForMake(tfull) << '\n';
+                       << ConvertToMakefilePath(tfull) << '\n';
         // if building for more than one architecture
         // then remove those executables as well
         if (this->Architectures.size() > 1) {
@@ -4921,8 +4941,7 @@ void cmGlobalXCodeGenerator::CreateXCodeDependHackMakefile(
             std::string universalFile = cmStrCat(universal, architecture, '/',
                                                  gt->GetFullName(configName));
             makefileStream << "\t/bin/rm -f "
-                           << this->ConvertToRelativeForMake(universalFile)
-                           << '\n';
+                           << ConvertToMakefilePath(universalFile) << '\n';
           }
         }
         makefileStream << "\n\n";
@@ -5123,12 +5142,6 @@ cmDocumentationEntry cmGlobalXCodeGenerator::GetDocumentation()
            "Generate Xcode project files." };
 }
 
-std::string cmGlobalXCodeGenerator::ConvertToRelativeForMake(
-  std::string const& p)
-{
-  return cmSystemTools::ConvertToOutputPath(p);
-}
-
 std::string cmGlobalXCodeGenerator::RelativeToSource(const std::string& p)
 {
   std::string const& rootSrc =

Source/cmGlobalXCodeGenerator.h

@@ -163,7 +163,6 @@ private:
   std::string RelativeToSource(const std::string& p);
   std::string RelativeToRootBinary(const std::string& p);
   std::string RelativeToBinary(const std::string& p);
-  std::string ConvertToRelativeForMake(std::string const& p);
   void CreateCustomCommands(
     cmXCodeObject* buildPhases, cmXCodeObject* sourceBuildPhase,
     cmXCodeObject* headerBuildPhase, cmXCodeObject* resourceBuildPhase,

Source/cmLocalGenerator.cxx

@@ -4403,7 +4403,12 @@ void CreateGeneratedSource(cmLocalGenerator& lg, const std::string& output,
   }
 
   // Make sure the output file name has no invalid characters.
-  std::string::size_type pos = output.find_first_of("#<>");
+  const bool hashNotAllowed = lg.GetState()->UseBorlandMake();
+  std::string::size_type pos = output.find_first_of("<>");
+  if (pos == std::string::npos && hashNotAllowed) {
+    pos = output.find_first_of('#');
+  }
+
   if (pos != std::string::npos) {
     lg.GetCMakeInstance()->IssueMessage(
       MessageType::FATAL_ERROR,

Source/cmState.cxx

@@ -716,6 +716,16 @@ bool cmState::UseGhsMultiIDE() const
   return this->GhsMultiIDE;
 }
 
+void cmState::SetBorlandMake(bool borlandMake)
+{
+  this->BorlandMake = borlandMake;
+}
+
+bool cmState::UseBorlandMake() const
+{
+  return this->BorlandMake;
+}
+
 void cmState::SetWatcomWMake(bool watcomWMake)
 {
   this->WatcomWMake = watcomWMake;

Source/cmState.h

@@ -213,6 +213,8 @@ public:
   bool UseWindowsVSIDE() const;
   void SetGhsMultiIDE(bool ghsMultiIDE);
   bool UseGhsMultiIDE() const;
+  void SetBorlandMake(bool borlandMake);
+  bool UseBorlandMake() const;
   void SetWatcomWMake(bool watcomWMake);
   bool UseWatcomWMake() const;
   void SetMinGWMake(bool minGWMake);
@@ -294,6 +296,7 @@ private:
   bool WindowsShell = false;
   bool WindowsVSIDE = false;
   bool GhsMultiIDE = false;
+  bool BorlandMake = false;
   bool WatcomWMake = false;
   bool MinGWMake = false;
   bool NMake = false;

Tests/CustomCommand/CMakeLists.txt

@@ -588,3 +588,12 @@ add_custom_target(drive_mac_fw ALL DEPENDS mac_fw.txt)
 # Test empty COMMANDs are omitted
 add_executable(empty_command empty_command.cxx)
 add_custom_command(TARGET empty_command POST_BUILD COMMAND $<0:date>)
+
+# Test OUTPUT allows filenames containing "#" on generators that support this
+if(NOT CMAKE_GENERATOR MATCHES "Borland Makefiles")
+  add_custom_target(file_with_hash ALL DEPENDS "${PROJECT_BINARY_DIR}/hash#in#name.txt")
+  add_custom_command(
+    OUTPUT "${PROJECT_BINARY_DIR}/hash#in#name.txt"
+    COMMAND ${CMAKE_COMMAND} -E touch "${PROJECT_BINARY_DIR}/hash#in#name.txt"
+  )
+endif()

Tests/RunCMake/add_custom_command/BadByproduct-stderr.txt

@@ -1,9 +1,3 @@
-CMake Error at BadByproduct.cmake:2 \(add_custom_command\):
-  BYPRODUCTS containing a "#" is not allowed.
-Call Stack \(most recent call first\):
-  CMakeLists.txt:3 \(include\)
-
-
 CMake Error at BadByproduct.cmake:3 \(add_custom_command\):
   BYPRODUCTS containing a "<" is not allowed.
 Call Stack \(most recent call first\):
@@ -17,7 +11,7 @@ Call Stack \(most recent call first\):
 
 (
 CMake Error at BadByproduct.cmake:5 \(add_custom_command\):
-  BYPRODUCTS containing a "#" is not allowed.
+  BYPRODUCTS containing a ">" is not allowed.
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)
 

Tests/RunCMake/add_custom_command/BadByproduct.cmake

@@ -1,8 +1,8 @@
 set(CMAKE_DISABLE_SOURCE_CHANGES ON)
-add_custom_command(OUTPUT a BYPRODUCTS "a#")
+
 add_custom_command(OUTPUT b BYPRODUCTS "a<")
 add_custom_command(OUTPUT c BYPRODUCTS "a>")
-add_custom_command(OUTPUT d BYPRODUCTS "$<CONFIG>/#")
+add_custom_command(OUTPUT d BYPRODUCTS "$<CONFIG>/$<ANGLE-R>")
 add_custom_command(OUTPUT e BYPRODUCTS ${CMAKE_CURRENT_SOURCE_DIR}/f)
 add_custom_command(OUTPUT f BYPRODUCTS "$<TARGET_PROPERTY:prop>")
 add_custom_command(OUTPUT h BYPRODUCTS "$<OUTPUT_CONFIG:h>")

Tests/RunCMake/add_custom_command/BadOutput-stderr.txt

@@ -1,9 +1,3 @@
-CMake Error at BadOutput.cmake:2 \(add_custom_command\):
-  OUTPUT containing a "#" is not allowed.
-Call Stack \(most recent call first\):
-  CMakeLists.txt:3 \(include\)
-
-
 CMake Error at BadOutput.cmake:3 \(add_custom_command\):
   OUTPUT containing a "<" is not allowed.
 Call Stack \(most recent call first\):
@@ -17,7 +11,7 @@ Call Stack \(most recent call first\):
 
 (
 CMake Error at BadOutput.cmake:5 \(add_custom_command\):
-  OUTPUT containing a "#" is not allowed.
+  OUTPUT containing a ">" is not allowed.
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)
 

Tests/RunCMake/add_custom_command/BadOutput.cmake

@@ -1,8 +1,8 @@
 set(CMAKE_DISABLE_SOURCE_CHANGES ON)
-add_custom_command(OUTPUT "a#" COMMAND a)
+
 add_custom_command(OUTPUT "a<" COMMAND b)
 add_custom_command(OUTPUT "a>" COMMAND c)
-add_custom_command(OUTPUT "$<CONFIG>/#" COMMAND d)
+add_custom_command(OUTPUT "$<CONFIG>/$<ANGLE-R>" COMMAND d)
 add_custom_command(OUTPUT ${CMAKE_CURRENT_SOURCE_DIR}/e COMMAND f)
 add_custom_command(OUTPUT "$<TARGET_PROPERTY:prop>" COMMAND g)
 add_custom_command(OUTPUT "$<OUTPUT_CONFIG:h>" COMMAND h)

Tests/RunCMake/add_custom_target/BadByproduct-stderr.txt

@@ -1,9 +1,3 @@
-CMake Error at BadByproduct.cmake:2 \(add_custom_target\):
-  BYPRODUCTS containing a "#" is not allowed.
-Call Stack \(most recent call first\):
-  CMakeLists.txt:3 \(include\)
-
-
 CMake Error at BadByproduct.cmake:3 \(add_custom_target\):
   BYPRODUCTS containing a "<" is not allowed.
 Call Stack \(most recent call first\):
@@ -17,7 +11,7 @@ Call Stack \(most recent call first\):
 
 (
 CMake Error at BadByproduct.cmake:5 \(add_custom_target\):
-  BYPRODUCTS containing a "#" is not allowed.
+  BYPRODUCTS containing a ">" is not allowed.
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)
 

Tests/RunCMake/add_custom_target/BadByproduct.cmake

@@ -1,8 +1,8 @@
 set(CMAKE_DISABLE_SOURCE_CHANGES ON)
-add_custom_target(a BYPRODUCTS "a#" COMMAND b)
+
 add_custom_target(c BYPRODUCTS "a<" COMMAND d)
 add_custom_target(e BYPRODUCTS "a>" COMMAND f)
-add_custom_target(g BYPRODUCTS "$<CONFIG>/#" COMMAND h)
+add_custom_target(g BYPRODUCTS "$<CONFIG>/$<ANGLE-R>" COMMAND h)
 add_custom_target(i BYPRODUCTS ${CMAKE_CURRENT_SOURCE_DIR}/j COMMAND k)
 add_custom_target(l BYPRODUCTS "$<TARGET_PROPERTY:prop>" COMMAND m)
 add_custom_target(n BYPRODUCTS "$<OUTPUT_CONFIG:n>" COMMAND o)