Skip to content

Update branding to 7.0.1#44768

Merged
dougbu merged 3 commits into
dotnet:release/7.0from
vseanreesermsft:branding-7.0.1-2022-10-27-0951
Nov 4, 2022
Merged

Update branding to 7.0.1#44768
dougbu merged 3 commits into
dotnet:release/7.0from
vseanreesermsft:branding-7.0.1-2022-10-27-0951

Conversation

@vseanreesermsft

Copy link
Copy Markdown
Contributor

No description provided.

@vseanreesermsft vseanreesermsft requested review from a team, dougbu and wtgodbe as code owners October 27, 2022 16:52
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Oct 27, 2022
@wtgodbe

wtgodbe commented Oct 27, 2022

Copy link
Copy Markdown
Member

Is it time for this already @vseanreesermsft?

- change label to "servicing"
- check in 7.0.0 PackageOverrides.txt and PlatformManifest.txt files
@dougbu

dougbu commented Nov 1, 2022

Copy link
Copy Markdown
Contributor

Looks like we're missing a System.Text.Json.Serialization fix in this branch e.g. from local debugging of StatusCodeSampleTest.StatusCodePage_ProducesProblemDetails():

System.MissingMethodException: Method not found: 'Void System.Text.Json.Serialization.Metadata.JsonTypeInfo.MakeReadOnly()'.
   at Microsoft.AspNetCore.Http.DefaultProblemDetailsWriter.ProblemDetailsJsonContext.Create_ProblemDetails(JsonSerializerOptions options, Boolean makeReadOnly)
   at Microsoft.AspNetCore.Http.DefaultProblemDetailsWriter.ProblemDetailsJsonContext.get_ProblemDetails() in C:\\dd\\dnx\\aspnetcore\\src\\Http\\Http.Extensions\\src\\System.Text.Json.SourceGeneration\\System.Text.Json.SourceGeneration.JsonSourceGenerator\\ProblemDetailsJsonContext.ProblemDetails.g.cs:line 23
   at Microsoft.AspNetCore.Http.DefaultProblemDetailsWriter.WriteAsync(ProblemDetailsContext context) in C:\\dd\\dnx\\aspnetcore\\src\\Http\\Http.Extensions\\src\\DefaultProblemDetailsWriter.cs:line 57
   at Microsoft.AspNetCore.Http.ProblemDetailsService.WriteAsync(ProblemDetailsContext context) in C:\\dd\\dnx\\aspnetcore\\src\\Http\\Http.Extensions\\src\\ProblemDetailsService.cs:line 37
   at Microsoft.AspNetCore.Builder.StatusCodePagesOptions.<>c.<<-ctor>b__0_0>d.MoveNext() in C:\\dd\\dnx\\aspnetcore\\src\\Middleware\\Diagnostics\\src\\StatusCodePage\\StatusCodePagesOptions.cs:line 29
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Diagnostics.StatusCodePagesMiddleware.Invoke(HttpContext context) in C:\\dd\\dnx\\aspnetcore\\src\\Middleware\\Diagnostics\\src\\StatusCodePage\\StatusCodePagesMiddleware.cs:line 71
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddlewareImpl.Invoke(HttpContext context) in C:\\dd\\dnx\\aspnetcore\\src\\Middleware\\Diagnostics\\src\\DeveloperExceptionPage\\DeveloperExceptionPageMiddlewareImpl.cs:line 98","headers":{"Accept":["application/json"],"Host":["localhost"]},"path":"/","endpoint":null,"routeValues":null}]

The missing method isn't mentioned at all in this repo and DefaultProblemDetailsWriter.ProblemDetailsJsonContext inherits from JsonSerializerContext w/ only [JsonSerializable(typeof(ProblemDetails))] added.

@dotnet/area-system-text-json any suggestions on a workaround or fix❔

@dougbu

dougbu commented Nov 1, 2022

Copy link
Copy Markdown
Contributor

Also seeing a lot of the following in slightly lower-level tests:

System.MissingMethodException : Method not found: 'Void System.Text.Json.Serialization.Metadata.JsonTypeInfo.MakeReadOnly()'

@wtgodbe

wtgodbe commented Nov 1, 2022

Copy link
Copy Markdown
Member

Looks like that was added here: dotnet/runtime#76899. I suspect this might not work until we get the dependency updates included in the internal -> public merge

@dougbu

dougbu commented Nov 2, 2022

Copy link
Copy Markdown
Contributor

I suspect this might not work until we get the dependency updates included in the internal -> public merge

Why❔ Aren't we doing dependency flow in public now that we have a 7.0.0 build❔ Needing to wait for the internal ➡️ public merge before even branding can merge will likely cause a mad rush on GA day ☹️

Can we at least manually trigger the public runtime ➡️ aspnetcore flow to get the fix for the missing methods here❔ I could also darc update-dependencies in this branch if that's appropriate (once I find or learn the right BAR id of course).

/cc @vseanreesermsft @dkurepa @mmitche

@wtgodbe

wtgodbe commented Nov 2, 2022

Copy link
Copy Markdown
Member

Why❔ Aren't we doing dependency flow in public now that we have a 7.0.0 build❔

We do every servicing release from the internal repos - worst case scenario, if we do have to wait until GA day, we get an extra week before branches close for 7.0.1 (GA 11/8, branches close 11/14 EOD), so we could make that work. I'll leave it up to @mmitche on whether it's appropriate to do a pre-emptive update-dependencies here

@dougbu

dougbu commented Nov 2, 2022

Copy link
Copy Markdown
Contributor

If it matters, dotnet-runtime had a few successful builds after dotnet/runtime#76899 merged:

#20221018.4 failed but is the closest match for the chosen GA build. We could rerun failed jobs in that one to create a public build w/ most of the 7.0.0 content.

@wtgodbe

wtgodbe commented Nov 2, 2022

Copy link
Copy Markdown
Member

I can't think of a good reason not to do an update-dependencies here for one of the builds you linked. I don't think "closest match" matters, I'd just go with 153194. It does mean we'll have to resolve a merge conflict in the internal->public merge, and therefore do it manually, but the tradeoff seems worth it to me.

@dougbu

dougbu commented Nov 2, 2022

Copy link
Copy Markdown
Contributor

but the tradeoff seems worth it to me.

👍 will do (after lunch)

@dougbu

dougbu commented Nov 2, 2022

Copy link
Copy Markdown
Contributor

Something is very wrong here. We're already at #20221013.12, BAR id 153194 in this branch and @eiriktsarpalis's dotnet/runtime#76899 change was included in that build (see https://github.com/dotnet/runtime/commits/cd2d83798383716204eb580eb5c89ef5b73b8ec2). Are we doing something wrong here @jeffhandley @carlossanlop @eiriktsarpalis

Again, the problems are Method not found: 'Void System.Text.Json.Serialization.Metadata.JsonTypeInfo.MakeReadOnly()' all over the place though we make no direct use of JsonTypeInfo and definitely don't call MakeReadOnly().

@dougbu

dougbu commented Nov 2, 2022

Copy link
Copy Markdown
Contributor

Another, relatively easy stack trace to use

   at Microsoft.AspNetCore.StaticWebAssets.ManifestStaticWebAssetFileProvider.SourceGenerationContext.Create_StaticWebAssetManifest(JsonSerializerOptions options, Boolean makeReadOnly)
   at Microsoft.AspNetCore.StaticWebAssets.ManifestStaticWebAssetFileProvider.SourceGenerationContext.get_StaticWebAssetManifest() in /_/src/Hosting/Hosting/src/System.Text.Json.SourceGeneration/System.Text.Json.SourceGeneration.JsonSourceGenerator/SourceGenerationContext.StaticWebAssetManifest.g.cs:line 23
   at Microsoft.AspNetCore.StaticWebAssets.ManifestStaticWebAssetFileProvider.StaticWebAssetManifest.Parse(Stream manifest) in /_/src/Shared/StaticWebAssets/ManifestStaticWebAssetFileProvider.cs:line 325
   at Microsoft.AspNetCore.Hosting.Tests.StaticWebAssets.ManifestStaticWebAssetsFileProviderTest.ParseWorksWithNodesThatOnlyDifferOnCasing(String path, Boolean exists) in /_/src/Hosting/Hosting/test/StaticWebAssets/ManifestStaticWebAssetsFileProviderTests.cs:line 102
   at InvokeStub_ManifestStaticWebAssetsFileProviderTest.ParseWorksWithNodesThatOnlyDifferOnCasing(Object, Object, IntPtr*)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)

In this case, SourceGenerationContext is a bit more complicated:

    [JsonSourceGenerationOptions]
    [JsonSerializable(typeof(StaticWebAssetManifest))]
    [JsonSerializable(typeof(IDictionary<string, StaticWebAssetNode>))]
    internal sealed partial class SourceGenerationContext : JsonSerializerContext
    {
        public static readonly SourceGenerationContext DefaultWithConverter = new SourceGenerationContext(new JsonSerializerOptions
        {
            Converters = { new OSBasedCaseConverter() }
        });
    }

@eiriktsarpalis

Copy link
Copy Markdown
Member

JsonTypeInfo.MakeReadOnly() is new API that was added recently as part of dotnet/runtime#76899 and is invoked by STJ source generated code. Not sure what could be causing this exact issue, but I it's the type of error one might see when using a GA SDK at compile time and then executing it with an earlier version of the runtime.

