Skip to content

Commit

Permalink
build: fix MSVC UWP builds
Browse files Browse the repository at this point in the history
The MSVC UWP job in CI did not actually enable UWP. Fix this and
the fallouts discovered after enabling it.

- GHA/windows: make sure to enable UWP in MSVC vcpkg UWP job.
  Use the CMake options and C flags already used for mingw-w64, but use
  `WINAPI_FAMILY_PC_APP` instead of the deprecated `WINAPI_FAMILY_APP`.
  (The former is not supported by mingw-w64, so leave it there as-is.)
  Follow-up to cb22cfc curl#14077

- GHA/windows: by default the MSVC UWP job became 2x-3x slower than
  others after actually enabling UWP. Most of it is caused by
  CMake/MSBuild automatically building full APPX containers for each
  `.exe` target. This includes 21 CMake feature detections. Each
  detection app is built into a 15MB APPX project, with code signing,
  logos, etc. Example:
    https://github.com/curl/curl/actions/runs/12056968170/job/33620610958
  Disable this overhead for curl build targets via custom
  `CMAKE_VS_GLOBALS` options. I've found no way to apply them to feature
  detection targets, so those remain slow.

- cmake: automatically enable Unicode for UWP builds. It's required.
  Also stop enabling it manually in the existing CI job.

- tests: fix `getpid()` use for Windows UWP:
  ```
  tests\server\util.c(281,21): warning C4013: 'getpid' undefined; assuming extern returning int
  ```
  Ref: https://github.com/curl/curl/actions/runs/12061215311/job/33632904249#step:11:38

- src/tool_doswin: disable `GetLoadedModulePaths()` for UWP.
  mingw-w64 UWP was okay with this, but MS SDK headers are not.
  This makes `--dump-module-paths` return empty for UWP builds.
  ```
  src\tool_doswin.c(620,3): error C2065: 'MODULEENTRY32': undeclared identifier
  src\tool_doswin.c(626,11): warning C4013: 'CreateToolhelp32Snapshot' undefined; assuming extern returning int
  src\tool_doswin.c(626,36): error C2065: 'TH32CS_SNAPMODULE': undeclared identifier
  src\tool_doswin.c(632,7): warning C4013: 'Module32First' undefined; assuming extern returning int
  ```
  Ref: https://github.com/curl/curl/actions/runs/12055081933/job/33614629930#step:9:35

- examples: fix `websocket.c` to include `winsock2.h` before `windows.h`
  to make it build with MSVC UWP:
  ```
  include\curl\curl.h(143,16): error C2061: syntax error: identifier 'curl_socket_t'
  include\curl\curl.h(143,16): error C2059: syntax error: ';'
  include\curl\curl.h(417,52): error C2146: syntax error: missing ')' before identifier 'curlfd'
  include\curl\curl.h(417,38): error C2081: 'curl_socket_t': name in formal parameter list illegal
  ```
  Ref: https://github.com/curl/curl/actions/runs/12055317910/job/33615644427#step:14:126

- GHA/windows: silence linker warning with MSVC UWP builds:
  ```
  LINK : warning LNK4075: ignoring '/INCREMENTAL' due to '/OPT:ICF' specification
  ```
  Ref: https://github.com/curl/curl/actions/runs/12055696808/job/33616629610#step:11:38

- GHA/windows: set `/INCREMENTAL:NO` for all MSVC jobs to improve
  performance a little.

- cmake: show `UWP` platform flag.

Ref: curl#15652
Closes curl#15657
  • Loading branch information
