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

Package-supplied CMake support [was: opencv issues] #77

Closed
traversaro opened this issue Sep 22, 2016 · 12 comments
Closed

Package-supplied CMake support [was: opencv issues] #77

traversaro opened this issue Sep 22, 2016 · 12 comments
Assignees
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed

Comments

@traversaro
Copy link
Contributor

traversaro commented Sep 22, 2016

To test vcpkg, I tried to compile a simple cmake-based OpenCV example:
https://gist.github.com/traversaro/d8c7de12480433fc6820743f44a38d88

Following https://github.com/Microsoft/vcpkg/blob/master/docs/EXAMPLES.md#example-1-2-b ,
I successfully installed opencv with ./vcpkg install opencv.

I then tried to compile the CMake OpenCV example using:

mkdir build
cd build 
cmake .. "-DCMAKE_TOOLCHAIN_FILE=C:/Users/Traversaro/Documents/Code/vcpkg/scripts/buildsystems/vcpkg.cmake"

The first problem was that find_package(OpenCV REQUIRED) was not working due to a wrong value of CMAKE_PREFIX_PATH set in the toolchain file. I fixed this little issue (see #76) but then I was faced by a more serious error:

$  cmake .. "-DCMAKE_TOOLCHAIN_FILE=C:/Users/Traversaro/Documents/Code/vcpkg/scripts/buildsystems/vcpkg.cmake"
CMake Error at C:/Users/Traversaro/Documents/Code/vcpkg/installed/x86-windows/share/OpenCV/OpenCVModules.cmake:62 (message):
  The imported target "opencv_world" references the file
     "C:/Users/Traversaro/Documents/Code/vcpkg/installed/x86-windows/share/lib/opencv_world310d.lib"
  but this file does not exist.  Possible reasons include:
  * The file was deleted, renamed, or moved to another location.
  * An install or uninstall procedure did not complete successfully.
  * The installation package was faulty and contained
     "C:/Users/Traversaro/Documents/Code/vcpkg/installed/x86-windows/share/OpenCV/OpenCVModules.cmake"
  but not all the files it references.
Call Stack (most recent call first):
  C:/Users/Traversaro/Documents/Code/vcpkg/installed/x86-windows/share/OpenCV/OpenCVConfig.cmake:86 (include)
  CMakeLists.txt:6 (find_package)
-- Configuring incomplete, errors occurred!
See also "C:/Users/Traversaro/Documents/Code/d8c7de12480433fc6820743f44a38d88/build/CMakeFiles/CMakeOutput.log".

This error is caused by this line (and the following) in the portfile.cmake :

file(RENAME ${CURRENT_PACKAGES_DIR}/cmake/OpenCVModules.cmake ${CURRENT_PACKAGES_DIR}/share/opencv/OpenCVModules.cmake)

https://github.com/Microsoft/vcpkg/blob/master/ports/opencv/portfile.cmake#L57

The problem is that relocatable CMake imported target files (the one tipically created using install(TARGETS ...), see https://cmake.org/cmake/help/v3.5/manual/cmake-packages.7.html#creating-relocatable-packages) encode the relative positions in the installation layout of the include directories and of the libraries file with respect to themselves. For this reason, they cannot be moved relatively to the other installation includes and libraries.
Unfortunately trying to remove this arbitrary change of location of the OpenCVModules.cmake file in the portfile.cmake is impossible, because apparently there is a rule that imposes that all .cmake files should be in the share/<package> location.

At the moment this means that all the CMake libraries that do not install their installed targets directly in share/<package> cannot be found using their <package>Config.cmake in vcpkg. Note that CMake documentation itself suggests to install the CMake config files and the installed targets in lib/cmake/<package>*/ (see https://cmake.org/cmake/help/v3.5/manual/cmake-packages.7.html#id14). So, even a new package following exactly the CMake documentation cannot be properly packaged using CMake and vcpkg.

Possible solutions

Two possible solution I can think of:

Patch all projects to install their CMake config files in share/<package>

This solution involve patching (or calling the build with an appropriate option, if the build system has an option for that) all projects' CMake files to install their exports directly in share/<package>.
This would be undesirable as the vast majority of existing project would require a patch.

Do not constrain the port to install cmake files in share/<package>

This solution is the one used by all the package manager that I am aware of: they permit to install the cmake configuration files whenever the upstream prefer. As long as this path is in the find_package search path, the library can be easily found. This would drastically simplify the writing of the portfile.cmakes, but I do not know if this conflicts some other vcpkg requirement.

@traversaro
Copy link
Contributor Author

Slightly related to #6 , as the use of a highly atypical CMake setup in https://github.com/Microsoft/vcpkg/blob/master/docs/EXAMPLES.md#example-1-2-b has hidden the problem.

@ras0219-msft
Copy link
Contributor

This is a serious issue, thank you for bringing it up and laying out the situation so clearly.

While option 2 is the most immediate (with the assumption it doesn't prevent any other of our lint checks from working), I'd first like to make sure we completely explore the cost/benefit of option 1.

For option 1, it is certainly true that we would need to alter the installation of many packages that deploy CMake scripts. I'll make a ballpark guess that this is 20% of all packages. What's the minimum needed change (to each package) to make that work? Is it required to patch the CMakeLists.txt or is there a highly reliable post-processing that could be performed on the installed CMake scripts? If this could be made as straightforward as a single vcpkg_fixup_cmake() command either at the bottom of the portfile.cmake or before the configure, my feeling is that this is not a very high cost.

The benefits of option 1 are simplicity (for users), consistency, and reliability. It is easy to automatically determine post-installation whether a given package has provided CMake support. If there is an extremely common location that 50%+ of packages currently use (as you mentioned above, perhaps lib/cmake/<package>), we could lower the cost of option 1 further by requiring that directory pattern instead of share/<package>.

Taking a look at Debian, I see that (at least for opencv), the files are deployed to /usr/share/OpenCV/: https://packages.debian.org/jessie/amd64/libopencv-dev/filelist. ArchLinux uses similar (/usr/share/opencv/): https://www.archlinux.org/packages/extra/x86_64/opencv/.

@traversaro
Copy link
Contributor Author

traversaro commented Sep 25, 2016

For option 1, it is certainly true that we would need to alter the installation of many packages that deploy CMake scripts. I'll make a ballpark guess that this is 20% of all packages. What's the minimum needed change (to each package) to make that work?

The typical patch would involve changing the DESTINATION parameter of three commands (see also https://cmake.org/cmake/help/v3.3/manual/cmake-packages.7.html for a reference) :

However if we choose this option, I strongly suggest discussing with CMake upstream to modify the examples in https://cmake.org/cmake/help/v3.3/manual/cmake-packages.7.html to have a CMake variable (such as CMAKE_INSTALL_CMAKECONFDIR?) that can be used as "DESTINATION" in this three commands and that can be changed without patching the CMakeLists.txt file, similarly to what happens now if a projects uses the GNUInstallDirs CMake module.

Is it required to patch the CMakeLists.txt or is there a highly reliable post-processing that could be performed on the installed CMake scripts? If this could be made as straightforward as a single vcpkg_fixup_cmake() command either at the bottom of the portfile.cmake or before the configure, my feeling is that this is not a very high cost.

I am not aware of any "reliable post-processing" trick to change the relative position of the CMake configuration files in the install layout, but it may be worth to ask for the same question in the CMake mailing list.

If there is an extremely common location that 50%+ of packages currently use (as you mentioned above, perhaps lib/cmake/), we could lower the cost of option 1 further by requiring that directory pattern instead of share/.

Recently I raised a concern with the CMake devs about the fact that in Windows CMake was a better alternative for installing CMake conf files (see https://gitlab.kitware.com/cmake/cmake/issues/16212) but instead of modifying the documentation we ended up in modifying CMake find_package search path to cover the same use case by still using lib/cmake/<package>. So I think lib/cmake/<package> is a sort of "unofficially" recommended path, even if any path found by find_package is fully supported by CMake. Again, this would be a question that may be worth to ask on the CMake mailing list.

Taking a look at Debian, I see that (at least for opencv), the files are deployed to /usr/share/OpenCV/: https://packages.debian.org/jessie/amd64/libopencv-dev/filelist. ArchLinux uses similar (/usr/share/opencv/): https://www.archlinux.org/packages/extra/x86_64/opencv/.

OpenCV may not be the best example as it has a quite complicate installation logic for its CMake config files (see https://github.com/opencv/opencv/blob/3.1.0/cmake/OpenCVGenConfig.cmake and opencv/opencv#7331) but there is nothing Debian or Arch specific in those installation layouts : simply share/opencv is the default CMake configuration file installation path for OpenCV in Linux, and the distribution simply follow that behavior without modification, as they do with all other packages as long these conf files can be found by find_package(<package> ...) (i.e. solution 2).

@ras0219-msft ras0219-msft self-assigned this Sep 27, 2016
@ras0219-msft
Copy link
Contributor

I've massaged OpenCV a bit more, so I have some empirical findings to report.

First, Option 2 (in its purest form) will not work for multi-configuration generators (like Visual Studio). This is because we install to multiple roots in order to provide both Debug and Release versions of libraries and the respectively installed cmake scripts are tied to the particular version installed. For example, if we do not manipulate the outputs at all, we'll get something like

lib/cmake/opencv/OpenCVModules.cmake
lib/cmake/opencv/OpenCVModules-release.cmake
debug/lib/cmake/opencv/OpenCVModules.cmake
debug/lib/cmake/opencv/OpenCVModules-debug.cmake

When configuring a cmake project that uses OpenCV, it will only pull in the Debug or Release file, not both. This causes multi-configuration generators to fail in either Debug or Release mode.

Fortunately, the *-release.cmake and *-debug.cmake are autogenerated and follow a very regular format. They specifically rely on the ${_IMPORT_PREFIX} variable to point at the installation prefix. Because of our particular case where the debug version of _IMPORT_PREFIX is at the release version's prefix plus /debug/, I believe it's robust to perform a string replacement here. For OpenCV, this looks like

file(READ ${CURRENT_PACKAGES_DIR}/debug/share/opencv/OpenCVModules-debug.cmake OPENCV_DEBUG_MODULE)
string(REPLACE "\${_IMPORT_PREFIX}" "\${_IMPORT_PREFIX}/debug" OPENCV_DEBUG_MODULE "${OPENCV_DEBUG_MODULE}")
file(WRITE ${CURRENT_PACKAGES_DIR}/share/opencv/OpenCVModules-debug.cmake "${OPENCV_DEBUG_MODULE}")

I've confirmed that this works in an external project using the VS generator in both Debug and Release.

Secondly, OpenCV's default installation behavior on Windows tries to place binaries in subfolders based on architecture. Since we are handling architecture-specific locations, we need to modify this anyway. This causes the Windows-specific cmake files to no longer point to the correct locations, but the Unix ones are actually correct! So, I had to make approximately 4 lines of modification to adjust to using the Unix paths, with one more line of modification enabling external override of the cmake config directory.

I've pushed changes to the opencv port that include all the above modifications. Since OpenCV in particular is no longer an issue here, I think the right course of action is to continue requesting cmake files to be placed in /share/<package> until we find another concrete example which demonstrates that it's easier to take Option 2 than Option 1.

Does this sound reasonable to you? It may also make sense to open a thread on the CMake Mailing List to confirm my above conclusions.

@traversaro
Copy link
Contributor Author

When configuring a cmake project that uses OpenCV, it will only pull in the Debug or Release file, not both. This causes multi-configuration generators to fail in either Debug or Release mode.

I see. I wonder if the fix for the autogenerated targets file can be made generic by writing a cmake macro (as you previously proposed) to move the autogenerated targets file. In the end we need to replicate the logic in install(EXPORT ...) [1] to modify the series of get_filename_component(_IMPORT_PREFIX "${_IMPORT_PREFIX}" PATH) to account for the new (relative) path of the targets file.

On the other hand, automatically accounting for all the logic present in configure_package_config_file seems much harder to me.

Does this sound reasonable to you?

Yes. The ideal situation for me would be that a project with a "state-of-the-art" CMake build system (such as the one described in https://cmake.org/cmake/help/v3.3/manual/cmake-packages.7.html#creating-relocatable-packages) could be packaged in vcpkg with no modifications as in happens in homebrew, but apparently this is not realistic in the short term, so at least fixing the OpenCV package and then trying to package more CMake-based libraries to understand the cost of tradeoff is definitely a reasonable way of proceeding.

I think it may be worth to also add some notes to the packaging documentation (see https://github.com/Microsoft/vcpkg/blob/master/docs/EXAMPLES.md#suggested-example-portfiles), mentioning that for CMake-based libraries there are problems in correctly relocating config packages, that at the moment must be solved in a package-specific way.

It may also make sense to open a thread on the CMake Mailing List to confirm my above conclusions.

Yes, I think in general the CMake community has always been interested in sane ways of installing and finding C++ dependencies, so I think they could be very interested in finding the best possible solution to this issue.

Thanks a lot for your effort on this.

[1] https://github.com/Kitware/CMake/blob/e9dbb272e0075516f8d6a970a6eb70be6234c4b6/Source/cmExportInstallFileGenerator.cxx#L180

@ras0219-msft ras0219-msft changed the title Problems in compiling a cmake-based OpenCV example Package-supplied CMake support [was: opencv issues] Sep 28, 2016
@ras0219-msft ras0219-msft added the category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed label Sep 30, 2016
@traversaro
Copy link
Contributor Author

traversaro commented Oct 24, 2016

By the way, I just realized something (perhaps obvious): why install files that contain references to architecture-dependendent files such as <package>Targets.cmake in share/<package>, when share has been traditionally reserved for architecture-indipendent files (such as copyright notices)?

@ras0219-msft
Copy link
Contributor

The note about share/ is interesting, however we've generally taken the approach with vcpkg that it's not worth trying to extract architecture-independent from architecture-dependent files. Looking at it another way, we don't have share/<package>, we have <arch>/share/<package> :).

@picoharry
Copy link

I am having an issue on one of my dev machines, and I am wondering if anybody has some insight.

I am just following the doc (using CMAKE_TOOLCHAIN_FILE), and it works at least one machine (after setting CMAKE_PREFIX_PATH to "...vcpkg/installed/x86-windows/share/opencv"). But, on another machine, I get this error:

Found package configuration file: but it set OpenCV_FOUND to FALSE so package "OpenCV" is considered to be NOT FOUND.

The two machines have exactly the same setup in terms of VS 2017 update and vcpkg source snapshot, etc. And yet, it does not work on one machine. I tried everything I could think of (setting/unsetting OpenCV_DIR, etc.), but nothing helps. I have no problem using my own OpenCV builds or nuget package. It's very likely that the problem has something to do vcpkg.

Any input will be greatly appreciated.

@ras0219-msft
Copy link
Contributor

You shouldn't need to set cmake prefix path to that -- the toolchain file should take care of everything.

Just to sanity check: Vcpkg list returns the same on both machines? What opencv package version is returned by list? Can you run your project's cmake configure with debug information (or trace mode) and send any weird bits to [email protected]?

@picoharry
Copy link

picoharry commented Dec 23, 2016

Thanks for the quick reply, @ras0219-msft.

Although I didn't do side-by-side comparison (one is a work computer and the other is a computer I use at home), things appeared correct on my work computer (which I'm having this problem with). vcpkg list returns a reasonable list (I only installed x86-windows version), and everything seems more or less correct in the installed directory. I think the version was correct (3.1.0?). The only thing I can think of is, it's just a wildest guess, that something's "wrong" with my work computer, and the opencvconfig.cmake in the original opencv build is somewhat more "forgiving" than the version from vcpkg/port.

Come to think of it, there are a few differences: My home computer has VS2015 Community and VS2017 RC2 Community, and my work computer has VS2015 Enterprise and VS2017 RC2 Community. The home computer I used has Intel i7 and my work computer has an AMD processor (cannot remember the exact processor name right now). Also, I remember setting VCPKG(?) env variable on my home computer, but I didn't do that for the work computer. (I started using VS tools for cmake + vcpkg yesterday, and I've been trying out different things. The doc is not entirely clear as to what step is optional and what step is required, etc.) I installed vcpkg at the top level (C:) on my home computer, but it was installed in a subdirectory below some levels on the other (meaning a longer path). vcpkg.exe was included in the system path on home computer whereas it was not done on work computer. etc. (But, I did integrate install on both machines.) BTW, regarding the cmake prefix var, it might not have been necessary. I was just trying different options, and there might have been some cache issues. (I just learned that you could easily clean the cmake cache directory from VS cmake menu.) Let me double check. (Also, I have two versions of cmake on both machines, one from standalone install 3.7.1 and the other from VS2017 3.6.x, and I get the same results whether I use cmake from command line (using -D) or from VS2017 tool (using cmakesettings.json).)

When I get back to work next week, I'll try this again, and if I find anything notable, I'll send you the information. Thanks!

@ras0219-msft
Copy link
Contributor

Thanks for the very detailed information. We should insulate ourselves from all of those differences, if any of those end up being the root cause, that's a bug we need to fix!

For debugging, I would recommend using cmake from the command line and completely deleting the build directory between trials. If you run cmake --help, there are a few diagnostic switches you can throw to find out exactly where the import fails.

@picoharry
Copy link

Just to follow up, I no longer have this problem. I cleaned up all old opencv installations and removed all related opencv env vars, and rebooted the machine. Now I can cmake, build, and run a simple opencv test app with no problems. Thanks!

fwcd pushed a commit to fwcd/vcpkg that referenced this issue Aug 21, 2023
Updated port libdjinterop to version 0.19.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed
Projects
None yet
Development

No branches or pull requests

3 participants