-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
windows: fix compilation for Windows UWP platform #7006
Conversation
e34ad28
to
c92467d
Compare
@@ -50,6 +50,10 @@ | |||
# define in_addr_t unsigned long | |||
#endif | |||
|
|||
#if defined(WINAPI_FAMILY) && (WINAPI_FAMILY == WINAPI_FAMILY_APP) | |||
# include <afunix.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have never heard of that header before. A google search doesn't lead me to any official microsoft pages. Are you sure this is contained in stock Windows and if so since which version? This must also be checked!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's weird - about the only information about this header is on the MS blog: https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On my machine this header is located like this:
C:\Program Files (x86)\Windows Kits\10\Include\10.0.19041.0\shared
It is bundled with Windows 10 SDK (10.0.19041.0)
, standard installation via Visual Studio. Check more details here: https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
Usage example:
https://devblogs.microsoft.com/commandline/windowswsl-interop-with-af_unix/
This header is also mentioned in the sources too:
Lines 723 to 730 in 70cf50f
/* Replicating logic present in afunix.h of newer Windows 10 SDK versions */ | |
# define UNIX_PATH_MAX 108 | |
/* !checksrc! disable TYPEDEFSTRUCT 1 */ | |
typedef struct sockaddr_un { | |
ADDRESS_FAMILY sun_family; | |
char sun_path[UNIX_PATH_MAX]; | |
} SOCKADDR_UN, *PSOCKADDR_UN; | |
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is bundled with Windows 10 SDK (10.0.19041.0), standard installation via Visual Studio. Check more details here: https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
Then please check for that build and don't discriminate users of older versions of Windows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@emilengler, would you please elaborate on your comment?
Even though the SDK is one of the latest you can target the required min. Windows version for the build, in my case project is targeting 10.0.15063.0
which is fairly old 1703 Creators Update from early 2017 (https://en.wikipedia.org/wiki/Windows_10_version_history).
Without this correction curl can not be build successfully as explained in the PR description. Developers targeting the older versions could try to not define USE_UNIX_SOCKETS
.
lib/curl_addrinfo.c
Outdated
@@ -50,6 +50,10 @@ | |||
# define in_addr_t unsigned long | |||
#endif | |||
|
|||
#if defined(WINAPI_FAMILY) && (WINAPI_FAMILY == WINAPI_FAMILY_APP) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also check if compiling with USE_UNIX_SOCKETS
otherwise it won't compile on Windows systems that don't have the header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made an experiment and targeted the very first Windows 10 version (10.0.10240.0
) and tried to compile curl:
Without a fix I am getting the following failure:
curl_addrinfo.c(483,8): error C2027: use of undefined type 'sockaddr_un'
With a fix applied source compiles successfully. So targeting the initial Windows 10 version (10.0.10240.0
) with the standard latest Windows 10 SDK (10.0.19041.0)
shipped with Visual Studio works fine.
The proposed PR covers only Windows 10 case, so Win32 family is not affected by this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the proposed additional check for USE_UNIX_SOCKETS
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please run checksrc
./curl_addrinfo.c:53:96: warning: Longer than 79 columns (LONGLINE)
#if defined(USE_UNIX_SOCKETS) && defined(WINAPI_FAMILY) && (WINAPI_FAMILY == WINAPI_FAMILY_APP)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay as long as the CPP macros WINAPI_FAMILY
and WINAPI_FAMILY_APP
are available only on Windows 10 you have my go 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(WINAPI_FAMILY == WINAPI_FAMILY_APP)
is valid only for Windows 10 UWP platform and both defines are declared in winapifamily.h
:
It is used for quite long time in PortAudio project without any issue for several years:
https://github.com/PortAudio/portaudio/blob/39bfe6c63df5b85bb651f16327e7386504089276/src/hostapi/wasapi/pa_win_wasapi.c#L68
c92467d
to
80518c5
Compare
…unix.h which is necessary for sockaddr_un when USE_UNIX_SOCKETS is defined
80518c5
to
23f17cd
Compare
Thanks |
Fix compilation for Windows UWP platform by including the
afunix.h
which is necessary forsockaddr_un
whenUSE_UNIX_SOCKETS
is defined.