[PATCH] D101070: [llvm][cmake] Make `install_symlink` robust to absolute dirs.

[John Ericson via Phabricator via cfe-commits]

Ericson2314 updated this revision to Diff 447358.
Ericson2314 added a comment.

Rebase, make caller responsible for making paths absolute

This avoids module resolution issues.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101070/new/

https://reviews.llvm.org/D101070

Files:
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/LLVMInstallSymlink.cmake


Index: llvm/cmake/modules/LLVMInstallSymlink.cmake
===
--- llvm/cmake/modules/LLVMInstallSymlink.cmake
+++ llvm/cmake/modules/LLVMInstallSymlink.cmake
@@ -1,22 +1,26 @@
 # We need to execute this script at installation time because the
 # DESTDIR environment variable may be unset at configuration time.
 # See PR8397.
+#
+# `outdir` must be an absolute path. This module gets a very reduced
+# `CMAKE_MODULE_PATH` so it is easier to make the caller the responsible
+# for this.
 
 include(GNUInstallDirs)
 
 function(install_symlink name target outdir)
   set(DESTDIR $ENV{DESTDIR})
-  set(bindir "${DESTDIR}${CMAKE_INSTALL_PREFIX}/${outdir}")
+  set(outdir "${DESTDIR}${outdir}")
 
   message(STATUS "Creating ${name}")
 
   execute_process(
 COMMAND "${CMAKE_COMMAND}" -E create_symlink "${target}" "${name}"
-WORKING_DIRECTORY "${bindir}" ERROR_VARIABLE has_err)
+WORKING_DIRECTORY "${outdir}" ERROR_VARIABLE has_err)
   if(CMAKE_HOST_WIN32 AND has_err)
 execute_process(
   COMMAND "${CMAKE_COMMAND}" -E copy "${target}" "${name}"
-  WORKING_DIRECTORY "${bindir}")
+  WORKING_DIRECTORY "${outdir}")
   endif()
 
 endfunction()
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1950,7 +1950,7 @@
 function(add_lit_testsuites project directory)
   if (NOT LLVM_ENABLE_IDE)
 cmake_parse_arguments(ARG "EXCLUDE_FROM_CHECK_ALL" "FOLDER" 
"PARAMS;DEPENDS;ARGS" ${ARGN})
-
+
 if (NOT ARG_FOLDER)
   set(ARG_FOLDER "Test Subdirectories")
 endif()
@@ -2006,11 +2006,14 @@
 
   set(output_dir lib${LLVM_LIBDIR_SUFFIX})
   if(WIN32 AND "${type}" STREQUAL "SHARED")
-set(output_dir bin)
+set(output_dir "${CMAKE_INSTALL_BINDIR}")
   endif()
 
+  # `install_symlink` needs an absoute path.
+  extend_path(output_dir "${CMAKE_INSTALL_PREFIX}" "${output_dir}")
+
   install(SCRIPT ${INSTALL_SYMLINK}
-  CODE "install_symlink(${full_name} ${full_dest} ${output_dir})"
+  CODE "install_symlink(\"${full_name}\" \"${full_dest}\" 
\"${output_dir}\")"
   COMPONENT ${component})
 
 endfunction()
@@ -2045,8 +2048,11 @@
 set(full_dest llvm${CMAKE_EXECUTABLE_SUFFIX})
   endif()
 
+  # `install_symlink` needs an absoute path.
+  extend_path(output_dir "${CMAKE_INSTALL_PREFIX}" 
"${${project}_TOOLS_INSTALL_DIR}")
+
   install(SCRIPT ${INSTALL_SYMLINK}
-  CODE "install_symlink(${full_name} ${full_dest} 
${${project}_TOOLS_INSTALL_DIR})"
+  CODE "install_symlink(\"${full_name}\" \"${full_dest}\" 
\"${output_dir}\")"
   COMPONENT ${component})
 
   if (NOT LLVM_ENABLE_IDE AND NOT ARG_ALWAYS_GENERATE)


Index: llvm/cmake/modules/LLVMInstallSymlink.cmake
===
--- llvm/cmake/modules/LLVMInstallSymlink.cmake
+++ llvm/cmake/modules/LLVMInstallSymlink.cmake
@@ -1,22 +1,26 @@
 # We need to execute this script at installation time because the
 # DESTDIR environment variable may be unset at configuration time.
 # See PR8397.
+#
+# `outdir` must be an absolute path. This module gets a very reduced
+# `CMAKE_MODULE_PATH` so it is easier to make the caller the responsible
+# for this.
 
 include(GNUInstallDirs)
 
 function(install_symlink name target outdir)
   set(DESTDIR $ENV{DESTDIR})
-  set(bindir "${DESTDIR}${CMAKE_INSTALL_PREFIX}/${outdir}")
+  set(outdir "${DESTDIR}${outdir}")
 
   message(STATUS "Creating ${name}")
 
   execute_process(
 COMMAND "${CMAKE_COMMAND}" -E create_symlink "${target}" "${name}"
-WORKING_DIRECTORY "${bindir}" ERROR_VARIABLE has_err)
+WORKING_DIRECTORY "${outdir}" ERROR_VARIABLE has_err)
   if(CMAKE_HOST_WIN32 AND has_err)
 execute_process(
   COMMAND "${CMAKE_COMMAND}" -E copy "${target}" "${name}"
-  WORKING_DIRECTORY "${bindir}")
+  WORKING_DIRECTORY "${outdir}")
   endif()
 
 endfunction()
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1950,7 +1950,7 @@
 function(add_lit_testsuites project directory)
   if (NOT LLVM_ENABLE_IDE)
 cmake_parse_arguments(ARG "EXCLUDE_FROM_CHECK_ALL" "FOLDER" "PARAMS;DEPENDS;ARGS" ${ARGN})
-
+
 if (NOT ARG_FOLDER)
   set(ARG_FOLDER "Test Subdirectories")
 endif()
@@ -2006,11 +2006,14 @@
 
   set(output_dir lib${LLVM_LIBDIR_SUFFIX})
   if(WIN32 AND "${type}" 

[PATCH] D101070: [llvm][cmake] Make `install_symlink` robust to absolute dirs.

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added a comment.

@sebastian-ne Thanks yes I was worried that might happen. I think I will just 
move the `extend_path` to the caller then.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101070/new/

https://reviews.llvm.org/D101070

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101070: [llvm][cmake] Make `install_symlink` robust to absolute dirs.

[Sebastian Neubauer via Phabricator via cfe-commits]

sebastian-ne added a comment.

In D101070#3675462 , @sebastian-ne 
wrote:

> Hi, not sure if you saw D130256 , but I 
> needed to extend CMAKE_MODULE_PATH, otherwise the ExtendPath module was not 
> found when running LLVMInstallSymlink.cmake.

Just checked, if I run `ninja install` with this patch, it fails with

  CMake Error at /llvm-project/llvm/cmake/modules/LLVMInstallSymlink.cmake:5 
(include):
include could not find requested file:
  
  ExtendPath
  Call Stack (most recent call first):
tools/llvm-ar/cmake_install.cmake:56 (include)
tools/cmake_install.cmake:49 (include)
cmake_install.cmake:78 (include)
  
  
  FAILED: CMakeFiles/install.util


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101070/new/

https://reviews.llvm.org/D101070

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101070: [llvm][cmake] Make `install_symlink` robust to absolute dirs.

[Sebastian Neubauer via Phabricator via cfe-commits]

sebastian-ne added a comment.

Hi, not sure if you saw D130256 , but I 
needed to extend CMAKE_MODULE_PATH, otherwise the ExtendPath module was not 
found when running LLVMInstallSymlink.cmake.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101070/new/

https://reviews.llvm.org/D101070

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101070: [llvm][cmake] Make `install_symlink` robust to absolute dirs.

[John Ericson via Phabricator via cfe-commits]

Ericson2314 updated this revision to Diff 447108.
Ericson2314 added a comment.

Rebase to retrigger CI

I think it tried to cherry-pick an already-landed patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101070/new/

https://reviews.llvm.org/D101070

Files:
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/LLVMInstallSymlink.cmake


Index: llvm/cmake/modules/LLVMInstallSymlink.cmake
===
--- llvm/cmake/modules/LLVMInstallSymlink.cmake
+++ llvm/cmake/modules/LLVMInstallSymlink.cmake
@@ -2,21 +2,22 @@
 # DESTDIR environment variable may be unset at configuration time.
 # See PR8397.
 
-include(GNUInstallDirs)
+include(ExtendPath)
 
 function(install_symlink name target outdir)
   set(DESTDIR $ENV{DESTDIR})
-  set(bindir "${DESTDIR}${CMAKE_INSTALL_PREFIX}/${outdir}")
+  extend_path(outdir_abs "${CMAKE_INSTALL_PREFIX}" "${outdir}")
+  set(outdir_abs "${DESTDIR}${outdir_abs}")
 
   message(STATUS "Creating ${name}")
 
   execute_process(
 COMMAND "${CMAKE_COMMAND}" -E create_symlink "${target}" "${name}"
-WORKING_DIRECTORY "${bindir}" ERROR_VARIABLE has_err)
+WORKING_DIRECTORY "${outdir_abs}" ERROR_VARIABLE has_err)
   if(CMAKE_HOST_WIN32 AND has_err)
 execute_process(
   COMMAND "${CMAKE_COMMAND}" -E copy "${target}" "${name}"
-  WORKING_DIRECTORY "${bindir}")
+  WORKING_DIRECTORY "${outdir_abs}")
   endif()
 
 endfunction()
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -2006,7 +2006,7 @@
 
   set(output_dir lib${LLVM_LIBDIR_SUFFIX})
   if(WIN32 AND "${type}" STREQUAL "SHARED")
-set(output_dir bin)
+set(output_dir "${CMAKE_INSTALL_LIBDIR}")
   endif()
 
   install(SCRIPT ${INSTALL_SYMLINK}


Index: llvm/cmake/modules/LLVMInstallSymlink.cmake
===
--- llvm/cmake/modules/LLVMInstallSymlink.cmake
+++ llvm/cmake/modules/LLVMInstallSymlink.cmake
@@ -2,21 +2,22 @@
 # DESTDIR environment variable may be unset at configuration time.
 # See PR8397.
 
-include(GNUInstallDirs)
+include(ExtendPath)
 
 function(install_symlink name target outdir)
   set(DESTDIR $ENV{DESTDIR})
-  set(bindir "${DESTDIR}${CMAKE_INSTALL_PREFIX}/${outdir}")
+  extend_path(outdir_abs "${CMAKE_INSTALL_PREFIX}" "${outdir}")
+  set(outdir_abs "${DESTDIR}${outdir_abs}")
 
   message(STATUS "Creating ${name}")
 
   execute_process(
 COMMAND "${CMAKE_COMMAND}" -E create_symlink "${target}" "${name}"
-WORKING_DIRECTORY "${bindir}" ERROR_VARIABLE has_err)
+WORKING_DIRECTORY "${outdir_abs}" ERROR_VARIABLE has_err)
   if(CMAKE_HOST_WIN32 AND has_err)
 execute_process(
   COMMAND "${CMAKE_COMMAND}" -E copy "${target}" "${name}"
-  WORKING_DIRECTORY "${bindir}")
+  WORKING_DIRECTORY "${outdir_abs}")
   endif()
 
 endfunction()
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -2006,7 +2006,7 @@
 
   set(output_dir lib${LLVM_LIBDIR_SUFFIX})
   if(WIN32 AND "${type}" STREQUAL "SHARED")
-set(output_dir bin)
+set(output_dir "${CMAKE_INSTALL_LIBDIR}")
   endif()
 
   install(SCRIPT ${INSTALL_SYMLINK}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits