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 the error for targeting net9 in 17.11 #43143

Open
wants to merge 8 commits into
base: release/9.0.1xx
Choose a base branch
from

Conversation

marcpopMSFT
Copy link
Member

@marcpopMSFT marcpopMSFT commented Aug 30, 2024

Update the minimum MSBuild version to 17.11.3
Update the supported TFM so net9 doesn't show up when you're in 17.11 Update the min vs for unsupported version to N+4 since we don't know what vs version net10 will ship in

Summary
Per the support rules, GA always supports N-1 of the Visual Studio versions when targeting downlevel. This is to support customers on day 0 who have not updated their TF yet but will get a new SDK through the install dotnet GH action.

Customer Impact

Customers using 17.11 and trying to target net9.0 will get a warning that they need to update their Visual studio

Regression

Intentional change

Testing

Automated.

Risk

Low.

Update the minimum MSBuild version to 17.11.3
Update the supported TFM so net9 doesn't show up when you're in 17.11
Update the min vs for unsupported version to N+4 since we don't know what vs version net10 will ship in
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Request triage from a team member label Aug 30, 2024
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@marcpopMSFT
Copy link
Member Author

@dotnet/kitten how worried should I be that I couldn't build this with minimum msbuild being 17.11.0 or 17.11.3. I'm trying 17.11.4 as that appears to be what it's finding on the feeds. What are customers likely to have as this will make it so that the 9 sdk won't even load if we find anything older than 17.11.4

@maridematte
Copy link
Contributor

From what I can find on NuGet we only published 17.11.4, and nothing earlier in 17.11. But it seems that we did the same thing with earlier versions, so I don't think this should be a problem. But I'll double check that.

@marcpopMSFT
Copy link
Member Author

/azp run dotnet-sdk-public-ci,sdk-source-build,sdk-unified-build

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@marcpopMSFT
Copy link
Member Author

This Pr has to wait until the test images are on 17.12 or on the GA 17.11 as right now they are still 17.11 previews.

@marcpopMSFT marcpopMSFT added Servicing-consider breaking-change Using this label will notify dotnet/compat and trigger a request to file a compat bug labels Sep 17, 2024
Copy link
Contributor

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

  1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
  2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

@rainersigwald
Copy link
Member

@dotnet/kitten how worried should I be that I couldn't build this with minimum msbuild being 17.11.0 or 17.11.3. I'm trying 17.11.4 as that appears to be what it's finding on the feeds. What are customers likely to have as this will make it so that the 9 sdk won't even load if we find anything older than 17.11.4

Looks like we shipped VS 17.11.0 with MSBuild 17.11.2 and didn't bump VS until MSBuild 17.11.9 in VS 17.11.4. MSBuild 17.11.4 was an SDK-only release as I understand it.

I'm not sure why that's the one that got published to NuGet.org. @MichalPavlik do you recall?

@MichalPavlik
Copy link
Member

@rainersigwald, I remember we published 17.11.4 to nuget and some 17.11.2 packages as we were asked by Sylvia... Let's discuss it offline today.

@rainersigwald
Copy link
Member

@marcpopMSFT we may actually want to restrict the MSBuild version to 17.11.9, which has the STJ bump fix that fixed 9.0.100-rc.1. We could publish those packages if you need them.

@marcpopMSFT
Copy link
Member Author

@marcpopMSFT we may actually want to restrict the MSBuild version to 17.11.9, which has the STJ bump fix that fixed 9.0.100-rc.1. We could publish those packages if you need them.

Hmm, @rainersigwald we typically haven't used this for specific version restrictions in the past. It's really been more about just setting a floor for a VS release and in the past has been the .0 version where possible. Only recently is MSBuild shipping more .N releases in the VS .0 version because you stabilize versions earlier so it makes it look like I want 17.11.4 but I really just want 17.11.0

@marcpopMSFT
Copy link
Member Author

@rainersigwald concerns if I leave it as is or did you want me to update it to ensure folks have a latest 17.11?

Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

.4 is fine by me.

@marcpopMSFT
Copy link
Member Author

@rainersigwald Looks like the resolver uses an unstable build number rather than the stable version. Did that change at some point as I don't think we had this sort of problem before?

The VS images did update but they have msbuild version 17.11.0.27902 rather than 17.11.4. Suggestions on how we resolve this?

https://github.com/dotnet/sdk/blob/main/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs#L168
C:\h\w\B5990A03\p\d\sdk\9.0.100-ci\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.ImportWorkloads.props(14,3): error : Version 9.0.100-ci of the .NET SDK requires at least version 17.11.4 of MSBuild. The current available version of MSBuild is 17.11.0.27902. Change the .NET SDK specified in global.json to an older version that requires the MSBuild version currently available.

@rainersigwald
Copy link
Member

Looks like the resolver uses an unstable build number rather than the stable version. Did that change at some point as I don't think we had this sort of problem before?

I also don't remember this, and wouldn't expect us to have changed it . . .

@marcpopMSFT
Copy link
Member Author

I also don't remember this, and wouldn't expect us to have changed it . . .
That surprised me as well. There was a change somewhere but I'm not sure how to track it down. Is it worth digging into or should I just switch to separating these (it was always strange that we tied the msbuild build to the actual min version)

@rainersigwald
Copy link
Member

I'm completely on board with separating.

@marcpopMSFT
Copy link
Member Author

This works but we have tests targeting net9 that check for a warning. I either need an image with 17.12 in it so we don't get the warnings or suppress that specific warning in the impacted tests. Checking with dnceng on the dates for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Using this label will notify dotnet/compat and trigger a request to file a compat bug needs-breaking-change-doc-created untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants