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

windows: fix compilation for Windows UWP platform #7006

Closed
wants to merge 1 commit into from

Conversation

dmitrykos
Copy link
Contributor

Fix compilation for Windows UWP platform by including the afunix.h which is necessary for sockaddr_un when USE_UNIX_SOCKETS is defined.

@dmitrykos dmitrykos force-pushed the fix_compile_sockaddr_un_uwp branch from e34ad28 to c92467d Compare May 4, 2021 14:59
@@ -50,6 +50,10 @@
# define in_addr_t unsigned long
#endif

#if defined(WINAPI_FAMILY) && (WINAPI_FAMILY == WINAPI_FAMILY_APP)
# include <afunix.h>
Copy link
Contributor

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!

Copy link
Contributor

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/

Copy link
Contributor Author

@dmitrykos dmitrykos May 4, 2021

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:

curl/lib/config-win32.h

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

Copy link
Contributor

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

Copy link
Contributor Author

@dmitrykos dmitrykos May 4, 2021

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.

@@ -50,6 +50,10 @@
# define in_addr_t unsigned long
#endif

#if defined(WINAPI_FAMILY) && (WINAPI_FAMILY == WINAPI_FAMILY_APP)
Copy link
Contributor

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

Copy link
Contributor Author

@dmitrykos dmitrykos May 4, 2021

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:
image

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.

Copy link
Contributor Author

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.

Copy link
Member

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)

Copy link
Contributor

@emilengler emilengler May 5, 2021

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 👍

Copy link
Contributor Author

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

@dmitrykos dmitrykos force-pushed the fix_compile_sockaddr_un_uwp branch from c92467d to 80518c5 Compare May 4, 2021 19:46
@jay jay added build Windows Windows-specific labels May 5, 2021
…unix.h which is necessary for sockaddr_un when USE_UNIX_SOCKETS is defined
@dmitrykos dmitrykos force-pushed the fix_compile_sockaddr_un_uwp branch from 80518c5 to 23f17cd Compare May 5, 2021 06:55
@jay jay closed this in 79a05e1 May 5, 2021
@jay
Copy link
Member

jay commented May 5, 2021

Thanks

@dmitrykos dmitrykos deleted the fix_compile_sockaddr_un_uwp branch May 6, 2021 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Windows Windows-specific
Development

Successfully merging this pull request may close these issues.

None yet

4 participants