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

MSVC: Fix build with recent versions of libcurl #1722

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
3 participants
@lephilousophe
Copy link
Member

commented Jun 30, 2019

The missing CURL_STATICLIB prevents linking with static libcurl.lib
(which is what is used for 32-bit builds)
The missing libraries raise external symbol errors with recent versions
of libcurl due to curl/curl@7921659.
More explanations can be found in curl/curl#2474.

MSVC: Fix build with recent versions of libcurl
The missing CURL_STATICLIB prevents linking with static libcurl.lib
(which is what is used for 32-bit builds)
The missing libraries raise external symbol errors with recent versions
of libcurl due to curl/curl@7921659.
More explanations can be found in curl/curl#2474.
@SupSuper

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2019

These should be conditional, as it's possible to build project with curl disabled. See for example:

if (curlEnabled && projectType == kProjectMSVC)
setup.defines.push_back("CURL_STATICLIB");
if (sdlnetEnabled && projectType == kProjectMSVC)
setup.libraries.push_back("iphlpapi");

Extra libraries should be added via setup. I don't know why defines need to be "hacked" into x64 though.

@lephilousophe

This comment has been minimized.

Copy link
Member Author

commented Jun 30, 2019

Well, I just added a hack over the hack.
It seems that, to filter out some stuff for x64, MSVC generator rebuild a defines list.
At this stage we don't have curlEnabled anymore to add define conditionnally.
It's the whole hack section which should be removed.

@bluegr

This comment has been minimized.

Copy link
Member

commented Jun 30, 2019

I agree with @SupSuper that these libraries should not be enforced, and should be conditionally enabled

@SupSuper

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

@lephilousophe I made a new PR to fix the hack and solve your issues, let me know if it works for you: #1725

@lephilousophe

This comment has been minimized.

Copy link
Member Author

commented Jul 1, 2019

Yours is totally better.
I will check if everything compile well but I can already close this PR.

@lephilousophe lephilousophe deleted the lephilousophe:fix-libcurl-link branch Jul 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.