Fix up NativeAOT testing

[MichalStrehovsky]

This should work around #79513 (comment).

Cc @dotnet/ilc-contrib

[ghost]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

null

Author:	MichalStrehovsky
Assignees:	-
Labels:	area-NativeAOT-coreclr
Milestone:	-

[MichalStrehovsky]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[MichalStrehovsky]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ViktorHofer]

Can we come up with a heuristic that automatically determines which CoreLib to pick, similar to how this works for Mono today?

I.e. when building clr.aot, always bind against NativeAOT's CoreLib.

[MichalStrehovsky]

I.e. when building clr.aot, always bind against NativeAOT's CoreLib.

There is a heuristic, but it stopped kicking in because we're building CoreCLR as well:

runtime/eng/Subsets.props

Line 106 in 7e10f81

<UseNativeAotCoreLib Condition="$(_subset.Contains('+clr.nativeaotlibs+')) and !$(_subset.Contains('+clr.native+')) and !$(_subset.Contains('+clr.runtime+'))">true</UseNativeAotCoreLib>

[ViktorHofer]

There is a heuristic, but it stopped kicking in because we're building CoreCLR as well:

Right. I was asking if we should change it to pick NativeAOT's CoreLib when the nativeaot subset is built, even if clr.runtime is built as well?

[MichalStrehovsky]

There is a heuristic, but it stopped kicking in because we're building CoreCLR as well:

Right. I was asking if we should change it to pick NativeAOT's CoreLib when the nativeaot subset is built, even if clr.runtime is built as well?

This is after macro expansion. If one builds clr, then clr.aot will also be visible here.

[ViktorHofer]

This is our own build system. We can just move it before we expand the Subset msbuild property, if that's what we want.

Of course, ideally we wouldn't build clr.runtime at all: #79144.

[MichalStrehovsky]

This is our own build system. We can just move it before we expand the Subset msbuild property, if that's what we want.

Right, that also has problems because clr.aot is a macro and one could be doing nativeaot development by just manually specifying what it expands to. I think the current placement is better.

[ViktorHofer]

When this workaround is merged in, what would be your preferred solution to clean this up? Requiring this extra property to be set (think about the local dev innerloop) doesn't seem ideal to me.

[MichalStrehovsky]

When this workaround is merged in, what would be your preferred solution to clean this up? Requiring this extra property to be set (think about the local dev innerloop) doesn't seem ideal to me.

We should stop building the clr.runtime and host subsets - #79575.

This is not much of an issue for nativeaot inner loop - the missing apphost is only a problem for the System.Diagnostics.DiagnosticSource.NativeAotTests project that is not part of inner devloop. Both normal libraries testing and running the src\tests tests work fine without apphost.

[VSadov]

Did this fix the failures in System.Data.DataSetExtensions.Tests ?

I think I still see the failures after merging and running tests. I tried twice in different enlistments.

[MichalStrehovsky]

Did this fix the failures in System.Data.DataSetExtensions.Tests ?

I think I still see the failures after merging and running tests. I tried twice in different enlistments.

It did - exhibit 1 in the CI results.

If you can still repro it, your local workflow is running into the same issue CI did (my local workflows don't run into it and it took a while to repro locally). I don't know how you build things, but make sure to pass /p:UseNativeAotCoreLib=true to all build command that end up building libs. You shouldn't need to pass it, but you're probably building in a way that makes it necessary.

[VSadov]

I just do build.cmd clr+libs+libs.tests -rc Checked -lc Release /p:TestNativeAot=true -test

is that not the right way? it worked in the past.

[MichalStrehovsky]

Replace clr with clr.aot. With clr you are building libs against CoreCLR's CoreLib and you should have been running into this issue for months (we shipped 7.0 with it, so that's at minimum how long this was a problem).

[VSadov]

I think the failures are recent - not more than month old. The root issue could be older, but maybe something else changed and made it to show up as a test failure.

I typically do a trial PR these days with a fake noop change before running tests on the actual changes - it saves time investigating whether the failures are new or old and the trial PR is often morphed into a PR where I fix tests.

I think the last time the trial PR did not catch these. (#78005)

[VSadov]

I've rerun things with clr.aot and it looks like System.Data.DataSetExtensions.Tests problem went away. Thanks !!!

[MichalStrehovsky]

This started to be a problem for the CI only 11 days ago after 5435aa8 because we started building all of coreclr in the CI with that.

It would be a problem that you can see locally if you were building all of clr locally before but you must have changed your local workflow if you didn't see it before.

[VSadov]

I do not know, maybe it would be useful for further investigations, but I am sure I used the same command before. My workflow is basically copy/pasting commands from the notepad. I am not a fast typist and make typos, so I am compensating for that :-).
Even command in #79523 (comment) was pasted from that same .txt file.

Perhaps the change that had effect on the CI had effect on local builds as well.