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

fix(dotnet): Ensure that packages can be updated when referencing .NET workloads #10649

Merged
merged 8 commits into from
Oct 22, 2024

Conversation

jeromelaban
Copy link
Contributor

@jeromelaban jeromelaban commented Sep 20, 2024

What are you trying to accomplish?

This PR is adding an MSBuild property that will prevent this .NET SDK target to execute in order to continue getting the dependency graph.

Fixes #10117

Anything you want to highlight for special attention from reviewers?

There does not seem to be another way to avoid getting the NETSDK1147 error.

This PR uses two specifics to get around workloads not being installed:

  • It uses DesignTimeBuild to skip the workloads validation
  • It defines a fake TargetPlatformVersion in order for nuget to still consider the platform-specific target frameworks.

How will you know you've accomplished your goal?

Both tests that are added will succeed, as they reference target frameworks that require the use of .NET workloads.

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

@github-actions github-actions bot added the L: dotnet:nuget NuGet packages via nuget or dotnet label Sep 20, 2024
@jeromelaban jeromelaban marked this pull request as ready for review September 20, 2024 19:38
@jeromelaban jeromelaban requested a review from a team as a code owner September 20, 2024 19:38
@JonDouglas
Copy link

lgtm, @brettfo is there a point of contact who manages the .NET dependabot stuff nowadays? I think Stephen P did last I chatted.

@brettfo
Copy link
Contributor

brettfo commented Oct 16, 2024

@JonDouglas I'm a good point of contact for the NuGet updater.

@jeromelaban Could you explain more (or links so I can read more) about the $(TargetPlatformVersion) being set to 1.0? Setting $(DesignTimeBuild) to true I'm good with, but I want to make sure I fully understand the other.

@jeromelaban
Copy link
Contributor Author

@jeromelaban Could you explain more (or links so I can read more) about the $(TargetPlatformVersion) being set to 1.0? Setting $(DesignTimeBuild) to true I'm good with, but I want to make sure I fully understand the other.

Sure! The issue here is that NuGet requires that any target framework with a platform specifier also has an SDK that defines an msbuild property with the currently supported default platform version, when none is defined in the TargetFramework.

For instance in .NET 9.0, net9.0-ios means net9.0-ios18.0 where TargetPlatformVersion is set to 18.0 by default when workloads are installed. When that property is not defined, nuget complains that it can't resolve references, and 1.0 is chosen to allow the resolver to match packages that explicitly define a version (it's generally >= 1.0).

@brettfo
Copy link
Contributor

brettfo commented Oct 18, 2024

I requested one test change and you'll also have to update latest from main; there was a rename that will likely cause a conflict.

@jeromelaban jeromelaban marked this pull request as draft October 18, 2024 17:26
@jeromelaban jeromelaban force-pushed the dev/jela/nuget-workloads branch 2 times, most recently from 04c5b05 to 0f0a4a2 Compare October 18, 2024 17:39
@jeromelaban jeromelaban marked this pull request as ready for review October 18, 2024 18:38
@brettfo
Copy link
Contributor

brettfo commented Oct 18, 2024

Thank you for this! I should be able to get it merged and deployed early next week.

@randhircs randhircs merged commit eb77cb3 into dependabot:main Oct 22, 2024
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: dotnet:nuget NuGet packages via nuget or dotnet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for dotnet project requiring workload installation for builds
4 participants