vszakats committed Nov 28, 2024
1 parent 2f03242 commit a72b479
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 14 deletions.
13 changes: 11 additions & 2 deletions .github/workflows/windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ jobs:
- { build: 'cmake' , sys: 'mingw64', env: 'x86_64' , tflags: '' , config: '-DENABLE_DEBUG=ON -DBUILD_SHARED_LIBS=OFF -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON -DENABLE_ARES=ON', type: 'Debug', name: 'schannel c-ares U' }
- { build: 'cmake' , sys: 'ucrt64' , env: 'ucrt-x86_64' , tflags: 'skiprun' , config: '-DENABLE_DEBUG=OFF -DBUILD_SHARED_LIBS=ON -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON -DENABLE_CURLDEBUG=ON', type: 'Release', name: 'schannel R TrackMemory' }
- { build: 'cmake' , sys: 'clang64', env: 'clang-x86_64', tflags: 'skiprun' , config: '-DENABLE_DEBUG=ON -DBUILD_SHARED_LIBS=OFF -DCURL_USE_OPENSSL=ON -DENABLE_UNICODE=OFF', type: 'Release', name: 'openssl' }
- { build: 'cmake' , sys: 'ucrt64' , env: 'ucrt-x86_64' , tflags: 'skiprun' , config: '-DENABLE_DEBUG=OFF -DBUILD_SHARED_LIBS=ON -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON', type: 'Release', test: 'uwp', name: 'schannel' }
- { build: 'cmake' , sys: 'ucrt64' , env: 'ucrt-x86_64' , tflags: 'skiprun' , config: '-DENABLE_DEBUG=OFF -DBUILD_SHARED_LIBS=ON -DCURL_USE_SCHANNEL=ON', type: 'Release', test: 'uwp', name: 'schannel' }
- { build: 'cmake' , sys: 'mingw64', env: 'x86_64' , tflags: 'skiprun' , config: '-DENABLE_DEBUG=ON -DBUILD_SHARED_LIBS=ON -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON -DCMAKE_VERBOSE_MAKEFILE=ON', type: 'Debug', cflags: '-DCURL_SCHANNEL_DEV_DEBUG', name: 'schannel dev debug' }
- { build: 'cmake' , sys: 'mingw32', env: 'i686' , tflags: 'skiprun' , config: '-DENABLE_DEBUG=OFF -DBUILD_SHARED_LIBS=ON -DCURL_USE_SCHANNEL=ON -DENABLE_UNICODE=ON', type: 'Release', name: 'schannel R' }
fail-fast: false
Expand Down Expand Up @@ -676,11 +676,20 @@ jobs:
timeout-minutes: 5
run: |
PATH="/c/msys64/usr/bin:$PATH"
if [ '${{ matrix.plat }}' = 'uwp' ]; then
options+=' -DCMAKE_SYSTEM_NAME=WindowsStore -DCMAKE_SYSTEM_VERSION=10.0'
cflags='-DWINAPI_FAMILY=WINAPI_FAMILY_PC_APP'
ldflags='/OPT:NOREF /OPT:NOICF /APPCONTAINER:NO'
vsglobals=';AppxPackage=false;WindowsAppContainer=false'
fi
cmake -B bld ${options} \
"-DCMAKE_TOOLCHAIN_FILE=$VCPKG_INSTALLATION_ROOT/scripts/buildsystems/vcpkg.cmake" \
"-DVCPKG_INSTALLED_DIR=$VCPKG_INSTALLATION_ROOT/installed" \
'-DVCPKG_TARGET_TRIPLET=${{ matrix.arch }}-${{ matrix.plat }}' \
-DCMAKE_VS_GLOBALS=TrackFileAccess=false \
"-DCMAKE_C_FLAGS=${cflags}" \
"-DCMAKE_EXE_LINKER_FLAGS=/INCREMENTAL:NO ${ldflags}" \
"-DCMAKE_SHARED_LINKER_FLAGS=/INCREMENTAL:NO ${ldflags}" \
"-DCMAKE_VS_GLOBALS=TrackFileAccess=false${vsglobals}" \
'-DCMAKE_BUILD_TYPE=${{ matrix.type }}' \
-DCMAKE_UNITY_BUILD=ON -DCURL_TEST_BUNDLES=ON \
-DCURL_WERROR=ON \
Expand Down
6 changes: 6 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ endif()
if(WIN32)
set(_target_flags "${_target_flags} WIN32")
endif()
if(WINDOWS_STORE)
set(_target_flags "${_target_flags} UWP")
endif()
if(CYGWIN)
set(_target_flags "${_target_flags} CYGWIN")
endif()
Expand Down Expand Up @@ -178,6 +181,9 @@ if(WIN32)
endif()

option(ENABLE_UNICODE "Use the Unicode version of the Windows API functions" OFF)
if(WINDOWS_STORE)
set(ENABLE_UNICODE ON)
endif()
if(ENABLE_UNICODE)
add_definitions("-DUNICODE" "-D_UNICODE")
if(MINGW)
Expand Down
1 change: 1 addition & 0 deletions docs/examples/websocket.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <stdio.h>
#include <string.h>
#ifdef _WIN32
#include <winsock2.h>
#include <windows.h>
#define sleep(s) Sleep((DWORD)(s))
#else
Expand Down
6 changes: 6 additions & 0 deletions lib/curl_setup.h
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,12 @@
#include <curl/stdcheaders.h>
#endif

