[WIP][vcpkg] vcpkg can link wrong libraries (issue #5540 b) by Neumann-A · Pull Request #5543 · microsoft/vcpkg

Neumann-A · 2019-03-05T12:20:08Z

fixes #5540 b

Feel free to change. Is just an example how to solve part of the problem.

Overview
TODOs: #5543 (comment)
OLD FIND_PACKAGE PORTS: #5543 (comment)
AFFECTED PORTS: #5543 (comment) (not updated; too many)
CMAKE_DEBUG_SUFFIX LIST: #5543 (comment)

Some Numbers:
Number of packages which needed changed variables: around 60; (Thats somewhere around 6% of all packages)
Number of packages which consume those variables: around 120; (which is more than 10% of all packages)
Number of packages which actually fix the linkage issue in their target files: ???

Status
REMAINING ISSUES: all current regressions are errors on the package side logic to find a library and must be fixed.
CURRENT STATUS: closed due to wrong approach

cenit · 2019-03-06T00:17:34Z

another path is working with every upstream package to export proper config file and getting in contact with Kitware to update many modules to find package and deal with release/debug libraries (which should have different names also...). I tried this path, it is very long, and very very slow. Some shortcuts would be extremely appreciated.

Neumann-A · 2019-03-06T07:54:14Z

@cenit The problem is not only to update find_package and that libraries should export configs but also consuming libraries need to update their build scripts. I think that is something which will never 100% happen so vcpkg must support a shortcut.

the complete fix will probably look something like:

get_cmake_property(ALL_VARS VARIABLES)
list(FILTER ${ALL_VARS} INCLUDE REGEX "^[${name}|${uppercasename}|${lowercasename}][0-9]*_[INC|LIB]")
foreach(PACKAGE_VAR ${ALL_VARS})
 #fix the paths here
endforeach()
unset(ALL_VARS)

will implement it if i have the time.

cenit · 2019-03-06T07:57:29Z

@Neumann-A why do you say so? optimized;path_to_release_lib;debug;path_to_debug_lib works as expected and is filled inside <PackageName>_LIBRARY by the wonderful select_library_configurations(basename) function inside a proper Find<PackageName>.cmake... so if the downstream project just links to <PackageName>_LIBRARY everything works as expected and you have config support "for free".
Anyway, I fully support you when you say that this is a way-too-long road, which will never achieve 100% completion

Neumann-A · 2019-03-06T09:46:59Z

hmm can somebody get the regex correctly to also include HDF5 and possible others?
https://cmake.org/cmake/help/v3.14/module/FindHDF5.html

something like:
"^[${name}|${uppercasename}|${lowercasename}][^INC|^LIB]_[INC|LIB]"

cenit · 2019-03-06T09:59:50Z

The procedure is to open a PR on https://gitlab.kitware.com/cmake/cmake
Unfortunately at the moment I am too busy on other tasks, otherwise I'd have helped you

Neumann-A · 2019-03-11T20:53:04Z

Can somebody from the team supply me with all cmake config logs from CI after the last commit?
I would like to filter them for Value of to see which package variables are not properly set.

Rastaban · 2019-03-14T06:33:19Z

@Neumann-A, the CI system does not keep around the logs from passing builds and the files edited are not part of the set that triggers a rebuild of ports so nothing was actually built as part of the CI test for the last commit. Supporting this kind of thing in the test system is on my backlog, but I am not sure when I will be able to make those changes. You can run the ci command on your own machine (vcpkg ci [triplet].) It takes about 14 hours to finish on our VMs but you would have all of the logs. If you don't want to tie up your machine for that long then let me know and I can run it on my own over the weekend.

Neumann-A · 2019-04-08T06:53:50Z

DECIDED:

~~decide if <packagename>_LIBRARY|LIBRARIES should always contain keywords debug and optimized even if not required to build (could solve the problem of debug/release targets being overwritten)~~ Yes -> Multi configuration generators require that to work correctly
decide if variables <packagename>_LIBRARY_RELEASE|DEBUG should be defined if CMake is not doing it (does not really matter since nobody will use those/depend on those since CMake does not define them.) No. Why should they be defined?
decide if MinSizeRelease and RelWithDebInfo should always be mapped to Release using CMAKE_MAP_IMPORTED_CONFIG_<config> if not set (issue vcpkg doesn't install CMake targets for RelWithDebInfo and MinSizeRel #5621) Yes because we should not suprise users

Neumann-A · 2019-04-08T09:03:26Z

decide if _LIBRARY|LIBRARIES should always contain keywords debug and optimized even if not required to build (could solve the problem of debug/release targets being overwritten)

this is required because of multi configuration generators like visual studio. Linkage of libraries for debug/release may not be correct if the cmake files link directly to either debug or release library files instead of targets or using optimized/debug. (old find_package style)

Neumann-A · 2019-04-08T12:10:41Z

note to #5625 (comment)
1 and 2 (fix at start): fixes the multi configuration generator issue of find_package
3 and 4 (fix at end): will not fix this issue

Neumann-A · 2019-04-08T13:45:58Z

@cenit: This fixes your opencv issue ... .

Lets see what CI says to this one (probably needs to be triggered manually?)

Neumann-A · 2019-04-08T14:10:37Z

TODOs:

Wait for CI regressions (if any)
~~Remove/Comment out cmake messages.~~ Too helpful to comment out ....
Bump Control for all ports in ([WIP][vcpkg] vcpkg can link wrong libraries (issue #5540 b) #5543 (comment))
cleanup dependent portfiles with unnecessary string replacements for dependencies in target files.
(Can be another issue/pr)
Understand CMake Warning: CMake Warning (dev) at G:/vcpkg_test/vcpkg/scripts/buildsystems/vcpkg.cmake:257 (if):
Policy CMP0054 is not set: Only interpret if() arguments as variables or
keywords when unquoted. Run "cmake --help-policy CMP0054" for policy
details. Use the cmake_policy command to set the policy and suppress this
warning.
add warning to vcpkg_fixup_cmake if debug and release *.cmake files do not have the same content! (Probably different PR/Issue)
fix/check _RELEASE and _DEBUG variables (Example: netcdf-c -> SZIP, CURL)
skip variable loop if _CONFIG is found and hope for the best....

Neumann-A · 2019-04-09T06:06:09Z

Neumann-A · 2019-04-09T06:18:37Z

@Rastaban: Can you create a list of ports which have indirect dependencies (meaning dependencies which dependent on the list in #5543 (comment); second level dependencies; don't know how to call those)? All those ports must be checked for correct linkage of its second level dependencies because the target file of the direct dependent might be wrong if the target is not manually fixed within the portfile.

Don't mind I can simply do that while updating the list above and look at the folder name where I find replacements.

Neumann-A · 2019-04-09T07:38:08Z

porbably affected libraries
(incomplete list as of 740/958 (sdl ports dependencies not yet included due to cascade failure))

allegro 5
apr-util
aubio
avro-c
blaze
blosc
boost-iostream
caf
ccfits
ceres
cgal
clapack
clblas
clfft
collada-dom
cppfs
cpprestsdk
cppzmq
cutelyst2
devil
dlib
ensmallen
exiv2
fcl
fizz
fmi4cpp
folly
fontconfig
g2o
gdcm2
grpc
geogram
google-cloud-cpp
graphicsmagick
grpc
kd-soap
kf5plotting
leptonics
libarchive
libevent
libfreenect2
libgeotiff
libkml
libmysql
libodb-pgsql
libpng
libpq
libsndfile
libssh
libtheora
libtins
libwebp
libxml2
mlpack
opencv
pcl
plplot
poco
podofo
promotheus-cpp
sfml
signalrclient
sophus
suiteparse
tesseract
thrift
tiff
vtk
wavepack
yara

This list must be checked if any of those export dependencies to any of the ports in the list #5543 (comment)

seanwarren · 2019-04-09T10:46:24Z

@Neumann-A, I've tested this on quite a large project and it seems to do a pretty good job. I'm seeing two issues:

Building with x64-windows-static using visual studio I'm seeing two regressions.

qt5
In the link line for the debug configuration I have:
${VCPKG_ROOT}\installed\x64-windows-static\plugins\platforms\qwindows.lib
where it should be:
${VCPKG_ROOT}\installed\x64-windows-static\debug\plugins\platforms\qwindows.lib

libpq
In the link line for the debug configuration I have:
C:\Users\CIMLab\Documents\vcpkg-pristine\installed\x64-windows-static\debug\lib\libpq.lib
where it should be
C:\Users\CIMLab\Documents\vcpkg-pristine\installed\x64-windows-static\debug\lib\libpqd.lib

Neumann-A · 2019-04-09T10:52:58Z

@seanwarren: Can you attach the CMake configurations logs (config-triplet-(detailed-stuff).out.log) for those failing projects? There should be lines reading Replacing: <Something> \n with: <Someotherthing>

# Conflicts: # ports/libgeotiff/portfile.cmake # ports/pdal/portfile.cmake

# Conflicts: # ports/ceres/CONTROL # ports/ceres/portfile.cmake # ports/gdal/portfile.cmake

…hout debug suffix

…ch-2

…version. names position is sometimes not the second one. in these cases additional unused arguments are passe to find_library. Due to that the NAMES position is variable and must be found.

# Conflicts: # ports/glib/CONTROL # ports/libraw/CONTROL # scripts/buildsystems/vcpkg.cmake

# Conflicts: # ports/fizz/CONTROL # ports/folly/CONTROL # ports/libmysql/CONTROL # ports/zstd/CONTROL

Neumann-A · 2019-06-13T10:19:05Z

documenting another approach:

do not fix find_library -> fix target_link_libraries instead
requires correct imported targets defined in findModule.cmake (will require set_property fix from this PR)

# Conflicts: # ports/vtk/CONTROL

CI logs told me they were not even rebuild the last time

Fleischner · 2019-06-17T21:03:20Z

pybind11 installed cmake targets look suspicious. There seems to be a quite complicated machinery that searches for an python.exe and once found it searches for the python libraries relative to this python.exe.

However vcpkg port python3 currently only builds python.dlls. So pybind11 will certainly find completely wrong .lib files in this case.
Also it seems debug libraries are not handled.

It should just use the python libraries of the python3 port instead

angelmixu · 2019-07-04T07:58:44Z

Hi @Neumann-A ! Thanks for all the work!
What happened with this PR? Has it been merged, or are you working on a different approach?

Neumann-A · 2019-07-04T09:13:35Z

@angelmixu.
It requires a different approach. It is not possibly to simply inject the variable returned by find_library with a generator expression because of:
Antipattern: if(EXISTS LIBRARY_VAR) after a find_library call
Usage of CMAKE_REQUIRED_LIBRARIES for CheckFunctionExists not working with generator expressions and small other minor blockers.

So an alternativ approach is necessary with the following attributes:

fix find_library calls with variables:
LIBRARY_VAR -> point to release
LIBRARY_VAR_RELEASE -> point to release
LIBRARY_VAR_DEBUG ->point to debug
generate VCPKG_LIB_NAMES_libname_VAR with VCPKGPATH/.lib and set it to names used in the find_library call. (is properly not required but it is good to have the search names stored somewhere)
generate VCPKG_LIB_GENEXP_libname_VAR generator expression with the correct paths to release/debug

fix set_property on targets and target_set_properties:
IMPORTED_LOCATION -> Generate IMPORTED_LOCATION_RELEASE and IMPORTED_LOCATION_DEBUG (properly requires the searchnames to do an extra find_library call in release or debug folder)
IMPORTED_LOCATION_RELEASE -> check if release path
IMPORTED_LOCATION_DEBUG -> check if debug path

fix target_link_libraries and link_libraries:
Check every library which is linked having the vcpkg base path and replace it with VCPKG_LIB_GENEXP_libname_VAR if necessary (e.g. if debug/optimized keywords are not used)

So I have a plan but no time to do it currently

Rastaban added category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) wip labels Mar 5, 2019

Rastaban changed the title ~~WIP: Solve #5540 b~~ [WIP][vcpkg] vcpkg can link wrong libraries (issue #5540 b) Mar 5, 2019

This was referenced Mar 11, 2019

[lzma] Locate correct release/debug libraries in static build #5625

Closed

[assimp] fix linux build #5632

Merged

Neumann-A force-pushed the patch-2 branch from 491bc2a to d693db5 Compare March 26, 2019 23:42

Neumann-A force-pushed the patch-2 branch from d693db5 to e9de439 Compare April 8, 2019 12:39

Neumann-A mentioned this pull request Apr 9, 2019

[SDL2] Running CMake configuration RelWithDebInfo causes error #3463

Closed

Neumann-A and others added 17 commits May 29, 2019 12:04

Merge remote-tracking branch 'upstream/master' into patch-2

b04f091

# Conflicts: # ports/libgeotiff/portfile.cmake # ports/pdal/portfile.cmake

Merge remote-tracking branch 'upstream/master' into patch-2

99be7c2

# Conflicts: # ports/ceres/CONTROL # ports/ceres/portfile.cmake # ports/gdal/portfile.cmake

make finding _DEBUG libs in release non fatal and retry to search wit…

20b713c

…hout debug suffix

Merge remote-tracking branch 'upstream/master' into patch-2

0f5cd95

fix arrow zstd linkage

c4c958e

fix poco include dir

a7fbc50

reenable ninja

bb1136d

fix openexr find module for openimageio

046df30

find_library patch. variable was not set into parent scope

935e55f

Merge branch 'patch-2' of https://github.com/Neumann-A/vcpkg into pat…

a6eb795

…ch-2

Merge remote-tracking branch 'origin/patch-2' into patch-2

7c9a7fc

sometimes cmake_parse_arguments PARSE_ARGV fails so retry with other …

2b39a93

…version. names position is sometimes not the second one. in these cases additional unused arguments are passe to find_library. Due to that the NAMES position is variable and must be found.

moved some code around. Code was actually required by add_executable

36a51f9

Merge remote-tracking branch 'upstream/master' into patch-2

cccaf3e

# Conflicts: # ports/glib/CONTROL # ports/libraw/CONTROL # scripts/buildsystems/vcpkg.cmake

Merge remote-tracking branch 'upstream/master' into patch-2

7181ec7

bump control to force rebuild and ci cache refresh.

02d8b03

Merge remote-tracking branch 'upstream/master' into patch-2

a707c39

# Conflicts: # ports/fizz/CONTROL # ports/folly/CONTROL # ports/libmysql/CONTROL # ports/zstd/CONTROL

Neumann-A added 6 commits June 14, 2019 22:30

Merge remote-tracking branch 'upstream/master' into patch-2

e607acf

fix find_library argument parsing.

b0bb03e

Merge remote-tracking branch 'upstream/master' into patch-2

85d4ab6

# Conflicts: # ports/vtk/CONTROL

bump control of regressing ports to force a rebuild of those ports.

0b592ab

CI logs told me they were not even rebuild the last time

fix openssl target

a395dfc

bump openssl control

f3b06b8

Neumann-A closed this Jun 21, 2019

tetsuh mentioned this pull request Jun 30, 2019

[poco] Fix the release libraries are linked to debug pcred.dll #7099

Closed

Neumann-A mentioned this pull request Aug 2, 2019

WIP: Trying to fix linkage issues with multi configuration generators and others #7524

Closed

Conversation

Neumann-A commented Mar 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cenit commented Mar 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Neumann-A commented Mar 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cenit commented Mar 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Neumann-A commented Mar 6, 2019

Uh oh!

cenit commented Mar 6, 2019

Uh oh!

Neumann-A commented Mar 11, 2019

Uh oh!

Rastaban commented Mar 14, 2019

Uh oh!

Neumann-A commented Apr 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Neumann-A commented Apr 8, 2019

Uh oh!

Neumann-A commented Apr 8, 2019

Uh oh!

Neumann-A commented Apr 8, 2019

Uh oh!

Neumann-A commented Apr 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Neumann-A commented Apr 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Neumann-A commented Apr 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Neumann-A commented Apr 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seanwarren commented Apr 9, 2019

Uh oh!

Neumann-A commented Apr 9, 2019

Uh oh!

Neumann-A commented Jun 13, 2019

Uh oh!

Fleischner commented Jun 17, 2019

Uh oh!

angelmixu commented Jul 4, 2019

Uh oh!

Neumann-A commented Jul 4, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Neumann-A commented Mar 5, 2019 •

edited

Loading

cenit commented Mar 6, 2019 •

edited

Loading

Neumann-A commented Mar 6, 2019 •

edited

Loading

cenit commented Mar 6, 2019 •

edited

Loading

Neumann-A commented Apr 8, 2019 •

edited

Loading

Neumann-A commented Apr 8, 2019 •

edited

Loading

Neumann-A commented Apr 9, 2019 •

edited

Loading

Neumann-A commented Apr 9, 2019 •

edited

Loading

Neumann-A commented Apr 9, 2019 •

edited

Loading