stress dependencies download duplication/confusion

[BruceForstall]

To run GCStress testing, for x86 and x64, the CoreDisTools library must be downloaded and placed into the runtime directory (or on the path). This is done by the src\coreclr\tests\setup-stress-dependencies.cmd/sh scripts, invoked by the src\coreclr\build-test.cmd/sh scripts.

In the CI system, this step is done many times, sometimes where the result isn't used, so it is confusing where the correct location is that does the required work.

For example, in the "runtime-coreclr gcstress0x3-gcstress0xc" pipeline, consider the Windows x64 case:

• Job: CoreCLR Product Build Windows_NT x64 checked
  • "Build native test components" invokes F:\workspace\_work\1\s/src/coreclr\build-test.cmd skipmanaged checked x64 -priority=1 skipgeneratelayout which runs src\coreclr\tests\setup-stress-dependencies.cmd and puts coredistools.dll
    in artifacts\bin\coreclr\Windows_NT.x64.Checked
  • "Zip product build" archives it with the rest of the product
• Job: CoreCLR Common Pri1 Test Build Windows_NT x64 checked
  • "Build managed test components" invokes F:\workspace\_work\1\s/src/coreclr\build-test.cmd skipnative skipgeneratelayout skiptestwrappers checked x64 -priority=1 ci /p:LibrariesConfiguration=Release which runs src\coreclr\tests\setup-stress-dependencies.cmd but the result appears to be unused.
• Job: CoreCLR Pri1 Runtime Tests Run Windows_NT x64 checked
  • "Unzip product build"
    • unzips the product, including coredistools.dll, into artifacts\bin\coreclr\Windows_NT.x64.Checked
  • "Copy native test components to test output folder"
    • invokes F:\workspace\_work\1\s/src/coreclr\build-test.cmd copynativeonly checked x64 -priority=1 /p:LibrariesConfiguration=Release which runs src\coreclr\tests\setup-stress-dependencies.cmd. This overwrites the version that was downloaded in the "Unzip product build" step.
  • "Generate test wrappers"
    • invokes F:\workspace\_work\1\s/src/coreclr\build-test.cmd buildtestwrappersonly checked x64 -priority=1 /p:LibrariesConfiguration=Release which runs src\coreclr\tests\setup-stress-dependencies.cmd. This overwrites the version that was created in the "Copy native test components to test output folder" step.

I can't figure out where we create the tests\Core_Root directory, and copy in the product files, so I can't tell which of these steps is required, and which is not.

(Doesn't anyone know how/when Core_Root is created?)

It seems like we should only do this coredistools download once. Maybe it should be its own step, with its own archive/unzip steps, to make it more clear.

[BruceForstall]

@jashook @trylek @ViktorHofer @safern Can anyone help here?

[jashook]

This should only be done when we create the core_root. Its references should be removed everywhere else.

[BruceForstall]

@jashook (and others): do you know where in the process we create Core_Root? I couldn't find it.

[BruceForstall]

Ok, I couldn't find it because (1) someone recently changed it to not be verbose, so "Creating test overlay" is the only output, and (2) none of the output includes "Core_Root" or the path to "Core_Root" so you can't just search for that.

It looks like even this step is duplicated: we create Core_Root twice, in both of:

• Copy native test components to test output folder
• Generate test wrappers

The code deletes the Core_Root directory before creating it, so the first time is pure waste.

[trylek]

Thanks Bruce for digging into this. You're right, I wasn't careful enough when splitting the steps long ago. I think that part of the problem is the negative nature of many build-test switches (e.g. skipmanaged, skipnative) that basically make the syntax something like "skip this step and do everything else". My hope is that over time we'll siphon away most of build-test logic into an msbuild script so I haven't thought about refactoring the command-line syntax yet. If we're about to keep the same command-line logic, we could add something like "skipstressdependencies" and "skipgeneratelayout" and modify the pipelines as appropriate. If we agree this is the right way to process, I can easily make the PR, the change is trivial. And I guess you've probably already found out that CORE_ROOT gets populated here:

runtime/src/coreclr/tests/src/Common/Directory.Build.targets

Line 89 in 45e2047

<Copy

[BruceForstall]

@trylek Do you want to keep this open until your change in dev/infrastructure makes it to master? Or, do you want to close it now?

[trylek]

@BruceForstall I think I should keep it open at least until tomorrow to remind myself that I need to make a small follow-up change to my checkin, I have noticed in my private lab testing that the CORE_ROOT population step unnecessarily duplicates crossgenning framework in R2R / Crossgen2 runs so I want to fix that; I guess I'll use that follow-up PR to close this issue as it should kind of wrap up the effort.