@carlossanlop

Copy link
Copy Markdown
Contributor

This was an unusual case of adding new APIs after preview7. What does aspnetcore need to do (maybe consume dependencies in a certain order) to be able to detect these new APIs?

@wtgodbe

wtgodbe commented Nov 3, 2022

Copy link
Copy Markdown
Member

Oh, maybe it's this?

<DefaultRuntimeFrameworkVersion Condition=" '$(IsServicingBuild)' != 'true' AND
'%(TargetFramework)' == '${DefaultNetCoreTargetFramework}' AND
'$(TargetLatestDotNetRuntime)' != 'false' ">${MicrosoftNETCoreAppRuntimeVersion}</DefaultRuntimeFrameworkVersion>

Now that this is a servicing build, we're no longer defaulting to using the incoming runtime version, and are instead using what's in the SDK, which doesn't have the new API?

@eiriktsarpalis

Copy link
Copy Markdown
Member

and are instead using what's in the SDK, which doesn't have the new API?

Seems like a plausible explanation, the RC2 sdk doesn't come with the new API.

@jeffhandley

Copy link
Copy Markdown
Member

@wtgodbe Can you confirm that 7.0 GA does have the visibility into this API that was added during that milestone, and this only surfaced in servicing because the process for the build changed?

@wtgodbe

wtgodbe commented Nov 3, 2022

Copy link
Copy Markdown
Member

Can you confirm that 7.0 GA does have the visibility into this API that was added during that milestone, and this only surfaced in servicing because the process for the build changed?

I have no reason to believe otherwise - the API change will have flowed to the SDK as part of the normal process just like any other API change. This mismatch is caused by us using an SDK from rc2, and a runtime from RTM. Normally we have a workaround that avoids these types of mismatches, but we turn that workaround off in servicing based on the assumption that API surface area won't change.

@eiriktsarpalis

Copy link
Copy Markdown
Member

This mismatch is caused by us using an SDK from rc2, and a runtime from RTM.

I suspect it should be the other way around? An RTM source generator producing calls to JsonTypeInfo.MakeReadOnly() which is then run against an rc2 runtime.

@dougbu

dougbu commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

I'll try using a more recent SDK in this PR.

@dougbu

dougbu commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

We always use the same SDK and that's 7.0.100-rtm.22478.12 at the moment. The issue is more that we update the targeting pack version unconditionally (based on dependency flow) but rely on the servicing SDK to provide the latest runtime. I'm assuming that SDK version doesn't have a runtime including the JsonTypeInfo.MakeReadOnly() method.

@dougbu

dougbu commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

@marcpopMSFT what 7.0.1xx SDK is actually the latest❔ I tried both the installer and SDK versions from https://aka.ms/dotnet/7.0.1xx/daily/productCommit-win-x64.txt w/o success. 7.0.101 is shown on the dotnet/installer home page (matching the installer version), but no luck there in particular.

@dougbu

dougbu commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

@marcpopMSFT what 7.0.1xx SDK is actually the latest❔ I tried both the installer and SDK versions from https://aka.ms/dotnet/7.0.1xx/daily/productCommit-win-x64.txt w/o success. 7.0.101 is shown on the dotnet/installer home page (matching the installer version), but no luck there in particular.

@dsplaisted since @marcpopMSFT is under the weather

@wtgodbe

wtgodbe commented Nov 3, 2022

Copy link
Copy Markdown
Member

I grabbed the one from the latest build of release/7.0.1xx: https://dev.azure.com/dnceng/internal/_build/results?buildId=2034545&view=results

@dougbu

dougbu commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

Thanks @wtgodbe. Still wondering what's going on w/ the dotnet/installer home page and the productCommit-win-x64.txt file.

@wtgodbe

wtgodbe commented Nov 3, 2022

Copy link
Copy Markdown
Member

Still wondering what's going on w/ the dotnet/installer home page and the productCommit-win-x64.txt file.

I think it lists the "shipping" version of the SDK, which (since branding is now stable), will always be unsuffixed. But dotnet/installer produces both a stable-versioned & non-stable-versioned SDK every time when branding is stable, and only the non-stable one can be picked up by dotnet-install because dotnet-install uses the SDK version you list to look for a folder in blob storage. It's only on release day that we create such a folder w/ stable branding.

@wtgodbe

wtgodbe commented Nov 3, 2022

Copy link
Copy Markdown
Member

I added the stable windowsdesktop feed from https://github.com/dotnet/installer/blob/release/7.0.1xx/NuGet.config#L10, I think that should do it

@wtgodbe

wtgodbe commented Nov 3, 2022

Copy link
Copy Markdown
Member

On Tuesday we should go to the 7.0.100 SDK (and make sure the windowsdesktop feed is removed)

@wtgodbe

