Remove references to CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS

[vicroms]

Cherry-picked from PR #5169

[cenit]

Thanks for cherry picking. I am more than busy these days, but let me know if I broke anything so that I can try to fix it

[vicroms]

I just hoping that it won't become too troublesome to fix the Git history in your OpenCV 4 PR.

[cenit]

No problem. After this merge I will clean up there everything and force push something much more manageable. I just hope you will see the light after this tunnel. I am sure you are finding like me so many broken ports...

[Fleischner]

I'm not sure if I like this. I see the points and I followed the discussion in #5784. But this seems to target a lot of libraries, and I'm concerned that we might hit problems with mixing static and dynamic libraries.

From #5784:

above said, it is important to enable users to opt into advanced or custom scenarios assuming they are fully aware of the downsides. In this case, it is as simple as removing the vcpkg_check_linkage() line and adding a -DCMAKE_WINDOWS_EXPORT_ALL_SYMBOLS=ON declaration to the vcpkg_install_cmake() command.

@ras0219-msft @vicroms @qis Can we find a way to allow this choice without touching any port files? Having changed files is such a mess during upgrade. E.g an triplet option. MAKE_WINDOWS_EXPORT_ALL_SYMBOLS might be a hack, but disallowing dynamic linkage for so many ports is a huge change that many people might be surprised about.

Also is there any roadmap on the compiler side to handle this issue, or will these libraries stay with static linkage for ever?

[cenit]

@Fleischner

Being the author of the original commit, I agree with you that the rationale behind it should be really explained at least in the documentation. for me, the work I did was more of a demonstration of the huge changes required because of the policy change than a real PR (almost automated changes, I didn’t really had to use any intelligence and I did it like an anti-stress...).
What @ras0219-msft was saying and what I accepted was that we should not export symbols like we were the authors. So no CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS written by us. That’s ok and perfectly acceptable. Of course if the author creates the library exploiting that cmake functionality, i’d really not touch his baby. But since here we were us doing it, I can see the point.
Besides blocked optimizations paths, there’s also the risk of exposing internal API, which the author would not like to support maybe...?

[Fleischner]

What @ras0219-msft was saying and what I accepted was that we should not export symbols like we were the authors. So no CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS written by us. That’s ok and perfectly acceptable. Of course if the author creates the library exploiting that cmake functionality, i’d really not touch his baby. But since here we were us doing it, I can see the point.

I agree this sounds reasonable. But still that should be done with care. If the library comes from linux and could be build there as linked library it should be assumed that the library is intended to be used as linked library is certain scenarios. Vcpkg still doesn't build and run tests #3107. If dependent libraries consist of multiple dlls, this could result in undetected errors from duplicated global state.

Probably the best option, but also with highest effort, would be to forward issues to the libraries itself. E.g. a couple of month ago I tried to apply WINDOWS_EXPORT_ALL_SYMBOLS to ompl and it failed on the symbol limit. I asked the authors if they can support windows dll by adding exports. They did, and now there is an (still unmerged) branch https://github.com/ompl/ompl/tree/symbol_visibility which needs some further work. It seems a lot of Linux focused libraries are not aware of the need to specify exports explicitly.

[cenit]

would you mind cherry-picking also b994fb6 in this PR?

[cenit]

I am working towards cleaning up the OpenCV4 PR commit history (which is huge anyway), removing all "external" to the project itself, while at the same time I'd like to save all the useful fruits, if you consider them tasty

[vicroms]

@cenit
I cherry-picked your second commit. I'll keep working on ironing out the regressions caused by these changes.

[cenit]

Thanks. I sincerely can’t understand how reducing options is causing regressions... i was convinced that many are not regressions, the ports were failing also in master.
If only you could put out status, I would help you.
I will push the clenaed up commit history in the opencv4 PR today. Thanks for the great help along this journey

[vicroms]

Current Status

All "regressions" fixed.

[cenit]

@qis the fact that we just reduced build options should not introduce any problem from this point of view. I would agree with you in case of an expansion of the build options! On the contrary, this PR just emerged many ports with small problems, which would have never worked in any case...

@Fleischner I agree with you that a clear stance on it is necessary. In any case, with ports properly written, it's extremely easy to revert this PR or (example) just adopt a "bypass" mechanism that can opt-in to CMAKE_EXPORT_ALL_SYMBOLS and enable dynamic libs also when declared static-only, with very little effort.

[Fleischner]

@cenit @ras0219-msft I would suggest to allow an per port option that can be enabled in the triplet as described in the
Per-port customization.

Proposal: Extend vcpkg_check_linkage

1. To prevent combinatory explosion and keep testing effort manageable, vcpkg should only support one tested default configuration per triplet. With vcpkg_check_linkage in all ports those default configurations are now defined. Therefore show a warning (unsupported/untested) whenever the user triggers an per port change of the linking. To enable detection, the user should not touch VCPKG_LIBRARY_LINKAGE per port basis. Instead there should be an a per port option for the triplet file VCPKG_LIBRARY_LINKAGE_PER_PORT_OVERRIDE. vcpkg will show an warning (unsupported/untested) if the linkage was successfully changed to the requested per port override. Or throw an error if this was not possible.

2. Add an additional option ONLY_STATIC_LIBRARY_SUPPORTS_UNCLEAN_AUTO_EXPORT. This allows the user to set a per port option for the triplet file VCPKG_LIBRARY_LINKAGE_PER_PORT_FORCE_UNCLEAN_AUTO_EXPORT . Throws error if port does not have vcpkg_check_linkage(ONLY_STATIC_LIBRARY_SUPPORTS_UNCLEAN_AUTO_EXPORT). Otherwise sets internal variable that gets forwarded to CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS in the port file or passed to the corresponding library option for configure. Also show a warning that states the problems with auto export.

Furthermore:

• Forward problems to library authors, to request dllexport additions for dynamic support on windows
• Internal forward to the compiler team, if this could be solved with upcoming c++20 modules (my last info is that export and dllexport will have no relationship)
• Special treatment, if static linkage would raise issues with GLP or LGPL licenses.

[vicroms]

The following packages will now fail to build for x64-windows-static:

• angle
• fann
• halide
• hpx
• jemalloc
• libepoxy
• openblas
• opencl
• openjpeg
• sciter
• tbb
• unrar

This is intended behavior as vcpkg_check_linkage(ONLY_DYNAMIC_LIBRARY) will refuse to build a dynamic library with static CRT linkage, this validation can be bypassed by explicitly requesting the configuration in a custom triplet file.

[cenit]

great job Victor @vicroms !

[JackBoosY]

My point is: using CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS will break the permissions of the function interface, which may not be what the author hopes, but setting vcpkg_check_linkage(ONLY_STATIC_LIBRARY) causes the relative port to generate only static libraries, which is limited.
Some link problems(such as LNK2019 LNK2001) are due to the fact that the author does not declare that the port is exportable. I think these are code errors. It is not appropriate to use CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS to fix these problems.

[NNemec]

I believe a problem that I'm currently struggling with is caused by this change:

glibmm is build with ONLY_STATIC_LIBRARY but at least three libraries (pangomm, atkmm and gtkmm) are built as dynamic libraries. That means that an application using gtkmm which pulls in all of them ends up with three copies of glibmm and therefore three copies of all static variables that are inside.

I see various options:

• give up attempting to use DLLs and go with windows-static altogether
• make sure that all libraries depending on an ONLY_STATIC_LIBRARY (at least those with static state) are sat to ONLY_STATIC_LIBRARY as well.
• hand-craft a .def file that collects all the symbols that are supposed to be exported by the DLL
• revert this change

I'm fairly new to the topic, but my understanding is that CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS is intended exactly for this use case: a libraries developed on Linux where every symbol is exported when building a dynamic library. Sure, it is a hack, but it seems a bit premature to generically remove it before a clean replacement exists.

[NNemec]

Correction: the three dependent libraries cannot be built as static libraries. They have ONLY_DYNAMIC_LIBRARY set and naively replacing that by ONLY_STATIC_LIBRARY leads to errors that i do not want to attempt digging into.

Simply reverting this patch selectively for glibmm fixes all my problems.

I would ask to consider reverting this patch completely and only selectively removing CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS in those cases where it actually causes problems or a clean solution without it exists. Just forcing ONLY_STATIC_LIBRARY for all cases does not appear to be a viable solution.

[Fleischner]

This is more or less exactly the situation I expected to happen.

give up attempting to use DLLs and go with windows-static altogether

Is not an option for us, because of LGPL license issues

make sure that all libraries depending on an ONLY_STATIC_LIBRARY (at least those with static state) are sat to ONLY_STATIC_LIBRARY as well.

same reason

Additional options:

• Extend vcpkg_check_linkage as suggested above
• Just wait for incoming issues reporting obscure crashes and strange behavior to find all problematic ports

[cbezault]

@NNemec @Fleischner We actually provide a build system replacement for glibmm, which overrides their build system which does export all symbols when producing dlls. In this case we should have left CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS.