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

Win32: enable IPv6 and LDAPS for non-configure builds #3137

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@MarcelRaad
Member

MarcelRaad commented Oct 14, 2018

As done by default in the autotools and CMake builds.

IPv6 is enabled for Winsock2 when targeting Windows XP or later, which introduced stable IPv6 support. Only when HAVE_GETADDRINFO is defined to avoid a linker error as Curl_ipv4_resolve_r is used but undefined otherwise.

LDAPS is enabled unconditionally when using WinLDAP.

@jay

This comment has been minimized.

Member

jay commented Oct 16, 2018

IPv6 is supposed to be enabled by USE_IPV6 so isn't the real issue that those projects should do it by default? IIRC Visual Studio 2010 or later we know the default SDKs include IPv6 so what if I add that define to those project files?

@MarcelRaad

This comment has been minimized.

Member

MarcelRaad commented Oct 16, 2018

Yes, for Visual Studio 2010 and later it's safe to define it as the minimum target Windows version is XP. However, IIRC it's also supported in much older Visual Studio versions (at least 2005) as long as the target Windows version is at least XP. Maybe add it to the 2008+ projects and see what AppVeyor says - if one patches the project to target Windows 2000, they can surely also remove that define.

The reason I put it there was that it looked like config-win32.h seems to support many more build environments than Visual Studio. I thought there were at least non-configure MinGW makefiles, but it seems they have been removed? But then there would be no way to disable it except patching config-win32.h if we don't introduce a macro for that. Adding it to the Visual Studio projects would definitely be the more backward compatible way.

@jay

This comment has been minimized.

Member

jay commented Oct 16, 2018

AppVeyor uses cmake so I don't think it applies. winbuild/ the makefile already uses USE_IPV6 and I tried with VS2008 and it builds fine (as long as IDN is disabled). The pre-generated project files in projects/ I added USE_IPV6 to the VS2008 project and it builds fine. I am using VS2008 SP1 though I don't know about when SP1 isn't applied.

@MarcelRaad

This comment has been minimized.

Member

MarcelRaad commented Oct 16, 2018

Oh, of course, then AppVeyor is already using IPv6. Thanks for confirming! I'll add it to the project files instead then.

MarcelRaad added some commits Oct 14, 2018

config_win32: enable LDAPS
As done in the autotools and CMake builds by default.

Closes #3137
VS projects: add USE_IPV6
The Visual Studio builds didn't use IPv6. Add it to all projects since
Visual Studio 2008, which is verified to build via AppVeyor.

Closes #3137
@MarcelRaad

This comment has been minimized.

Member

MarcelRaad commented Oct 17, 2018

Replaced the commit enabling IPv6 via config-win32.h by one adding USE_IPV6 to all Visual Studio libcurl project files since Visual Studio 2008.

@jay

This comment has been minimized.

Member

jay commented Oct 19, 2018

Ok LGTM

MarcelRaad added a commit that referenced this pull request Oct 19, 2018

VS projects: add USE_IPV6
The Visual Studio builds didn't use IPv6. Add it to all projects since
Visual Studio 2008, which is verified to build via AppVeyor.

Closes #3137
@MarcelRaad

This comment has been minimized.

Member

MarcelRaad commented Oct 19, 2018

Thanks!

@MarcelRaad MarcelRaad deleted the MarcelRaad:visualstudio_missingfeatures branch Oct 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment