rapids_test_install_relocatable now handles SOVERSION target properties

[robertmaynard]

Description

Previously the rapids_test_install_relocatable would presume the file(INSTALL command would always be a single line. But cmake generates a multi line entry when doing the SOVERSION symlinks, and rapids-cmake would crash when encountering them.

Now the determine_install_location_of_all_targets internal function does proper parsing to extract the file(INSTALL commands so we can support multi-line entries.

Checklist

• [x] I am familiar with the Contributing Guidelines.
• [x] New or existing tests cover these changes.
• [x] The documentation is up to date with these changes.

[robertmaynard]

Regarding the overall design, would it be possible to use the File API for this use case? Parsing cmake_install.cmake in this manner is very brittle, and the File API now allows the project itself to request API output (though I realize it might not be feasible to use this until we require CMake 3.27.)

Looking at the file api I am seeing some holes that would be required to switch over to it ( a welcome change ).

baseline

cmake_file_api(
  QUERY
  API_VERSION 1
  CODEMODEL 2.3
  TOOLCHAINS 1
)
add_library(preload SHARED usage.cu)
set_target_properties(preload PROPERTIES
  VERSION 1.1.1
  SOVERSION 1
  LIBRARY_OUTPUT_DIRECTORY my_libs
  )
install(TARGETS preload DESTINATION lib/nested)

First the install component has a minor bug where double lists the relative part:

"install" : {
  "destinations" : [
    {
      "backtrace" : 2,
      "path" : "lib/nested"
    },
    {
      "backtrace" : 2,
      "path" : "lib/nested"
    }
  ],
  "prefix" : {
    "path" : "/usr/local"
  }
 }

A serious part is that I can't find the build directory location. I believe the nameOnDisk and paths components would be how I compute that, but it isn't correct:

"nameOnDisk" : "libpreload.so",
"paths" : 
{
  "build" : ".",
  "source" : "."
},

Lastly is that target properties like SOVERSION which cause side-effects in the binary and install directory aren't representated in the file API. We need to know about these to properly fix up things like LD_PRELOAD env settings that exist on tests

[KyleFromNVIDIA]

First the install component has a minor bug where double lists the relative part:

Is this something we could fix upstream?

A serious part is that I can't find the build directory location. I believe the nameOnDisk and paths components would be how I compute that, but it isn't correct:

What part specifically is not correct? By "build directory location" do you mean the location of CMakeCache.txt? I believe the File API provides this elsewhere.

Lastly is that target properties like SOVERSION which cause side-effects in the binary and install directory aren't representated in the file API. We need to know about these to properly fix up things like LD_PRELOAD env settings that exist on tests

Again, could we contribute this upstream?

[robertmaynard]

What part specifically is not correct? By "build directory location" do you mean the location of CMakeCache.txt? I believe the File API provides this elsewhere.

I would need the relative path of the shared library on disk from the root of CMAKE_BINARY_DIR. Currently It looks like "build" : ".", is the field but it shoudl be `"build" : "./my_libs/".

Again, could we contribute this upstream?

Absolutley we can add fixing these things upstream to our timeline :)

[KyleFromNVIDIA]

I would need the relative path of the shared library on disk from the root of CMAKE_BINARY_DIR. Currently It looks like "build" : ".", is the field but it shoudl be `"build" : "./my_libs/".

"build": is merely telling you the add_subdirectory() that the target is part of. It doesn't include the output directory. However, "artifacts": does include the output directory.

[robertmaynard]

Ahh perfect

On Thu, Jun 27, 2024 at 2:42 PM Kyle Edwards @.***> wrote:

I would need the relative path of the shared library on disk from the root of CMAKE_BINARY_DIR. Currently It looks like "build" : ".", is the field but it shoudl be `"build" : "./my_libs/".

"build": is merely telling you the add_subdirectory() that the target is part of. It doesn't include the output directory. However, "artifacts": does include the output directory.

— Reply to this email directly, view it on GitHub https://github.com/rapidsai/rapids-cmake/pull/636#issuecomment-2195445099, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABUVTAWE3FLKMCITZ7SNKLZJRMIJAVCNFSM6AAAAABKAJDAFCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJVGQ2DKMBZHE . You are receiving this because you authored the thread.Message ID: @.***>

[KyleFromNVIDIA]

Lastly is that target properties like SOVERSION which cause side-effects in the binary and install directory aren't representated in the file API. We need to know about these to properly fix up things like LD_PRELOAD env settings that exist on tests

Again, could we contribute this upstream?

I've opened https://gitlab.kitware.com/cmake/cmake/-/issues/26087 to discuss this.

[robertmaynard]

/merge