Collection of common port issues

[Neumann-A]

Trying to get a list together of common port issues which have been seen in the past and have been fixed or are still in need of a fix (maybe the fix is already known and it it just must be applied):

Oberserved Problems?

• Changing the debug name of a library, find_package broken: Renaming of debug libraries through CMAKE_DEBUG_POSTFIX or other means #6014
• Manually changing the name of a library / dll leading to the problems described in No postfix in debug dll of libwebm (followup) #6553
• Changing the exported symbols: Missing documentation for WINDOWS_EXPORT_ALL_SYMBOLS guidelines #6043
• Wrong library linkage: [WIP][vcpkg] vcpkg can link wrong libraries (issue #5540 b) #5543, vcpkg (silently) links wrong libraries #5540
• packageConfig.cmake in wrong directory: fix find_package for fftw3 and add arch features #4783
• package*.cmake different for release and debug and release version is overwritten by debug version: (probably also mentioned in [WIP][vcpkg] vcpkg can link wrong libraries (issue #5540 b) #5543 or add a better reference)
• single configuration triplets fail because the portfile trys to install/copy files which do not exist in a single configuration build

What should new ports always test?
(after of course building successfully)

• Port\Package should be correctly be found by find_package
• Linkage of packages should be correct for debug/release (must also be checked in any generated target/config file)
• single configuration triplets (either release or debug not both)

Should I ping people for more visibility?

Please add comments so that I can extend the list (and some of you have probably more experience than me fixing ports).

[vicroms]

There's also the case when consuming libraries cannot find a dependency due to static and shared libraries having different names.

"Fixable" by using a vcpkg-cmake-wrapper.cmake file, e.g.: rocksdb.

[Neumann-A]

@vicroms

"Fixable" by using a vcpkg-cmake-wrapper.cmake file, e.g.: rocksdb.

Please don't do that. Target RocksDB::rocksdb is probably intended to be always a static target while RocksDB::rocksdb-shared should always be a shared target. I had the same issue with hdf5 once and somebody thought it would be a good idea to redefine the targets. In x64-windows hdf5:hdf5-static was mapped to hdf5:hdf5-shared while in x64-windows-static it was the other way around. If you have problems with consuming packages not thinking about how to link correctly to rocksdb in all cases patch their behavior instead. If rocksdb is building shared as well as static targets (in e.g. x64-windows) do not deactivate that in vcpkg because of the missing target you generate with that. (Furthermore default behavior is unnecessary changed.)

[vicroms]

@Neumann-A thanks for the explanation. I haven't actually done it myself, it's just something I've noticed while reviewing PRs.

I agree, that we shouldn't change the default behavior of libraries, however producing both shared and static libs will fail during post-build validation. We could probably add a variable that should be explicitly set for ports that produce both libraries, to bypass this post-build validation.

[ras0219-msft]

Target RocksDB::rocksdb is probably intended to be always a static target while RocksDB::rocksdb-shared should always be a shared target.

Within vcpkg, it is our policy to "erase" the differences between static and shared targets. The reasoning for this is as follows:

1. It is critical to avoid symbol duplication to avoid ODR violations
2. Therefore, there needs to be a single owner for a given symbol (i.e. either a single static or a single shared library)
3. At the end of the day, it is the process owner (i.e. the person running vcpkg.exe) who should have total authority over all these choices.

Vcpkg provides a mechanism for that user to specify the single (shared/static) binary they want: the triplet file. For most libraries, this works perfectly.

However, libraries (like rocksdb) which change their name between static and shared cause problems. It means that somehow, higher level libraries that build upon rocksdb need to be "told" which of rocksdb's targets to use. In practice, 99% of the time, this configuration is not properly expressed -- when it is switched upon, it usually is done as "if I'm building as static, then of course my dependency must be static". This is obviously over-restrictive (take the extremely common case of building static libraries on top of the shared system copy of openssl).

Thus, leaving things as they are by default simply results in many failed builds for absolutely no reason. We therefore as a policy "erase" these distinctions by aliasing identical targets together, such as in the case of hdf5 and rocksdb. We will still recommend to the user that they use the "appropriate" target name for their configuration (so if you install rocksdb:x64-windows-static we recommend you use rocksdb::rocksdb and similar for rocksdb:x64-windows and rocksdb::rocksdb-shared), however we opt to follow the choice made in the triplet file when these do not exactly match.

[Neumann-A]

@vicroms:

I agree, that we shouldn't change the default behavior of libraries, however producing both shared and static libs will fail during post-build validation. We could probably add a variable that should be explicitly set for libraries that produce both libraries, to bypass this post-build validation.

I think this problem only occurs on -static triplets. In this case the port could try to set BUILD_SHARED_LIBS to OFF and observe if the package/port is doing the right thing (not installing DLLs).

@ras0219-msft:

Within vcpkg, it is our policy to "erase" the differences between static and shared targets.

That breaks downstream expectations. There might be a good reason why a library is actually linked statically with a shared CRT. One of those reasons is DLL-Hell which I don't know if it is still in issue but I saw strange behavior using MATLAB (HDF 1.8.12) and my program at the same time which both use hdf.dll if I link hdf dynamically so I assume yes. (Side Note: My program actual can use both MATLAB APIs and the newer HDF5 APIs (1.10.5), that is why I actually link hdf5-static).
Erasing the difference between static and shared targets is the wrong approach if the difference actual matters. It would be better to delete the other target completely to be consistent (But this of course would require a lot of extra patching).

1. It is critical to avoid symbol duplication to avoid ODR violations
2. Therefore, there needs to be a single owner for a given symbol (i.e. either a single static or a single shared library)
3. At the end of the day, it is the process owner (i.e. the person running vcpkg.exe) who should have total authority over all these choices.

Yes but it isn't solved by erasing the difference within vcpkg (see #5540 -> ##5485 (comment)). The main culprit here is the lazy linkage (VcpkgAutolink) within the visual studio integration which also requires the manual-link folder (which is prone to error as seen by the last mentioned issue). CMake does not require that because it explicitly links the correct library so there are no symbol clashes.
Furthermore, as far as I know, if a shared library is build properly on windows __declspec(dllexport) is required which adds extra decorations to the exported symbols (correct me if I am wrong here). So the symbols are already different.
If you are redirecting targets you are in reality taking away authority from downstream targets/packages/projects which I think is bad.

In the end in might just come down to change the implementation of how vcpkg achieves the visual studio integration. At the same time you probably also want to fix the /include path and overwriting of headers file be moving everything into /include/packagename. Again this choice was probably made to have a lazy VS integration despite the obvious problems.

Vcpkg provides a mechanism for that user to specify the single (shared/static) binary they want: the triplet file. For most libraries, this works perfectly.

That is already untrue. Because in x64-windows some libraries are build as static libraries and can only be build as that. I assume that mantra lead to, what I call, the great vcpkg dll fiasko #6043. Be a bit more strict with your own guidelines which can be reinterpreted as:

Do not change default package behavior if it is reasonable because you will break downstream targets or expectations

and see x64-windows as all libraries (static/shared) with shared CRT linkage instead.

[cbezault]

@NancyLi1013 Don't unpin this.

[heydojo]

single configuration triplets fail because the portfile trys to install/copy files which do not exist in a single configuration build

And

single configuration triplets (either release or debug not both)

Wasn't there a single issue thread at one point (not so long ago) to bundle all of those issues together? I was looking for it to check if python3 was on it:
https://github.com/microsoft/vcpkg/blob/master/ports/python3/portfile.cmake
because it has the issue where the port will try but fail to deal with the debug files if there is a triplet stopping debug from being built.

I fixed it in my copy of the tree and was going to push up a patch via merge. But I wanted to check that someone else hadn't flagged it in that thread. Unfortunately, the issue search function is woeful and didn't turn up anything when I tried. The search function worked much better before all the weird changes were made to github.

Does anyone know where the thread is please? (Hopefully maybe someone bookmarked it?)
Maybe it could be linked here by adding it to the thread's parent?

EDIT: I found it: #6368 but it looks like it has been closed.
#8165 details a discussion around using a function to fix the issue.

I've already pushed a pull request for python3.

[jmoguillansky-gpsw]

Within vcpkg, it is our policy to "erase" the differences between static and shared targets. The reasoning for this is as follows:

It is critical to avoid symbol duplication to avoid ODR violations
Therefore, there needs to be a single owner for a given symbol (i.e. either a single static or a single shared library)
At the end of the day, it is the process owner (i.e. the person running vcpkg.exe) who should have total authority over all these > choices.

I think this approach will lead to some issues in terms of maintanance and stability.
I understand that currently there's different triplets: "x64-windows" and "x64-windows-static".
The first issue is: won't there be a lot of duplicate files in these two locations?
For example, the include files and data files, etc. can be the same regardless of static or shared configuration.

Second: some libraries might support static build, some might support shared, and some: both.
So if someone wants to use libabc which is shared-only, and libxyz which is static only, then they need to
reference two different prefix targets: installed/x64-windows and installed/x64-windows-static, and this will be a messy configuration .

I understand that one of the motivations is to support vcpkg integrate install.
I suggest to follow a similar approach as Linux (e.g. apt, pacman, etc).
Have one install prefix (e.g. x64-windows). Put static libs, shared libs, debug libs, etc all in x64-windows/lib.
Add an extra file for vcpkg integration support (similar to how there's an extra file for pkg-config support) under lib/vcpkg/
for example, lib/vcpkg/avcodec.info that has extra information used by vcpkg integrate install, such as the default library for auto-linking via vcpkg integrate install, etc.

Maybe one other thing that could be useful is to support "vcpkg integrate install" on a per-package basis.
This way the user can choose exactly which libs they want to automatically add as linker params.
I see vcpkg integrate install as a useful development feature.
I'm not sure it's the right approach for production builds, but it's useful for a development environment.

[Neumann-A]

The first issue is: won't there be a lot of duplicate files in these two locations?

doesn't matter. Also the VS integration won't work otherwise due to requiring #define SOMEAPI __declspec(dllimport) in the main header of some libraries quite often.

Second: some libraries might support static build, some might support shared, and some: both.
So if someone wants to use libabc which is shared-only, and libxyz which is static only, then they need to
reference two different prefix targets: installed/x64-windows and installed/x64-windows-static, and this will be a messy configuration .

No the solution here is a custom triplet with per port customization. Do not mix different vcpkg installed folders!

Have one install prefix (e.g. x64-windows). Put static libs, shared libs, debug libs, etc all in x64-windows/lib.

won't work since you have not fixed the filename collision of import lib and static libs. You also have problems using find_library and finding the correct one in cmake.. Also you can only ever link one library so it does not make sense to have both!

(similar to how there's an extra file for pkg-config support)

pkg-config on linux is kind of broken considering you have to pass --static without actually knowing if you have to pass it. vcpkg fixes this by never requiring to --static since the pkgconfig are fixed to reflect the actual select library type.

[dg0yt]

Oberserved Problems?

Typo.

* single configuration triplets fail because the portfile trys to install/copy files which do not exist in a single configuration build

Now there is only "release" as single-config.

What should new ports always test? (after of course building successfully)

• Linkage of packages should be correct for debug/release (must also be checked in any generated target/config file)

This is systematically broken by vcpkg for INTERFACE_LINK_LIBRARIES in the main config file, because debug variant is silently dropped (#22546).

• single configuration triplets (either release or debug not both)

Now there is only "release" as single-config.

Additional problems:

• The find wrapper does not properly set the policies it needs.
• The find wrapper sets <Pkg>_LIBRARY with select_library_configurations to a list, but either the find module (e.g. FindFontconfig.cmake) or a consumer expect a single file path.
• pkg-config files missing for some triplets (usually windows).
• pkg-config data invalid (e.g. lib names do not match).

[JackBoosY]

We also should run post check in non-Windows to check the binary formats.