Skip to content

Commit

Permalink
Fix composite Crossgen2 runs on Windows and clean up R2R pipelines (#…
Browse files Browse the repository at this point in the history
…43261)

Some time ago I introduced a new option "crossgenframeworkonly"
in the test build scripts and I modified the CoreCLR pipeline
to use it. Turns out it was never fully functional, crossgenning
framework continued taking place in the CORE_ROOT population phase
(generatelayoutonly) and only on Windows it got duplicated in the
subsequent "crossgen framework" step. This duplication moreover
uncovered a bug in the R2RTest tool - when compiling the framework
in composite mode twice in a row, the tool was incorrectly
manipulating the composite file "framework-r2r.dll", causing an
error in the second framework crossgenning step.

I believe that it's completely logical to crossgen the framework
as part of populating CORE_ROOT. In light of this fact I have just
deleted the separate "crossgen framework" step and the corresponding
build script options. I have also fixed the described bug in R2RTest
related to repeated framework crossgenning in composite mode to
restore the invariant that framework crossgenning should be idempotent.

Thanks

Tomas
  • Loading branch information
trylek authored Oct 11, 2020
1 parent 58f28f6 commit 6108083
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 19 deletions.
8 changes: 2 additions & 6 deletions eng/pipelines/common/templates/runtimes/run-test-job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -254,16 +254,12 @@ jobs:


# Compose the Core_Root folder containing all artifacts needed for running
# CoreCLR tests.
# CoreCLR tests. This step also compiles the framework using Crossgen / Crossgen2
# in ReadyToRun jobs.
- script: $(Build.SourcesDirectory)/src/tests/build$(scriptExt) generatelayoutonly $(runtimeFlavorArgs) $(crossgenArg) $(buildConfig) $(archType) $(crossArg) $(priorityArg) $(librariesOverrideArg)
displayName: Generate CORE_ROOT


# Crossgen framework assemblies prior to triggering readyToRun execution runs.
- ${{ if eq(parameters.readyToRun, true) }}:
- script: $(Build.SourcesDirectory)/src/tests/build$(scriptExt) crossgenframeworkonly $(crossgenArg) $(buildConfig) $(archType) $(crossArg) $(priorityArg) $(librariesOverrideArg)
displayName: Crossgen framework assemblies

# Overwrite coreclr runtime binaries with mono ones
- ${{ if eq(parameters.runtimeFlavor, 'mono') }}:
- script: $(_msbuildCommand)
Expand Down
9 changes: 5 additions & 4 deletions src/coreclr/src/tools/r2rtest/BuildFolderSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ namespace R2RTest
{
public class BuildFolderSet
{
const string FrameworkOutputFileName = "framework-r2r.dll";

private readonly IEnumerable<BuildFolder> _buildFolders;

private readonly IEnumerable<CompilerRunner> _compilerRunners;
Expand Down Expand Up @@ -221,6 +223,9 @@ public bool CompileFramework()
Stopwatch stopwatch = Stopwatch.StartNew();

string coreRoot = _options.CoreRootDirectory.FullName;

File.Delete(Path.Combine(coreRoot, FrameworkOutputFileName));

string[] frameworkFolderFiles = Directory.GetFiles(coreRoot);

IEnumerable<CompilerRunner> frameworkRunners = _options.CompilerRunners(isFramework: true, overrideOutputPath: _options.OutputDirectory.FullName);
Expand All @@ -239,10 +244,6 @@ public bool CompileFramework()

if (_options.Composite)
{
const string FrameworkOutputFileName = "framework-r2r.dll";

File.Delete(Path.Combine(_options.CoreRootDirectory.FullName, FrameworkOutputFileName));

var processes = new ProcessInfo[(int)CompilerIndex.Count];
foreach (CompilerRunner runner in frameworkRunners)
{
Expand Down
1 change: 0 additions & 1 deletion src/tests/build.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ if /i "%1" == "skipgeneratelayout" (set __SkipGenerateLayout=1&set processedA
if /i "%1" == "copynativeonly" (set __CopyNativeTestBinaries=1&set __SkipStressDependencies=1&set __SkipNative=1&set __CopyNativeProjectsAfterCombinedTestBuild=false&set __SkipGenerateLayout=1&set __SkipTestWrappers=1&set __SkipCrossgenFramework=1&set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)
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 __SkipStressDependencies=1&set processedArgs=!processedArgs! %1&shift&goto Arg_Loop)
if /i "%1" == "crossgenframeworkonly" (set __SkipRestorePackages=1&set __SkipStressDependencies=1&set __SkipNative=1&set __SkipManaged=1&set __SkipGenerateLayout=1&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)
Expand Down
8 changes: 0 additions & 8 deletions src/tests/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,6 @@ usage_list+=("-skiptestwrappers: Don't generate test wrappers.")
usage_list+=("-buildtestwrappersonly: only build the test wrappers.")
usage_list+=("-copynativeonly: Only copy the native test binaries to the managed output. Do not build the native or managed tests.")
usage_list+=("-generatelayoutonly: only pull down dependencies and build coreroot.")
usage_list+=("-crossgenframeworkonly: only compile the framework in CORE_ROOT with Crossgen / Crossgen2.")

usage_list+=("-crossgen: Precompiles the framework managed assemblies in coreroot.")
usage_list+=("-crossgen2: Precompiles the framework managed assemblies in coreroot using the Crossgen2 compiler.")
Expand Down Expand Up @@ -493,13 +492,6 @@ handle_arguments_local() {
__SkipCrossgenFramework=1
;;

crossgenframeworkonly|-crossgenframeworkonly)
__SkipStressDependencies=1
__SkipNative=1
__SkipManaged=1
__SkipGenerateLayout=1
;;

crossgen|-crossgen)
__DoCrossgen=1
__TestBuildMode=crossgen
Expand Down

0 comments on commit 6108083

Please sign in to comment.