Skip to content

Conversation

@sin-ack
Copy link
Contributor

@sin-ack sin-ack commented Mar 6, 2025

These are a couple fixes for when working with non-latest SDKs such as net8.0. We faced these issues when upgrading from rules_dotnet 0.15.1 to 0.17.5.

@sin-ack sin-ack requested a review from purkhusid as a code owner March 6, 2025 17:35
sin-ack added 2 commits March 6, 2025 17:37
Without this change, the following scenario fails:

- csharp_library rule targets net8.0 + depends on 3rd-party package
- 3rd-party package has targets ["net9.0", "netstandard2.0"]

In this case, MSBuild will fall back to netstandard2.0. This change
replicates that behavior.
dotnet.targeting_packs.bzl contains a nuget_repo declaration for the
different targeting packs available on different .NET versions. However,
when generating the repo, the latter versions would silently override
the former versions' targeting_pack_overrides and framework_list, and
thus older versions like microsoft.aspnetcore.app.ref.v8.0.11 would have
targeting_pack_overrides containing version 9.0.0 and such.

This change ensures that different packages within the same repo do not
clobber each other's parameters.
@purkhusid
Copy link
Collaborator

Do you have an example of what you are exactly fixing with this?

@sin-ack
Copy link
Contributor Author

sin-ack commented Mar 7, 2025

For the first commit, an example has been given in the commit message: Our projects are targeting net8.0 at the moment, but many packages have upgraded to only support net9.0, and those packages need to fall back to netstandard2.0. Otherwise we'd have to manually pin everything in paket.dependencies because many packages in the Microsoft namespace have dependency versions >= 8.0 which automatically pull in 9.0 versions. This works the same way in MSBuild.

For the second commit, we have target_frameworks = ["net8.0"] for our csharp_library targets, but 9.0 versions of targeting pack packages would be pulled in because the targeting pack version 8.0 was being clobbered by version 9.0 in nuget_repo.

If you'd like I can try to prepare an example project (and hopefully not disappear this time 😅).

@purkhusid
Copy link
Collaborator

If we could add a test that guards us from a regression then that would be awesome.

@sin-ack
Copy link
Contributor Author

sin-ack commented Mar 7, 2025

I'll try to do that later today.

@sin-ack
Copy link
Contributor Author

sin-ack commented Mar 12, 2025

Sorry for the delay, here are the tests.

@sin-ack sin-ack force-pushed the targeting-fixes branch 2 times, most recently from cca6b49 to d949aa1 Compare March 12, 2025 13:34
@purkhusid
Copy link
Collaborator

Nice! Thanks a lot for your contribution!

@purkhusid purkhusid merged commit 54cf66c into bazel-contrib:master Mar 13, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants