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

Update SDK to 6.0 RC2 #60256

Merged
merged 8 commits into from
Nov 3, 2021
Merged

Update SDK to 6.0 RC2 #60256

merged 8 commits into from
Nov 3, 2021

Conversation

ViktorHofer
Copy link
Member

Updating the SDK now to see if there's anything that needs attention for dotnet/runtime to consume RC2 and the final 6.0 SDK later.

@ghost
Copy link

ghost commented Oct 11, 2021

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

Updating the SDK now to see if there's anything that needs attention for dotnet/runtime to consume RC2 and the final 6.0 SDK later.

Author: ViktorHofer
Assignees: ViktorHofer
Labels:

area-Infrastructure

Milestone: -

global.json Show resolved Hide resolved
@ViktorHofer
Copy link
Member Author

src/mono/sample/Android/AndroidSampleApp.csproj(0,0): error NU1102: (NETCORE_ENGINEERING_TELEMETRY=Restore) Unable to find package Microsoft.NETCore.App.Runtime.Mono.android-arm with version (= 6.0.0-rc.2.21480.5)

from https://github.com/dotnet/runtime/pull/60256/checks?check_run_id=3873792305. @steveisok I spoke with @akoeplinger offline and he mentioned that you are probably the right person to ask about why the samples try to restore a runtime pack as presumably they shouldn't and instead use the live built one later.

@ViktorHofer
Copy link
Member Author

artifacts\bin\testPackages\packageTest.targets(113,5): error : (NETCORE_ENGINEERING_TELEMETRY=Build) Duplicate entries for System.Runtime.CompilerServices.Unsafe : D:\a_work\1\s\artifacts\bin\testPackages\cache\system.runtime.compilerservices.unsafe\7.0.0-ci\lib\net7.0\System.Runtime.CompilerServices.Unsafe.dll & D:\a_work\1\s\artifacts\bin\microsoft.netcore.app.ref\ref\net7.0\System.Runtime.CompilerServices.Unsafe.dll

@ericstj @Anipik FYI that the SDK upgrade causes the package testing to fail. Haven't yet looked into what's causing the duplicate assets.

@ericstj
Copy link
Member

ericstj commented Oct 12, 2021

Looks like conflict resolution isn't picking the ref pack. I would expect dotnet/sdk@fb094f4 to have fixed this scenario, which was in RC2.

@ericstj
Copy link
Member

ericstj commented Oct 12, 2021

Interesting, conflict resolution is working for the reference.

Encountered conflict between 'Reference:D:\a\_work\1\s\artifacts\bin\testPackages\cache\system.runtime.compilerservices.unsafe\7.0.0-ci\lib\net7.0\System.Runtime.CompilerServices.Unsafe.dll' and 'Reference:D:\a\_work\1\s\artifacts\bin\microsoft.netcore.app.ref\ref\net7.0\System.Runtime.CompilerServices.Unsafe.dll'. Choosing 'Reference:D:\a\_work\1\s\artifacts\bin\microsoft.netcore.app.ref\ref\net7.0\System.Runtime.CompilerServices.Unsafe.dll' because it comes from a package that is preferred.

But not working for runtime, it doesn't drop the copy-local asset.

_ReferenceCopyLocalPathsWithoutConflicts
    D:\a\_work\1\s\artifacts\bin\testPackages\cache\microsoft.extensions.configuration.abstractions\7.0.0-ci\lib\net7.0\Microsoft.Extensions.Configuration.Abstractions.dll
        DestinationSubPath = Microsoft.Extensions.Configuration.Abstractions.dll
        NuGetPackageVersion = 7.0.0-ci
        AssetType = runtime
        NuGetPackageId = Microsoft.Extensions.Configuration.Abstractions
        CopyLocal = true
        PathInPackage = lib/net7.0/Microsoft.Extensions.Configuration.Abstractions.dll
    D:\a\_work\1\s\artifacts\bin\testPackages\cache\microsoft.extensions.primitives\7.0.0-ci\lib\net7.0\Microsoft.Extensions.Primitives.dll
        DestinationSubPath = Microsoft.Extensions.Primitives.dll
        NuGetPackageVersion = 7.0.0-ci
        AssetType = runtime
        NuGetPackageId = Microsoft.Extensions.Primitives
        CopyLocal = true
        PathInPackage = lib/net7.0/Microsoft.Extensions.Primitives.dll
    D:\a\_work\1\s\artifacts\bin\testPackages\cache\system.runtime.compilerservices.unsafe\7.0.0-ci\lib\net7.0\System.Runtime.CompilerServices.Unsafe.dll
        DestinationSubPath = System.Runtime.CompilerServices.Unsafe.dll
        NuGetPackageVersion = 7.0.0-ci
        AssetType = runtime
        NuGetPackageId = System.Runtime.CompilerServices.Unsafe
        CopyLocal = true
        PathInPackage = lib/net7.0/System.Runtime.CompilerServices.Unsafe.dll

We later test runtime closure against the refpack (to avoid having to restore all runtimes) and that's what's failing.

Looking at the call to conflict resolution, it's missing the PlatformManifest. If I compare a 6.0 TFM to the failing 7.0 they differ in this way. I don't see a platform manifest returned from the ResolveTargetingPackAssets task in the failing net7.0 project. Is it produced?

@ericstj
Copy link
Member

ericstj commented Oct 12, 2021

Yeah, that's the difference. I checked a passing all configurations build from a random PR and see this:
image

Compared to this failing PR:
image

@am11
Copy link
Member

am11 commented Oct 12, 2021

dotnet/sdk@70f204b is new in rc2 which seems to be related.

@ericstj
Copy link
Member

ericstj commented Oct 12, 2021

Yep, that looked suspicious to me too. cc @rainersigwald

@rainersigwald
Copy link
Member

Yeah, looks fishy to me too. I'll investigate.

Is it possible to (temporarily) hack the build script to set the environment variable DOTNETSDK_ALLOW_TARGETING_PACK_CACHING=0 to see if the problem is the caching, or the refactoring?

@rainersigwald
Copy link
Member

Would you mind pointing me in the direction of which binlog you were looking at and what project was causing the problems?

@ericstj
Copy link
Member

ericstj commented Oct 12, 2021

I wonder if MSBuild caches this information at a point before we've built things like the platform manifest and then never refreshes it later on. @ViktorHofer would that theory make sense with how the live ref-pack works?

@rainersigwald I think we owe some investigation on our side first. dotnet/runtime is rather special with how it uses framework references which could be interfering here.

Here's a binlog: https://dev.azure.com/dnceng/_apis/resources/Containers/8452875/Windows_NT_Libraries%20Build%20windows%20allConfigurations%20x64%20Debug?itemPath=Windows_NT_Libraries%20Build%20windows%20allConfigurations%20x64%20Debug%2FBuild.binlog

@rainersigwald
Copy link
Member

I will definitely admit that "what about during the runtime build" was not something I considered when building that caching so it seems likely I created a sharp corner for you here.

@jkoritzinsky
Copy link
Member

I believe I found out why the cache is a problem. In our build, we have some of the Microsoft.Extensions.* projects implicitly reference the ref pack, as shown here:

<!-- Use targeting pack references instead of granular ones in the project file. -->
<DisableImplicitAssemblyReferences>false</DisableImplicitAssemblyReferences>

These projects are built during the libs.src subsets, but we don't produce the platform manifest until the libs.pretest subset here:

<!-- Generate the ref pack's PlatformManifest -->
<Target Name="GenerateFileVersionPropsRefPack"
DependsOnTargets="GetSharedFrameworkRuntimeFiles"
AfterTargets="BuildExternalsProject"
Inputs="@(SharedFrameworkRuntimeFile)"
Outputs="$(MicrosoftNetCoreAppRefPackDataDir)PlatformManifest.txt"
Condition="'$(BuildTargetFramework)' == '$(NetCoreAppCurrent)' or '$(BuildTargetFramework)' == ''">
<GenerateFileVersionProps Files="@(SharedFrameworkRuntimeFile)"
PackageId="$(MicrosoftNetCoreAppFrameworkName).Ref"
PackageVersion="$(ProductVersion)"
PlatformManifestFile="$(MicrosoftNetCoreAppRefPackDataDir)PlatformManifest.txt"
PreferredPackages="$(MicrosoftNetCoreAppFrameworkName).Ref"
PermitDllAndExeFilesLackingFileVersion="true" />
</Target>

As a result, we cache the ref pack contents before we generate the platform manifest. We basically fall into the exact case of "ref pack changes during a build" that the caching step explicitly doesn't support:
https://github.com/dotnet/sdk/blob/41de3649581a9d1f0651d969bb8cb0a4fa7623c9/src/Tasks/Microsoft.NET.Build.Tasks/ResolveTargetingPackAssets.cs#L71-L76

I've validated locally that disabling the cache fixes the AllConfigurations build. I'll finish up the change and push it to this branch. For now, we'll probably have to disable the cache. A better long term solution would be to finish constructing the ref pack with the platform manifest and all related data files before building any assemblies that would reference Microsoft.NETCore.App through a FrameworkReference.

Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @dsplaisted @marcpopMSFT -- the targeting pack caching I added to SDK broke runtime but the escape hatch seems like it's keeping things ok.

eng/build.ps1 Outdated Show resolved Hide resolved
eng/build.sh Outdated Show resolved Hide resolved
@jkoritzinsky
Copy link
Member

@mmitche are we uploading our shipped artifacts to our dotnet-public mirror feed? The shipped runtime packs for RC2 aren't on the dotnet-public feed, which is causing our builds to fail when attempting to consume them. The dotnet-6 feed also doesn't have the builds. It has the prior build from the day, but not the final build (maybe due to the security fix). As a result, using that feed doesn't solve our problem either.

@mmitche
Copy link
Member

mmitche commented Oct 21, 2021

@mmitche are we uploading our shipped artifacts to our dotnet-public mirror feed? The shipped runtime packs for RC2 aren't on the dotnet-public feed, which is causing our builds to fail when attempting to consume them. The dotnet-6 feed also doesn't have the builds. It has the prior build from the day, but not the final build (maybe due to the security fix). As a result, using that feed doesn't solve our problem either.

We are, but these are new (or new-ish) packages and so they aren't auto-mirrored

@jkoritzinsky
Copy link
Member

Can you ping this thread once they're uploaded? That looks to be the next blocker on this PR.

@mmitche
Copy link
Member

mmitche commented Oct 21, 2021

Can you ping this thread once they're uploaded? That looks to be the next blocker on this PR.

I'm resolving this now by uploading all the shipping packages to public

@mmitche
Copy link
Member

mmitche commented Oct 21, 2021

Can you ping this thread once they're uploaded? That looks to be the next blocker on this PR.

I'm resolving this now by uploading all the shipping packages to public

Done.

@jkoritzinsky jkoritzinsky force-pushed the ViktorHofer-sdk-update-6.0-RC2 branch 2 times, most recently from 1095c8e to 5bba0c7 Compare October 22, 2021 01:32
@agocke
Copy link
Member

agocke commented Nov 2, 2021

do the host tests take into account the global machine state including the globally installed SDKs?

They shouldn't directly use that... but it's possible something leaked in...

@agocke
Copy link
Member

agocke commented Nov 2, 2021

FYI I do not think these machines are running on ARM64 hardware actions/runner-images#2187

@agocke
Copy link
Member

agocke commented Nov 2, 2021

This PR fails on my x64 Mac with the following error:

[EXEC       >] [....] [00:14:37.908] /Users/angocke/code/runtime/artifacts/tests/Release/ha/nativeHosting/0/nativehost get_hostfxr_path false nullptr x
[EXEC       -] [....] [00:14:38.174] Waiting for process 87321 to exit...
get_hostfxr_path failed: 0x80008083
[EXEC       <] [FAIL] [00:14:38.343] > /Users/angocke/code/runtime/artifacts/tests/Release/ha/nativeHosting/0/nativehost get_hostfxr_path false nullptr x exited with 1
=== COMMAND LINE ===

@jkoritzinsky
Copy link
Member

@agocke what macOS version do you have?

@vitek-karas
Copy link
Member

The failure is obviously unexpected, but the exit code might be expected. Can you get the XML file from the tests - that will have much more detail about the failure.

@agocke
Copy link
Member

agocke commented Nov 2, 2021

Unfortunately, I don't see an XML file in the artifacts/TestResults/Release directory. Looks like it may have failed poorly

@jkoritzinsky
Copy link
Member

@agocke can you try running ./dotnet test src/installer/tests/HostActivation.Tests/HostActivation.Tests.csproj -c Release -r artifacts/TestResults/Release --logger xml? I think that should generate the test results file. Otherwise try using --logger trx instead of --logger xml. That should generate a test results file that VS can open.

@jkoritzinsky
Copy link
Member

Okay I got a repro locally. The repro requires OSX 11.6 x64 for some reason.

There is one test failure and another failure that causes the test host to crash:

Test failure: MultilevelSharedFxLookup.SharedMultilevelFxLookup_Must_Not_RollForward_if_Framework_Version_is_Specified_Through_Argument.

The test host process crashes during this test: FrameworkResolution.MultipleHives.FrameworkHiveSelection_GlobalHiveWithBetterMatch.

@jkoritzinsky
Copy link
Member

jkoritzinsky commented Nov 2, 2021

It looks like the failing tests are only supposed to run on Windows. Not sure why they're running at all...

I forgot to explicitly filter out the attributed tests on my dotnet test invocation. Going back to the drawing board here

@vitek-karas
Copy link
Member

So I guess there's no local repro yet - right?

@jkoritzinsky
Copy link
Member

I'm finishing up one now. Was waiting to post again until I have finalized results. I think I've narrowed it down to a particular portion of the test infra for the installer tests. Will share more once I've validated that I can get a clean build by disabling some of the tests.

@jkoritzinsky
Copy link
Member

I've validated that I found the failures. The test crashes are all contained in the Nethost.cs file and are due to the infrastructure around the install_location config file. Something in the infrastructure for setting up the temporary file causes the test process to crash.

@vitek-karas @agocke to unblock this PR, can I open an issue for the failures and disable the tests on macOS against the new issue?

@agocke
Copy link
Member

agocke commented Nov 2, 2021

Fine with me

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants