[llvm-bugs] [Bug 41030] New: Wrong install path expansion in multi-configuration generated build (VS 2017)

Mon Mar 11 09:01:12 PDT 2019

https://bugs.llvm.org/show_bug.cgi?id=41030

            Bug ID: 41030
           Summary: Wrong install path expansion in multi-configuration
                    generated build (VS 2017)
           Product: Build scripts
           Version: trunk
          Hardware: PC
                OS: Windows NT
            Status: NEW
          Severity: normal
          Priority: P
         Component: cmake
          Assignee: unassignedbugs at nondot.org
          Reporter: richard.musil at gmail.com
                CC: llvm-bugs at lists.llvm.org

Created attachment 21583
  --> https://bugs.llvm.org/attachment.cgi?id=21583&action=edit
Patch file

I am using Visual Studio 2017 build tools (cmake version 3.12.18081601-MSVC_2)
to build LLVM project(s) by running this commands:

> cmake -G "Visual Studio 15 2017" -A x64 -Thost=x64 -DCMAKE_INSTALL_PREFIX=D:\Dev\LLVM-ng -DLLVM_ENABLE_PROJECTS=clang;clang-tools-extra;compiler-rt;debuginfo-tests;libclc;libcxx;libcxxabi;libunwind;lld;llvm;parallel-libs;polly;pstl ..
> cmake --build . --target INSTALL --config Release

Build phase passes alright (--target ALL_BUILD) but install phase bugs out with
an error about missing file source(s). This happens at two occasions when
compiler-rt libs and clang headers are going to be installed.

When looking at corresponding cmake_install.cmake file for compiler-rt libs I
noticed following snippets:

<build-1>/projects/compiler-rt/lib/asan/cmake_install.cmake

> if("x${CMAKE_INSTALL_COMPONENT}x" STREQUAL "xasanx" OR NOT CMAKE_INSTALL_COMPONENT)
>   if("${CMAKE_INSTALL_CONFIG_NAME}" MATCHES "^([Dd][Ee][Bb][Uu][Gg])$")
>     file(INSTALL DESTINATION "${CMAKE_INSTALL_PREFIX}/lib/clang/9.0.0/lib/windows" TYPE STATIC_LIBRARY FILES "D:/Work_OSS/llvm-project/llvm/build-1/$(Configuration)/lib/clang/9.0.0/lib/windows/clang_rt.asan-x86_64.lib")
>   elseif("${CMAKE_INSTALL_CONFIG_NAME}" MATCHES "^([Rr][Ee][Ll][Ee][Aa][Ss][Ee])$")
>     file(INSTALL DESTINATION "${CMAKE_INSTALL_PREFIX}/lib/clang/9.0.0/lib/windows" TYPE STATIC_LIBRARY FILES "D:/Work_OSS/llvm-project/llvm/build-1/$(Configuration)/lib/clang/9.0.0/lib/windows/clang_rt.asan-x86_64.lib")
>   elseif("${CMAKE_INSTALL_CONFIG_NAME}" MATCHES "^([Mm][Ii][Nn][Ss][Ii][Zz][Ee][Rr][Ee][Ll])$")
>     file(INSTALL DESTINATION "${CMAKE_INSTALL_PREFIX}/lib/clang/9.0.0/lib/windows" TYPE STATIC_LIBRARY FILES "D:/Work_OSS/llvm-project/llvm/build-1/$(Configuration)/lib/clang/9.0.0/lib/windows/clang_rt.asan-x86_64.lib")
>   elseif("${CMAKE_INSTALL_CONFIG_NAME}" MATCHES "^([Rr][Ee][Ll][Ww][Ii][Tt][Hh][Dd][Ee][Bb][Ii][Nn][Ff][Oo])$")
>     file(INSTALL DESTINATION "${CMAKE_INSTALL_PREFIX}/lib/clang/9.0.0/lib/windows" TYPE STATIC_LIBRARY FILES "D:/Work_OSS/llvm-project/llvm/build-1/$(Configuration)/lib/clang/9.0.0/lib/windows/clang_rt.asan-x86_64.lib")
>   endif()
> endif()

where the problem is $(Configuration) in the path. So even though the multiple
configurations (Release, Debug, etc.) are properly expanded in the script, they
are all set with the same (wrong) path.

The corresponding script responsible for this is
compiler-rt/cmake/Modules/AddCompilerRT.cmake, this function in particular:

> function(set_target_output_directories target output_dir)
>   # For RUNTIME_OUTPUT_DIRECTORY variable, Multi-configuration generators
>   # append a per-configuration subdirectory to the specified directory.
>   # To avoid the appended folder, the configuration specific variable must be
>   # set 'RUNTIME_OUTPUT_DIRECTORY_${CONF}':
>   # RUNTIME_OUTPUT_DIRECTORY_DEBUG, RUNTIME_OUTPUT_DIRECTORY_RELEASE, ...
>   if(CMAKE_CONFIGURATION_TYPES)
>     foreach(build_mode ${CMAKE_CONFIGURATION_TYPES})
>       string(TOUPPER "${build_mode}" CONFIG_SUFFIX)"${output_dir}")
>       set_target_properties("${target}" PROPERTIES
>           "ARCHIVE_OUTPUT_DIRECTORY_${CONFIG_SUFFIX}" ${output_dir}
>           "LIBRARY_OUTPUT_DIRECTORY_${CONFIG_SUFFIX}" ${output_dir}
>           "RUNTIME_OUTPUT_DIRECTORY_${CONFIG_SUFFIX}" ${output_dir})
>     endforeach()
>   else()
>     set_target_properties("${target}" PROPERTIES
>         ARCHIVE_OUTPUT_DIRECTORY ${output_dir}
>         LIBRARY_OUTPUT_DIRECTORY ${output_dir}
>         RUNTIME_OUTPUT_DIRECTORY ${output_dir})
>   endif()
> endfunction()

Where ${output_dir} contains the path with "$(Configuration)". It seems that
the author's idea was that this will get expanded, but when looking at cmake
doc, and in particular at target properties
(https://cmake.org/cmake/help/latest/prop_tgt/RUNTIME_OUTPUT_DIRECTORY.html) it
says only 'generator expression' gets expanded, but not platform specific build
time variables.

It seems that some cmake scripts authors were already aware of that as for
example llvm/cmake/modules/AddLLVM.cmake, which took this approach:

> if(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
>   foreach(build_mode ${CMAKE_CONFIGURATION_TYPES})
>     string(TOUPPER "${build_mode}" CONFIG_SUFFIX)
>     if(ARG_BINARY_DIR)
>       string(REPLACE ${CMAKE_CFG_INTDIR} ${build_mode} bi ${ARG_BINARY_DIR})
>       set_target_properties(${target} PROPERTIES "RUNTIME_OUTPUT_DIRECTORY_${CONFIG_SUFFIX}" ${bi})
>     endif()
>     if(ARG_LIBRARY_DIR)
>       string(REPLACE ${CMAKE_CFG_INTDIR} ${build_mode} li ${ARG_LIBRARY_DIR})
>       set_target_properties(${target} PROPERTIES "ARCHIVE_OUTPUT_DIRECTORY_${CONFIG_SUFFIX}" ${li})
>     endif()
>     if(module_dir)
>       string(REPLACE ${CMAKE_CFG_INTDIR} ${build_mode} mi ${module_dir})
>       set_target_properties(${target} PROPERTIES "LIBRARY_OUTPUT_DIRECTORY_${CONFIG_SUFFIX}" ${mi})
>     endif()
>   endforeach()
> else()

I.e. generating configuration specific target properties and replacing
"$(Configuration)" by the desired config.

When looking at the cmake doc, I also noticed that while non
multi-configuration target properties (e.g. RUNTIME_OUTPUT_DIRECTORY) are
supposed  to be suffixed by multi-configuration generator with the
configuration name, this _does not_ happen, when the property contains
expression generator.

In short, the original function can be fixed by using "$<CONFIG>" expression
generator, like this:

> function(set_target_output_directories target output_dir)
>   # If we are using multi-configuration generator ensure proper folder
>   # expansion by using $<CONFIG> generator expression
>   if(CMAKE_CONFIGURATION_TYPES)
>     string(REPLACE "${CMAKE_CFG_INTDIR}" "$<CONFIG>" output_dir "${output_dir}")
>   endif()
>   set_target_properties("${target}" PROPERTIES
>       ARCHIVE_OUTPUT_DIRECTORY ${output_dir}
>       LIBRARY_OUTPUT_DIRECTORY ${output_dir}
>       RUNTIME_OUTPUT_DIRECTORY ${output_dir})
> endfunction()

The other occurrence related to clang headers is of the similar kind (applied
to "install" cmake command), so I am not going to dissect it here, only
interesting point is that it was not present in 7.0.1 release and apparently
crept into the current branch later.

As the bug only manifests itself in the install phase I assume the install
phase is not normally validated before release. I would suggest adding it to
the validation.

Proposed patch attached is generated against current master.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20190311/971167f5/attachment.html>