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 branding to 6.0.0 Alpha1 #24983

Merged
merged 5 commits into from
Oct 12, 2020
Merged

Update branding to 6.0.0 Alpha1 #24983

merged 5 commits into from
Oct 12, 2020

Conversation

dougbu
Copy link
Member

@dougbu dougbu commented Aug 17, 2020

  • hold the TFM back at net5.0
  • update site extensions to split 5.0 and 6.0
  • remove unnecessary @(SuppressBaselineReference) items
    • either 3.1.7 baselines we're using don't include reference or still using package
    • fix some comments and Condition attributes to make remainder easy to find
    • I'll undo any changes that were over-zealous

@dougbu dougbu requested review from BrennanConroy, jkotalik and a team August 17, 2020 23:30
@dougbu dougbu requested review from halter73, SteveSandersonMS and a team as code owners August 17, 2020 23:30
@dougbu
Copy link
Member Author

dougbu commented Aug 18, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@dougbu
Copy link
Member Author

dougbu commented Aug 18, 2020

/fyi This PR needs more work to get the tests passing ☹️

@Pilchie Pilchie added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Aug 18, 2020
@dougbu dougbu force-pushed the dougbu/update.branding.6.0 branch from e7bd316 to 32776fa Compare August 19, 2020 04:14
@@ -15,7 +15,7 @@
<RepoTasks.RemoveSharedFrameworkDependencies Condition="@(_BuildOutput->Count()) != 0"
Files="@(_BuildOutput)"
FrameworkOnlyPackages="@(AspNetCoreAppReference)"
SharedFrameworkTargetFramework="net$(AspNetCoreMajorVersion).$(AspNetCoreMinorVersion)" />
SharedFrameworkTargetFramework="net5.0" />
Copy link
Member Author

Choose a reason for hiding this comment

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

Use of $(AspNetCoreMajorVersion) or $(AspNetCoreMajorMinorVersion) was only occasionally correct. Since the concepts aren't related, we shouldn't change this back in the future

@dougbu dougbu force-pushed the dougbu/update.branding.6.0 branch 2 times, most recently from b789b64 to 09a4145 Compare August 19, 2020 22:25
@dougbu
Copy link
Member Author

dougbu commented Aug 19, 2020

Internal build https://dev.azure.com/dnceng/internal/_build/results?buildId=778919 should help confirm changes in site extensions build

@dougbu dougbu force-pushed the dougbu/update.branding.6.0 branch 2 times, most recently from d463b74 to 8e3917f Compare August 20, 2020 19:00
@dougbu
Copy link
Member Author

dougbu commented Aug 20, 2020

🆙📅 though I doubt tests will work without further changes. I'm building locally to see if I can get more information about template failures in previous builds. Suspect we have additional places where TFMs get mixed up with package versions.

Worst case, we might need a runtime update containing dotnet/runtime#40950. I triggered that (daily) subscription just in case.

@dougbu
Copy link
Member Author

dougbu commented Aug 20, 2020

@pranavkm I seem to remember you looking at test failures like and that you resolved them.

The active test run was aborted. Reason: Test host process crashed : Unhandled exception. System.InvalidOperationException: There is no currently active test.

Any idea why I'm seeing them in the gRPC interop tests e.g. https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-aspnetcore-refs-pull-24983-merge-c76e36077044499e8d/InteropTests--net5.0/console.74b37675.log?sv=2019-02-02&se=2020-09-09T20%3A09%3A24Z&sr=c&sp=rl&sig=89ThpfLWPzA57TSNhBUDUXyo5Zl7VM%2FE5dV4aEzYBKI%3D❔

@dougbu
Copy link
Member Author

dougbu commented Aug 20, 2020

@HaoK I'm seeing gRPC test failures on other platforms that seem to indicate the newly-built 6.0.0-ci copy of Microsoft.AspNetCore.App.Runtime was installed to the wrong location. What needs to change❔

For example, from https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-aspnetcore-refs-pull-24983-merge-e4e47cb7c8b8484ca9/InteropTests--net5.0/console.ab71631b.log?sv=2019-02-02&se=2020-09-09T20%3A08%3A14Z&sr=c&sp=rl&sig=YqU17c32Tp8rRlY3EZdxU9Vo%2BSHqmJYs5K7gpC6QeuE%3D:

Found AspNetRuntime: Microsoft.AspNetCore.App.Runtime.win-x64.6.0.0-ci.nupkg, extracting *.txt,json,dll to /datadisks/disk1/work/B8880A11/w/B18309D8/e/.dotnet28088/shared/Microsoft.AspNetCore.App/5.0.0-rc.1.20417.14

@HaoK
Copy link
Member

HaoK commented Aug 20, 2020

The versions haven't been updated yet? -sdk 5.0.100-rc.1.20379.10 --runtime 5.0.0-rc.1.20417.14

https://github.com/dotnet/aspnetcore/blob/master/eng/targets/Helix.targets#L120 I think we use MicrosoftNETCoreAppRuntimeVersion

@dougbu
Copy link
Member Author

dougbu commented Aug 20, 2020

The versions haven't been updated yet? -sdk 5.0.100-rc.1.20379.10 --runtime 5.0.0-rc.1.20417.14

We should be able to rebrand independent of the dotnet/runtime repo. Helix.targets and the scripts used on the Helix agents should not use $(MicrosoftNETCoreAppRuntimeVersion) when installing Microsoft.AspNetCore.App.Runtime. Guess I've got another thing to separate in this PR…


/fyi the same messed up install location for the new Microsoft.AspNetCore.App.Runtime bits is hurting the project template tests too e.g. https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-aspnetcore-refs-pull-24983-merge-56d6b0427e38425e94/ProjectTemplates.Tests--net5.0/console.2800c965.log?sv=2019-02-02&se=2020-09-09T20%3A09%3A23Z&sr=c&sp=rl&sig=fJ7Q1BIJ1qJNSQxW8J2SP801r31rmo3kHfbakOY0szw%3D

On Windows, the project template downstream errors are usually "failed to run EF migrations" due to an inability to load the Microsoft.AspNetCore.Diagnostics.Abstractions.dll assembly that Microsoft.AspNetCore.Diagnostics.EntityFrameworkCore.dll needs. The gRPC downstream errors are thrown TimeoutExceptions.

@HaoK
Copy link
Member

HaoK commented Aug 20, 2020

Right both GRPC and templates depend on the aspnet runtime, that currently use that version, i think if you just replace that with whatever the aspnet specific version is, things shoudl work

@dougbu dougbu force-pushed the dougbu/update.branding.6.0 branch from 8e3917f to 869eaaa Compare August 21, 2020 00:26
@dougbu
Copy link
Member Author

dougbu commented Aug 21, 2020

@HaoK please have a look @ the last commit

@dougbu
Copy link
Member Author

dougbu commented Aug 21, 2020

@HaoK please have a look @ the last commit

Now the second-to-last commit

@dougbu
Copy link
Member Author

dougbu commented Aug 21, 2020

New test internal build is https://dev.azure.com/dnceng/internal/_build/results?buildId=781173 Previous one didn't uncover any issues in the packages but I'd like to double-check

@dougbu
Copy link
Member Author

dougbu commented Sep 28, 2020

/fyi @mmitche this probably needs another iteration but is much closer than it was.

@dougbu dougbu force-pushed the dougbu/update.branding.6.0 branch from 7b42067 to 6fdf569 Compare September 29, 2020 16:50
@dougbu dougbu force-pushed the dougbu/update.branding.6.0 branch from 6fdf569 to da9a43e Compare September 30, 2020 01:20
dougbu added a commit to dotnet/efcore that referenced this pull request Oct 7, 2020
- without this, generated assemblies are unusable in aspnetcore after that brands to 6.0.0
- in other words, this is needed for dotnet/aspnetcore#24983 to test correctly
- hold the TFM back at `net5.0`
- correct `TargetFrameworkVersion` in FrameworkList.xml; don't use `$(AspNetCoreMajorMinorVersion)`
- move `$(DefaultNetCoreTargetFramework)` to eng/Versions.props
  - avoid repeating value in AfterSolutionBuild.targets and Directory.Build.props files
- add new versions to `TemplatePackageInstaller`
- update project template test scripts
- basically, remove test assumptions that branding == target TFM
- `TestData` will need another update once 6.0.0.0 assembly versions arrive from dotnet/runtime
- allow sharedFx assemblies in Microsoft.AspNetCore.Components.WebAssembly.DevServer package
- see #26448 about reducing this baggage later in 6.0
@dougbu dougbu force-pushed the dougbu/update.branding.6.0 branch from da9a43e to d357dfa Compare October 10, 2020 20:15
@dougbu
Copy link
Member Author

dougbu commented Oct 10, 2020

/ping reviewers

This PR addresses additional version confusions that have crept into the repo, specifically mixing the default .NET TFM with the shared framework version. Switching to 6.0.0 does not mean we're targeting net6.0 (yet).

This PR also introduces follow-up items #26448 and #26776.

dougbu added a commit to dotnet/efcore that referenced this pull request Oct 10, 2020
- without this, generated assemblies are unusable in aspnetcore after that brands to 6.0.0
- in other words, this is needed for dotnet/aspnetcore#24983 to test correctly
@dougbu
Copy link
Member Author

dougbu commented Oct 11, 2020

PR is green.

Only aspnetcore-quarantined-pr tests failed in last validation runs. Failures in https://dev.azure.com/dnceng/public/_build/results?buildId=848137 look very similar to those in https://dev.azure.com/dnceng/public/_build/results?buildId=848129 (the last rolling build of the master branch), including the Helix Linux and Windows timeouts. IOW there's no new failures in this PR compared to the existing master branch.

dougbu added a commit to dotnet/efcore that referenced this pull request Oct 12, 2020
* Upgrade to a 6.0 SDK
- without this, generated assemblies are unusable in aspnetcore after that brands to 6.0.0
- in other words, this is needed for dotnet/aspnetcore#24983 to test correctly

* Check for `IOException` instead of missing `NetworkException`
- `NetworkException` unavailable in shared framework or a shipping package

* Update .NET SDK version used in Helix tests
- eliminate need to change the .NET SDK version in two files
- `$(DotNetCliVersion)` must be set explicitly
  - Helix SDK does not pick up .NET SDK version from our global.json
- fortunately, `$(NETCoreSdkVersion)` is available in helix.proj
<ReferencePackSharedFxVersion Condition="'$(VersionSuffix)' != ''">$(ReferencePackSharedFxVersion)-$(VersionSuffix)</ReferencePackSharedFxVersion>
</PropertyGroup>

<ItemGroup>
<FrameworkListRootAttributes Include="Name" Value="$(AspNetCoreAppFrameworkBrandName)" />
<FrameworkListRootAttributes Include="TargetFrameworkIdentifier" Value="$(NETCoreAppFrameworkIdentifier)" />
<FrameworkListRootAttributes Include="TargetFrameworkVersion" Value="$(AspNetCoreMajorMinorVersion)" />
<FrameworkListRootAttributes Include="TargetFrameworkVersion" Value="$(DefaultNetCoreTargetFramework.Substring(3))" />
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look resilient. Granted we probably won't change the format netX.Y to something else, I guess I'm okay with it.

</PropertyGroup>

<ItemGroup>
<FrameworkReference Include="Microsoft.AspNetCore.App" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did this reference change? I'm not too sure about the changes in this file but as long as @pranavkm signs off it should be fine.

Copy link
Member

Choose a reason for hiding this comment

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

Probably putting an error above that indicating that we expect "netN" so that we catch this location if something changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

The whole project changed because we have a mess of conflicts because this project attempted to use AspNetCore bits from the SDK as well as reference projects elsewhere in the repo. @pranavkm signed off on doing this in Teams.

#26448 is about trimming the package correctly at some point in the 6.0 train but only if this project doesn't move elsewhere.

@dougbu dougbu merged commit da7fead into master Oct 12, 2020
@dougbu dougbu deleted the dougbu/update.branding.6.0 branch October 12, 2020 19:07
dougbu added a commit to dougbu/razor-compiler that referenced this pull request Nov 17, 2021
* Update branding to 6.0.0 Alpha1
- hold the TFM back at `net5.0`
- correct `TargetFrameworkVersion` in FrameworkList.xml; don't use `$(AspNetCoreMajorMinorVersion)`
- move `$(DefaultNetCoreTargetFramework)` to eng/Versions.props
  - avoid repeating value in AfterSolutionBuild.targets and Directory.Build.props files
- add new versions to `TemplatePackageInstaller`
- update project template test scripts

* Update / add 6.0 package versions in test expectations
- basically, remove test assumptions that branding == target TFM
- `TestData` will need another update once 6.0.0.0 assembly versions arrive from dotnet/runtime

* Update `Condition`s on `@(SuppressBaselineReference)` items to 6.0

* Undo some of dotnet/aspnetcore#24816
- allow sharedFx assemblies in Microsoft.AspNetCore.Components.WebAssembly.DevServer package
- see dotnet/aspnetcore#26448 about reducing this baggage later in 6.0

* Skip IndividualAuth tests on Helix
- dotnet/aspnetcore#26776

Commit migrated from dotnet/aspnetcore@da7fead48635
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants