Skip to content

WIP: try x64 build with clang-cl#972

Closed
Neumann-A wants to merge 2 commits intomicrosoft:mainfrom
Neumann-A:test-build-with-clang-cl
Closed

WIP: try x64 build with clang-cl#972
Neumann-A wants to merge 2 commits intomicrosoft:mainfrom
Neumann-A:test-build-with-clang-cl

Conversation

@Neumann-A
Copy link
Copy Markdown
Contributor

No description provided.

call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=x64 -host_arch=x64
rmdir /s /q build.x64.debug > nul 2> nul
cmake.exe -G Ninja -DCMAKE_C_COMPILER=clang-cl -DCMAKE_CXX_COMPILER=clang-cl -DCMAKE_BUILD_TYPE=Debug -DBUILD_TESTING=ON -DVCPKG_DEVELOPMENT_WARNINGS=ON -DVCPKG_WARNINGS_AS_ERRORS=ON -DVCPKG_BUILD_BENCHMARKING=ON -DVCPKG_BUILD_FUZZING=ON -DVCPKG_BUILD_TLS12_DOWNLOADER=ON -DVCPKG_ARTIFACTS_DEVELOPMENT=ON -B build.x64.debug
ninja.exe -C build.x64.debug all generate-message-map
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

who had the idea to call ninja directly instead of calling ninja through cmake? is there a good reason to do it this way?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Adding more indirection didn't seem to be helpful?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

depends if you want to use presets or not. All these cmake calls look like a prime example to use cmake presets so that people can easily reproduce them locally.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This script long predates cmake presets being a thing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

New in version 3.19.

From 222bf361e4f0f75698eab0bcad55064cec539923 Mon Sep 17 00:00:00 2001
From: Brad King <brad.king@kitware.com>
Date: Wed, 18 Nov 2020 07:46:23 -0500
Subject: [PATCH] CMake 3.19.0
[Add azure-pipelines and e2e tests to vcpkg-tool.](https://github.com/microsoft/vcpkg-tool/commit/7881dfc9e2ce719907c0b5391df6b5706e7fda13)

@[BillyONeal](https://github.com/microsoft/vcpkg-tool/commits?author=BillyONeal)
BillyONeal committed on Feb 3, 2021

Seems like BillyONeal committed on Feb 3, 2021 predates Date: Wed, 18 Nov 2020 07:46:23 -0500 in your frame of reference. How fast are you moving that time dilatation is that big? (I am just evil at this point but lets call it that both were created around the same time and the benefits of presets weren't know yet)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems like BillyONeal committed on Feb 3, 2021 predates Date: Wed, 18 Nov 2020 07:46:23 -0500 in your frame of reference.

When it was committed in CMake's tree vs. when it was available for us to use are very different things, but fine, drop "long" from my statement.

rmdir /s /q build.x86.debug > nul 2> nul
cmake.exe -G Ninja -DCMAKE_BUILD_TYPE=Debug -DBUILD_TESTING=ON -DVCPKG_DEVELOPMENT_WARNINGS=ON -DVCPKG_WARNINGS_AS_ERRORS=ON -DVCPKG_BUILD_BENCHMARKING=ON -DVCPKG_BUILD_FUZZING=ON -DVCPKG_BUILD_TLS12_DOWNLOADER=ON -DVCPKG_ARTIFACTS_DEVELOPMENT=ON -B build.x86.debug
ninja.exe -C build.x86.debug all generate-message-map
call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=x64 -host_arch=x64
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is only x64 so that i don't need extra flags for clang-cl (e.g. -m32 or something like that)

@Neumann-A
Copy link
Copy Markdown
Contributor Author

Conclusion:
Werror and clang-cl won't work together. Somebody might want to fix these errors here.

BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this pull request Apr 5, 2023
When we first set vcpkg's executable architecture to x86, we chose that
option because it was the only way we could build that would run on all
Windows.

Since then, arm64 has gained the ability to run amd64 programs, and
arm has lost relevancy. Platforms like nanoserver without a wow64
subsystem are also increasing in importance. And we don't really expect
x86 hosted dev boxes to be a thing in 2023.

See also: microsoft#972 (no plans to
change official compiler to clang-cl though)

Resolves microsoft/vcpkg#25521
@BillyONeal
Copy link
Copy Markdown
Member

I don't expect us to ever change our official compilers for Windows like this.

@Neumann-A
Copy link
Copy Markdown
Contributor Author

I don't expect us to ever change our official compilers for Windows like this.

This PR was just a proof of concept that if you want code coverage you could use clang-cl on windows for it since you claimed that switching to clang-cl wouldn't be easy since there was no way to mutate the runners and my argument was that there is no need to do so since the runners already have it installed ;)
The second argument here ist that you might want to add a second windows ci using clang-cl since that gets you more interesting warnings.

BillyONeal added a commit that referenced this pull request Apr 5, 2023
When we first set vcpkg's executable architecture to x86, we chose that
option because it was the only way we could build that would run on all
Windows.

Since then, arm64 has gained the ability to run amd64 programs, and
arm has lost relevancy. Platforms like nanoserver without a wow64
subsystem are also increasing in importance. And we don't really expect
x86 hosted dev boxes to be a thing in 2023.

See also: #972 (no plans to
change official compiler to clang-cl though)

Resolves microsoft/vcpkg#25521
@Neumann-A Neumann-A closed this Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants