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.

Sign up for GitHub

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

Jump to bottom

Update Ninja to 1.12.1. #39260

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Update Ninja to 1.12.1. #39260

wants to merge 2 commits into from
+37 −19

Conversation

BillyONeal
Copy link
Member

Fixes #38494

Resurrects #38538
@BillyONeal @xb284524239
Update Ninja to 1.12.
883f01d 
Fixes microsoft#38494

Resurrects microsoft#38538

Co-authored-by: xb284524239 <40262910+xb284524239@users.noreply.github.com>
@BillyONeal BillyONeal added info:internal This PR or Issue was filed by the vcpkg team. category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly labels Jun 13, 2024
@BillyONeal
1.12.1
a632c24 
Also fix vcpkgTools.xml
@BillyONeal BillyONeal changed the title Update Ninja to 1.12. Jun 13, 2024
@BillyONeal BillyONeal mentioned this pull request Jun 13, 2024
Closed
@LilyWangLL LilyWangLL self-assigned this Jun 13, 2024
dg0yt
dg0yt reviewed Jun 13, 2024
View reviewed changes
Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

The build_arch thing may come from vcpkg_configure_meson. But meson uses a different terminology.
Cf. git grep 'ARM|arm'.
scripts/cmake/vcpkg_find_acquire_program(NINJA).cmake
else()
set(build_arch $ENV{PROCESSOR_ARCHITECTURE})
endif()
if((build_arch MATCHES "^(ARM|arm)64$") OR (build_arch MATCHES "^(ARM|arm)$"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use arm64 binaries on ^(ARM|arm)$?
(This regex is used a few times with VCPKG_TARGET_ARCHITECTURE but never for the host.)
scripts/cmake/vcpkg_find_acquire_program(NINJA).cmake
Comment on lines +5 to +10
if(DEFINED ENV{PROCESSOR_ARCHITEW6432})
set(build_arch $ENV{PROCESSOR_ARCHITEW6432})
else()
set(build_arch $ENV{PROCESSOR_ARCHITECTURE})
endif()
if((build_arch MATCHES "^(ARM|arm)64$") OR (build_arch MATCHES "^(ARM|arm)$"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if(DEFINED ENV{PROCESSOR_ARCHITEW6432})
set(build_arch $ENV{PROCESSOR_ARCHITEW6432})
else()
set(build_arch $ENV{PROCESSOR_ARCHITECTURE})
endif()
if((build_arch MATCHES "^(ARM|arm)64$") OR (build_arch MATCHES "^(ARM|arm)$"))
if(DEFINED ENV{PROCESSOR_ARCHITEW6432})
set(HOST_ARCH $ENV{PROCESSOR_ARCHITEW6432})
else()
set(HOST_ARCH $ENV{PROCESSOR_ARCHITECTURE})
endif()
if(HOST_ARCH MATCHES "^(ARM|arm)64$")

"host" is the term used in vcpkg. HOST_ARCH is the variable used for the other platforms here. Non-64 arm is not a relevant host arch for windows
@dg0yt
Copy link
Contributor

dg0yt commented Jun 13, 2024

I was surprised to see new android errors. Now I checked the fine print. The errors are related to module support in CMake since 3.28. Module support is enabled under some conditions. For Ninja generators, it needs at least Ninja 1.11. This is what is changed by this PR.

The immediate mitigation might be adding -DCMAKE_CXX_SCAN_FOR_MODULES=OFF to the affected modules.

https://cmake.org/cmake/help/latest/manual/cmake-cxxmodules.7.html
@BillyONeal BillyONeal marked this pull request as draft June 17, 2024 19:20
@dg0yt dg0yt mentioned this pull request Jun 18, 2024
Closed
@xb284524239
Copy link

The "libxkbcommon" library failed to build after we upgraded Ninja to a version greater than 1.10.2. Perhaps we need some helpful ideas from the library's authors.
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:internal This PR or Issue was filed by the vcpkg team.
4 participants
@BillyONeal @dg0yt @xb284524239 @LilyWangLL