-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
[vcpkg-cmake] Fix windows toolchain chainloading for uwp #21857
Conversation
as a testament to how much uwp is used… unfortunately |
CC @BillyONeal @ras0219-msft for accelerated review, if uwp matters. |
Related: #19818. |
@JackBoosY I don't think #19818 is related to this PR. Maybe you meant to add the comment to #21507? |
@dg0yt For uwp, there are some problems when using |
(I do separate commits to facilitate reviews. Please make use of it while reviewing.) |
@BillyONeal Another vcpkg ci exception again on agent vcpkg-eg-mac-02, https://dev.azure.com/vcpkg/public/_build/results?buildId=64286&view=logs&j=7b75bd19-17d3-53d4-00fd-23f1a49a8ba4&t=a23dba4e-3a62-59dc-a73c-a7222676d7a4. |
@BillyONeal Even worse, the windows builds are green despite the build failing for qtinterfaceframework. This even happened in yesterdays run. Failure logs are uploaded for all triplets. |
qt* failures in baseline has been fixed by #21925. |
Please merge to master first. |
Ideally we would use the right |
Well, do you want me to trigger another world rebuild for this PR, or do we want to address that for netcdf-c and possibly other ports in another PR? |
:sigh: I would really like to see it fixed given that lots of validation tools will reject binaries built with /wd4996, but given that it was there before and doesn't seem to be the true focus of your PR I suppose it's OK if that's not addressed in this particular PR. |
LGTM. |
Thanks for your help and clear explanations, @dg0yt! |
What does your PR fix?
Due to a wrong variable name, the windows toolchain file was not chainloaded for uwp. Note the empty
VCPKG_CHAINLOAD_TOOLCHAIN_FILE
passed here:[1/2] "D:/downloads/tools/cmake-3.21.1-windows/cmake-3.21.1-windows-i386/bin/cmake.exe" -S "D:/buildtrees/libjxl/src/v0.6.1-bf1b66f693.clean" -B "../../arm-uwp-dbg" "... "-DCMAKE_SYSTEM_NAME=WindowsStore" ... "-DVCPKG_CHAINLOAD_TOOLCHAIN_FILE=" "-DVCPKG_TARGET_TRIPLET=arm-uwp" ...
Without the chainloading, the
VCPKG_C_FLAGS
etc. are not added toCMAKE_C_FLAGS
etc. This is even detected during cmake configuration:This hurts twice on uwp because it often needs extra definitions or flags to turn of warnings/errors.
Current case: [lodepng,lodepng-c, libjxl] Update, merge lodepng-c into lodepng #21846 build https://dev.azure.com/vcpkg/public/_build/results?buildId=64141&view=results
I couldn't resist switching to the common definitions for targets in order to deduplicate target detection.
Which triplets are supported/not supported? Have you updated the CI baseline?
all (fixing uwp), no
Does your PR follow the maintainer guide?
yes
If you have added/updated a port: Have you run
./vcpkg x-add-version --all
and committed the result?yes