#ifdef _WIN32
#define Curl_getpid() GetCurrentProcessId()
#else
#define Curl_getpid() getpid()
#endif

/*
* Large file (>2Gb) support using Win32 functions.
*/
Expand Down
8 changes: 1 addition & 7 deletions lib/smb.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@

#if !defined(CURL_DISABLE_SMB) && defined(USE_CURL_NTLM_CORE)

#ifdef _WIN32
#define Curl_getpid() ((unsigned int)GetCurrentProcessId())
#else
#define Curl_getpid() ((unsigned int)getpid())
#endif

#include "smb.h"
#include "urldata.h"
#include "sendf.h"
Expand Down Expand Up @@ -548,7 +542,7 @@ static void smb_format_message(struct Curl_easy *data, struct smb_header *h,
h->flags2 = smb_swap16(SMB_FLAGS2_IS_LONG_NAME | SMB_FLAGS2_KNOWS_LONG_NAME);
h->uid = smb_swap16(smbc->uid);
h->tid = smb_swap16(req->tid);
pid = Curl_getpid();
pid = (unsigned int)Curl_getpid();
h->pid_high = smb_swap16((unsigned short)(pid >> 16));
h->pid = smb_swap16((unsigned short) pid);
}
Expand Down
4 changes: 3 additions & 1 deletion src/tool_doswin.c
Original file line number Diff line number Diff line change
Expand Up @@ -616,9 +616,10 @@ CURLcode FindWin32CACert(struct OperationConfig *config,
*/
struct curl_slist *GetLoadedModulePaths(void)
{
struct curl_slist *slist = NULL;
#if !defined(CURL_WINDOWS_UWP)
HANDLE hnd = INVALID_HANDLE_VALUE;
MODULEENTRY32 mod = {0};
struct curl_slist *slist = NULL;

mod.dwSize = sizeof(MODULEENTRY32);

Expand Down Expand Up @@ -661,6 +662,7 @@ struct curl_slist *GetLoadedModulePaths(void)
cleanup:
if(hnd != INVALID_HANDLE_VALUE)
CloseHandle(hnd);
#endif
return slist;
}

Expand Down
2 changes: 1 addition & 1 deletion tests/server/rtspd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1394,7 +1394,7 @@ int main(int argc, char *argv[])

if(got_exit_signal) {
logmsg("========> %s rtspd (port: %d pid: %ld) exits with signal (%d)",
ipv_inuse, (int)port, (long)getpid(), exit_signal);
ipv_inuse, (int)port, (long)Curl_getpid(), exit_signal);
/*
* To properly set the return status of the process we
* must raise the same signal SIGINT or SIGTERM that we
Expand Down
2 changes: 1 addition & 1 deletion tests/server/sws.c
Original file line number Diff line number Diff line change
Expand Up @@ -2492,7 +2492,7 @@ int main(int argc, char *argv[])

if(got_exit_signal) {
logmsg("========> %s sws (%s pid: %ld) exits with signal (%d)",
socket_type, location_str, (long)getpid(), exit_signal);
socket_type, location_str, (long)Curl_getpid(), exit_signal);
/*
* To properly set the return status of the process we
* must raise the same signal SIGINT or SIGTERM that we
Expand Down
2 changes: 1 addition & 1 deletion tests/server/tftpd.c
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@ int main(int argc, char **argv)

if(got_exit_signal) {
logmsg("========> %s tftpd (port: %d pid: %ld) exits with signal (%d)",
ipv_inuse, (int)port, (long)getpid(), exit_signal);
ipv_inuse, (int)port, (long)Curl_getpid(), exit_signal);
/*
* To properly set the return status of the process we
* must raise the same signal SIGINT or SIGTERM that we
Expand Down
2 changes: 1 addition & 1 deletion tests/server/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ curl_off_t our_getpid(void)
{
curl_off_t pid;

pid = (curl_off_t)getpid();
pid = (curl_off_t)Curl_getpid();
#if defined(_WIN32)
/* store pid + 65536 to avoid conflict with Cygwin/msys PIDs, see also:
* - https://cygwin.com/git/?p=newlib-cygwin.git;a=commit; ↵
Expand Down

0 comments on commit a72b479

Please sign in to comment.