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

Enable using the Ninja generator instead of Visual Studio on Windows builds. #41897

Merged
merged 65 commits into from
Nov 2, 2020

Conversation

jkoritzinsky
Copy link
Member

@jkoritzinsky jkoritzinsky commented Sep 4, 2020

Summary for Infra Rollout

We're enabling users to use Ninja across the coreclr, libraries, and host native builds instead of Visual Studio or Make. Ninja is a slimmed down build orchestrator that performs much faster than MSBuild on our native Windows CoreCLR build and performs comparably to MSBuild on the Windows libraries and host builds and Make on all non-Windows builds.

You can use the -ninja flag to use Ninja to run the native builds instead of MSBuild or Make. For this rollout, we're not changing the default experience for local development, but we are enabling Ninja builds in CI for the CoreCLR build.

There is one breaking change and one non-breaking change for developers:

  • Breaking: If you are using the src/coreclr/build-runtime.cmd script to build on Windows, you'll need to build either the clr.dactools or clr.runtime subset once from the root script first. The DacTableGen tool used in the Windows build has moved out of the CMake build and is now a regular C# project. As a result, it needs to be built via the root build script once so it is available for the native build.
  • Non breaking: The minimum recommended CMake version is 3.16. If you use CMake 3.16 precompiled headers will be used in all builds. If you use CMake 3.15.5 or lower, precompiled headers will not be used. This is a behavior change, since we used to always have precompiled headers on Windows and never have them on non-Windows. If you notice slowness in your Windows build after pulling down this PR and have not updated CMake, try installing CMake 3.16 to re-enable precompiled headers.

Original Summary

This PR enables using the Ninja generator on Windows instead of the Visual Studio CMake generators via the -ninja switch (for the build-runtime.cmd script) and via a Ninja MSBuild global property. To keep pre-compiled headers working on Windows, this PR bumps the minimum recommended CMake version to 3.16. Using an older CMake version is still possible, but your build will not support precompiled headers. However, now precompiled headers are enabled on all platforms (not just Windows) when using CMake 3.16 or higher.

Since the C# support in CMake is only for the Visual Studio generators, I've moved the DacTableGen project into a subset that's included with the clr.runtime subset and also moved the PIA for DIALib into a checked-in ilproj project by decompiling it with ildasm.

To support opening the CoreCLR VS-generated solution in Visual Studio, I've augmented the root -vs flag. If you pass coreclr.sln (case-insensitive) to the -vs flag, the build script will generate the CoreCLR solution for the provided parameters using the VS generator (if it hasn't been generated already) and open the solution in VS.

I've updated the CoreCLR, Libraries, and Host builds to all enable using Ninja.

For this PR, Ninja usage will be opt-in. In the future, we will make Ninja the default generator and make the Visual Studio generator opt-in.

TODO:

  • CI validation
    • The CI machines don't have Ninja installed yet, so this is blocked on getting it installed.
  • Documentation of new options

Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
…urning on /W3 and disabling any new warnings we hit.
…BRARY abstraction feature in CMake 3.14+ (below the minimum requirement on Windows).
@jkoritzinsky jkoritzinsky added this to the 6.0.0 milestone Sep 4, 2020
@jkoritzinsky jkoritzinsky requested a review from a team September 4, 2020 21:57
@jkoritzinsky jkoritzinsky changed the title [WIP] Enable using the Ninja generator instead of Visual Studio on Windows builds. Enable using the Ninja generator instead of Visual Studio on Windows builds. Oct 22, 2020
@jkoritzinsky jkoritzinsky marked this pull request as ready for review October 22, 2020 18:39
@jkoritzinsky
Copy link
Member Author

This PR is ready for review!

@@ -15,7 +16,7 @@

<Target Name="BuildNativeUnix"
BeforeTargets="Build"
Condition="'$(TargetOS)' != 'Windows_NT'">
Condition="!$([MSBuild]::IsOsPlatform(Windows))">
Copy link
Member

Choose a reason for hiding this comment

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

Unsure if this ever worked but the TargetOS check doesn't assert the current OS but the target one that is passed in via the -os switch.

Copy link
Member

Choose a reason for hiding this comment

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

Can I build the native bits targeting Unix on Windows?

Copy link
Member Author

Choose a reason for hiding this comment

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

This broke with the WASM on Windows build, since it was going down the build-native.sh path when it should have gone down the build-native.cmd path.

src/coreclr/build-runtime.cmd Show resolved Hide resolved
eng/pipelines/coreclr/templates/build-job.yml Outdated Show resolved Hide resolved
eng/native/functions.cmake Outdated Show resolved Hide resolved
src/coreclr/CMakeLists.txt Outdated Show resolved Hide resolved
src/coreclr/runtime.proj Show resolved Hide resolved
src/coreclr/src/debug/daccess/dacfn.cpp Outdated Show resolved Hide resolved
src/libraries/Native/build-native.cmd Show resolved Hide resolved
Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
@BruceForstall
Copy link
Member

Looks like you need to add more documentation on Ninja, e.g.:

  1. From where do I install it?
  2. How do I install it?
  3. What is the minimum version?

The answers to these questions need to cover all platforms.

The documents at the various "Requirements" links on https://github.com/dotnet/runtime/blob/master/docs/workflow/README.md should be updated.

@BruceForstall
Copy link
Member

The minimum recommended CMake version is 3.16

I didn't look thoroughly: is there an easily visible warning on the console if you don't have the recommended minimum cmake version? (i.e., it seems like there should be)

@jkoritzinsky
Copy link
Member Author

We have an error for the minimum required CMake, but not a warning for the minimum recommended CMake (this is the first time they differ). I'll add a warning output.

@jkoritzinsky
Copy link
Member Author

Test failures are #43983 and #43927

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants