Skip to content

Conversation

@wtgodbe
Copy link
Member

@wtgodbe wtgodbe commented Sep 27, 2021

No description provided.

@wtgodbe wtgodbe requested a review from a team September 27, 2021 17:21
@wtgodbe
Copy link
Member Author

wtgodbe commented Sep 27, 2021

CC @adityamandaleeka

@wtgodbe
Copy link
Member Author

wtgodbe commented Sep 27, 2021

Fixes #35506

@Pilchie Pilchie added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Sep 27, 2021
Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

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

This is a great doc to add. I've added some questions that might be helpful to answer in the doc inline.

Also, can you add a link to this file in docs/README.md?


## Updating Major Version

Typically, we will update the Major Version before updating the TFM. This is because updating Major Version only requires a logical agreement that `main` is no longer the place for work on the previous major release, while updating the TFM requires waiting for dotnet/runtime to update their TFM so that we can ingest that change. For an example, [this](https://github.com/dotnet/aspnetcore/pull/35402) is the PR where we updated our branding from 6.0.0 to 7.0.0.
Copy link
Member

Choose a reason for hiding this comment

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

Some questions to help me understand this a little bit more:

  • I assume because of the TFM updates requirement on dotnet/runtime, TFM updates can only happen in a dependency update PR. AKA it's part of the process of ingesting an update that goes from 6.x.x to 7.x.x.

  • What does the Major Version impact? Is that just what gets set in local dev builds as the package version? So 7.0.0-dev or 6.0.0-dev.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, in a dependency update PR. MajorVersion just updates the branding of the assets we produce in all builds, local & official

4. Change `PreReleaseBrandingLabel` to `Alpha $(PreReleaseVersionIteration)`.
* In [src/Framework/test/TestData.cs](/src/Framework/test/TestData.cs), update `ListedTargetingPackAssemblies` by incrementing the AssemblyVersion of all aspnetcore assemblies by 1 major version. Once dotnet/runtime updates their AssemblyVersions, we also need to update those in this file. They typically make that change at the same time as their TFM update, but we change our AssemblyVersions as soon as we update branding.
* Add entries to [nuget.config](/nuget.config) for the new Major Version's feed. This just means copying the current feeds (e.g. `dotnet7` and `dotnet7-transport`) and adding entries for the new feeds (`dotnet8` and `dotnet8-transport`). Make an effort to remove old feeds here at the same time.
* In [src/ProjectTemplates/Shared/TemplatePackageInstaller.cs](/src/ProjectTemplates/Shared/TemplatePackageInstaller.cs), add entries to `_templatePackages ` for `Microsoft.DotNet.Web.ProjectTemplates` and `Microsoft.DotNet.Web.Spa.ProjectTemplates` matching the new version.
Copy link
Member

Choose a reason for hiding this comment

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

Is there a validation that you can run after this point to make sure you did everything right? I assume a full test run is sufficient enough validation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point on validation - full test run is generally sufficient, though you should also check that the build produces packages with the new version. I'll add that.


## Updating TFM

Once dotnet/runtime has updated their TFM, we update ours in the dependency update PR ingesting that change. We won't be able to ingest new dotnet/runtime dependencies in `main` until this is done. For an example, [this](https://github.com/dotnet/aspnetcore/pull/36328) is the PR where we updated our TFM to `net7.0`. This step can be tricky - we have workarounds in [eng/tools/GenerateFiles/Directory.Build.targets.in](/eng/tools/GenerateFiles/Directory.Build.targets.in) to make the build work before we get an SDK containing runtime bits with the new TFM. We copy the `KnownFrameworkReference`, `KnownRuntimePack`, and `KnownAppHostPack` from the previous TFM, give them the incoming runtime dependency versions, and give them the new TFM. This allows us to build against the new TFM before we get an SDK with a reference to it, but there are often problems that arise in this area. The best way to debug build errors related to FrameworkReferences it to get a binlog of a failing project (`dotnet build /bl`) and look at the inputs to the task that failed. Confirm that the `Known___` items look as expected (there is an entry with the current TFM & the current dotnet/runtime dependency version), and look at the source code of the task in [dotnet/sdk](https://github.com/dotnet/sdk) for hints.
Copy link
Member

Choose a reason for hiding this comment

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

I see my earlier question is answered in the first sentence. Might help to preview this point earlier for the impatient reader 🤪 but not necessary.


## Ingesting an SDK with the new TFM

Typically we update the SDK we use in `main` every Monday. Once we have one that contains a runtime using the new TFM, we can update [eng/tools/RepoTasks/RepoTasks.csproj](/eng/tools/RepoTasks/RepoTasks.csproj), [eng/tools/RepoTasks/RepoTasks.tasks](/eng/tools/RepoTasks/RepoTasks.tasks), and [eng/helix/content/RunTests/RunTests.csproj](/eng/helix/content/RunTests/RunTests.csproj) to use `DefaultNetCoreTargetFramework` again rather than hard-coding the previous TFM.
Copy link
Member

Choose a reason for hiding this comment

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

Are you meant to undo the workarounds with the KnownX attributes at this point or is that something that sticks around even after an SDK update?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, those stick around - those particular workarounds no-op unless we're using an SDK that doesn't know about the new TFM yet

@wtgodbe
Copy link
Member Author

wtgodbe commented Sep 29, 2021

@captainsafia updated for feedback (thanks!)

1. Add references to [src/SiteExtensions/LoggingAggregate/src/Microsoft.AspNetCore.AzureAppServices.SiteExtension/Microsoft.AspNetCore.AzureAppServices.SiteExtension.csproj](/src/SiteExtensions/LoggingAggregate/src/Microsoft.AspNetCore.AzureAppServices.SiteExtension/Microsoft.AspNetCore.AzureAppServices.SiteExtension.csproj) to `Microsoft.AspNetCore.AzureAppServices.SiteExtension.{PreviousMajorVersion}.0.x64` and `Microsoft.AspNetCore.AzureAppServices.SiteExtension.{PreviousMajorVersion}.0.x86`.
2. Add entries in [eng/Versions.props](/eng/Versions.props) similar to [these](https://github.com/dotnet/aspnetcore/blob/216c92b78bce31d5e81a70b589707ec2ae5ab21a/eng/Versions.props#L224-L226) - the version should be from the latest released build of .Net.
3. Add entries in [eng/Dependencies.props](/eng/Dependencies.props) similar to [these](https://github.com/dotnet/aspnetcore/blob/a47c0a58d7002b9a530c67532366b9db96d73cc6/eng/Dependencies.props#L119-L120).
* Update AssemblyVersions for dotnet/runtime assemblies in [src/Framework/test/TestData.cs](/src/Framework/test/TestData.cs).
Copy link
Contributor

Choose a reason for hiding this comment

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

Mention need to update the template configuration in the dotnet/spa-templates repo and to pick up the new commit in dotnet/aspnetcore. That must happen after other dotnet/aspnetcore branches stop using dotnet/spa-templates' 'main' branch.

Copy link
Member Author

Choose a reason for hiding this comment

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

What does updating the template config in spa-templates look like? I didn't do that, did somebody else?

Copy link
Contributor

Choose a reason for hiding this comment

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

You changed the TFM in the SPA templates

@wtgodbe
Copy link
Member Author

wtgodbe commented Sep 30, 2021

@dougbu ok, I think this should be good to go now.

Copy link
Contributor

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

Just one nit.

For the future: Please wrap Markdown lines to make it easier to comment on small parts of a paragraph.

@wtgodbe
Copy link
Member Author

wtgodbe commented Oct 1, 2021

Merging on red since it's a doc change only

@wtgodbe wtgodbe merged commit 52921ed into main Oct 1, 2021
@wtgodbe wtgodbe deleted the wtgodbe/TFMDoc branch October 1, 2021 14:58
@ghost ghost added this to the 7.0-preview1 milestone Oct 1, 2021
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.

5 participants