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

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

Closed

Conversation

MarcelRaad
Copy link
Member

@MarcelRaad 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.

@MarcelRaad MarcelRaad added build LDAP Windows Windows-specific labels Oct 14, 2018
@jay
Copy link
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
Copy link
Member Author

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
Copy link
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
Copy link
Member Author

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

As done in the autotools and CMake builds by default.

Closes curl#3137
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 curl#3137
@MarcelRaad
Copy link
Member Author

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
Copy link
Member

jay commented Oct 19, 2018

Ok LGTM

MarcelRaad added a commit that referenced this pull request Oct 19, 2018
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
Copy link
Member Author

Thanks!

@MarcelRaad MarcelRaad deleted the visualstudio_missingfeatures branch October 19, 2018 07:29
@lock lock bot locked as resolved and limited conversation to collaborators Jan 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
build LDAP Windows Windows-specific
Development

Successfully merging this pull request may close these issues.

None yet

2 participants