[glibmm] fix #6550 by partially reverting #5937 (+minor correction in glibmmconfig.h)#6621
[glibmm] fix #6550 by partially reverting #5937 (+minor correction in glibmmconfig.h)#6621grdowns merged 9 commits intomicrosoft:masterfrom
Conversation
(partially reverts change discussed in microsoft#5937)
|
The build appears to have some issues. Unfortunately, I don't have access to the details. |
|
It's failing on Linux x64, I'm away from a work computer so I can't get you logs right now. |
|
P. S. Here's a link to the beta public pipeline that should give you access to Linux build results https://dev.azure.com/vcpkg/public/_build?definitionId=8&_a=summary please give us any feedback you have. |
|
Thanks, I've found the logfile and it leaves me puzzled: The log file config-x64-linux-dbk-out.log does indeed state I don't know how to continue from here without digging deep into the build pipelines... |
|
Agreed is seems weird that those configure_files are getting run, I'll look into it more tomorrow. |
ports/glibmm/CMakeLists.txt
Outdated
| endif() | ||
|
|
||
| # A hacky solution for building Windows DLLs from sources ported from Linux. | ||
| # The cleaner approach would be to manually add __declspec(dllexport) to all public symbols in the header files or |
There was a problem hiding this comment.
It's not so much that it's hacky, it's more that vcpkg doesn't feel like we should export symbols when the library maintainer chooses not to. In the case of glibmm the library does actually choose to export all symbols. vcpkg provides a full build system replacement for glibmm and so we also need to export all symbols. This comment should reflect that.
There was a problem hiding this comment.
I admit that the comment can be phrased better, but I don't agree with your reasoning:
The library maintainer did not make a choice. They simply did not consider Windows. On Linux, you always export everything. This flag is never about overriding upstream decisions, but about mimicking Linux default behaviour on Windows. Without the flag, the DLL exports nothing, which is clearly not the authors intention.
To this point, glibmm is in no way special. The only special aspect is that static linking it's not an option.
There was a problem hiding this comment.
"hacky" is also still true, because this is kind of lazy: Of you really want to know the authors intention, you could simply go through all the public headers and handcraft a .def file from there. That is what the author would have done, if that option existed on Linux.
There was a problem hiding this comment.
That's untrue the library does export everything. Look at MSVC_NMake/gendef/gendef.cc.
There was a problem hiding this comment.
I had no idea that file existed! Anyhow, it does not change my main point: Any DLL ported from Linux needs to add export defs. For glibmm, this was obviously already done upstream. If it had not been done, vcpkg would still have to do it, to make the DLL work.
Anyhow, I won't press this any further. I'll rephrase the comment and pull out of the more general discussion.
|
Hey @NNemec, I had the chance to dig into this problem and resolve the issues. As per the prior conversation, I've removed |
|
@grdowns the whole point of this PR was to add |
|
I totally misunderstood the discussion -- it seems pretty obvious in retrospect. I'll make that change 👍 |
No description provided.