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

[Fix uwp toolchain|world rebuild] make ninja work for uwp #22831

Merged
merged 74 commits into from
Apr 28, 2022

Conversation

Neumann-A
Copy link
Contributor

@Neumann-A Neumann-A commented Jan 27, 2022

depends #23001
depends #23846 (for less changes)
closes #19601

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for vcpkg-cmake but no changes to version or port version.
-- Version: 2021-12-20
-- Old SHA: a35eb7c761372dc64526d59fa918a13c0dfbba1b
-- New SHA: bec39c4d9e3a770f7647b6944eadae5df6e9be08
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@JackBoosY JackBoosY added category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly info:world-rebuild labels Jan 28, 2022
@JackBoosY
Copy link
Contributor

This may fix #19818.

@Neumann-A
Copy link
Contributor Author

This may fix #19818.

be aware that the Fortran runtime libraries are not build against WindowsApp.lib which makes them not suiteable for UWP. So currently, Fortran cannot support UWP.

Comment on lines -543 to +552
string(REPLACE "\\" "/" VCToolsInstallDir "$ENV{VCToolsInstallDir}")
file(TO_CMAKE_PATH "$ENV{VCToolsInstallDir}" VCToolsInstallDir)
set(_replacement -FU\"${VCToolsInstallDir}/lib/x86/store/references/platform.winmd\")
string(REPLACE "${_replacement}" "" VCPKG_DETECTED_CMAKE_CXX_FLAGS_DEBUG "${VCPKG_DETECTED_CMAKE_CXX_FLAGS_DEBUG}")
string(REPLACE "${_replacement}" "" VCPKG_DETECTED_CMAKE_C_FLAGS_DEBUG "${VCPKG_DETECTED_CMAKE_C_FLAGS_DEBUG}")
string(REPLACE "${_replacement}" "" VCPKG_DETECTED_CMAKE_CXX_FLAGS_RELEASE "${VCPKG_DETECTED_CMAKE_CXX_FLAGS_RELEASE}")
string(REPLACE "${_replacement}" "" VCPKG_DETECTED_CMAKE_C_FLAGS_RELEASE "${VCPKG_DETECTED_CMAKE_C_FLAGS_RELEASE}")
# Can somebody please check if CMake's compiler flags for UWP are correct?
set(ENV{_CL_} "$ENV{_CL_} /D_UNICODE /DUNICODE /DWINAPI_FAMILY=WINAPI_FAMILY_APP /D__WRL_NO_DEFAULT_LIB_ -FU\"${VCToolsInstallDir}/lib/x86/store/references/platform.winmd\"")
string(APPEND VCPKG_DETECTED_CMAKE_CXX_FLAGS_RELEASE " -ZW:nostdlib")
string(APPEND VCPKG_DETECTED_CMAKE_CXX_FLAGS_DEBUG " -ZW:nostdlib")
set(ENV{_LINK_} "$ENV{_LINK_} ${VCPKG_DETECTED_CMAKE_C_STANDARD_LIBRARIES} ${VCPKG_DETECTED_CMAKE_CXX_STANDARD_LIBRARIES} /MANIFEST /DYNAMICBASE /WINMD:NO /APPCONTAINER")
set(ENV{_CL_} "$ENV{_CL_} -FU\"${VCToolsInstallDir}/lib/x86/store/references/platform.winmd\"")
set(ENV{_LINK_} "$ENV{_LINK_} ${VCPKG_DETECTED_CMAKE_C_STANDARD_LIBRARIES} ${VCPKG_DETECTED_CMAKE_CXX_STANDARD_LIBRARIES}")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Theoretically this needs to be generalized to every flag containing spaces in its arguments.

@Neumann-A
Copy link
Contributor Author

@BillyONeal: Please take a look at the uwp errors (except boost) and tell me which class of errors should be marked as supports: "!uwp"

@BillyONeal
Copy link
Member

Unfortunately I know next to nothing about UWP; although if things built in the old msbuild world order it doesn't seem like a "shared" change like this should change which things are built successfully?

@Neumann-A
Copy link
Contributor Author

built in the old msbuild

As far as i know normal CI won't tell me if i passed a former regression and subsequent ports now fail.
Also cmake msbuild != vs default msbuild. Cmake is probably just using /ZW which is why it works. I'll compare some cmake msbuild build flags against this PR. Currently this PR uses just the vs defaults.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for vcpkg-cmake but no changes to version or port version.
-- Version: 2022-01-19
-- Old SHA: b7c050fe60f91dcedef6d87a3f87584151bf8aee
-- New SHA: 73901e3427bc4e9dc315a84843193d011b47d53d
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@Neumann-A
Copy link
Contributor Author

@BillyONeal I see a lot of Error: curl failed to put file to https://vcpkgbinarycache.blob.core.windows.net/cache/449c37a85a2ae47ab26223514fef805ea0d0f856cf3039497e58535ce80ce45c.zip?*** SECRET *** with exit code '0' and http code '403' errors in the uwp ci run

also fix bug in build_make
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for vcpkg-cmake but no changes to version or port version.
-- Version: 2022-01-19
-- Old SHA: b7c050fe60f91dcedef6d87a3f87584151bf8aee
-- New SHA: 73901e3427bc4e9dc315a84843193d011b47d53d
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for vcpkg-cmake but no changes to version or port version.
-- Version: 2022-01-19
-- Old SHA: b7c050fe60f91dcedef6d87a3f87584151bf8aee
-- New SHA: 73901e3427bc4e9dc315a84843193d011b47d53d
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for vcpkg-cmake but no changes to version or port version.
-- Version: 2022-01-19
-- Old SHA: b7c050fe60f91dcedef6d87a3f87584151bf8aee
-- New SHA: 73901e3427bc4e9dc315a84843193d011b47d53d
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@JackBoosY
Copy link
Contributor

Since microsoft/vcpkg-tool#350 was merged, can we continue this PR?

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for vcpkg-cmake but no changes to version or port version.
-- Version: 2022-04-08
-- Old SHA: 11b8114c46aae5abcbff8a6c985a63cc1f030f9b
-- New SHA: 9350fa30123000ea0c075b4aa43ce5b6dc462d34
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/cpprestsdk/vcpkg.json
  • ports/libcerf/vcpkg.json
  • ports/liblbfgs/vcpkg.json
  • ports/orc/vcpkg.json
  • ports/physfs/vcpkg.json
  • ports/physx/vcpkg.json
  • ports/sdl2/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/cpprestsdk/vcpkg.json
  • ports/libcerf/vcpkg.json
  • ports/liblbfgs/vcpkg.json
  • ports/orc/vcpkg.json
  • ports/physfs/vcpkg.json
  • ports/physx/vcpkg.json
  • ports/sdl2/vcpkg.json

Valid values for the license field can be found in the documentation

Alexander Neumann added 2 commits April 20, 2022 09:25
# Conflicts:
#	ports/physx/vcpkg.json
#	ports/vcpkg-cmake/vcpkg.json
#	scripts/ci.baseline.txt
#	versions/baseline.json
#	versions/p-/physx.json
#	versions/v-/vcpkg-cmake.json
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/cpprestsdk/vcpkg.json
  • ports/libcerf/vcpkg.json
  • ports/liblbfgs/vcpkg.json
  • ports/orc/vcpkg.json
  • ports/physfs/vcpkg.json
  • ports/sdl2/vcpkg.json

Valid values for the license field can be found in the documentation

# Conflicts:
#	ports/boost-modular-build-helper/vcpkg.json
#	versions/b-/boost-modular-build-helper.json
#	versions/baseline.json
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/cpprestsdk/vcpkg.json
  • ports/libcerf/vcpkg.json
  • ports/liblbfgs/vcpkg.json
  • ports/orc/vcpkg.json
  • ports/physfs/vcpkg.json
  • ports/sdl2/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/cpprestsdk/vcpkg.json
  • ports/libcerf/vcpkg.json
  • ports/liblbfgs/vcpkg.json
  • ports/orc/vcpkg.json
  • ports/physfs/vcpkg.json
  • ports/sdl2/vcpkg.json

Valid values for the license field can be found in the documentation

@BillyONeal BillyONeal added the info:reviewed Pull Request changes follow basic guidelines label Apr 26, 2022
@BillyONeal
Copy link
Member

Thanks for making this work @Neumann-A : Words can't describe how much I appreciate the pain you went through to get here trying to demystify the UWP black box. I'm going to try to confirm with some folks who own what UWP means tomorrow.

@BillyONeal
Copy link
Member

image

Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

I am giving folks who understand UWP better a few days to confirm that this is reasonable, but this is awesome.

THANK YOU SO MUCH!

message(FATAL_ERROR "Invalid setting for VCPKG_CRT_LINKAGE: \"${VCPKG_CRT_LINKAGE}\". It must be \"static\" or \"dynamic\"")
endif()

set(_vcpkg_charset "/utf-8")
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit sad that this one is called _vcpkg_charset but the equivalent in the Windows toolchain is CHARSET_FLAG. No change requested (unless you want).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you can change this with the /MP change below however you like. I just mimicked the windows toolchain.

Copy link
Member

Choose a reason for hiding this comment

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

The flag is pointless, but isn't worth another world rebuild to change if that's the only remaining nitpick. You got it to be green, so I'm going to run with it :)

endif()

set(_vcpkg_cpp_flags "/DWIN32 /D_WINDOWS /D_UNICODE /DUNICODE /DWINAPI_FAMILY=WINAPI_FAMILY_APP /D__WRL_NO_DEFAULT_LIB__" ) # VS adds /D "_WINDLL" for DLLs;
set(_vcpkg_common_flags "/nologo /Z7 /MP /GS /Gd /Gm- /W3 /WX- /Zc:wchar_t /Zc:inline /Zc:forScope /fp:precise /Oy- /EHsc")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
set(_vcpkg_common_flags "/nologo /Z7 /MP /GS /Gd /Gm- /W3 /WX- /Zc:wchar_t /Zc:inline /Zc:forScope /fp:precise /Oy- /EHsc")
set(_vcpkg_common_flags "/nologo /Z7 /GS /Gd /Gm- /W3 /WX- /Zc:wchar_t /Zc:inline /Zc:forScope /fp:precise /Oy- /EHsc")

/MP is a workaround for an MSBuildism that doesn't apply here.

Copy link
Member

Choose a reason for hiding this comment

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

(Don't worry about applying this yet; I'll just apply it before merging. Just avoiding building the world again until I'm ready to push the merge button)

@JackBoosY JackBoosY added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Apr 27, 2022
@BillyONeal BillyONeal added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Apr 27, 2022
@BillyONeal
Copy link
Member

@JackBoosY I'm waiting for internal folks to OK it, @Neumann-A doesn't need to respond unless they want. :)

@BillyONeal BillyONeal merged commit 05c93c5 into microsoft:master Apr 28, 2022
@BillyONeal
Copy link
Member

Thanks again for getting UWP to not be a special snowflake!

@Neumann-A Neumann-A deleted the fix_ninja_uwp branch April 28, 2022 05:07
BillyONeal added a commit to strega-nil/vcpkg that referenced this pull request Apr 28, 2022
BillyONeal added a commit that referenced this pull request Apr 29, 2022
* [toolchain windows] set CMAKE_SYSTEM_NAME and CMAKE_SYSTEM_PROCESSOR

In specific, I did this for the cpuinfo PR - I realized the reason
that cpuinfo doesn't support arm64 windows cross compilation is because
we don't set CMAKE_SYSTEM_PROCESSOR.

* correctly set CMAKE_CROSSCOMPILING

* start fixin libraries

* more changes:

- gainput: remove line
- glog: remove try_run when cross compiling
- windows.cmake: set CMAKE_SYSTEM_VERSION

* more patches

- mapnik: set BOOST_REGEX_HAS_ICU to avoid check_cxx_source_runs
- orc: set HAS_PRE_1970 and HAS_POST_2038 for same
- seal: change out check_cxx_source_runs for check_cxx_source_compiles

* more changes

* fix x86-windows

* fix qpid-proton, glog

* build corrade-rc

* fix x64-uwp ports

* forgot to _actually_ always build corrade-rc .,.

* Replay #22831

* Dedupe CMAKE_SYSTEM_NAME settings.

* Add quotes for corrade_rc_param

Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>

* Update version DB.

Co-authored-by: nicole mazzuca <mazzucan@outlook.com>
Co-authored-by: Billy Robert O'Neal III <bion@microsoft.com>
Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly info:reviewed Pull Request changes follow basic guidelines requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[vcpkg|uwp] Fix toolchain so that ninja can be used
7 participants