Runtime: Update Arcade subscriptions to not point at legacy repos

[jkoritzinsky]

We have a number of Arcade subscriptions that currently point at legacy repositories (dotnet/coreclr, dotnet/corefx, dotnet/core-setup) for various tools used in the repository or NuGet packages.

Here are the various subscriptions we have:

• IL tools (which we're planning using the live version per #1137)
• Hosting layer (plans to switch to live version per #487)
• Various libraries (runtime.native.System.IO.Ports, System.Text.Encodings.Web, System.Text.Json) that are referenced as NuGet packages to avoid issues around packages being built across multiple systems or packages that need non-.NET 5 TFMs because they're consumed by the SDK. There's no current plan to update these references to be live.

The third category in particular is blocking https://github.com/dotnet/sdk/pull/3884

cc: @dotnet/runtime-infrastructure

[ericstj]

There's no current plan to update these references to be live.

Does it make sense to move all managed code into Libraries?

[jkoritzinsky]

I'm not sure if it does. The three libraries we have that build in installer are:

• Microsoft.DotNet.PlatformAbstractions
• Microsoft.Extensions.DependencyModel
• Microsoft.NET.HostModel

All three libraries are netstandard2.0 (or older for compat reasons) libraries. I don't know how well the libraries innerloop dev experience would be for building these and running their unit tests. Additionally, it does move them further away from their corresponding native code (for DependencyModel and HostModel) but since it's in the same repository, it's probably not a huge issue.

@vitek-karas @dagood what do you think?

[dagood]

@eerhardt for DependencyModel (+ PlatformAbstractions) area

[ViktorHofer]

Various libraries (runtime.native.System.IO.Ports, System.Text.Encodings.Web, System.Text.Json) that are referenced as NuGet packages to avoid issues around packages being built across multiple systems or packages that need non-.NET 5 TFMs because they're consumed by the SDK. There's no current plan to update these references to be live.

I might be misunderstanding something, why can't we just change the target-repo in the subscription from corefx to runtime so that we take a dependency on ourselves in the BAR graph?

[jkoritzinsky]

That's what I was thinking of doing to fix this, but I wasn't sure if anyone had any qualms about subscribing to ourselves in the new repo since we still haven't set it up yet. What frequency do we want to that subscription to update?

[safern]

We use to have it once daily so it might make sense to do it that way.

[dagood]

It is a bit fishy from a source-build perspective now that you mention it. It sounds like the self-dependency is only used to compile against, is that right? (Then source-build can provide them as reference-only packages built by e.g. a GenAPI workflow.)

/cc @dseefeld

[ViktorHofer]

Also there's the issue with live changes not taking affect immediately for consumers of these packages. I hit that with the IL.SDK in the past when I updated a property name in the targets file. Switching to live dependencies might be the better solution long term.

[safern]

Switching to live dependencies might be the better solution long term.

And we already have issues for some of them, IL.SDK, is one of them.

[ViktorHofer]

Right, I'm talking about the ones that @jkoritzinsky mentioned at last, ie runtime.native.System.IO.Ports.

[eerhardt]

Does it make sense to move all managed code into Libraries?

I'm not sure if it does. The three libraries we have that build in installer are:
Microsoft.DotNet.PlatformAbstractions
Microsoft.Extensions.DependencyModel

@eerhardt for DependencyModel (+ PlatformAbstractions) area

I have a desire to remove PlatformAbstractions altogether and combine it with RuntimeInformation (or similar). However, the current proposal hasn't made much traction. See https://github.com/dotnet/core-setup/issues/5213 and https://github.com/dotnet/corefx/issues/31002.

However, DependencyModel needs to live on. I have no problem with it moving to src\libraries and being more like every other library we ship. It would definitely cut down on the different build systems/environments we use to build the product.

Additionally, it does move them further away from their corresponding native code

Note that we have other examples where the "managed code" lives in a different src\XXX directory than its corresponding native code. For example, AssemblyDependencyResolver's ref assemblies live in src\libraries, it's managed code lives in src\coreclr, and its corresponding native code lives in src\installer.

I don't know how well the libraries innerloop dev experience would be for building these and running their unit tests.

The experience would be equivalent to every other library we build from src\libraries, which is why I would prefer to move DependencyModel there.

[jkoritzinsky]

The other problem with moving HostModel is the tests for it use the hosts built within installer and the test infrastructure in installer. The tests would have to be changed pretty significantly to work within the libraries test infra IMO.

[ericstj]

the tests for it use the hosts built within installer and the test infrastructure in installer

Sounds like a problem rather than a constraint. Do we actually like having tests that depend on full stack build? I bet we can still get pretty good (maybe better?) test coverage if those were written as unit tests. We can still benefit from functional tests that depend on host / installer, but that can probably stay with the installer.

[jkoritzinsky]

the tests for it use the hosts built within installer and the test infrastructure in installer

Sounds like a problem rather than a constraint. Do we actually like having tests that depend on full stack build? I bet we can still get pretty good (maybe better?) test coverage if those were written as unit tests. We can still benefit from functional tests that depend on host / installer, but that can probably stay with the installer.

I think it's more of a constraint than a problem since the code in Microsoft.NET.HostModel is the code used by the SDK to customize the hosts for each built application. So even the tests that are unit tests use copies of the built hosts to test the host customization code.

[vitek-karas]

/cc @swaroop-sridhar who sort of owns the HostModel.

I do agree with @jkoritzinsky that this is almost a requirement.

We could test on a mock host binaries - which would be the "unit test" way of doing things, but that would not guard against live-build breaks where changes to the host binaries need to be reflected in the HostModel (and vice-versa).

That said we could probably split the tests and run most of them on mocks in the libraries subset and have only a small number of end-2-end validation tests in the installer subset.

Or even better - we should move the host product sources/build into the coreclr subset (a long standing discussion that this should be done) and then simply consume them from the libraries subset like the rest of the runtime. The tricky bit in that approach is the need for a fully functional dotnet install layout on disk, but I think coreclr tests have the ability to do that already.

[ViktorHofer]

Or even better - we should move the host product sources/build into the coreclr subset (a long standing discussion that this should be done) and then simply consume them from the libraries subset like the rest of the runtime. The tricky bit in that approach is the need for a fully functional dotnet install layout on disk, but I think coreclr tests have the ability to do that already.

I would prefer that option over the others as it also helps with the composition of our testhost.

[jkoritzinsky]

I don’t think we can move the host binaries into the coreclr subset since they’ll also be used with mono, but I’m very much for moving them into their own “host” subset that we can build before libraries and then consume from the libraries subset. That would also enable moving the libraries tests to use the live hosts very easily.

[safern]

I would also like to call out that for libraries all configurations package testing we depend on Microsoft.NETCore.App which currently is just pinned to a version (I'm updating it to latest), but I can't take a dependency on dotnet/runtime since we need to sort out what to do with all these dependencies that are produced in dotnet/runtime.

@ericstj will work on: https://github.com/dotnet/runtime/issues/2017 and after that we will explore the idea of consuming live bits, but for that we would need to make package testing depend on all the installer builds that produce the whole product.

[swaroop-sridhar]

@jkoritzinsky and I just talked about it; I agree that it is a good idea to mode HostModel to libraries partition, and divide the tests between libraries and installer tests.

• The Host tests should remain in the installer partition.

• The AppHost rewriter tests should be in the library partition

• The Bundler tool tests should be in the library partition

  • AppHost singability test should be moved to live along with Host Tests in the installer partition.

  • The BundleExtractRun tests should be in the libraries partition. Here, the "Run" part was just a convenient way to ensure all files are extracted OK. We can instead check the contents of the extraction manually, and get rid of the run part.

In this scheme, I think we'll not need a mock CoreCLR layer to facilitate testing.
There may be some code duplication because tests in both partitions will use the Bundle helpers.

[jkotas]

The Bundler tool tests should be in the library partition

This does not make sense to me. Bundler is not a library and it is CoreCLR specific. The tests for it should not be in a library partition.

[jkotas]

mock CoreCLR layer

I think the problem is that we are conflating inner loop testing and the final product testing.

The final product testing should always work on the final build of the product (ie bits produced by installers).

Now, the problem is how to make inner loop efficient - you do not want to go through building installers for each one line change. I believe that scalable way to make the inner loop testing work is:

1. First, you need to build the whole product. We are there today already.
2. The inner loop testing should take the whole product build, install it (e.g. by unziping it) and patch the specific binaries in it that you are iterating on.

This side-steps all the circular dependencies that we are otherwise going to deal with forever.

BTW: This is how .NET Framework inner loop worked. The product construction of .NET Core is very similar to .NET Framework product construction, so we can borrow all other things that came with it.

[jkoritzinsky]

The Bundler tool tests should be in the library partition

This does not make sense to me. Bundler is not a library and it is CoreCLR specific. The tests for it should not be in a library partition.

The .NET Core 3.0 bundler is not CoreCLR-specific. It lives completely at the hosting layer with its implementation in the apphost and Microsoft.NET.HostModel. The specific test that would need a mock is the test that the Microsoft.NET.HostModel bundle creation is compatible with the apphost bundle extraction, which runs the apphost to extract. We would need a simple hostfxr mock that no-ops and then we’d be able to validate that the bundling works.

[jkotas]

Ok, the .NET Core 3.0 bundler is not CoreCLR-specific. But I still believe that libraries are a wrong place for anything that has to do with it.

Why do we need all these mocks? Can we build the whole product and run the test against that?

[jkoritzinsky]

Most of the mocks are in the hosting layer to help us enable more granular tests for the apphost, hostfxr, hostpolicy, coreclr interactions. We could probably reuse the libraries test harness to put together a fully functional bundle for an app, but it would add a lot of complexity in comparison to using a simple mock hostfxr that just returns “success” from its “run the application” entry point. I just checked the hosts source tree and we already build a mock hostfxr that does exactly what we want for usage in the host activation tests that we can reuse.

[jkotas]

the libraries test harness to put together a fully functional bundle for an app, but it would add a lot of complexity in comparison to using a simple mock hostfxr that just returns “success” from its “run the application” entry point.

Do I read correctly that you agree that the tests listed in https://github.com/dotnet/runtime/issues/1823#issuecomment-577451034 should not live under libraries?

I believe that the src/library/... should contain public API definitions, implementations (that are not runtime specific) and their tests.

If there are pieces of libraries test infrastructure that we would like to reuse for other parts of the product, we should factor them out into a shared place (e.g. .eng).

[jkoritzinsky]

I think the tests that test the code in Microsoft.NET.HostModel should live in the same subset as Microsoft.NET.HostModel. If Microsoft.NET.HostModel moves to the libraries subset, then as many of the tests of the code in Microsoft.NET.HostModel should move as well. The tests listed in https://github.com/dotnet/runtime/issues/1823#issuecomment-577451034 are all tests of the Microsoft.NET.HostModel assembly. So by my logic, they should move along with Microsoft.NET.HostModel.

The HostActivation.Tests test suite that directly tests the various native hosting components will either stay where it is (since it currently uses the final product-built shared framework layout) or move along with the hosts to the host partition (to make a tight innerloop for developing the hosting layer that doesn't require rebuilding the installers).

[jkoritzinsky]

If we want to solve the problem of "managed libraries in the hosting layer depend on live packages built in libraries" in another manner, I would also be fine with that. I might be able to make it be a live package dependency like we do with Microsoft.NETCore.App's dependency on Microsoft.NETCore.Platforms and Microsoft.NETCore.Targets.

[jkoritzinsky]

That would allow us to keep the hosting-layer managed libraries right where they are and not have to move things around at the moment.

[swaroop-sridhar]

I believe that the src/library/... should contain public API definitions, implementations (that are not runtime specific) and their tests.

The HostModel library currently has parts that are CoreCLR specific, and others that are not. For example:

• The AppHost rewriting is completely CoreCLR specific, and would make sense to live with the installer. There are plans to improve the AppHost rewriting seem more like a generic binary rewrite tool for -- write certain bits at a given offset, transfer resources from one PE image to another , etc. https://github.com/dotnet/core-setup/issues/8763, https://github.com/dotnet/core-setup/issues/8764. But this work is still not done, and the sole known use of this code is to update the CoreCLR hosts.

• The Bundler theoritically is an independent managed tool that can be used to pack and extract files at the end of any binary. In future it may also support extraction. So, this can be considered a valid generic public API, but has no known use outside of single-file bundling.

That would allow us to keep the hosting-layer managed libraries right where they are and not have to move things around at the moment.

If we can manage setting up the builds without moving the HostModel library around, that's great.

However, even if that doesn't work, the host tests noted in https://github.com/dotnet/runtime/issues/1823#issuecomment-577451034, shouldn't use mock components. These are the only tests that test running single-file apps in automation, and it's better if they are run against the actual product.

[jkoritzinsky]

I just tested the "no move" scenario out locally, and going the "depend on the System.Text.Json package created by the libraries build" would require everyone to do a libraries -allconfigurations build from the root to create the package, which would add about 5 minutes (at least on my machine) to the initial "first local build" loop. It would also complicate PR builds by requiring the installer builds to wait for the allconfigurations build as well as the individual libraries build to finish before running.

If that's something everyone's OK with, we can go down this path.

• The AppHost rewriting is completely CoreCLR specific, and would make sense to live with the installer. There are plans to improve the AppHost rewriting seem more like a generic binary rewrite tool for -- write certain bits at a given offset, transfer resources from one PE image to another , etc. dotnet/core-setup#8763, dotnet/core-setup#8764. But this work is still not done, and the sole known use of this code is to update the CoreCLR hosts.
• The Bundler theoritically is an independent managed tool that can be used to pack and extract files at the end of any binary. In future it may also support extraction. So, this can be considered a valid generic public API, but has no known use outside of single-file bundling.

The AppHost rewriting is at the layer of the apphost. It doesn't touch any of the other hosting libraries or coreclr.dll/libcoreclr.so/libcoreclr.dylib directly. The COM Host is the same way.

Since Mono is planning on reusing the same hosting layer that CoreCLR uses and the AppHost rewriting only touches the apphost, the AppHost rewriting isn't CoreCLR specific from what I can tell.

[swaroop-sridhar]

The AppHost rewriting is at the layer of the apphost. It doesn't touch any of the other hosting libraries or coreclr.dll/libcoreclr.so/libcoreclr.dylib directly. The COM Host is the same way.

Since Mono is planning on reusing the same hosting layer that CoreCLR uses and the AppHost rewriting only touches the apphost, the AppHost rewriting isn't CoreCLR specific from what I can tell.

Sorry, I said CoreCLR, but didn't mean coreclr.dll specifically.
I meant to make a distinction between: utilities specifically needed to update .net core hosts, and those that are generically useful to be used by .net developers. The bundler can be considered to be generically useful in the later sense, but not the AppHost writer.

[jkotas]

-allconfigurations build from the root to create the package, which would add about 5 minutes (at least on my machine)

We do not need to build all packages. We just need to build the few packages that matter for the rest of the runtime repo.

Or we can use a fixed existing version of the package, like what Microsoft.NET.HostModel does for System.Reflection.Metadata :
https://github.com/dotnet/runtime/blob/master/src/installer/managed/Microsoft.NET.HostModel/Microsoft.NET.HostModel.csproj#L16 .

[jkoritzinsky]

If we’re ok with updating to a fixed version, I would be totally fine with that. @ericstj do you have any thoughts about fixing the version of System.Text.Json used by the managed hosting libraries (DependencyModel/HostModel)?

[jkoritzinsky]

I'm going to go ahead with the hardcoding of System.Text.Json to 4.7.0 for the hosting libraries since that fixes some of the other scenarios (including dotnet/sdk#3884 that prompted me to open this issue).

[vitek-karas]

Microsoft.NET.HostModel is weird - it's a purely managed assembly, but we don't intend it to be consumed by users directly (as in, we don't document its public API for example). It's basically something like an MSBuild task implementation (whether it's actually a task or not doesn't matter). It's supposed to be consumed only by our SDK.

As such I don't think it belongs to libraries (based on the definition what goes there by @jkotas). Logically it belongs to the same place where host lives (currently installer, maybe elsewhere in the future). Note that it's a component which Mono will also need (if they're going to use the same hosting layer as CoreCLR, then the SDK will need HostModel even for Mono targets).

As to why we're using mocks in the host tests:

• We actually have quite a few tests which run against a full fledged .NET Core install (that's how these tests were done in 2.* timeframe). While functional, the tests are very slow (~400 ms for one iteration). This makes it cumbersome to work with.
• The new tests (in 3.* timeframe) started to use mocks to make tests much faster (~20ms for one iteration depending on the exact test setup). This enabled "matrix" style testing for some cases (for example framework resolution and roll forward is a huge matrix of possibilities, so to get a good test coverage we need lot of test cases).
• The native hosting APIs already need native test components since it's not practical (and in some cases not possible) to test these through PInvokes. So having native mocks doesn't really add much complexity to anything.
• We don't have much testing for backward/forward compatibility between hosting components, but we're slowly adding these. These tests can't work on live-built .NET Core install, as they need to validate how new hostfxr interacts with 2.* hostpolicy for example. The most practical way to test this on the feature level is mocks (that is mocks for different versions of hostpolicy/hostfxr, ...).

[eerhardt]

This issue has come up again with https://github.com/dotnet/runtime/issues/35006.

Both Microsoft.Extensions.DependencyModel and Microsoft.NET.HostModel are both used by the dotnet/sdk's Microsoft.NET.Build.Tasks.dll. However, they each reference different versions of System.Text.Json. Microsoft.Extensions.DependencyModel references the "live" System.Text.Json, 5.0.0, while Microsoft.NET.HostModel references the old System.Text.Json, 4.7.0. This causes problems on .NET Framework's MSBuild because a single "Tasks" assembly doesn't get a unified closure of dependencies. And according to @rainersigwald, binding redirects do not work with MSBuild Tasks. Instead, the Tasks.dll should have its dependencies next to it. However, since there is only 1 directory here, we can only put 1 version of System.Text.Json in the directory.

So, basically we need to unify the System.Text.Json versions used by Microsoft.Extensions.DependencyModel and Microsoft.NET.HostModel, since they are used by the same MSBuild Tasks assembly.

[eerhardt]

This issue has come up again in #41041. Since HostModel is referencing an old System.Text.Json - it was using an API that has been marked as Obsolete in 5.0. But we didn't know since it was still using a Preview4 version.

We should really take a look at this library and its dependencies and decide on a long-term plan for it. What we are doing today isn't working very well.

Note that PlatformAbstractions and DependencyModel have been resolved and have found their permanent home (PlatformAbstractions was discontinued and DependencyModel is under src/libraries). The only remaining library to fix is Microsoft.NET.HostModel.

cc @ericstj

[vitek-karas]

cc @agocke

[ericstj]

This came up again in https://github.com/dotnet/sdk/pull/14574. Because of the different assembly version of dependencies of HostModel (checked in, 5.0 RC) and DependencyModel (live 6.0) there's a mismatch.

@jkoritzinsky @agocke is this issue only tracking HostModel re-homing? If so, can we change the title (and ideally fix in 6.0 😺)

[jkoritzinsky]

This issue also tracks using live built hosts for the libraries test host instead of consuming them via packages.