Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[halide:x64-linux] Cmake find_package fails - Libs installed to the wrong folder #19797

Closed
ghost opened this issue Aug 30, 2021 · 18 comments
Closed
Assignees
Labels
category:port-bug The issue is with a library, which is something the port should already support requires:discussion

Comments

@ghost
Copy link

ghost commented Aug 30, 2021

Host Environment

  • OS: Ubuntu 20.04
  • Compiler: GCC 9.3
  • cmake 3.21.1

To Reproduce
Steps to reproduce the behavior:

  1. $ ./vcpkg install Halide:x64-linux

  2. use cmake code in a project:


set(CMAKE_TOOLCHAIN_FILE "$ENV{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake")
project(Halide_lab VERSION 0.1.0)
find_package(Halide REQUIRED)

After I installed Halide that way, in my project, cmake find_package still failed:
The autoscheduler libraries were expected to be in the vcpkg/installed/x64-linux/tools/halide folder, which they weren't.
So I had to copy them by hand from the vcpkg/installed/x64-linux/lib folder to the expected place. But that's a different issue I guess.

@gebdadendo -- Please @ me in that issue as it's my responsibility to fix

Originally posted by @alexreinking in #19635 (comment)

@alexreinking
Copy link
Contributor

alexreinking commented Aug 30, 2021

The problem is with vcpkg_cmake_config_fixup() being very greedy on Linux whereas on Windows it only rewrites bin/*.exe. I posted the following on Slack the other day... need a response:

Where does vcpkg want me to put loadable modules on Linux? On Windows, putting them in bin was fine, since they had a .dll extension. But vcpkg_cmake_config_fixup() apparently breaks my package on Linux since it rewrites bin/ to tools/<port>, whereas on Windows it rewrites bin/*.exe to tools/<port>/*.exe.

@NancyLi1013 NancyLi1013 added the category:port-bug The issue is with a library, which is something the port should already support label Aug 30, 2021
@alexreinking
Copy link
Contributor

alexreinking commented Aug 30, 2021

I wonder if the right solution is to make vcpkg_cmake_config_fixup() smarter about selecting executable targets on Linux. This could be done by shelling out to file, making sure regex matches only add_executable(), etc. etc. AFAICT, these are the offending lines:

foreach(release_target IN LISTS release_targets)
file(READ "${release_target}" contents)
string(REPLACE "${CURRENT_INSTALLED_DIR}" "\${_IMPORT_PREFIX}" contents "${contents}")
string(REGEX REPLACE "\\\${_IMPORT_PREFIX}/bin/([^ \"]+${EXECUTABLE_SUFFIX})" "\${_IMPORT_PREFIX}/tools/${PORT}/\\1" contents "${contents}")
file(WRITE "${release_target}" "${contents}")
endforeach()

@alexreinking
Copy link
Contributor

alexreinking commented Oct 4, 2021

It's been a bit over a month... can I get some advice re: how to resolve this issue? I'm happy to put in the work on either the Halide port or on reworking vcpkg_cmake_config_fixup as I suggested above, or both.

@ras0219-msft @ras0219 @NancyLi1013 @JackBoosY

@dg0yt
Copy link
Contributor

dg0yt commented Oct 4, 2021

IIUC the problem is not generally rewriting bin to tools/port/bin on Linux, but having bin in the original exported config when libraries (or plugins?) indeed belong to lib.

@alexreinking
Copy link
Contributor

alexreinking commented Oct 4, 2021

@dg0yt - Vcpkg's policies require (or at least used to require) that loadable modules be placed in bin, not lib. See the commit message in 3bfdc2f

@dg0yt
Copy link
Contributor

dg0yt commented Oct 4, 2021

Vcpkg's policies require (or at least used to require) that loadable modules be placed in bin, not lib. See the commit message in 3bfdc2f

The comment refers to DLLs which are in bin on Windows. This is not quite the same as shared objects or loadable modules on Linux. For reference, qt5-base installs plugins to plugins on Linux.

@alexreinking
Copy link
Contributor

@dg0yt - so is the resolution to use two different folders based on the platform, or would placing them somewhere like lib/halide all the time be okay?

@dg0yt
Copy link
Contributor

dg0yt commented Oct 5, 2021

so is the resolution to use two different folders based on the platform, or would placing them somewhere like lib/halide all the time be okay?

@alexreinking I can't say for sure for vcpkg. But you already pinged the vcpkg maintainers 9 hours ago so you let us wait for a response.

@JackBoosY JackBoosY self-assigned this Oct 8, 2021
@JackBoosY
Copy link
Contributor

I will take over this issue.

@JackBoosY
Copy link
Contributor

root@usr:/home/usr/work/vcpkg# find packages/halide_x64-linux/tools/halide/
featurization_to_sample    get_host_target            retrain_cost_model         weightsdir_to_weightsfile
root@usr:/home/usr/work/vcpkg# find packages/halide_x64-linux/share/ | xargs grep "featurization_to_sample"
grep: packages/halide_x64-linux/share/: Is a directory
grep: packages/halide_x64-linux/share/HalideHelpers: Is a directory
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:# Import target "Halide::featurization_to_sample" for configuration "Release"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:set_property(TARGET Halide::featurization_to_sample APPEND PROPERTY IMPORTED_CONFIGURATIONS RELEASE)
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:set_target_properties(Halide::featurization_to_sample PROPERTIES
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:  IMPORTED_LOCATION_RELEASE "${_IMPORT_PREFIX}/tools/halide/featurization_to_sample"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:list(APPEND _IMPORT_CHECK_TARGETS Halide::featurization_to_sample )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:list(APPEND _IMPORT_CHECK_FILES_FOR_Halide::featurization_to_sample "${_IMPORT_PREFIX}/tools/halide/featurization_to_sample" )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:# Import target "Halide::featurization_to_sample" for configuration "Debug"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:set_property(TARGET Halide::featurization_to_sample APPEND PROPERTY IMPORTED_CONFIGURATIONS DEBUG)
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:set_target_properties(Halide::featurization_to_sample PROPERTIES
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:  IMPORTED_LOCATION_DEBUG "${_IMPORT_PREFIX}/tools/halide/featurization_to_sample"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:list(APPEND _IMPORT_CHECK_TARGETS Halide::featurization_to_sample )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:list(APPEND _IMPORT_CHECK_FILES_FOR_Halide::featurization_to_sample "${_IMPORT_PREFIX}/tools/halide/featurization_to_sample" )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces.cmake:foreach(_expectedTarget Halide::Tools Halide::ImageIO Halide::RunGenMain Halide::Runtime Halide::retrain_cost_model Halide::featurization_to_sample Halide::get_host_target Halide::weightsdir_to_weightsfile)
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces.cmake:# Create imported target Halide::featurization_to_sample
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces.cmake:add_executable(Halide::featurization_to_sample IMPORTED)
grep: packages/halide_x64-linux/share/halide: Is a directory
grep: packages/halide_x64-linux/share/halide/tools: Is a directory
packages/halide_x64-linux/share/halide/tools/autotune_loop.sh:    ${AUTOSCHED_BIN}/featurization_to_sample ${D}/${FNAME}.featurization $R $P $S ${D}/${FNAME}.sample || echo "featurization_to_sample failed for ${D} (probably because benchmarking failed)"
root@usr:/home/usr/work/vcpkg# find packages/halide_x64-linux/share/ | xargs grep "get_host_target"
grep: packages/halide_x64-linux/share/: Is a directory
grep: packages/halide_x64-linux/share/HalideHelpers: Is a directory
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:# Import target "Halide::get_host_target" for configuration "Release"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:set_property(TARGET Halide::get_host_target APPEND PROPERTY IMPORTED_CONFIGURATIONS RELEASE)
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:set_target_properties(Halide::get_host_target PROPERTIES
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:  IMPORTED_LOCATION_RELEASE "${_IMPORT_PREFIX}/tools/halide/get_host_target"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:list(APPEND _IMPORT_CHECK_TARGETS Halide::get_host_target )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:list(APPEND _IMPORT_CHECK_FILES_FOR_Halide::get_host_target "${_IMPORT_PREFIX}/tools/halide/get_host_target" )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:# Import target "Halide::get_host_target" for configuration "Debug"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:set_property(TARGET Halide::get_host_target APPEND PROPERTY IMPORTED_CONFIGURATIONS DEBUG)
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:set_target_properties(Halide::get_host_target PROPERTIES
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:  IMPORTED_LOCATION_DEBUG "${_IMPORT_PREFIX}/tools/halide/get_host_target"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:list(APPEND _IMPORT_CHECK_TARGETS Halide::get_host_target )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:list(APPEND _IMPORT_CHECK_FILES_FOR_Halide::get_host_target "${_IMPORT_PREFIX}/tools/halide/get_host_target" )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces.cmake:foreach(_expectedTarget Halide::Tools Halide::ImageIO Halide::RunGenMain Halide::Runtime Halide::retrain_cost_model Halide::featurization_to_sample Halide::get_host_target Halide::weightsdir_to_weightsfile)
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces.cmake:# Create imported target Halide::get_host_target
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces.cmake:add_executable(Halide::get_host_target IMPORTED)
grep: packages/halide_x64-linux/share/halide: Is a directory
grep: packages/halide_x64-linux/share/halide/tools: Is a directory
packages/halide_x64-linux/share/halide/tools/autotune_loop.sh:HL_TARGET=`${AUTOSCHED_BIN}/get_host_target avx512 avx512_knl avx512_skylake avx512_cannonlake`
root@usr:/home/usr/work/vcpkg# find packages/halide_x64-linux/share/ | xargs grep "retrain_cost_model"
grep: packages/halide_x64-linux/share/: Is a directory
grep: packages/halide_x64-linux/share/HalideHelpers: Is a directory
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:# Import target "Halide::retrain_cost_model" for configuration "Release"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:set_property(TARGET Halide::retrain_cost_model APPEND PROPERTY IMPORTED_CONFIGURATIONS RELEASE)
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:set_target_properties(Halide::retrain_cost_model PROPERTIES
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:  IMPORTED_LOCATION_RELEASE "${_IMPORT_PREFIX}/tools/halide/retrain_cost_model"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:list(APPEND _IMPORT_CHECK_TARGETS Halide::retrain_cost_model )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:list(APPEND _IMPORT_CHECK_FILES_FOR_Halide::retrain_cost_model "${_IMPORT_PREFIX}/tools/halide/retrain_cost_model" )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:# Import target "Halide::retrain_cost_model" for configuration "Debug"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:set_property(TARGET Halide::retrain_cost_model APPEND PROPERTY IMPORTED_CONFIGURATIONS DEBUG)
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:set_target_properties(Halide::retrain_cost_model PROPERTIES
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:  IMPORTED_LOCATION_DEBUG "${_IMPORT_PREFIX}/tools/halide/retrain_cost_model"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:list(APPEND _IMPORT_CHECK_TARGETS Halide::retrain_cost_model )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:list(APPEND _IMPORT_CHECK_FILES_FOR_Halide::retrain_cost_model "${_IMPORT_PREFIX}/tools/halide/retrain_cost_model" )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces.cmake:foreach(_expectedTarget Halide::Tools Halide::ImageIO Halide::RunGenMain Halide::Runtime Halide::retrain_cost_model Halide::featurization_to_sample Halide::get_host_target Halide::weightsdir_to_weightsfile)
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces.cmake:# Create imported target Halide::retrain_cost_model
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces.cmake:add_executable(Halide::retrain_cost_model IMPORTED)
grep: packages/halide_x64-linux/share/halide: Is a directory
grep: packages/halide_x64-linux/share/halide/tools: Is a directory
packages/halide_x64-linux/share/halide/tools/autotune_loop.sh:            ${AUTOSCHED_BIN}/retrain_cost_model \
root@usr:/home/usr/work/vcpkg# find packages/halide_x64-linux/share/ | xargs grep "weightsdir_to_weightsfile"
grep: packages/halide_x64-linux/share/: Is a directory
grep: packages/halide_x64-linux/share/HalideHelpers: Is a directory
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:# Import target "Halide::weightsdir_to_weightsfile" for configuration "Release"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:set_property(TARGET Halide::weightsdir_to_weightsfile APPEND PROPERTY IMPORTED_CONFIGURATIONS RELEASE)
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:set_target_properties(Halide::weightsdir_to_weightsfile PROPERTIES
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:  IMPORTED_LOCATION_RELEASE "${_IMPORT_PREFIX}/tools/halide/weightsdir_to_weightsfile"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:list(APPEND _IMPORT_CHECK_TARGETS Halide::weightsdir_to_weightsfile )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-release.cmake:list(APPEND _IMPORT_CHECK_FILES_FOR_Halide::weightsdir_to_weightsfile "${_IMPORT_PREFIX}/tools/halide/weightsdir_to_weightsfile" )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:# Import target "Halide::weightsdir_to_weightsfile" for configuration "Debug"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:set_property(TARGET Halide::weightsdir_to_weightsfile APPEND PROPERTY IMPORTED_CONFIGURATIONS DEBUG)
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:set_target_properties(Halide::weightsdir_to_weightsfile PROPERTIES
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:  IMPORTED_LOCATION_DEBUG "${_IMPORT_PREFIX}/tools/halide/weightsdir_to_weightsfile"
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:list(APPEND _IMPORT_CHECK_TARGETS Halide::weightsdir_to_weightsfile )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces-debug.cmake:list(APPEND _IMPORT_CHECK_FILES_FOR_Halide::weightsdir_to_weightsfile "${_IMPORT_PREFIX}/tools/halide/weightsdir_to_weightsfile" )
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces.cmake:foreach(_expectedTarget Halide::Tools Halide::ImageIO Halide::RunGenMain Halide::Runtime Halide::retrain_cost_model Halide::featurization_to_sample Halide::get_host_target Halide::weightsdir_to_weightsfile)
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces.cmake:# Create imported target Halide::weightsdir_to_weightsfile
packages/halide_x64-linux/share/HalideHelpers/Halide-Interfaces.cmake:add_executable(Halide::weightsdir_to_weightsfile IMPORTED)
grep: packages/halide_x64-linux/share/halide: Is a directory
grep: packages/halide_x64-linux/share/halide/tools: Is a directory
root@usr:/home/usr/work/vcpkg#

Is there anything I missed?
All tools are in tools/halide folder and the cmake configurations are correct.

@JackBoosY JackBoosY added the requires:more-information This Issue requires more information to solve label Oct 9, 2021
@dg0yt
Copy link
Contributor

dg0yt commented Oct 9, 2021

@JackBoosY The question was about finding the autoscheduler library (probably module/plugin), and about the rewriting done by vcpkg_cmake_config_fixup for this library. It is not about a particular tool.

@JackBoosY
Copy link
Contributor

JackBoosY commented Oct 9, 2021

In windows:

  • dynamic libraries -> bin.
  • static libraries -> lib.
  • tools -> tools/${PORT}

In non-Windows:

  • dynamic libraries -> lib.
  • static libraries -> lib.
  • tools -> tools/${PORT}.

The module file should be placed in share (called by the library) or placed in tools/${PORT} (called by the tool).
Of course, we may need to modify the code or configuration.

@ras0219-msft
Copy link
Contributor

ras0219-msft commented Oct 13, 2021

Unfortunately I don't think we have solid advice to give here; plugins are still very exceptional and vcpkg still does not have a fully-supported model for shared libraries on Linux due to many of these troubles.

I can at least factually note that:

  1. Our advice to put runtime libraries in bin/ is strictly for linked .dlls; we have no such advice for .sos and it's much more nebulous for plugin .dlls
  2. .sos that are intended to be linked should be in lib/.
  3. As noted above, Qt uses a completely separate folder (/plugins) to house its plugins on all platforms

I think using /plugins/halide/ is a reasonable starting place for all platforms and should escape fixup_cmake's knowledge. Aligning with Qt is a good direction to take because as we continue to refine our approach we will certainly have a solution that works for it.

@alexreinking
Copy link
Contributor

alexreinking commented Oct 14, 2021

I think using /plugins/halide/ is a reasonable starting place for all platforms and should escape fixup_cmake's knowledge. Aligning with Qt is a good direction to take because as we continue to refine our approach we will certainly have a solution that works for it.

But will this not trip a check for port validity? IIRC the reason they were moved to bin is because vcpkg didn't like .dll files sitting elsewhere; that is, will I need to add set(VCPKG_POLICY_EMPTY_PACKAGE enabled) back to the Halide port?

@JackBoosY JackBoosY added requires:discussion and removed requires:more-information This Issue requires more information to solve labels Oct 14, 2021
@alexreinking
Copy link
Contributor

alexreinking commented Nov 3, 2021

@JackBoosY - it looks like the Halide port is going to be updated in #20749... it would be nice to get this fixed at the same time. What is the agreed-upon resolution? Re-add set(VCPKG_POLICY_EMPTY_PACKAGE enabled) and set -DHalide_INSTALL_PLUGINDIR=plugins/${PORT}?

@ras0219-msft - does that sound reasonable?

ATTN @yurybura

@jacobkahn
Copy link
Contributor

@JackBoosY any updates here? This is also causing #22228 which makes automating things downstream with the package impossible.

@JackBoosY
Copy link
Contributor

I'm not sure about that.
I think we cannot effectively solve this problem before we solve rpath.
Any other suggestions here?

@JackBoosY
Copy link
Contributor

This issue should be fixed by #23035. Please ping me to reopen this issue if it still bother you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support requires:discussion
Projects
None yet
Development

No branches or pull requests

6 participants