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

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

Merged
merged 22 commits into from
Apr 5, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f4cb7bc
Make ninja the default generator for product builds.
jkoritzinsky Mar 16, 2021
42b79c6
Make ninja the default of the test build as well.
jkoritzinsky Mar 16, 2021
1293ccd
Change over the CI scripts to use ninja by default but preserve the M…
jkoritzinsky Mar 16, 2021
1718b57
Update src/coreclr/build-runtime.cmd
jkoritzinsky Mar 16, 2021
9a02908
Fix -vs CoreCLR.sln to match new path.
jkoritzinsky Mar 16, 2021
ec50dc0
Update help for -msbuild flag.
jkoritzinsky Mar 16, 2021
a20b54d
Fix /EHsc flag in the host build.
jkoritzinsky Mar 16, 2021
9eddcc9
Merge branch 'ninja-default' of github.com:jkoritzinsky/runtime into …
jkoritzinsky Mar 16, 2021
394742a
Update the mechanism for copying the native test binaries into the ri…
jkoritzinsky Mar 17, 2021
8d0768a
Fix batch syntax
jkoritzinsky Mar 17, 2021
1cb5694
Fixes for CI.
jkoritzinsky Mar 17, 2021
7df5899
Include the mainfest files for the COM tests.
jkoritzinsky Mar 17, 2021
47f4f22
Fix compiler command line warnings.
jkoritzinsky Mar 18, 2021
02863a5
Fix incremental module index generation with Ninja.
jkoritzinsky Mar 18, 2021
c64fd1a
Fix MSBuild test build native files location.
jkoritzinsky Mar 18, 2021
f81d629
Fix manifest/tlb copying to copy to the right folder.
jkoritzinsky Mar 18, 2021
7070c2c
Update docs that point to CoreCLR.sln to point to the new path for Co…
jkoritzinsky Mar 19, 2021
3a4fd89
Merge branch 'main' into ninja-default
jkoritzinsky Mar 19, 2021
201e3f2
Fix generating runtimeinfo. Centralize the setup in runtimeinfo/CMake…
jkoritzinsky Mar 19, 2021
f15c897
Fix cross-component build.
jkoritzinsky Mar 19, 2021
b4563e0
Don't double-specify optimization levels. We already specify a level …
jkoritzinsky Mar 22, 2021
6b617aa
Apply suggestions from code review
jkoritzinsky Mar 24, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions eng/build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Param(
[ValidateSet("Debug","Release")][string][Alias('lc')]$librariesConfiguration,
[ValidateSet("CoreCLR","Mono")][string][Alias('rf')]$runtimeFlavor,
[switch]$ninja,
[switch]$msbuild,
[string]$cmakeargs,
[switch]$pgoinstrument,
[Parameter(ValueFromRemainingArguments=$true)][String[]]$properties
Expand Down Expand Up @@ -79,7 +80,8 @@ function Get-Help() {

Write-Host "Native build settings:"
Write-Host " -cmakeargs User-settable additional arguments passed to CMake."
Write-Host " -ninja Use Ninja instead of MSBuild to run the native build."
Write-Host " -ninja Use Ninja to drive the native build. (default)"
Write-Host " -msbuild Use MSBuild to drive the native build."
Write-Host " -pgoinstrument Build the CLR with PGO instrumentation."

Write-Host "Command-line arguments not listed above are passed through to MSBuild."
Expand Down Expand Up @@ -234,7 +236,9 @@ foreach ($argument in $PSBoundParameters.Keys)
"properties" { $arguments += " " + $properties }
"verbosity" { $arguments += " -$argument " + $($PSBoundParameters[$argument]) }
"cmakeargs" { $arguments += " /p:CMakeArgs=`"$($PSBoundParameters[$argument])`"" }
"ninja" { $arguments += " /p:Ninja=$($PSBoundParameters[$argument])" }
# The -ninja switch is a no-op since Ninja is the default generator on Windows.
"ninja" { }
"msbuild" { $arguments += " /p:Ninja=false" }
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
"pgoinstrument" { $arguments += " /p:PgoInstrument=$($PSBoundParameters[$argument])"}
# configuration and arch can be specified multiple times, so they should be no-ops here
"configuration" {}
Expand Down Expand Up @@ -265,4 +269,8 @@ if ($failedBuilds.Count -ne 0) {
exit 1
}

if ($ninja) {
Write-Host "The -ninja option has no effect on Windows builds since the Ninja generator is the default generator."
}

exit 0
10 changes: 2 additions & 8 deletions eng/pipelines/coreclr/templates/build-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,6 @@ jobs:
- name: clrBuildPALTestsBuildArg
value: '-paltests '

- name: ninjaArg
value: ''
- ${{ if eq(parameters.osGroup, 'windows') }}:
- name: ninjaArg
value: '-ninja'

- name: pgoInstrumentArg
value: ''
- ${{ if eq(parameters.pgoType, 'PGO' )}}:
Expand Down Expand Up @@ -194,10 +188,10 @@ jobs:

# Build CoreCLR Runtime
- ${{ if ne(parameters.osGroup, 'windows') }}:
- script: $(Build.SourcesDirectory)/src/coreclr/build-runtime$(scriptExt) $(buildConfig) $(archType) $(crossArg) $(osArg) -ci $(compilerArg) $(clrBuildPALTestsBuildArg) $(ninjaArg) $(officialBuildIdArg) $(clrInterpreterBuildArg)
- script: $(Build.SourcesDirectory)/src/coreclr/build-runtime$(scriptExt) $(buildConfig) $(archType) $(crossArg) $(osArg) -ci $(compilerArg) $(clrBuildPALTestsBuildArg) $(officialBuildIdArg) $(clrInterpreterBuildArg)
displayName: Build CoreCLR Runtime
- ${{ if eq(parameters.osGroup, 'windows') }}:
- script: set __TestIntermediateDir=int&&$(Build.SourcesDirectory)/src/coreclr/build-runtime$(scriptExt) $(buildConfig) $(archType) -ci $(ninjaArg) $(enforcePgoArg) $(pgoInstrumentArg) $(officialBuildIdArg) $(clrInterpreterBuildArg)
- script: set __TestIntermediateDir=int&&$(Build.SourcesDirectory)/src/coreclr/build-runtime$(scriptExt) $(buildConfig) $(archType) -ci $(enforcePgoArg) $(pgoInstrumentArg) $(officialBuildIdArg) $(clrInterpreterBuildArg)
displayName: Build CoreCLR Runtime

- ${{ if in(parameters.osGroup, 'OSX', 'iOS','tvOS') }}:
Expand Down
15 changes: 15 additions & 0 deletions eng/pipelines/global-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,21 @@ jobs:
buildArgs: -c release -runtimeConfiguration debug
timeoutInMinutes: 90

#
# Build with Release config and runtimeConfiguration with MSBuild generator
#
- template: /eng/pipelines/common/platform-matrix.yml
parameters:
jobTemplate: /eng/pipelines/common/global-build-job.yml
buildConfig: release
platforms:
- windows_x86
jobParameters:
testGroup: innerloop
nameSuffix: MSBuild_CMake
buildArgs: -c Release -msbuild
timeoutInMinutes: 90

#
# Build with Debug config and Release runtimeConfiguration
#
Expand Down
9 changes: 6 additions & 3 deletions src/coreclr/build-runtime.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ set __CrossArch2=
set __CrossOS=0
set __PgoOptDataPath=
set __CMakeArgs=
set __Ninja=0
set __Ninja=1

@REM CMD has a nasty habit of eating "=" on the argument list, so passing:
@REM -priority=1
Expand Down Expand Up @@ -151,7 +151,9 @@ if /i "%1" == "-skipnative" (set __BuildNative=0&set processedArgs=!pro
if /i "%1" == "-skipcrossarchnative" (set __SkipCrossArchNative=1&set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)
if /i "%1" == "-skipgenerateversion" (set __SkipGenerateVersion=1&set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)
if /i "%1" == "-skiprestoreoptdata" (set __RestoreOptData=0&set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)
if /i "%1" == "-ninja" (set __Ninja=1&set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)
REM -ninja is a no-op option since Ninja is now the default generator on Windows.
if /i "%1" == "-ninja" (set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)
if /i "%1" == "-msbuild" (set __Ninja=0&set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)
if /i "%1" == "-pgoinstrument" (set __PgoInstrument=1&set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)
if /i "%1" == "-enforcepgo" (set __EnforcePgo=1&set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)
if /i "%1" == "-nopgooptimize" (set __PgoOptimize=0&set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)
Expand Down Expand Up @@ -279,7 +281,7 @@ set "__IntermediatesDir=%__RootBinDir%\obj\coreclr\%__TargetOS%.%__BuildArch%.%_
set "__LogsDir=%__RootBinDir%\log\!__BuildType!"
set "__MsbuildDebugLogsDir=%__LogsDir%\MsbuildDebugLogs"
set "__ArtifactsIntermediatesDir=%__RepoRootDir%\artifacts\obj\coreclr\"
if "%__Ninja%"=="1" (set "__IntermediatesDir=%__RootBinDir%\nmakeobj\%__TargetOS%.%__BuildArch%.%__BuildType%")
if "%__Ninja%"=="0" (set "__IntermediatesDir=%__IntermediatesDir%\ide")
set "__PackagesBinDir=%__BinDir%\.nuget"
set "__CrossComponentBinDir=%__BinDir%"
set "__CrossCompIntermediatesDir=%__IntermediatesDir%\crossgen"
Expand Down Expand Up @@ -700,6 +702,7 @@ REM ============================================================================

echo %__MsgPrefix%Build succeeded. Finished at %TIME%
echo %__MsgPrefix%Product binaries are available at !__BinDir!

exit /b 0

REM =========================================================================================
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/runtime.proj
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
'$(PgoInstrument)' != 'true'"
Include="-enforcepgo" />
<_CoreClrBuildArg Condition="$([MSBuild]::IsOsPlatform(Windows)) and '$(CrossDac)' != ''" Include="-$(CrossDac)dac" />
<_CoreClrBuildArg Condition="'$(Ninja)' == 'true'" Include="-ninja" />
<_CoreClrBuildArg Condition="'$(Ninja)' == 'true' and !$([MSBuild]::IsOsPlatform(Windows))" Include="-ninja" />
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
<_CoreClrBuildArg Condition="'$(Ninja)' == 'false' and $([MSBuild]::IsOsPlatform(Windows))" Include="-msbuild" />
<_CoreClrBuildArg Condition="'$(ClrRuntimeSubset)' != 'true'" Include="-skipruntime" />
<_CoreClrBuildArg Condition="'$(ClrJitSubset)' != 'true'" Include="-skipjit" />
<_CoreClrBuildArg Condition="'$(ClrPalTestsSubset)' == 'true'" Include="-paltests" />
Expand Down
2 changes: 1 addition & 1 deletion src/installer/corehost.proj
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
<BuildArgs Condition="'$(IncrementalNativeBuild)' == 'true'">$(BuildArgs) incremental-native-build</BuildArgs>
<BuildArgs>$(BuildArgs) rootdir $(RepoRoot)</BuildArgs>
<BuildArgs>$(BuildArgs) coreclrartifacts $(CoreCLRArtifactsPath)</BuildArgs>
<BuildArgs Condition="'$(Ninja)' == 'true'">$(BuildArgs) ninja</BuildArgs>
<BuildArgs Condition="'$(Ninja)' == 'false'">$(BuildArgs) msbuild</BuildArgs>
<BuildArgs>$(BuildArgs) runtimeflavor $(RuntimeFlavor)</BuildArgs>
<BuildArgs>$(BuildArgs) runtimeconfiguration $(RuntimeConfiguration)</BuildArgs>
</PropertyGroup>
Expand Down
7 changes: 5 additions & 2 deletions src/libraries/Native/build-native.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ set __TargetOS=windows
set CMAKE_BUILD_TYPE=Debug
set "__LinkArgs= "
set "__LinkLibraries= "
set __Ninja=0
set __Ninja=1

:Arg_Loop
:: Since the native build requires some configuration information before msbuild is called, we have to do some manual args parsing
Expand All @@ -37,7 +37,7 @@ if /i [%1] == [Browser] ( set __TargetOS=Browser&&shift&goto Arg_Loop)

if /i [%1] == [rebuild] ( set __BuildTarget=rebuild&&shift&goto Arg_Loop)

if /i [%1] == [ninja] ( set __Ninja=1&&shift&goto Arg_Loop)
if /i [%1] == [msbuild] ( set __Ninja=0&&shift&goto Arg_Loop)

shift
goto :Arg_Loop
Expand All @@ -60,6 +60,9 @@ if %__CMakeBinDir% == "" (
if %__IntermediatesDir% == "" (
set "__IntermediatesDir=%__artifactsDir%\obj\native\%__outConfig%"
)
if %__Ninja% == 0 (
set "__IntermediatesDir=%__IntermediatesDir%\ide"
)
set "__CMakeBinDir=%__CMakeBinDir:\=/%"
set "__IntermediatesDir=%__IntermediatesDir:\=/%"

Expand Down
7 changes: 5 additions & 2 deletions src/libraries/Native/build-native.proj
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
<TargetFramework>$(BuildTargetFramework)</TargetFramework>
<TargetFramework Condition="'$(TargetFramework)' == ''">$(NetCoreAppCurrent)</TargetFramework>
<_BuildNativeArgs>$(TargetArchitecture) $(Configuration) outconfig $(TargetFramework)-$(TargetOS)-$(Configuration)-$(TargetArchitecture) -os $(TargetOS)</_BuildNativeArgs>
<_BuildNativeArgs Condition="'$(Ninja)' == 'true'">$(_BuildNativeArgs) ninja</_BuildNativeArgs>
</PropertyGroup>

<ItemGroup>
Expand All @@ -18,6 +17,7 @@
BeforeTargets="Build"
Condition="!$([MSBuild]::IsOsPlatform(Windows))">
<PropertyGroup>
<_BuildNativeArgs Condition="'$(Ninja)' == 'true'">$(_BuildNativeArgs) ninja</_BuildNativeArgs>
<!--
MSBuildNodeCount should a good approximation for how many procs to use for native build, if we find that doesn't work
then we should consider calling Environment.ProcessorCount
Expand All @@ -44,7 +44,10 @@
BeforeTargets="Build"
Condition="$([MSBuild]::IsOsPlatform(Windows)) and
'$(TargetFramework)' == '$(NetCoreAppCurrent)'">
<!-- Run script that invokes Cmake to create VS files, and then calls msbuild to compile them -->
<PropertyGroup>
<_BuildNativeArgs Condition="'$(Ninja)' == 'false'">$(_BuildNativeArgs) msbuild</_BuildNativeArgs>
</PropertyGroup>
<!-- Run script that uses CMake to generate and build the native files. -->
<Message Text="&quot;$(MSBuildThisFileDirectory)build-native.cmd&quot; $(_BuildNativeArgs)" Importance="High"/>
<Exec Command="&quot;$(MSBuildThisFileDirectory)build-native.cmd&quot; $(_BuildNativeArgs)" />
</Target>
Expand Down
7 changes: 5 additions & 2 deletions src/native/corehost/build.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ set "__LinkArgs= "
set "__LinkLibraries= "
set __PortableBuild=0
set __IncrementalNativeBuild=0
set __Ninja=0
set __Ninja=1

:Arg_Loop
if [%1] == [] goto :InitVSEnv
Expand All @@ -43,7 +43,7 @@ if /i [%1] == [commit] (set __CommitSha=%2&&shift&&shift&goto Arg_Loop)
if /i [%1] == [incremental-native-build] ( set __IncrementalNativeBuild=1&&shift&goto Arg_Loop)
if /i [%1] == [rootDir] ( set __rootDir=%2&&shift&&shift&goto Arg_Loop)
if /i [%1] == [coreclrartifacts] (set __CoreClrArtifacts=%2&&shift&&shift&goto Arg_Loop)
if /i [%1] == [ninja] (set __Ninja=1)
if /i [%1] == [msbuild] (set __Ninja=0)
if /i [%1] == [runtimeflavor] (set __RuntimeFlavor=%2&&shift&&shift&goto Arg_Loop)
if /i [%1] == [runtimeconfiguration] (set __RuntimeConfiguration=%2&&shift&&shift&goto Arg_Loop)

Expand Down Expand Up @@ -73,6 +73,9 @@ if %__CMakeBinDir% == "" (
if %__IntermediatesDir% == "" (
set "__IntermediatesDir=%__objDir%\%__TargetRid%.%CMAKE_BUILD_TYPE%\corehost"
)
if %__Ninja% == 0 (
set "__IntermediatesDir=%__IntermediatesDir%\ide"
)
set "__ResourcesDir=%__objDir%\%__TargetRid%.%CMAKE_BUILD_TYPE%\hostResourceFiles"
set "__CMakeBinDir=%__CMakeBinDir:\=/%"
set "__IntermediatesDir=%__IntermediatesDir:\=/%"
Expand Down
22 changes: 20 additions & 2 deletions src/tests/build.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ set __SkipGenerateLayout=0
set __LocalCoreFXConfig=%__BuildType%
set __SkipFXRestoreArg=
set __GenerateLayoutOnly=0
set __Ninja=1

@REM CMD has a nasty habit of eating "=" on the argument list, so passing:
@REM -priority=1
Expand Down Expand Up @@ -99,6 +100,7 @@ if /i "%1" == "copynativeonly" (set __CopyNativeTestBinaries=1&set __Skip
if /i "%1" == "generatelayoutonly" (set __SkipManaged=1&set __SkipNative=1&set __CopyNativeProjectsAfterCombinedTestBuild=false&set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)
if /i "%1" == "buildtestwrappersonly" (set __SkipNative=1&set __SkipManaged=1&set __BuildTestWrappersOnly=1&set __SkipGenerateLayout=1&set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)

if /i "%1" == "-msbuild" (set __Ninja=0&set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)
if /i "%1" == "buildagainstpackages" (echo error: Remove /BuildAgainstPackages switch&&exit /b1)
if /i "%1" == "crossgen" (set __DoCrossgen=1&set __TestBuildMode=crossgen&set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)
if /i "%1" == "crossgen2" (set __DoCrossgen2=1&set __TestBuildMode=crossgen2&set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)
Expand Down Expand Up @@ -161,6 +163,7 @@ if not defined __TestIntermediateDir (
set "__TestIntermediateDir=tests\coreclr\obj\%__TargetOS%.%__BuildArch%.%__BuildType%"
)
set "__NativeTestIntermediatesDir=%__RootBinDir%\%__TestIntermediateDir%\Native"
if "%__Ninja%"=="0" (set "__NativeTestIntermediatesDir=%__NativeTestIntermediatesDir%\ide")
set "__ManagedTestIntermediatesDir=%__RootBinDir%\%__TestIntermediateDir%\Managed"

REM Generate path to be set for CMAKE_INSTALL_PREFIX to contain forward slash
Expand Down Expand Up @@ -213,7 +216,13 @@ echo %__MsgPrefix%Using environment: "%__VCToolsRoot%\vcvarsall.bat" %__VCBuildA
call "%__VCToolsRoot%\vcvarsall.bat" %__VCBuildArch%
@if defined _echo @echo on

set __ExtraCmakeArgs="-DCMAKE_SYSTEM_VERSION=10.0"
set __ExtraCmakeArgs=

if %__Ninja% EQU 1 (
set __ExtraCmakeArgs="-DCMAKE_SYSTEM_VERSION=10.0 -DCMAKE_BUILD_TYPE=!__BuildType!"
) else (
set __ExtraCmakeArgs="-DCMAKE_SYSTEM_VERSION=10.0"
)
call "%__RepoRootDir%\eng\native\gen-buildsys.cmd" "%__ProjectFilesDir%" "%__NativeTestIntermediatesDir%" %__VSVersion% %__BuildArch% !__ExtraCmakeArgs!

if not !errorlevel! == 0 (
Expand All @@ -239,8 +248,17 @@ set __MsbuildWrn=/flp1:WarningsOnly;LogFile=!__BuildWrn!
set __MsbuildErr=/flp2:ErrorsOnly;LogFile=!__BuildErr!
set __Logging=!__MsbuildLog! !__MsbuildWrn! !__MsbuildErr!

set __CmakeBuildToolArgs=

if %__Ninja% EQU 1 (
set __CmakeBuildToolArgs=
) else (
REM We pass the /m flag directly to MSBuild so that we can get both MSBuild and CL parallelism, which is fastest for our builds.
set __CmakeBuildToolArgs=/nologo /m !__Logging!
)

REM We pass the /m flag directly to MSBuild so that we can get both MSBuild and CL parallelism, which is fastest for our builds.
"%CMakePath%" --build %__NativeTestIntermediatesDir% --target install --config %__BuildType% -- /nologo /m !__Logging!
"%CMakePath%" --build %__NativeTestIntermediatesDir% --target install --config %__BuildType% -- !__CmakeBuildToolArgs!

if errorlevel 1 (
echo %__ErrMsgPrefix%%__MsgPrefix%Error: native test build failed.
Expand Down