Make Ninja the default CMake generator on Windows for the repo #49715

jkoritzinsky · 2021-03-16T18:05:41Z

Part of April's batched infrastructure rollout: #49889

This PR switches Ninja to be the default generator on Windows for all native build components.

MSBuild is still available on Windows with the -msbuild flag. To prevent conflicts from using both the Ninja and MSBuild generators in the same dev workflow, I've moved the MSBuild-generated sources to an ide subfolder of each native build's intermediate directory.

Fixes #45955
Contributes to #44761
Contributes to #47022

If a developer wants to easily create an MSBuild solution for CoreCLR's native code and open it in VS, they can run ./build.cmd -vs coreclr.sln -c <config> -a <arch> to create and open a solution for the given arch and config in VS.

…SBuild generator in a dev-innerloop job

ghost · 2021-03-16T18:05:46Z

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR switches Ninja to be the default generator on Windows for all native build components.

MSBuild is still available on Windows with the -msbuild flag. To prevent conflicts from using both the Ninja and MSBuild generators in the same dev workflow, I've moved the MSBuild-generated sources to an ide subfolder of each native build's intermediate directory.

Fixes #45955
Contributes to #44761

Since this fixes #45955, it might require a jitutils update as well. cc @dotnet/jit-contrib

This requires an infrastructure rollout to merge as it is an infra behavior change.

Author:	jkoritzinsky
Assignees:	-
Labels:	`area-Infrastructure`
Milestone:	6.0.0

src/coreclr/build-runtime.cmd

safern

Looks reasonable.

MichalStrehovsky · 2021-03-16T18:50:47Z

