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 TFMs #4394

Merged
merged 6 commits into from
Sep 13, 2023
Merged

Update TFMs #4394

merged 6 commits into from
Sep 13, 2023

Conversation

RussKie
Copy link
Member

@RussKie RussKie commented Sep 13, 2023

  • Drop support for .NET Core 3.1
  • Add support for .NET 6.0
  • Run tests for only supported TFMs - i.e., net462.net6.0,net8.0
Microsoft Reviewers: Open in CodeFlow

@ghost ghost assigned RussKie Sep 13, 2023
Comment on lines -89 to +84
<!-- This target will make sure that projects targeting netcoreapp3.1 will also have the Microsoft.Extensions.Logging.Abstractions analyzer removed. -->
<!-- This target will make sure that projects targeting net462 will also have the Microsoft.Extensions.Logging.Abstractions analyzer removed. -->
Copy link
Member Author

Choose a reason for hiding this comment

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

@joperezr I kept this target because the comment in Microsoft.Extensions.Telemetry.Abstractions.csproj refers to both net462 and netcoreapp3.1:
image

Please let me know, if I misunderstood it, and these should be removed.

* Drop support for .NET Core 3.1
* Add support for .NET 6.0
* Run tests for only supported TFMs - i.e., net462;net6.0;net8.0
@RussKie

This comment was marked as resolved.

@geeknoid
Copy link
Member

geeknoid commented Sep 13, 2023

@RussKie Where do the warnings come from? This did these just appear as a result of an analyzer upgrade?

@RussKie
Copy link
Member Author

RussKie commented Sep 13, 2023

There are also hundreds of TBD warnings:

error TBD: (NETCORE_ENGINEERING_TELEMETRY=Build) 'Microsoft.Extensions.Options.Contextual.IOptionsContext' is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed. (https://aka.ms/dotnet-extensions-warnings/TBD)

These are the result of (likely because of the newer SDK?):

<ItemGroup Condition="'$(Stage)' == 'dev' AND '$(OutputType)' != 'Exe' AND '$(Api)' != 'false'">
<AssemblyAttribute Include="System.Diagnostics.CodeAnalysis.ExperimentalAttribute">
<_Parameter1>TBD</_Parameter1>
<_Parameter2>UrlFormat = "https://aka.ms/dotnet-extensions-warnings/{0}"</_Parameter2>
<_Parameter2_IsLiteral>true</_Parameter2_IsLiteral>
</AssemblyAttribute>
</ItemGroup>

As an immediate workaround I'm going to suppress TBD warning, but we need to generalise how the assembly-level attribute is applied based on the workstream? We'll need a mapping between workstreams and the experiment areas listed in https://github.com/dotnet/extensions/blob/main/docs/list-of-diagnostics.md. I'll raise a tracking issue.


[EDIT] Raised #4395

@RussKie
Copy link
Member Author

RussKie commented Sep 13, 2023

@RussKie Where do the warnings come from? This did these just appear as a result of an analyzer upgrade?

The newer SDK:

   "sdk": {
-    "version": "8.0.100-rc.1.23381.2"
+    "version": "8.0.100-rc.2.23462.28"
   },

@geeknoid
Copy link
Member

I think we should fix the IDE0028 warnings (I'm guessing VS can do this over the whole solution).

And please suppress the TBD warning for now, I'll find a suitable workaround.

@RussKie
Copy link
Member Author

RussKie commented Sep 13, 2023

I think we should fix the IDE0028 warnings (I'm guessing VS can do this over the whole solution).

Sure.
Personally, I am not too fond of the new style, but I guess I may get used to it :)

@RussKie
Copy link
Member Author

RussKie commented Sep 13, 2023

The build is green, good to merge 😉

@RussKie RussKie merged commit b8b7166 into dotnet:release/8.0 Sep 13, 2023
5 checks passed
@RussKie RussKie deleted the update_tfms branch September 13, 2023 23:31
@joperezr
Copy link
Member

Thanks for fixing this @RussKie

FYI @ericstj

<ConditionalNet462 Condition="'$(CustomTargetFrameworks)' == '' OR $(CustomTargetFrameworks.Contains('net462'))">;net462</ConditionalNet462>

<!-- All the .NET TFMs we're testing against -->
<TestNetCoreTargetFrameworks>$(NetCoreTargetFrameworks)</TestNetCoreTargetFrameworks>
<TestNetCoreTargetFrameworks Condition="'$(CustomTargetFrameworks)' == ''">$(TestNetCoreTargetFrameworks);net7.0;net6.0</TestNetCoreTargetFrameworks>
<TestNetCoreTargetFrameworks Condition="'$(CustomTargetFrameworks)' == ''">$(TestNetCoreTargetFrameworks);net6.0</TestNetCoreTargetFrameworks>
Copy link
Contributor

Choose a reason for hiding this comment

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

@RussKie now this emits a warning that looks like:

Error occurred while restoring NuGet packages: Invalid restore input. The original target frameworks value must match the aliases. Original target frameworks: net8.0;net6.0;net6.0;net462, aliases: net8.0;net6.0;net462. Input files: C:\Users\...\source\dotnet-ext\test\Generators\Microsoft.Gen.AutoClient\Generated\Microsoft.Gen.AutoClient

I.e., the net6.0 TFM is listed twice for test projects that have <TargetFrameworks>$(TestNetCoreTargetFrameworks)</TargetFrameworks>

Copy link
Member

Choose a reason for hiding this comment

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

Can either you or @RussKie log an issue to fix this so that we make sure it is fixed? How come we aren't seeing this error in CI?

Copy link
Member Author

Choose a reason for hiding this comment

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

@ghost ghost locked as resolved and limited conversation to collaborators Oct 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants