[boost-modular-build-helper] use system layout
Describe the pull request
Boost layout switched to system
-
What does your PR fix?
This will allow linking boost libraries on Windows without guessing compiler and other suffixes, just like on any other platform
-
Which triplets are supported/not supported? Have you updated the CI baseline?
all
-
Does your PR follow the maintainer guide?
Yes
-
If you have added/updated a port: Have you run
./vcpkg x-add-version --alland committed the result?Yes
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
:x: vitalyster sign now
You have signed the CLA already but the status is still pending? Let us recheck it.
The reason we do not use the system layout on Windows is because it is incompatible with Multi-Configuration CMake Generators like MSBuild while using the built-in FindBoost.cmake module.
The FindBoost module needs to be able to find both the release and debug library files for Multi-Config Generators to work, but the system layout does not have any naming difference between them. This is why our current approach is to "pin" all the relevant values (compiler, etc) such that FindBoost is able to find them while also being able to distinguish between Release and Debug.
Can you please go into detail about the precise problem this PR resolves including repro steps? Thanks for the PR!
find both the release and debug library files for Multi-Config Generators
Everything works just fine, tested with minimal CMake project: Release directory contains release version of boost dlls and Debug directory contains Debug dlls, no need to have these creepy suffixes.
precise problem this PR resolves
This PR resolves problem with other build systems: without system layout each build system should implement giant FindBoost.cmake analog for no reason.
- Most of ports worked perfectly without suffixes, some ports like Boost have that only for legacy reasons which are not actual for vcpkg in my opinion.
@vitalyster You don't understand the problem. Multi Config cmake generators need the suffix to be able to correctly link against the libraries and generate correctly setup <package>-target.cmake files. If it is the same name CMake can only find one version of the libraries on its own.
@Neumann-A why it works for me and for thousands of other ports?
why it works for me and for thousands of other ports?
Because you don't know where the issues are and don't understand the multi config issues / <package>-target.cmake generation issue. If you only ever use single configuration stuff and consumers link against the boost target instead of the _LIBRARY vars you probably never see these issue. But those issues exists and are annoying. At least the d.lib must stay for cmake to do the right thing. (I really don't care about the rest of the suffix)
@Neumann-A I tested a multi-config CMake build and it works. If you know some issues that can happen - please provide an example.
At least the d.lib
I see the only reason to have a suffix is when all libraries are stored in a single directory. But in the vcpkg world debug libraries are stored in debug directory and it is enough to distinct debug and release binaries
I tested a multi-config CMake build and it works.
probably only tested debug config.....
I see the only reason to have a suffix is when all libraries are stored in a single directory.
wrong.. not the only reason. The reason to have d suffix is so that cmake can distinguish _LIBRARY_DEBUG from _LIBRARY_RELEASE and actually find the correct DEBUG and RELEASE version to setup the targets. Otherwise a vcpkg-cmake-wrapper.cmake is necessary to fix the targets/variables
If you know some issues that can happen - please provide an example.
feel free to read #6045 and all associated issues/pr. This should keep you busy for at least a week :P
cmake can distinguish _LIBRARY_DEBUG from _LIBRARY_RELEASE
They are stored in totally different directories: <triplet>/lib/library_name.dll and <triplet>/debug/lib/library_name.dll. All information previously encoded in suffixes are now in the file path.
https://github.com/microsoft/vcpkg/issues/6045 and all associated issues/pr.
The reason of postfix issues is postfix presence at all, it is not needed. Well, if I see the port which does not have debug suffixes - is it a bug?
They are stored in totally different directories:
You are missing the point. CMake does not have a default concept of "different directories" for libraries.
So what will find_library(somelib_RELEASE NAMES somelib) and find_library(somelib_DEBUG NAMES somelib) find?
If you don't get it after that please look into your cmake module directory and look at some Find<Modules>.cmake. It doesn't matter if the information is encoded in the directory for cmake since cmake knows nothing about that and there is no way to tell it to cmake in a general configuration dependent way.
Well, if I see the port which does not have debug suffixes - is it a bug?
no but it either has a vcpkg-cmake-wrapper.cmake if it has a cmake Find<Module>.cmake or it just has pkgconfig files which are ok but don't work correctly either in cmake multi config.
find_library(somelib_RELEASE NAMES somelib) and find_library(somelib_DEBUG NAMES somelib) find?
Why I should do these strange things? find_library(Boost COMPONENTS ...) with my patch just works. And Visual Studio builds successfully both Release and Debug targets.
it either has a vcpkg-cmake-wrapper.cmake
Great! So vcpkg-cmake-wrapper do the right thing and we don't need to have suffixes.
pkgconfig files which are ok but don't work correctly either in cmake multi config
Yes, and that is the job of vcpkg to provide a wrapper for multi config cmake and take the information from library directory or suffix. You are missing the point that these suffixes makes vcpkg builds unusable from other build systems. While I can provide a triplet directory root (either release or debug one) with clean names for any build system and it will find libraries without any specific changes. Yes it will not work for "multi config" builds, but it is not needed because there is no such concept in other build systems.
Let's get back to the core problem seeking a solution.
This PR resolves problem with other build systems: without system layout each build system should implement giant FindBoost.cmake analog for no reason.
Which buildsystem are you specifically having an issue with? I'm not sure if it has changed in recent versions, but the last time I checked the prebuilt binaries provided by Boost for Windows were built using the default layout and not the system layout. I assume that all C++ buildsystems must have a solution for using those well known binaries?
@ras0219-msft prebuilt Boost binaries are using auto linking by default and that is assumed by a lot of custom build tools. These suffixes were made for Boost auto linking feature. Quoting your answer in https://github.com/microsoft/vcpkg/issues/694#issuecomment-281235155
we want to embrace the common, cross-platform core that CMake defines. Therefore, when using CMake, we model the same experience (i.e. no-auto-linking) as on other platforms so you can minimize per-platform differences
Without suffixes I can use vcpkg Boost binaries in a custom build environment without platform-specific code, and there is no any reason to keep suffixes when auto linking is disabled.
@ras0219-msft ,Could you please help review this pr?
Sorry for taking a long time to get back to this issue, but we've thought more about the problem and come to the following conclusions:
- Relying on the publicly documented boost feature of autolinking (which is supported by the official boost distribution) makes sense and is something that vcpkg should support. If that will resolve your case, then I move that we retitle this issue to be "Re-enable boost autolinking with MSVC".
- While it would be convenient for boost to officially have the same binary names on all platforms, the official prebuilt windows binaries do not use the system layout, so it doesn't seem that us providing the system layout will overlap with existing users. Therefore, we do not see moving to the system layout as worth the costs to CMake's Multi-config generators. (MSBuild users are fine either way via our MSBuild integration).
- Yes, I'm ok with autolinking support, but that may require a lot more work
- Still not clear why we need such concept as "multi-config build", because everything just works with system layout
/azp run
Azure Pipelines successfully started running 1 pipeline(s).
@vitalyster, please run command ./vcpkg x-add-version boost-modular-build-helper --overwrite-versionand commit again
@vitalyster, Can you please resolve the conflicts against master?
@vitalyster, Can you please resolve the conflicts against master?
FTR, there is a draft PR to get a little bit of multi-config usage covered, #27408. It doesn't include boost. In any case, multi-config support shouldn't degrade with new PRs. After all, that's the whole point why vcpkg builds release and debug by default.
I will be converting your PR to draft status. The above suggested changes are only recommendations. If you are willing to adopt them, please click "ready for review" after making the modifications. If you do not wish to make any changes, please click "ready for review" directly. That way, I can be aware that you've responded since you can't modify the tags.
@vitalyster, Could you pleasae fix the conflicting files?
@vitalyster, please change the port-version of both boost-uninstall/vcpkg.json and boost-modular-build-helper/vcpkg.json files, then run command ./vcpkg x-add-version --all --overwrite-version and commit again.