[release/3.0] Stabilize package versions

[mmitche]

No description provided.

[mmitche]

@dougbu Can you take a look at this? This is a worrying number of errors for stabilization

[dougbu]

@mmitche I suspect one of two things:

1. Might related to a typo in one of my recent changes. @wtgodbe's Disable ref pack build in 3.0 #14746 contains the right fix at https://github.com/aspnet/AspNetCore/pull/14746/files#diff-4b2f402501d23abbede6eac0f47783e4R94
2. Our eng/targets/ResolveReferences.targets logic may contain untested logic for the case where we're stabilizing a build other than x.y.0. I thought we used the same logic as 2.x but it's entirely possible some "improvements" messed up a different scenario.

Probably need to download one of the binary logs (whatever one looks smallest) to see why the builds couldn't find the expected projects. I'm at home and the download will be slower than slow. Do you have a better connection?

[pranavkm]

@mmitche is this PR still relevant?

@JunTaoLuo \ @wtgodbe since @dougbu is going to be away, can one of you take over investigating this?

[mmitche]

@mmitche is this PR still relevant?

@JunTaoLuo \ @wtgodbe since @dougbu is going to be away, can one of you take over investigating this?

Definitely, this must be merged before 3.0.1 can go out.

[mmitche]

/azp run

[mmitche]

@JunTaoLuo This needs to go next

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[mmitche]

@JunTaoLuo Lots of fun errors :/

[wtgodbe]

Likely related to IsServicingBuild being flipped to true: https://github.com/aspnet/AspNetCore/blob/affd7e00df310cdf53744ee22cd4631a414f6c05/eng/Versions.props#L27-L31

Still looking

[wtgodbe]

Looks like it's because we turn off using project references during servicing builds: https://github.com/aspnet/AspNetCore/blob/602cb1dea59751a1297d674cddf7f0729c23e6ca/eng/targets/ResolveReferences.targets#L49

Which means all of these References to other AspNetCore projects don't get resolved, and so https://github.com/aspnet/AspNetCore/blob/1e2e9a4f2fbef76f8d9c158b62905d95419bf030/eng/targets/ResolveReferences.targets#L279-L281 fires. I don't know enough about the logic here to know why we do things this way for servicing builds, or how this would normally work, but I'm guessing it's because we normally wouldn't build the ref pack during servicing builds (whereas this time around, we are). So maybe we should add IsTargetingPackBuilding as one of the conditions to set UseProjectReferences to true here: https://github.com/aspnet/AspNetCore/blob/1e2e9a4f2fbef76f8d9c158b62905d95419bf030/eng/targets/ResolveReferences.targets#L49-L50

@JunTaoLuo @BrennanConroy @ryanbrandenburg does that sound reasonable?

[mmitche]

Feel free to make changes to this branch.

[wtgodbe]

I think ab1f127 will unblock the build, but I'm not confident it's correct as far as what we want to do logically. Let's wait until we get eyes from somebody else on @aspnet/build

[wtgodbe]

Nope, I'm wrong, same errors are still happening. Looking

[wtgodbe]

Interestingly, when I build locally, the errors transform from mentioning AspNetCore projects to Extensions projects. e.g.,

/Users/runner/runners/2.159.2/work/1/s/eng/targets/ResolveReferences.targets(279,5): error MSB3245: Could not resolve this reference. Could not locate the package or project for "Microsoft.AspNetCore.Hosting.Server.Abstractions". Did you update baselines and dependencies lists? See docs/ReferenceResolution.md for more details. [/Users/runner/runners/2.159.2/work/1/s/src/Hosting/Abstractions/src/Microsoft.AspNetCore.Hosting.Abstractions.csproj]

Becomes:

E:\code\aspnet\aspnetcore\eng\targets\ResolveReferences.targets(280,5): error MSB3245: Could not resolve this reference. Could not locate the package or project for "Microsoft.Extensions.Hosting.Abstractions". Did you update baselines and dependencies lists? See docs/ReferenceResolution.md for more details. [E:\code\aspnet\aspnetcore\src\Hosting\Abstractions\src\Microsoft.AspNetCore.Hosting.Abstractions.csproj]

[wtgodbe]

Just pushed a commit that fixed this for me locally - @JunTaoLuo @ryanbrandenburg @BrennanConroy I updated the conditions in ResolveReferences.targets for UseProjectReferences and UseLatestPackageRefences so that they're also true when we're building the targeting pack. Normally they'd be false for servicing builds, but that breaks us here since we build the targeting pack in 3.0.1. Is this the right fix?

[wtgodbe]

Hmm, we're still getting errors because of https://github.com/aspnet/AspNetCore/blob/f676c249d25eb438ffc282edb551d86eea1d9709/eng/SharedFramework.External.props#L67-L82. Plus Source build isn't fixed yet

[JunTaoLuo]

The approach seems to contradict the comment here though:

We don't use project references between components in servicing builds between compontents to preserve the baseline as much as possible.

I'm looking at how this works in Extensions.

[JunTaoLuo]

It looks like we need package dependencies for transitive corefx packages that is referenced for the framework. I fixed that but am now running into baseline errors due to missing dependencies:

C:\gh\AspNetCore\eng\targets\ResolveReferences.targets(277,5): error BUILD002: Package references changed since the last release. This could be a breaking change and is not allowed in a servicing update. References removed: [C:\gh\AspNetCore\src\Components\Components\src\Microsoft.AspNetCore.Components.csproj]
C:\gh\AspNetCore\eng\targets\ResolveReferences.targets(277,5): error BUILD002:  - Microsoft.AspNetCore.Components.Analyzers [C:\gh\AspNetCore\src\Components\Components\src\Microsoft.AspNetCore.Components.csproj]
C:\gh\AspNetCore\eng\targets\ResolveReferences.targets(277,5): error BUILD002:  -Microsoft.AspNetCore.Authorization [C:\gh\AspNetCore\src\Components\Components\src\Microsoft.AspNetCore.Components.csproj]
C:\gh\AspNetCore\eng\targets\ResolveReferences.targets(277,5): error BUILD002:  -Microsoft.JSInterop [C:\gh\AspNetCore\src\Components\Components\src\Microsoft.AspNetCore.Components.csproj]
C:\gh\AspNetCore\eng\targets\ResolveReferences.targets(277,5): error BUILD002:  -System.ComponentModel.Annotations [C:\gh\AspNetCore\src\Components\Components\src\Microsoft.AspNetCore.Components.csproj]

Will look into this more in the evening.

[wtgodbe]

I hit that when I updated the baseline in master last month, looks like we need to backport e233890. Will do that now

[wtgodbe]

Actually wait, it's already present in this branch: https://github.com/aspnet/AspNetCore/blob/37bdd3675e87928a6b3ed43d6d0c56c8c57078e0/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj#L26-L32. Looking

[wtgodbe]

I see, we don't honor SuppressBaselineReference in servicing builds:

https://github.com/aspnet/AspNetCore/blob/37bdd3675e87928a6b3ed43d6d0c56c8c57078e0/eng/targets/ResolveReferences.targets#L148

Guess we need the same logic w/ IsTargetingPackBuilding there too.

[JunTaoLuo]

Win-arm build is also fixed. I think this is ready to go in if all tests come back clean.

[wtgodbe]

New test failure in Microsoft.AspNetCore.Components.E2ETests 😢

InputDateInteractsWithEditContext_NullableDateTimeOffset

Assert.Equal() Failure\r\nExpected: modified invalid\r\nActual: modified valid

Could be transient, will look after retry

[pranavkm]

InputDateInteractsWithEditContext_NullableDateTimeOffset

Assert.Equal() Failure\r\nExpected: modified invalid\r\nActual: modified valid

Some of Blazor's tests are fairly problematic in 3.0. @javiercn made a bunch of reliability improvements in 3.1, we could consider porting these over (separately) if this will be an ongoing issue.

[wtgodbe]

we could consider porting these over (separately) if this will be an ongoing issue.

I'd be in favor of doing that for 3.0.2, for 3.0.1 we should try to minimize churn since we're already so behind

[wtgodbe]

These tests are baked in a buttery, flaky crust

[wtgodbe]

We are green

[JunTaoLuo]

We need to do something about the flakiness in this branch but we can address that later.

[wtgodbe]

If we can get an internal build w/ this change, then scrap everything in favor of @dougbu's change for 3.0.2, I think we'll be alright. This definitely hasn't been ideal, though

[JunTaoLuo]

FYI, we might want to spot check the artifacts that are produced.

@nguerrera I see a potential issue in the PackageOverrides.txt:
PackageOverrides.txt

It looks like the internal transport package for the extensions reference assemblies was included in that file. Will that have any functional impact and need to be fixed or will it be safely ignored?

[nguerrera]

I think it should be ok if that internal package is not shipped publicly and not supported. It will just be ready to override it, but it will never see it. @dsplaisted should check my work here.

[JunTaoLuo]

As part of the spot checks we found that there were issues with the contents of the ref pack before stabilization.

• Extra ref assemblies in the ref pack that was brought in via the Microsoft.Internal.Extensions.Refs direct reference. We ended up taking all of the extensions ref assemblies instead of only the ones we need
• PackageOverrides contains incorrect entries. The extensions references were replaced by Microsoft.Internal.Extensions.Refs.
• targeting pack now includes System.Security.Cryptography.Pkcs.dll, System.Drawing.Common.dll and System.Win32.SystemEvents.dll, need to figure out where these came from and whether they should be filtered.

[wtgodbe]

targeting pack now includes System.Security.Cryptography.Pkcs.dll, System.Drawing.Common.dll and System.Win32.SystemEvents.dll, need to figure out where these came from and whether they should be filtered.

System.Drawing.Common & System.Win32.SystemEvents are new entries to LatestPackageReference this time around, which explains them. I don't know about Pkcs though - that's been present there for 9 months. Still looking.

[nguerrera]

Stating the obvious, but there definitely cannot be any new (or missing) assemblies in 3.0.1 ref pack vs 3.0.0.

[wtgodbe]

Actually, System.Security.Cryptography.Pkcs.dll, System.Drawing.Common.dll and System.Win32.SystemEvents.dll are present because of this:

https://github.com/aspnet/AspNetCore/blob/54c2d9d1b2af3aeccba3a73d7333102bd0d6b021/eng/SharedFramework.External.props#L67-L81

https://github.com/aspnet/AspNetCore/blob/602cb1dea59751a1297d674cddf7f0729c23e6ca/src/Framework/ref/Microsoft.AspNetCore.App.Ref.csproj#L56

Seems to be intentional. Do we need to explicitly make sure these 3 get excluded? Maybe 54c2d9d will take care of it.

[wtgodbe]

On Ubuntu:

IdentityUI_ScriptTags_SubresourceIntegrityCheck

System.Threading.Tasks.TaskCanceledException : The operation was canceled.

[wtgodbe]

Will resolve the conflict as soon as the current Windows build finishes, so I can inspect the ref-pack

[JunTaoLuo]

@wtgodbe no won't exclude the extra System.Security.Cryptography.Pkcs.dll, System.Drawing.Common.dll and System.Win32.SystemEvents.dll. That change excludes the extra extensions assemblies only.

[wtgodbe]

no won't exclude the extra System.Security.Cryptography.Pkcs.dll, System.Drawing.Common.dll and System.Win32.SystemEvents.dll. That change excludes the extra extensions assemblies only.

Looks like you're right, just confirmed this with the ref-pack from the new build

[dsplaisted]

It's OK if there are extra non-used entries in PackageOverrides.txt (of course it would be better if they weren't there). All of the items that were there for 3.0 need to be there though.

[JunTaoLuo]

I think the latest fix should resolve all the remaining issues. I did a spot check of the produced artifacts locally and all looks good. Would be a good idea to double check the artifacts produced by the CI.

[wtgodbe]

I did a spot check of the ref pack and it matches 3.0.0, so we should be good to go once CI is green

[wtgodbe]

This is as good as it's going to be, merging in the hopes of getting a 3.0.1 build this weekend