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

Jump to bottom

curl-win32.h: Enable Unix Domain Sockets based on the Window SDK version #3939

Closed
wants to merge 1 commit into from
Closed

curl-win32.h: Enable Unix Domain Sockets based on the Window SDK version #3939

wants to merge 1 commit into from

Conversation

captain-caveman2k
Copy link
Contributor

@captain-caveman2k captain-caveman2k commented May 24, 2019 •
edited

Microsoft added support for Unix Domain Sockets in Windows 10 1803 (RS4).

Rather than expect the user to enable Unix Domain Sockets by uncommenting the #define that was added in 0fd6221 we use the RS4 pre-processor variable that is present in newer versions of the Windows SDK.

@MarcelRaad
Copy link
Member

Note: We might want to consider not making this dependent on the MSVC as well - any thoughts?

Yes, as the feature is not compiler-dependent, I'd rather remove the existing check.

sdkddkver.h has always been there and only winsdkver.h has been introduced in a Windows SDK later than the lowest one we support, right? I always confuse these headers.

@captain-caveman2k
Copy link
Contributor Author

captain-caveman2k commented May 24, 2019 •
edited

Yes, as the feature is not compiler-dependent, I'd rather remove the existing check.

Yes that was pretty much view as well.

sdkddkver.h has always been there and only winsdkver.h has been introduced in a Windows SDK later than the lowest one we support, right? I always confuse these headers.

I've only got the latest SDK's installed on my home workstation and laptop, but here I have v5.0, v7.0, v7.0A, etc... Both are present in v7.0+ but not v5.0. As such I think we can use WINVER / _WIN32_WINNT to include sdkddkver.h - I will play about with that tonight.

MarcelRaad
MarcelRaad approved these changes May 25, 2019
View reviewed changes
Copy link
Member

@MarcelRaad MarcelRaad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

MarcelRaad
MarcelRaad reviewed May 25, 2019
View reviewed changes
lib/config-win32.h Outdated
#if defined(_MSC_VER) && _MSC_VER >= 1900
/* #define USE_UNIX_SOCKETS */
#if defined(_MSC_VER) && (_MSC_VER >= 1500)
/* sdlddkver.h first shipped with Platform SDK v6.0A included with VS2008 */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo

@bagder
Copy link
Member

bagder commented May 25, 2019

Coveralls is silly again.

@captain-caveman2k captain-caveman2k changed the title curl-win32.h: Automatically enable Unix Domain Sockets based on the Window SDK version curl-win32.h: Enable Unix Domain Sockets based on the Window SDK version May 28, 2019
@captain-caveman2k captain-caveman2k deleted the win_domain_sockets branch May 31, 2019 11:43
php-pulls pushed a commit to php/php-src that referenced this pull request Jun 6, 2019
@cmb69
Fix test regarding Unix Domain Sockets on Windows
2df3294 
Recent Windows versions actually support Unix Domain Sockets.  Cf.
<curl/curl#3939>.
@lock lock bot locked as resolved and limited conversation to collaborators Aug 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MarcelRaad MarcelRaad MarcelRaad approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@captain-caveman2k @MarcelRaad @bagder