[llvm][cmake] Make `install_symlink` workflow work with absolute install dirs

[Ericson2314]

Summary

If CMAKE_INSTALL_BINDIR is a different absolute path per project, as
it is with NixOS when we install every package to its own prefix, the
old way fails when the absolute path gets prepended with CMAKE_INSTALL_PREFIX.

The extend_path function does what we want, but it is currently internal-only. So easier to just inline the one small case of it we need.

Also fix one stray bin -> CMAKE_INSTALL_BINDIR

[Ericson2314]

1. Updating D101070: Make llvm_install_symlink robust with respect to absolute dirs. #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

rebase

[Ericson2314]

Much simpler now

Code taken from D99484 and then to D117419 and now to here.

[Ericson2314]

Continuing my bisect of D99484 (since this change was in there at the time it was approved, but failing) I will land this one next.

[Ericson2314]

This seems to be the part of D99484 that wasn't working.

[Ericson2314]

Rebase, hopefully fix, and also fix stray GNUInstallDirs violation

[Ericson2314]

Rebase to retrigger CI

I think it tried to cherry-pick an already-landed patch marked as a parent revision.

[sebastian-ne]

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.

[sebastian-ne]

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

[Ericson2314]

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

[Ericson2314]

Rebase, make caller responsible for making paths absolute

This avoids module resolution issues.

[sebastian-ne]

I get a configure error with this version:

CMake Error at cmake/modules/AddLLVM.cmake:2052 (extend_path):
  Unknown CMake command "extend_path".
Call Stack (most recent call first):
  cmake/modules/AddLLVM.cmake:2148 (llvm_install_symlink)
  cmake/modules/AddLLVM.cmake:2154 (llvm_add_tool_symlink)
  tools/llvm-ar/CMakeLists.txt:22 (add_llvm_tool_symlink)

[Ericson2314]

Oops! ExtendPath was not already imported. Thanks, @sebastian-ne

[Ericson2314]

Also sorry, I had missed D130256 all along!

[sebastian-ne]

No problem, thanks for the fixes!
LGTM

[Ericson2314]

Avoid extend_path

llvm/cmake/modules/AddLLVM.cmake

add_lit_target(${target} ${comment}
     DEPENDS ${ARG_DEPENDS}
     ARGS ${ARG_ARGS}
     )
 endfunction()
 
 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()
 
     # Search recursively for test directories by assuming anything not
     # in a directory called Inputs contains tests.
     file(GLOB_RECURSE to_process LIST_DIRECTORIES true ${directory}/*)
     foreach(lit_suite ${to_process})
if(NOT component)
     set(component ${name})
   endif()
 
   set(full_name ${CMAKE_${type}_LIBRARY_PREFIX}${name}${CMAKE_${type}_LIBRARY_SUFFIX})
   set(full_dest ${CMAKE_${type}_LIBRARY_PREFIX}${dest}${CMAKE_${type}_LIBRARY_SUFFIX})
 
   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(SCRIPT ${INSTALL_SYMLINK}
-          CODE "install_symlink(${full_name} ${full_dest} ${output_dir})"
+          CODE "install_symlink(\"${full_name}\" \"${full_dest}\" \"${output_dir}\")"
           COMPONENT ${component})
 
 endfunction()
 
 function(llvm_install_symlink project name dest)
   get_property(LLVM_DRIVER_TOOLS GLOBAL PROPERTY LLVM_DRIVER_TOOLS)
   if(LLVM_TOOL_LLVM_DRIVER_BUILD AND ${dest} IN_LIST LLVM_DRIVER_TOOLS)
     set_property(GLOBAL APPEND PROPERTY LLVM_DRIVER_TOOL_SYMLINKS ${name})
function(llvm_install_symlink project name dest)
   endif()
 
   set(full_name ${name}${CMAKE_EXECUTABLE_SUFFIX})
   set(full_dest ${dest}${CMAKE_EXECUTABLE_SUFFIX})
   if (${dest} STREQUAL "llvm-driver")
     set(full_dest llvm${CMAKE_EXECUTABLE_SUFFIX})
   endif()
 
+  set(output_dir "${${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)
     add_llvm_install_targets(install-${name}
                              DEPENDS ${name} ${dest}
                              COMPONENT ${name}
                              SYMLINK ${dest})
   endif()

llvm/cmake/modules/LLVMInstallSymlink.cmake

 # We need to execute this script at installation time because the
 # DESTDIR environment variable may be unset at configuration time.
 # See PR8397.
 
 include(GNUInstallDirs)
 
 function(install_symlink name target outdir)
   set(DESTDIR $ENV{DESTDIR})
-  set(bindir "${DESTDIR}${CMAKE_INSTALL_PREFIX}/${outdir}")
+  if(NOT IS_ABSOLUTE "${outdir}")
+    set(outdir "${CMAKE_INSTALL_PREFIX}/${outdir}")
+  endif()
+  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()