Out of curiosity, why are we making this the default before CMake in VS user story is done (I see it's doesn't have the checkmark in #44761). Is it that much faster?

A lot of people (me included) use the generated projects to do work, but I don't use them on a daily basis. I can see myself hitting this a couple times until I reach the "acceptance" stage of grief:

build.cmd blah blah
Work on something that is not necessarily "the runtime" (crossgen2, NativeAOT, CoreLib work, etc.)
Figure out "oh, I need to open RyuJIT/the runtime in VS" because I don't like reverse engineering 3 layers of macros with findstr.
Oh shoot, I don't have the solution files, or they're out of sync because I forgot to pass -msbuild
Rebuild everything with vcxproj. Get coffee. Complain about build times to the toaster in the kitchen.

I invoke build.cmd manually a lot. Adding another mandatory parameter is a bit of a nuisance. It feels like the defaults in build.cmd are optimized for machines (the CI), not for humans. I would accept this if ninja is like 50% faster, but I don't know how much faster it is.

jashook

Lgtm. Are the binaries produced identical? If not what is the size difference?

hoyosjs

Couple small things. Also, this might be really rough on dev inner loop, if that many people use the SLN + proj files generated. Agree on the rollout part.

eng/build.ps1

src/coreclr/runtime.proj

jkoritzinsky · 2021-03-16T20:14:54Z

@MichalStrehovsky in addition to providing a speedup, Ninja can further enable #47022 by allowing us to let Ninja and CMake do an up to date check on the CMake build files instead of reconfiguring every build (and relying on CMake to not write out build files that didn't change).

For your use case, there's a (apparently little known) feature in the root build scripts:

If you run ./build -vs coreclr.sln, it will generate and open the VS solution for a full CoreCLR build. This also respects the flags for the configuration and architecture that the build script uses. With this switch, you never even need to go searching in the obj directories to find the solution file manually.

With the -vs coreclr.sln support, I don't worry too much about the lack of completion of the CMake-in-VS experience for the repo. Does the -vs coreclr.sln support satisfy your use case?

…ninja-default

jkoritzinsky · 2021-03-16T20:38:01Z

@jashook The last set of comparison numbers I had (#41897 (comment)) showed Ninja sizes as comparable if not better.

…ght place to understand the Ninja vs VS layout for the native test binaries and use the Ninja=false switch on Windows to switch to expecting the MSBuild layout (same as the rest of the build scripts).

MichalStrehovsky · 2021-03-17T08:06:40Z

Does the -vs coreclr.sln support satisfy your use case?

Yup, looks like it will do the trick. Thank you!

BruceForstall · 2021-03-18T05:47:32Z

I notice there are a ton of compiler warnings in the build. Can these get addressed before merge?

e.g.,

https://dev.azure.com/dnceng/public/_build/results?buildId=1044403&view=logs&j=388cb898-bfbc-5d7d-00fc-4978afab125f&t=e474b4ca-e4ef-54e1-ea7b-d237313199c0

Lots of:

cl : Command line warning D9025 : overriding '/W3' with '/W4'

and:

cl : Command line warning D9025 : overriding '/EHa' with '/EHs'

and:

cl : Command line warning D9025 : overriding '/Zc:wchar_t' with '/Zc:wchar_t-'

jkotas · 2021-03-18T05:51:49Z

Also, can we enable warnings as errors? I believe that the msbuild has warnings as errors enabled right now.

BruceForstall · 2021-03-18T16:02:24Z

Does ninja generate any build log files? I can't find any.

If I have a compiler error in the JIT, the build-runtime.cmd script outputs:

BUILD: Error: native component build failed. Refer to the build log files for details.
    "C:\gh\runtime3\src\coreclr\..\..\artifacts\log\Debug\CoreCLR_windows__x64__Debug.log"
    "C:\gh\runtime3\src\coreclr\..\..\artifacts\log\Debug\CoreCLR_windows__x64__Debug.wrn"
    "C:\gh\runtime3\src\coreclr\..\..\artifacts\log\Debug\CoreCLR_windows__x64__Debug.err"

but those files aren't there.

(Note, these are experiences without picking up this PR, which I have been assuming aren't fixed by this PR)

jkoritzinsky · 2021-03-18T16:24:50Z

I don't believe ninja generates any build log files by default. Similar to make, it stops on the first error so it's pretty easy to find the errors that caused the build failures.

I'll work on driving the warnings to 0 before merging this in.

jkoritzinsky · 2021-03-18T16:58:22Z

Warnings-as-errors is also enabled in the Ninja build, but this class of warnings are apparently excluded from warnings-as-errors behavior by CL

…reCLR.sln and update the docs of the -vs coreclr feature.

…Lists.txt. Add dependency on inject_debug_resources since that's what was touching coreclr.dll, making it dirty and leading to a rebuild/link of the single file host.

BruceForstall · 2021-03-19T20:54:27Z

I don't believe ninja generates any build log files by default. Similar to make, it stops on the first error so it's pretty easy to find the errors that caused the build failures.

If there are no log files, how do you debug issues with the build itself? ninja doesn't output the full list of commands and options to the command-line (nor would you want it to). What if I want to see exactly what the compile options are (e.g., warning levels, optimization levels, etc.)?

jkoritzinsky · 2021-03-19T21:04:54Z

The generated build.ninja file has all of the flags, dependencies, and command lines that ninja uses to build every file and target in the build. The file format is very simple and straightforward (very similar to a simple Makefile), so it is pretty easy to figure out where to look to check flags.

jkotas · 2021-03-20T06:19:38Z

warnings

The release build has still a bunch of warnings in the last CI run like:

2021-03-19T19:58:13.4594871Z cl : Command line warning D9025 : overriding '/Ox' with '/O1'
2021-03-19T19:58:13.4596777Z cl : Command line warning D9025 : overriding '/O2' with '/O1'

jkoritzinsky · 2021-03-20T06:27:27Z

I must have missed those last time I looked. I’ll do another pass over the warnings now that everything builds successfully.

…higher up in this file for Release builds.

akoeplinger · 2021-03-22T19:15:35Z

ninja doesn't output the full list of commands and options to the command-line (nor would you want it to). What if I want to see exactly what the compile options are (e.g., warning levels, optimization levels, etc.)?

@BruceForstall if something fails you should see the full compiler command line including all options:

  [226/346] Building C object mono/mini/CMakeFiles/monosgen-objects.dir/__/sgen/sgen-pinning.c.o
  [227/346] Building C object mono/mini/CMakeFiles/monosgen-objects.dir/__/sgen/sgen-nursery-allocator.c.o
  [228/346] Building C object mono/mini/CMakeFiles/monosgen-objects.dir/__/sgen/sgen-memory-governor.c.o
  [229/346] Building C object mono/mini/CMakeFiles/monosgen-objects.dir/Users/alexander/dev/runtime/src/libraries/Native/Unix/System.Globalization.Native/pal_calendarData.c.o
  FAILED: mono/mini/CMakeFiles/monosgen-objects.dir/Users/alexander/dev/runtime/src/libraries/Native/Unix/System.Globalization.Native/pal_calendarData.c.o
  /Library/Developer/CommandLineTools/usr/bin/clang -DHAVE_CONFIG_H -DMONO_DLL_EXPORT -D_THREAD_SAFE -DOSX_ICU_LIBRARY_PATH=\"/usr/lib/libicucore.dylib\" -Imono/mini -Imono/mini/../.. -Imono/mini/../../mono/eglib -I/Users/alexander/dev/runtime/src/mono/mono/mini/../.. -I/Users/alexander/dev/runtime/src/mono/mono/mini/.. -I/Users/alexander/dev/runtime/src/mono/mono/mini/../eglib -I/Users/alexander/dev/runtime/src/mono/mono/mini/../sgen -I/Users/alexander/dev/runtime/src/mono/mono/mini/../../../native -I/Users/alexander/dev/runtime/src/mono/mono/mini/../eventpipe -arch arm64 -std=gnu99 -fno-strict-aliasing -fwrapv -Wall -Wunused -Wmissing-declarations -Wpointer-arith -Wno-cast-qual -Wwrite-strings -Wno-switch -Wno-switch-enum -Wno-unused-value -Wno-attributes -Wmissing-prototypes -Wstrict-prototypes -Wnested-externs -Wno-format-zero-length -Wno-unused-function -Qunused-arguments -Wno-tautological-compare -Wno-parentheses-equality -Wno-self-assign -Wno-return-stack-address -Wno-constant-logical-operand -Wno-zero-length-array -Werror=return-type -Werror-implicit-function-declaration -g -arch arm64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX11.1.sdk -mmacosx-version-min=11.0   -g -fPIC -fvisibility=hidden -I -I/Users/alexander/dev/runtime/src/mono/mono/mini/../../../libraries/Native/Unix/System.Globalization.Native/ -I/Users/alexander/dev/runtime/src/mono/mono/mini/../../../libraries/Native/Unix/Common/ -DTARGET_UNIX -DU_DISABLE_RENAMING -Wno-reserved-id-macro -Wno-documentation -Wno-documentation-unknown-command -Wno-switch-enum -Wno-covered-switch-default -Wno-extra-semi-stmt -Wno-unknown-warning-option -Wno-deprecated-declarations -MD -MT mono/mini/CMakeFiles/monosgen-objects.dir/Users/alexander/dev/runtime/src/libraries/Native/Unix/System.Globalization.Native/pal_calendarData.c.o -MF mono/mini/CMakeFiles/monosgen-objects.dir/Users/alexander/dev/runtime/src/libraries/Native/Unix/System.Globalization.Native/pal_calendarData.c.o.d -o mono/mini/CMakeFiles/monosgen-objects.dir/Users/alexander/dev/runtime/src/libraries/Native/Unix/System.Globalization.Native/pal_calendarData.c.o -c /Users/alexander/dev/runtime/src/libraries/Native/Unix/System.Globalization.Native/pal_calendarData.c
  In file included from /Users/alexander/dev/runtime/src/libraries/Native/Unix/System.Globalization.Native/pal_calendarData.c:8:
  In file included from /Users/alexander/dev/runtime/src/libraries/Native/Unix/System.Globalization.Native/pal_locale_internal.h:6:
  /Users/alexander/dev/runtime/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal.h:22:10: fatal error: 'unicode/ucurr.h' file not found
  #include <unicode/ucurr.h>
           ^~~~~~~~~~~~~~~~~
  1 error generated.

If you want to see the options for a specific compilation unit without a failure then you can either look into the generated build.ninja file or pass --verbose option to ninja.

docs/workflow/debugging/coreclr/debugging.md

src/coreclr/debug/runtimeinfo/CMakeLists.txt

Co-authored-by: Juan Hoyos <[email protected]>

…shim_mono # By Aaron Robinson (10) and others # Via GitHub * upstream/main: (108 commits) [mbr] Add Apple sample (dotnet#50740) make EstablishProxyTunnelAsync throw on failure status code from proxy (dotnet#50763) Improve RGB Min Max evaluation performance by using 2 or 3 comparison… (dotnet#50622) [mono] More domain cleanups (dotnet#50479) Fix Crossgen2 of PlatformDefaultMemberFunction methods and calls. (dotnet#50754) Disable EventSource generator in design-time builds (dotnet#50741) Fix X509 test failures on Android (dotnet#50301) Do not confuse fgDispBasicBlocks in fgMorphBlocks (dotnet#50703) Enforce 64KB event payload size limit on EventPipe (dotnet#50600) Reorganize CoreCLR native build to reduce CMake reconfigures when the build system is untouched (dotnet#49906) [mbr] Turn on hot reload for iOS, tvOS and MacCatalyst (dotnet#50458) improve connection scavenge logic by doing zero-byte read (dotnet#50545) Resolve call mdtokens when making tier 1 inline observations (dotnet#50675) Annotate APIs in System.Private.Xml (dotnet#49682) Support compiling against OpenSSL 3 headers Change Configuration.Json to use a regular Dictionary. (dotnet#50611) Remove unused BigNumFromBinary P/Invoke (dotnet#50670) Make Ninja the default CMake generator on Windows for the repo (dotnet#49715) [AppleAppBuilder] Entitlements to run tests on catalyst using the JIT (dotnet#50637) [mono] Fix delegate invokes to dynamic methods in mixed mode. (dotnet#50547) ... # Conflicts: # src/mono/dlls/mscordbi/CMakeLists.txt

jkoritzinsky added 3 commits March 16, 2021 10:47

Make ninja the default generator for product builds.

f4cb7bc

Make ninja the default of the test build as well.

42b79c6

Change over the CI scripts to use ninja by default but preserve the M…

1293ccd

…SBuild generator in a dev-innerloop job

jkoritzinsky added the area-Infrastructure label Mar 16, 2021

jkoritzinsky added this to the 6.0.0 milestone Mar 16, 2021

jkoritzinsky requested a review from a team March 16, 2021 18:05

safern reviewed Mar 16, 2021

View reviewed changes

src/coreclr/build-runtime.cmd Outdated Show resolved Hide resolved

Update src/coreclr/build-runtime.cmd

1718b57

safern approved these changes Mar 16, 2021

View reviewed changes

jkoritzinsky mentioned this pull request Mar 16, 2021

Support the nmakeobj path and the standard intermediate output path for compile_commands.json on Windows. dotnet/jitutils#322

Merged

jashook approved these changes Mar 16, 2021

View reviewed changes

hoyosjs reviewed Mar 16, 2021

View reviewed changes

eng/build.ps1 Show resolved Hide resolved

src/coreclr/runtime.proj Show resolved Hide resolved

Fix -vs CoreCLR.sln to match new path.

9a02908

jkoritzinsky added 3 commits March 16, 2021 13:16

Update help for -msbuild flag.

ec50dc0

Fix /EHsc flag in the host build.

a20b54d

Merge branch 'ninja-default' of github.com:jkoritzinsky/runtime into …

9eddcc9

…ninja-default

Update the mechanism for copying the native test binaries into the ri…

394742a

…ght place to understand the Ninja vs VS layout for the native test binaries and use the Ninja=false switch on Windows to switch to expecting the MSBuild layout (same as the rest of the build scripts).

hoyosjs approved these changes Mar 17, 2021

View reviewed changes

jkoritzinsky added 3 commits March 17, 2021 09:29

Fix batch syntax

8d0768a

Fixes for CI.

1cb5694

Include the mainfest files for the COM tests.

7df5899

Fix compiler command line warnings.

47f4f22

jkoritzinsky added 4 commits March 18, 2021 12:29

Fix incremental module index generation with Ninja.

02863a5

Fix MSBuild test build native files location.

c64fd1a

Fix manifest/tlb copying to copy to the right folder.

f81d629

Update docs that point to CoreCLR.sln to point to the new path for Co…

7070c2c

…reCLR.sln and update the docs of the -vs coreclr feature.

jkoritzinsky mentioned this pull request Mar 19, 2021

Infrastructure - Rollout (April 2021) #49889

Closed

jkoritzinsky added 3 commits March 19, 2021 12:02

Merge branch 'main' into ninja-default

3a4fd89

Fix generating runtimeinfo. Centralize the setup in runtimeinfo/CMake…

201e3f2

…Lists.txt. Add dependency on inject_debug_resources since that's what was touching coreclr.dll, making it dirty and leading to a rebuild/link of the single file host.

Fix cross-component build.

f15c897

Don't double-specify optimization levels. We already specify a level …

b4563e0

…higher up in this file for Release builds.

hoyosjs approved these changes Mar 24, 2021

View reviewed changes

docs/workflow/debugging/coreclr/debugging.md Outdated Show resolved Hide resolved

docs/workflow/debugging/coreclr/debugging.md Outdated Show resolved Hide resolved

src/coreclr/debug/runtimeinfo/CMakeLists.txt Outdated Show resolved Hide resolved

jkoritzinsky commented Mar 24, 2021

View reviewed changes

src/coreclr/debug/runtimeinfo/CMakeLists.txt Outdated Show resolved Hide resolved

Apply suggestions from code review

6b617aa

Co-authored-by: Juan Hoyos <[email protected]>

jkoritzinsky merged commit 8c2158f into dotnet:main Apr 5, 2021

jkoritzinsky deleted the ninja-default branch April 5, 2021 16:28

vcsjones mentioned this pull request Apr 14, 2021

Linux build: The dependency target "inject_debug_resources_coreclr" of target "runtimeinfo" does not exist. #51248

Closed

ghost locked as resolved and limited conversation to collaborators May 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Ninja the default CMake generator on Windows for the repo #49715

Make Ninja the default CMake generator on Windows for the repo #49715

jkoritzinsky commented Mar 16, 2021 •

edited

Loading

ghost commented Mar 16, 2021

safern left a comment

MichalStrehovsky commented Mar 16, 2021

jashook left a comment

hoyosjs left a comment

jkoritzinsky commented Mar 16, 2021

jkoritzinsky commented Mar 16, 2021

MichalStrehovsky commented Mar 17, 2021

BruceForstall commented Mar 18, 2021

jkotas commented Mar 18, 2021

BruceForstall commented Mar 18, 2021

jkoritzinsky commented Mar 18, 2021

jkoritzinsky commented Mar 18, 2021

BruceForstall commented Mar 19, 2021

jkoritzinsky commented Mar 19, 2021

jkotas commented Mar 20, 2021

jkoritzinsky commented Mar 20, 2021

akoeplinger commented Mar 22, 2021

Make Ninja the default CMake generator on Windows for the repo #49715

Make Ninja the default CMake generator on Windows for the repo #49715

Conversation

jkoritzinsky commented Mar 16, 2021 • edited Loading

ghost commented Mar 16, 2021

safern left a comment

Choose a reason for hiding this comment

MichalStrehovsky commented Mar 16, 2021

jashook left a comment

Choose a reason for hiding this comment

hoyosjs left a comment

Choose a reason for hiding this comment

jkoritzinsky commented Mar 16, 2021

jkoritzinsky commented Mar 16, 2021

MichalStrehovsky commented Mar 17, 2021

BruceForstall commented Mar 18, 2021

jkotas commented Mar 18, 2021

BruceForstall commented Mar 18, 2021

jkoritzinsky commented Mar 18, 2021

jkoritzinsky commented Mar 18, 2021

BruceForstall commented Mar 19, 2021

jkoritzinsky commented Mar 19, 2021

jkotas commented Mar 20, 2021

jkoritzinsky commented Mar 20, 2021

akoeplinger commented Mar 22, 2021

jkoritzinsky commented Mar 16, 2021 •

edited

Loading