wtgodbe commented Nov 3, 2022

Copy link
Copy Markdown
Member

Argh, this won't work either - dotnet/runtime#76899 never flowed to the public dotnet/installer repo. I think we should revert the SDK updates & my nuget.config change, and special case the workaround at

<DefaultRuntimeFrameworkVersion Condition=" '$(IsServicingBuild)' != 'true' AND
'%(TargetFramework)' == '${DefaultNetCoreTargetFramework}' AND
'$(TargetLatestDotNetRuntime)' != 'false' ">${MicrosoftNETCoreAppRuntimeVersion}</DefaultRuntimeFrameworkVersion>
for 7.0.1 - @dougbu thoughts?

@dougbu

dougbu commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

@dougbu thoughts?

I think this is a convoluted mess that rhymes w/ source generators and late changes. I'll try changing some $(IsServicingBuild) checks. But, if that doesn't work, will give up and wait for the GA SDK to ship publicly.

@build-analysis build-analysis Bot mentioned this pull request Nov 3, 2022
2 tasks
- use incoming runtime even though this is a servicing build
@dougbu dougbu force-pushed the branding-7.0.1-2022-10-27-0951 branch from 7b0f75f to 841e438 Compare November 3, 2022 22:21
On the other hand, $(TargetLatestDotNetRuntime) is specific to this repo and controls only the update below.
-->
<DefaultRuntimeFrameworkVersion Condition=" '$(IsServicingBuild)' != 'true' AND
<DefaultRuntimeFrameworkVersion Condition=" ('$(IsServicingBuild)' != 'true' OR '$(NETCoreSdkVersion)' == '7.0.100-rtm.22478.12') AND

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note this hack automatically stops working as soon as we change the SDK version.

@marcpopMSFT

Copy link
Copy Markdown
Member

Will's analysis above is correct. We're now on stable branding which dotnet-install can't use (until it actually ships) so that installer table is no longer useful for 7.0.1xx. I'll work on getting it removed (it's the same reason we removed all the other servicing branches from that table).

@dougbu

dougbu commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

@marcpopMSFT is there a way to reliably get the latest public nonstable version (that dotnet-install scripts can use) other than @wtgodbe's method of visiting installer builds and finding the latest successful one for (say) the release/7.0.1xx branch❔ I'd like to update my Get-NightlySdkVersion.ps1 script so it works even during the X.0.0 to X.0.1 transition. (We stick w/ public stable versions once they exist.)

@dougbu

dougbu commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

/fyi all: Builds are looking good so far, except for some macOS issues w/ AzDO feeds. Expect I'll need to rerun failed jobs, but should be able to get this in tonight.

@dougbu dougbu merged commit 01b8d25 into dotnet:release/7.0 Nov 4, 2022
@marcpopMSFT

Copy link
Copy Markdown
Member

@marcpopMSFT is there a way to reliably get the latest public nonstable version (that dotnet-install scripts can use) other than @wtgodbe's method of visiting installer builds and finding the latest successful one for (say) the release/7.0.1xx branch❔ I'd like to update my Get-NightlySdkVersion.ps1 script so it works even during the X.0.0 to X.0.1 transition. (We stick w/ public stable versions once they exist.)

I'm not sure if there is. @YuliiaKovalova might have some idea or suggestion on if there's a channel or something you can use to get the latest daily build of a stable release.

@ghost

ghost commented Nov 4, 2022

Copy link
Copy Markdown

Hi @marcpopMSFT. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@YuliiaKovalova

Copy link
Copy Markdown
Member

Hi @marcpopMSFT ,

It is possible to specify -Channel "LTS" -Quality "daily" for getting the latest build of the stable version.

Please see for getting more details:
https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-install-script#:~:text=%2D-,Quality%7C%2D%2Dquality%20%3CQUALITY%3E,-Downloads%20the%20latest

@ghost

ghost commented Nov 7, 2022

Copy link
Copy Markdown

Hi @YuliiaKovalova. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@dougbu

dougbu commented Nov 7, 2022

Copy link
Copy Markdown
Contributor

Thanks @YuliiaKovalova, hadn't thought to use -quality quite that way. (Though it's actually ignored for the LTS channel.)

In any case, I'd reserve -quality ga for our regular SDK updates of 'main'. .\.dotnet\dotnet-install.ps1 -channel 7.0.1xx -dryrun -quality preview would have found the SDK build we tried (and eventually discarded) in this branch. -quality ga also looks like a great way to find the latest SDK for older branches.

@dougbu dougbu added this to the 7.0.1 milestone Nov 8, 2022
@dougbu dougbu added the tell-mode Indicates a PR which is being merged during tell-mode label Nov 8, 2022
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 tell-mode Indicates a PR which is being merged during tell-mode

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants