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

[geogram] Fix windows dynamic build usage #14962

Merged
merged 3 commits into from
Mar 29, 2021

Conversation

FabienPean
Copy link
Contributor

Fix the use of geogram library for triplet x64-windows. The bug has been reported upstream, but no news from maintainers so far. The PR fixes the package for current vcpkg version, and may need to be reverted when a new version of the library is released.

@FabienPean FabienPean marked this pull request as ready for review December 6, 2020 17:55
@NancyLi1013
Copy link
Contributor

Hi @FabienPean

Thanks for your PR.

Could you please describe what the problem is in dynamic usage?

@NancyLi1013 NancyLi1013 added category:port-bug The issue is with a library, which is something the port should already support requires:author-response labels Dec 7, 2020
@FabienPean
Copy link
Contributor Author

Hi, sure !
The preprocessor definition "GEO_DYNAMIC_LIBS" is never defined during the CMake config step alicevision/geogram#17,
which is necessary to have the proper declimport/declexport settings on windows in this file geogram/src/lib/geogram/api/defs.h#L69
That's at least my understanding of the situation.

@NancyLi1013
Copy link
Contributor

The regressions caused by gsoap will be fixed by PR #16270.

@NancyLi1013 NancyLi1013 added the info:reviewed Pull Request changes follow basic guidelines label Feb 20, 2021
@vicroms vicroms merged commit 1054567 into microsoft:master Mar 29, 2021
@FabienPean FabienPean deleted the fix/geogram_dynamic branch March 29, 2021 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants