Merge topic 'install-name-dir-no-separator-conversion'

726e6454b5 Ninja,Makefile: Do not convert separators in the install-name directory

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

Source/cmMakefileLibraryTargetGenerator.cxx

@@ -860,9 +860,10 @@ void cmMakefileLibraryTargetGenerator::WriteLibraryRules(
       if (install_name_dir.empty()) {
         vars.TargetInstallNameDir = "";
       } else {
-        // Convert to a path for the native build tool.
-        install_name_dir = this->LocalGenerator->ConvertToOutputFormat(
-          install_name_dir, cmOutputConverter::SHELL);
+        // The install name is target data embedded in the binary, not a
+        // host path: escape it for the shell without converting separators.
+        install_name_dir =
+          this->LocalGenerator->EscapeForShell(install_name_dir, true);
         vars.TargetInstallNameDir = install_name_dir.c_str();
       }
     }

Source/cmNinjaNormalTargetGenerator.cxx

@@ -1074,8 +1074,9 @@ void cmNinjaNormalTargetGenerator::WriteNvidiaDeviceLinkStatement(
       std::string install_dir =
         this->GetGeneratorTarget()->GetInstallNameDirForBuildTree(config);
       if (!install_dir.empty()) {
-        vars["INSTALLNAME_DIR"] = localGen.ConvertToOutputFormat(
-          install_dir, cmOutputConverter::SHELL);
+        // The install name is target data embedded in the binary,
+        // not a host path: escape it without converting separators.
+        vars["INSTALLNAME_DIR"] = localGen.EscapeForShell(install_dir, true);
       }
     }
   }
@@ -1437,8 +1438,9 @@ void cmNinjaNormalTargetGenerator::WriteLinkStatement(
     if (targetType == cm::TargetType::SHARED_LIBRARY) {
       std::string install_dir = gt->GetInstallNameDirForBuildTree(config);
       if (!install_dir.empty()) {
-        vars["INSTALLNAME_DIR"] = localGen.ConvertToOutputFormat(
-          install_dir, cmOutputConverter::SHELL);
+        // The install name is target data embedded in the binary,
+        // not a host path: escape it without converting separators.
+        vars["INSTALLNAME_DIR"] = localGen.EscapeForShell(install_dir, true);
       }
     }
   }

Tests/RunCMake/Make/InstallNameDir-check.cmake

@@ -0,0 +1,17 @@
+# The install-name directory is target data embedded in the binary, so it
+# must keep its forward slashes on every host.
+foreach(case "rpath_dir;@rpath/" "custom_dir;/custom/dir/")
+  list(GET case 0 target)
+  list(GET case 1 dir)
+  # Generators without link scripts inline the link command in build.make.
+  set(link_txt "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/${target}.dir/link.txt")
+  if(NOT EXISTS "${link_txt}")
+    set(link_txt "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/${target}.dir/build.make")
+  endif()
+  file(READ "${link_txt}" content)
+  string(FIND "${content}" "-install_name ${dir}" pos)
+  if(pos EQUAL -1)
+    string(APPEND RunCMake_TEST_FAILED
+      "${link_txt} does not contain '-install_name ${dir}':\n${content}")
+  endif()
+endforeach()

Tests/RunCMake/Make/InstallNameDir.cmake

@@ -0,0 +1,17 @@
+enable_language(C)
+
+# Pretend the platform supports install names, as Darwin does, so that the
+# generator writes the install-name directory on every host.
+set(CMAKE_PLATFORM_HAS_INSTALLNAME 1)
+set(CMAKE_SHARED_LIBRARY_SONAME_C_FLAG "-install_name ")
+set(CMAKE_SHARED_LIBRARY_RUNTIME_C_FLAG "-Wl,-rpath,")
+set(CMAKE_C_CREATE_SHARED_LIBRARY
+  "<CMAKE_C_COMPILER> <SONAME_FLAG><TARGET_INSTALLNAME_DIR><TARGET_SONAME> -o <TARGET> <OBJECTS>")
+
+add_library(rpath_dir SHARED hello.c)
+set_property(TARGET rpath_dir PROPERTY AIX_SHARED_LIBRARY_ARCHIVE OFF)
+
+add_library(custom_dir SHARED hello.c)
+set_property(TARGET custom_dir PROPERTY INSTALL_NAME_DIR "/custom/dir")
+set_property(TARGET custom_dir PROPERTY BUILD_WITH_INSTALL_NAME_DIR ON)
+set_property(TARGET custom_dir PROPERTY AIX_SHARED_LIBRARY_ARCHIVE OFF)

Tests/RunCMake/Make/RunCMakeTest.cmake

@@ -262,3 +262,5 @@ if(FAKE_MAKE AND RunCMake_GENERATOR MATCHES "Unix Makefiles|MinGW Makefiles|MSYS
   endfunction()
   run_OutputSyncUsesTerminal()
 endif()
+
+run_cmake(InstallNameDir)

Tests/RunCMake/Ninja/InstallNameDir-check.cmake

@@ -0,0 +1,12 @@
+# The install-name directory is target data embedded in the binary, so it
+# must keep its forward slashes on every host.
+set(build_ninja "${RunCMake_TEST_BINARY_DIR}/build.ninja")
+file(READ "${build_ninja}" content)
+foreach(expected "INSTALLNAME_DIR = @rpath/\n" "INSTALLNAME_DIR = /custom/dir/\n")
+  string(FIND "${content}" "${expected}" pos)
+  if(pos EQUAL -1)
+    string(STRIP "${expected}" expected)
+    string(APPEND RunCMake_TEST_FAILED
+      "${build_ninja} does not contain the line:\n  ${expected}\n")
+  endif()
+endforeach()

Tests/RunCMake/Ninja/InstallNameDir.cmake

@@ -0,0 +1,15 @@
+enable_language(C)
+
+# Pretend the platform supports install names, as Darwin does, so that the
+# generator writes the install-name directory on every host.
+set(CMAKE_PLATFORM_HAS_INSTALLNAME 1)
+set(CMAKE_SHARED_LIBRARY_SONAME_C_FLAG "-install_name ")
+set(CMAKE_SHARED_LIBRARY_RUNTIME_C_FLAG "-Wl,-rpath,")
+set(CMAKE_C_CREATE_SHARED_LIBRARY
+  "<CMAKE_C_COMPILER> <SONAME_FLAG><TARGET_INSTALLNAME_DIR><TARGET_SONAME> -o <TARGET> <OBJECTS>")
+
+add_library(rpath_dir SHARED hello.c)
+
+add_library(custom_dir SHARED hello.c)
+set_property(TARGET custom_dir PROPERTY INSTALL_NAME_DIR "/custom/dir")
+set_property(TARGET custom_dir PROPERTY BUILD_WITH_INSTALL_NAME_DIR ON)

Tests/RunCMake/Ninja/RunCMakeTest.cmake

@@ -465,3 +465,5 @@ endif()
 run_cmake(LINK_OPTIONSWithNewlines)
 
 run_cmake(StaticLibShort)
+
+run_cmake(InstallNameDir)