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 to net9.0 #75465

Merged
merged 5 commits into from
Oct 14, 2024
Merged

Update to net9.0 #75465

merged 5 commits into from
Oct 14, 2024

Conversation

jjonescz
Copy link
Member

@jjonescz jjonescz commented Oct 10, 2024

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 10, 2024
@DoctorKrolic
Copy link
Contributor

Does this also fix #75205 (are there any mentions of net7.0 in the codebase after this change)?

@jjonescz
Copy link
Member Author

are there any mentions of net7.0 in the codebase after this change

No, there don't seem to be any. I will mark the issue as fixed by this PR, thanks.

docs/contributing/Target Framework Strategy.md Outdated Show resolved Hide resolved
@@ -52,7 +52,7 @@
<MicrosoftExtensionsOptionsConfigurationExtensionVersion>8.0.0</MicrosoftExtensionsOptionsConfigurationExtensionVersion>
<MicrosoftExtensionsOptionsVersion>8.0.0</MicrosoftExtensionsOptionsVersion>
<MicrosoftExtensionsPrimitivesVersion>8.0.0</MicrosoftExtensionsPrimitivesVersion>
<MicrosoftNetCompilersToolsetVersion>4.11.0-2.24270.4</MicrosoftNetCompilersToolsetVersion>
<MicrosoftNetCompilersToolsetVersion>4.12.0-3.24473.3</MicrosoftNetCompilersToolsetVersion>
Copy link
Member

Choose a reason for hiding this comment

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

Curious: what if we just delete this and move to the compiler that comes with the .NET SDK.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can try that. But generally we sometimes want to consume new compiler features before we update our SDK, right? So then we would need to re-add this again.

Copy link
Member Author

Choose a reason for hiding this comment

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

Although I updated this to bring in the "better conversion from collection expression" feature that's needed because of some string.Join ambiguities - and that might not be even in RC1, right? Well, definitely it's not in preview 7 which we are currently at.

@@ -110,3 +110,22 @@ When the .NET SDK RTMs and Roslyn adopts it all occurrences of `$(NetRoslynNext)

**DO NOT** include both `$(NetRoslyn)` and `$(NetRoslynNext)` in the same project unless there is a very specific reason that both tests are adding value. The most common case is that the runtime has changed behavior and we simply need to update our baselines to match it. Adding extra TFMs for this just increases test time for very little gain.

## Checklist for updating TFMs (once a year)
Copy link
Member

Choose a reason for hiding this comment

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

One other item to check is that you don't regress the number of tests that are running in CI. Our logic for finding tests can include looking at TFM. In the past I messed this up and stopped running certain unit test assemblies.

@@ -40,10 +40,6 @@ internal static bool IsCoreClr8OrHigherRuntime
internal static bool IsCoreClr9OrHigherRuntime
=> CoreClrRuntimeVersion is { } v && v >= 9;

#if NET9_0_OR_GREATER
#error Make the above check be an #if NET9_OR_GREATER when we add net8 support to build
#endif
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what this was trying to say.

Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing it was important at some point when I was converting locally but not by the time I merged the change.

@jjonescz jjonescz marked this pull request as ready for review October 11, 2024 12:08
@jjonescz jjonescz requested review from a team as code owners October 11, 2024 12:08
@@ -4,7 +4,7 @@
<PropertyGroup>
<OutputType>Library</OutputType>
<RootNamespace>Microsoft.CodeAnalysis.MSBuild.UnitTests</RootNamespace>
<TargetFrameworks>$(NetRoslyn);net472</TargetFrameworks>
<TargetFrameworks>$(NetVS);net472</TargetFrameworks>
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't expect it to be necessary to hold this back. Was there a particular issue when moving this to 9.0?

Copy link
Member

Choose a reason for hiding this comment

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

We run the unit tests on a machine with VS installed, so that might matter somehow if maybe the regular SDK isn't there. If that somehow broke, do file a bug @jjonescz and we can investigate further.

Copy link
Member Author

Choose a reason for hiding this comment

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

We run the unit tests on a machine with VS installed, so that might matter somehow if maybe the regular SDK isn't there

Yes, that's exactly what happens. Created an issue: #75498

@@ -38,12 +38,14 @@ static Test()
// reasonable TLS protocol version for outgoing connections.
#pragma warning disable CA5364 // Do Not Use Deprecated Security Protocols
#pragma warning disable CS0618 // Type or member is obsolete
#pragma warning disable SYSLIB0014 // 'ServicePointManager' is obsolete
Copy link
Member

Choose a reason for hiding this comment

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

Is this obsoleted in a way that means this isn't working anymore? Is this just stale now?

Copy link
Member

Choose a reason for hiding this comment

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

(I have no idea what this is, but might be worth filing a bug as a follow up to track what this is.)

Copy link
Member Author

@jjonescz jjonescz Oct 14, 2024

Choose a reason for hiding this comment

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

WebRequest, WebClient, and ServicePoint are obsolete since .NET 6. See https://learn.microsoft.com/en-us/dotnet/core/compatibility/networking/6.0/webrequest-deprecated. The SYSLIB0014 is a pre-existing diagnostic that started to be reported for ServicePointManager in .NET 9 via dotnet/runtime#103456.

Filed an issue: #75499

@@ -4,7 +4,7 @@
<PropertyGroup>
<OutputType>Library</OutputType>
<RootNamespace>Microsoft.CodeAnalysis.MSBuild.UnitTests</RootNamespace>
<TargetFrameworks>$(NetRoslyn);net472</TargetFrameworks>
<TargetFrameworks>$(NetVS);net472</TargetFrameworks>
Copy link
Member

Choose a reason for hiding this comment

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

We run the unit tests on a machine with VS installed, so that might matter somehow if maybe the regular SDK isn't there. If that somehow broke, do file a bug @jjonescz and we can investigate further.

@jjonescz jjonescz merged commit daf100b into dotnet:main Oct 14, 2024
28 checks passed
@jjonescz jjonescz deleted the 75453-net9 branch October 14, 2024 09:39
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate the target framework in our builds remove net7.0 target
6 participants