Skip to content

[WIP] some libraries export <PackageName>LibraryDepends.cmake which needs fixing#4782

Closed
Neumann-A wants to merge 3 commits intomicrosoft:masterfrom
Neumann-A:fixup_cmake
Closed

[WIP] some libraries export <PackageName>LibraryDepends.cmake which needs fixing#4782
Neumann-A wants to merge 3 commits intomicrosoft:masterfrom
Neumann-A:fixup_cmake

Conversation

@Neumann-A
Copy link
Contributor

@Neumann-A Neumann-A commented Nov 21, 2018

instead of Targets.cmake.
Those file also need the fix of #1044

should close #4753 and maybe others.

related: #4622

Maybe just use the suggestion in #4622 and be done with it instead of having the same code 3 times for different *.cmake and other headaches if a package renames the config or target file.
related to the above is : #4633 (....targets-wt.cmake)

Bonus mention: #4150

instead of <PackageName>Targets.cmake.
Those file also need the fix of microsoft#1044

should close microsoft#4753
hopefully solved the issue within microsoft#4150
replaced the regex with something more readable
(also ident is lost)

should close:
microsoft#4753
microsoft#4633
microsoft#4150
and maybe more
@Neumann-A
Copy link
Contributor Author

lattest comit uses the general solution and fixes a few more issues

@Neumann-A
Copy link
Contributor Author

Neumann-A commented Dec 6, 2018

vcpkg_fixup_cmake_targets still has issues

It changes
;\$<\$<NOT:\$<CONFIG:DEBUG>>:G:/MPINew/mpi_vcpkg/installed/x64-windows/lib/zlib.lib>;\$<\$<CONFIG:DEBUG>:G:/MPINew/mpi_vcpkg/installed/x64-windows/debug/lib/zlibd.lib>;

to only

;${_IMPORT_PREFIX}/lib/zlib.lib;

which causes #4895

Hm could be that CMake is replacing the \$ any ideas how to circumvent that ?

@Neumann-A
Copy link
Contributor Author

Neumann-A commented Dec 6, 2018

hmm different hdf5-targets for dbg and rel build ......

release build it only states:
G:/MPINew/mpi_vcpkg/installed/x64-windows/lib/zlib.lib

vcpkg is simply overwriting the correct target file due to release being build after debug.
The question is why are those different at all ....

which means on the other hand that this pr is working as intended.
Question now is: Is this a CMake or a HDF5 bug?

@vicroms vicroms self-assigned this Jan 9, 2019
@vicroms vicroms changed the title some libraries export <PackageName>LibraryDepends.cmake which needs fixing [WIP] some libraries export <PackageName>LibraryDepends.cmake which needs fixing Jan 30, 2019
@vicroms
Copy link
Member

vicroms commented Jan 30, 2019

I've been testing these changes for the past weeks and it causes a bunch of regressions.

Changing the REGEX REPLACE fails to fix packages where IMPORT_PREFIX is only one level up.
Example, it wont fix packages where generated Targets.cmake file loos like this:

get_filename_component(_IMPORT_PREFIX "${CMAKE_CURRENT_LIST_FILE}" PATH)
get_filename_component(_IMPORT_PREFIX "${_IMPORT_PREFIX}" PATH)
if(_IMPORT_PREFIX STREQUAL "/")
  set(_IMPORT_PREFIX "")
endif()

@Neumann-A
Copy link
Contributor Author

@vicroms: since I don't know which packages fail the simplest solution is just to reverse it back to use regex replace as done by the lattest commit.

@vicroms vicroms added the wip label Feb 8, 2019
@vicroms
Copy link
Member

vicroms commented Feb 19, 2019

@Neumann-A

I've been working on this PR from the shadows for a while, and I'm confident that now it won't cause regressions.

Because the change to vcpkg_fixup_cmake_targets() causes a full-rebuild, it exposed a whole bunch of unrelated issues; which I had to address.

But, I would like you to review the code changes I made, before pushing to your branch.

@Neumann-A
Copy link
Contributor Author

@vicroms Since it is not yet a PR I cannot directly comment within the code:

here are my comments:

# Release folly-targets.cmake does not link to the right libraries in debug mode.
# We substitute with generator expressions so that the right libraries are linked for debug and release.
set(FOLLY_TARGETS_CMAKE "${CURRENT_PACKAGES_DIR}/share/folly/folly-targets.cmake")
FILE(READ ${FOLLY_TARGETS_CMAKE} _contents)
STRING(REPLACE 
[[
"Threads::Threads;Iphlpapi.lib;Ws2_32.lib;${_IMPORT_PREFIX}/lib/boost_context-vc140-mt.lib;${_IMPORT_PREFIX}/lib/boost_chrono-vc140-mt.lib;${_IMPORT_PREFIX}/lib/boost_date_time-vc140-mt.lib;${_IMPORT_PREFIX}/lib/boost_filesystem-vc140-mt.lib;${_IMPORT_PREFIX}/lib/boost_program_options-vc140-mt.lib;${_IMPORT_PREFIX}/lib/boost_regex-vc140-mt.lib;${_IMPORT_PREFIX}/lib/boost_system-vc140-mt.lib;${_IMPORT_PREFIX}/lib/boost_thread-vc140-mt.lib;${_IMPORT_PREFIX}/lib/boost_atomic-vc140-mt.lib;${_IMPORT_PREFIX}/lib/double-conversion.lib;${_IMPORT_PREFIX}/lib/ssleay32.lib;${_IMPORT_PREFIX}/lib/libeay32.lib;${_IMPORT_PREFIX}/lib/zlib.lib;gflags;glog::glog;event"
]]
[[
"Threads::Threads;Iphlpapi.lib;Ws2_32.lib;\$<\$<NOT:\$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/boost_context-vc140-mt.lib>;\$<\$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/boost_context-vc140-mt-gd.lib>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/boost_chrono-vc140-mt.lib>;\$<\$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/boost_chrono-vc140-mt-gd.lib>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/boost_date_time-vc140-mt.lib>;\$<\$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/boost_date_time-vc140-mt-gd.lib>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/boost_filesystem-vc140-mt.lib>;\$<\$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/boost_filesystem-vc140-mt-gd.lib>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/boost_program_options-vc140-mt.lib>;\$<\$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/boost_program_options-vc140-mt-gd.lib>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/boost_regex-vc140-mt.lib>;\$<\$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/boost_regex-vc140-mt-gd.lib>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/boost_system-vc140-mt.lib>;\$<\$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/boost_system-vc140-mt-gd.lib>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/boost_thread-vc140-mt.lib>;\$<\$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/boost_thread-vc140-mt-gd.lib>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/boost_atomic-vc140-mt.lib>;\$<\$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/boost_atomic-vc140-mt-gd.lib>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/double-conversion.lib>;\$<\$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/double-conversion.lib>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/ssleay32.lib>;\$<\$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/ssleay32.lib>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/libeay32.lib>;\$<\$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/libeay32.lib>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${_IMPORT_PREFIX}/lib/zlib.lib>;\$<\$<CONFIG:DEBUG>:${_IMPORT_PREFIX}/debug/lib/zlibd.lib>;gflags;glog::glog;event"
]]
    _contents "${_contents}")
FILE(WRITE ${FOLLY_TARGETS_CMAKE} "${_contents}")

Looks like the bug I encountered with HDF5 where the Targets.cmake is for some reason different for between Debug and Release builds and the Release version is missing the information for the Debug Targets. (see. #4782 (comment))
Do you have any idea why that is the case?

# _IMPORT_PREFIX needs to go up one extra level in the directory tree.
# These files should be modified.
#     /share/glbinding/glbinding-export.cmake 
#     /share/glbinding-aux/glbinding-aux-export.cmake
file(GLOB_RECURSE TARGET_CMAKES "${CURRENT_PACKAGES_DIR}/*-export.cmake")
foreach(TARGET_CMAKE IN LISTS TARGET_CMAKES)
    file(READ ${TARGET_CMAKE} _contents)
    string(REPLACE 
[[
get_filename_component(_IMPORT_PREFIX "${CMAKE_CURRENT_LIST_FILE}" PATH)
get_filename_component(_IMPORT_PREFIX "${_IMPORT_PREFIX}" PATH)
get_filename_component(_IMPORT_PREFIX "${_IMPORT_PREFIX}" PATH)
]]
[[
get_filename_component(_IMPORT_PREFIX "${CMAKE_CURRENT_LIST_FILE}" PATH)
get_filename_component(_IMPORT_PREFIX "${_IMPORT_PREFIX}" PATH)
get_filename_component(_IMPORT_PREFIX "${_IMPORT_PREFIX}" PATH)
get_filename_component(_IMPORT_PREFIX "${_IMPORT_PREFIX}" PATH)
]]
        _contents "${_contents}")
    file(WRITE ${TARGET_CMAKE} "${_contents}")
endforeach()

file(WRITE ${CURRENT_PACKAGES_DIR}/share/glbinding/glbinding-config.cmake "include(\${CMAKE_CURRENT_LIST_DIR}/glbinding/glbinding-export.cmake)\ninclude(\${CMAKE_CURRENT_LIST_DIR}/glbinding-aux/glbinding-aux-export.cmake)\nset(glbinding_FOUND TRUE)\n")

Maybe patching the *.cmake install location would be a better fix, since you are undoing changes done by vcpkg_fixup_cmake_targets. Maybe vcpkg_fixup_cmake_targets can detect those cases and conditionally change the files? But if it works its ok. Until now I was just assuming that all *.cmake files should go directly into share/<packagename> without any additional subfolders

# Remove unset of _IMPORT_PREFIX in VTKTargets.cmake
STRING(REPLACE [[set(_IMPORT_PREFIX)]] 
[[
# VCPKG: The value of _IMPORT_PREFIX should not be unset.
#set(_IMPORT_PREFIX)
]]
VTK_TARGETS_CONTENT "${VTK_TARGETS_CONTENT}")

Why is this required? I looked at different package with -targets.cmake and see this:

# Cleanup temporary variables.
set(_IMPORT_PREFIX)

So it shouldn't do any harm.

The rest of the changes seem unrelated

@vicroms
Copy link
Member

vicroms commented Feb 26, 2019

Closed in favor of #5459

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cmake integration for nlopt

2 participants