Remove the cast from the empty web project template #33393

davidfowl · 2021-06-09T04:54:55Z

Remove the cast from the empty web project template

davidfowl · 2021-06-09T08:20:18Z

src/ProjectTemplates/Web.ProjectTemplates/EmptyWeb-CSharp.csproj.in

@@ -4,6 +4,7 @@
    <TargetFramework>${DefaultNetCoreTargetFramework}</TargetFramework>
    <NoDefaultLaunchSettingsFile Condition="'$(ExcludeLaunchSettings)' == 'True'">True</NoDefaultLaunchSettingsFile>
    <RootNamespace Condition="'$(name)' != '$(name{-VALUE-FORMS-}safe_namespace)'">Company.WebApplication1</RootNamespace>
+    <LangVersion>preview</LangVersion>


I think we'll be able to remove this when we get a new SDK.

cc @jaredpar

This will go away in .NET 6 P7.

@jaredpar can you help me understand this better as the docs seem to say preview frameworks will always default to their matching preview language: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/configure-language-version

When your project targets a preview framework that has a corresponding preview language version, the language version used is the preview language version. You use the latest features with that preview in any environment, without affecting projects that target a released .NET Core version.

halter73 · 2021-06-09T16:50:44Z

It looks like Templates.Test.EmptyWebTemplateTest.EmptyWebTemplateCSharp is having issues again. error CS1593: Delegate 'RequestDelegate' does not take 0 arguments seems to imply we're still not testing against the MapGet overloads added in preview4 (Edit: or more likely not testing with the preview compiler). Is this actually testing against the updated template and aspnet runtime @dotnet/aspnet-build?

Templates.Test.EmptyWebTemplateTest.EmptyWebTemplateCSharp
Project new web  failed to publish. Exit code 1.
/datadisks/disk1/work/A1E60924/p/dotnet-cli/dotnet publish --no-restore -c Release /bl
StdErr:
StdOut: Microsoft (R) Build Engine version 17.0.0-preview-21256-03+d07c47ade for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

/datadisks/disk1/work/A1E60924/p/dotnet-cli/sdk/6.0.100-preview.5.21264.3/MSBuild.dll -maxcpucount -property:Configuration=Release -target:Publish -verbosity:m /bl ./AspNet.bzpld23ioa3.csproj
You are using a preview version of .NET. See: https://aka.ms/dotnet-core-preview
/datadisks/disk1/work/A1E60924/w/AC8C094C/e/Templates/BaseFolder/AspNet.bzpld23ioa3/Program.cs(13,17): error CS1593: Delegate 'RequestDelegate' does not take 0 arguments [/datadisks/disk1/work/A1E60924/w/AC8C094C/e/Templates/BaseFolder/AspNet.bzpld23ioa3/AspNet.bzpld23ioa3.csproj]

Expected: True
Actual:   False

dougbu · 2021-06-09T21:02:36Z

more likely not testing with the preview compiler

@halter73 we should use the Microsoft.Net.Compilers.Toolset version from eng/Versions.props everywhere, even on Helix machines when testing templates (because they're special-cased). If that covers the complete "preview compiler", everything should be fine.

Unfortunately the Helix logs are far too detailed to be useful. The one interesting bit is

[xUnit.net 00:00:56.67]         | [47.544s] Templates.Test.EmptyWebTemplateTest Information: [ERROR] Reconciling library Microsoft.Net.Compilers.Toolset/4.0.0-2.21263.6
[xUnit.net 00:00:56.67]         | [47.544s] Templates.Test.EmptyWebTemplateTest Information: [ERROR] Library Microsoft.Net.Compilers.Toolset/4.0.0-2.21263.6 does not exist

This may indicate the SDK (or something) attempts to override https://github.com/dotnet/aspnetcore/blob/main/eng/Versions.props#L159 (which hasn't changed in a couple of months). I'm not sure what version it falls back to.

davidfowl · 2021-06-10T01:03:21Z

@dougbu we need to check in changes that will work with the latest VS and SDK before preview6 ships. There seems be an issue with ingestion of newer bits here and that's blocking this change. I don't really understand how any of this works can you help diagnose this please?

davidfowl · 2021-06-10T04:20:42Z

/azp run

azure-pipelines · 2021-06-10T04:21:00Z

Azure Pipelines successfully started running 2 pipeline(s).

davidfowl · 2021-06-12T08:04:28Z

So I have a clue, when I build a sample in this repository with LangVersion set to preview I see this in a binlog:

CommandLineArguments = C:\dev\git\AspNetCore\.dotnet\dotnet.exe exec "C:\Users\david\.nuget\packages\microsoft.net.compilers.toolset\3.10.0-1.final\tasks\netcoreapp3.1\bincore\csc.dll" /noconfig /unsafe- /checked- /nowarn:1701,1702,1705,NU5105,CS1591,RS0041,CA1416,1701,1702 /fullpaths /nostdlib+ /errorreport:prompt /warn:5 /define:TRACE;DEBUG;NET;NET6_0;NETCOREAPP;NET5_0_OR_GREATER;NET6_0_OR_GREATER;NETCOREAPP1_0_OR_GREATER;NETCOREAPP1_1_OR_GREATER;NETCOREAPP2_0_OR_GREATER;NETCOREAPP2_1_OR_GREATER;NETCOREAPP2_2_OR_GREATER;NETCOREAPP3_0_OR_GREATER;NETCOREAPP3_1_OR_GREATER /highentropyva+ ..

Any ideas @dougbu ? This might be an arcade question... The exact same code builds fine outside of this repo with the same outer command line.

davidfowl · 2021-06-12T08:18:41Z

I think it's coming from here

aspnetcore/eng/Versions.props

Line 160 in 9fd548f

    
           <MicrosoftNetCompilersToolsetVersion>3.10.0-1.final</MicrosoftNetCompilersToolsetVersion>

davidfowl · 2021-06-12T08:45:02Z

OK I figured it out. Lets see if it breaks the entire repo 😄

davidfowl · 2021-06-12T13:46:31Z

/azp run

azure-pipelines · 2021-06-12T13:46:49Z

Azure Pipelines successfully started running 2 pipeline(s).

davidfowl · 2021-06-13T01:36:59Z

Feel free to let me know if this change is crazy because I removed some fundamental piece of infrastructure.

halter73 · 2021-06-14T16:39:18Z

eng/tools/GenerateFiles/Directory.Build.targets.in

-    <PackageReference Include="Microsoft.Net.Compilers.Toolset"
-      Version="${MicrosoftNetCompilersToolsetVersion}"
-      PrivateAssets="all"
-      IsImplicitlyDefined="true" />


Do we know why this was here before?

I don't know, we'll see if something breaks. I spoke to Doug and he's going to follow up and do some research.

@wtgodbe introduced this long ago (for a 3.0.0 preview) in 23a6e3e / #11818. We were reacting to a Roslyn breaking change and needed a newer toolset than Arcade (think we set $(UsingToolMicrosoftNetCompilers) at one point) or msbuild references by default. Maestro++ managed the version for a while but we stopped that at some point. @BrennanConroy and @pranavkm were among the few to change the version after we made the version manual.

I don't remember the details of the breaking change but it had something to do w/ nullability annotations and @roji did relevant work in the efcore repo. @jaredpar may know though he wasn't involved in the conversation back in the summer of 2019.

As a follow up, we should do one of

change the manually-controlled toolset version and re-introduce this package reference

clean up

aspnetcore/eng/Versions.props

Line 160 in 8ee0b03

<MicrosoftNetCompilersToolsetVersion>3.10.0-1.final</MicrosoftNetCompilersToolsetVersion>

enable $(UsingToolMicrosoftNetCompilers), meaning Arcade updates will bump the toolset version on occasion

I don't know enough about the toolset to have an opinion on which choice is best. @jaredpar❔

In general repositories should avoid explicitly referencing this package and instead prefer the one that just comes with arcade. The arcade version is updated at a regular cadence (roughly every other week). The idea being that this update cadence means we're regularly pushing new versions of the compiler to our partners in .NET.

This type of change where the version is pinned should be reserved for cases where a breaking change, bug, etc ... hits the repo and you need to pause the updates until we get a fix out. That appears to be what happened here except we forget to undo the pin once the bug was fixed.

The arcade version is updated at a regular cadence (roughly every other week).

Right now, the Arcade version (see https://github.com/dotnet/arcade/blob/05634736a53ee65b0d19cf913d77d1f5d4293ebc/eng/Versions.props#L37) is far behind what the SDK uses by default (see https://github.com/dotnet/sdk/blob/ef6249b3b034b12f127b0d3a72ea428979f44413/eng/Versions.props#L120). Right now Arcade is only at v3.10.0-4.21329.37 while the SDK is at v4.0.0-2.21354.7. I suggest we simply remove our confusing $(MicrosoftNetCompilersToolsetVersion) property and leave $(UsingToolMicrosoftNetCompilers) unset i.e. my option (2). Agreed @dotnet/aspnet-build @jaredpar❔

Remove the cast

7cd427a

davidfowl requested a review from Pilchie as a code owner June 9, 2021 04:54

ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jun 9, 2021

davidfowl requested a review from halter73 June 9, 2021 04:55

halter73 approved these changes Jun 9, 2021

View reviewed changes

Add language version preview

5876a19

davidfowl commented Jun 9, 2021

View reviewed changes

Remove implicit Microsoft.Net.Compilers.TooSet dependency

7edf1d9

davidfowl requested a review from a team as a code owner June 12, 2021 08:49

davidfowl merged commit 5f8fce6 into main Jun 13, 2021

davidfowl deleted the davidfowl/remove-the-cast branch June 13, 2021 01:36

ghost added this to the 6.0-preview6 milestone Jun 13, 2021

davidfowl mentioned this pull request Jun 13, 2021

Epic: Minimal hosting and actions #32378

Closed

29 tasks

halter73 reviewed Jun 14, 2021

View reviewed changes

dougbu mentioned this pull request Jul 6, 2021

Add support for enhanced line pragmas #33954

Merged

mindingdata mentioned this pull request Jul 15, 2021

Minmal .NET API - Delegate 'RequestDelegate' does not take 0 arguments #34390

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove the cast from the empty web project template #33393

Remove the cast from the empty web project template #33393

davidfowl commented Jun 9, 2021

davidfowl Jun 9, 2021

jaredpar Jun 9, 2021

DamianEdwards Jun 10, 2021 •

edited

Loading

halter73 commented Jun 9, 2021 •

edited

Loading

dougbu commented Jun 9, 2021

davidfowl commented Jun 10, 2021

davidfowl commented Jun 10, 2021

azure-pipelines bot commented Jun 10, 2021

davidfowl commented Jun 12, 2021 •

edited

Loading

davidfowl commented Jun 12, 2021

davidfowl commented Jun 12, 2021

davidfowl commented Jun 12, 2021

azure-pipelines bot commented Jun 12, 2021

davidfowl commented Jun 13, 2021

halter73 Jun 14, 2021

davidfowl Jun 14, 2021

dougbu Jun 14, 2021

jaredpar Jun 14, 2021

dougbu Jul 6, 2021

Remove the cast from the empty web project template #33393

Remove the cast from the empty web project template #33393

Conversation

davidfowl commented Jun 9, 2021

davidfowl Jun 9, 2021

Choose a reason for hiding this comment

jaredpar Jun 9, 2021

Choose a reason for hiding this comment

DamianEdwards Jun 10, 2021 • edited Loading

Choose a reason for hiding this comment

halter73 commented Jun 9, 2021 • edited Loading

dougbu commented Jun 9, 2021

davidfowl commented Jun 10, 2021

davidfowl commented Jun 10, 2021

azure-pipelines bot commented Jun 10, 2021

davidfowl commented Jun 12, 2021 • edited Loading

davidfowl commented Jun 12, 2021

davidfowl commented Jun 12, 2021

davidfowl commented Jun 12, 2021

azure-pipelines bot commented Jun 12, 2021

davidfowl commented Jun 13, 2021

halter73 Jun 14, 2021

Choose a reason for hiding this comment

davidfowl Jun 14, 2021

Choose a reason for hiding this comment

dougbu Jun 14, 2021

Choose a reason for hiding this comment

jaredpar Jun 14, 2021

Choose a reason for hiding this comment

dougbu Jul 6, 2021

Choose a reason for hiding this comment

DamianEdwards Jun 10, 2021 •

edited

Loading

halter73 commented Jun 9, 2021 •

edited

Loading

davidfowl commented Jun 12, 2021 •

edited

Loading