sentry-dotnet icon indicating copy to clipboard operation
sentry-dotnet copied to clipboard

.NET 7

Open mattjohnsonpint opened this issue 2 years ago • 10 comments

Should we add a target for .NET 7? When?

At minimum, we should at least make sure we can be used in a .NET 7 project, even if we're only target .NET 6. We should also make sure we can build the solution when the .NET 7 preview SDKs are installed.

mattjohnsonpint avatar May 26 '22 22:05 mattjohnsonpint

.NET 7 plan for GA is Nov 2022. https://github.com/dotnet/core/blob/main/roadmap.md

This will be more of a priority during the release candidate phase.

mattjohnsonpint avatar May 26 '22 22:05 mattjohnsonpint

Tentative plan:

  • When .NET 7 RC1 is released, we will add a net7.0 target to our test projects, and update CI to include it.
  • We will be able to then evaluate if we need a net7.0 target in Sentry or not.
  • Unless it is required, or if it gives us something we need in particular, we would prefer not to have a special target for net7.0 in the libraries. Otherwise we're on the path to someday having even more (net9.0;net8.0;net7.0;net6.0;net5.0, etc.)

mattjohnsonpint avatar Aug 03 '22 23:08 mattjohnsonpint

We should notice considerable perf improvements in our tests. https://devblogs.microsoft.com/dotnet/performance_improvements_in_net_7

The question remains if we'll have to target the libraries specifically for .NET 7 to see these perf gains or not.

mattjohnsonpint avatar Sep 01 '22 20:09 mattjohnsonpint

Unless it is required, or if it gives us something we need in particular, we would prefer not to have a special target for net7.0 in the libraries. Otherwise we're on the path to someday having even more (net9.0;net8.0;net7.0;net6.0;net5.0, etc.)

Agreed. Tried that in the past but always found myself having to add it everywhere. One reason was that:

A -> B -> C

If C has net7.0 and net6.0 but B only has 6.0. A couldn't bring C's net7.0. So if Sentry core gets net7.0, we need to add to all integrations too.

bruno-garcia avatar Sep 01 '22 20:09 bruno-garcia

The question remains if we'll have to target the libraries specifically for .NET 7 to see these perf gains or not.

i think the majority of perf fixed are runtime+library+jit. all of which we get by having the tests run on net7. we would not get any perf tweak that have been done in the compiler

SimonCropp avatar Sep 01 '22 21:09 SimonCropp

FYI, it is not currently possible to fully test because there are no workloads yet for platform-specific targets. In other words, we can't just add net7.0 to our targets list and update global.json, because then it won't find the packages needed to target net6.0-android, etc.

Under .NET 7 RC 1, dotnet workload install maui (or other workloads) fails with:

Advertising manifest not updated. Manifest package for microsoft.net.sdk.maccatalyst doesn't exist.
Advertising manifest not updated. Manifest package for microsoft.net.sdk.android doesn't exist.
Advertising manifest not updated. Manifest package for microsoft.net.sdk.maui doesn't exist.
Advertising manifest not updated. Manifest package for microsoft.net.sdk.tvos doesn't exist.
Advertising manifest not updated. Manifest package for microsoft.net.sdk.ios doesn't exist.
Advertising manifest not updated. Manifest package for microsoft.net.sdk.macos doesn't exist.
Skipping NuGet package signature verification.
Installing pack Microsoft.Maui.Core.Ref.android version 7.0.0-rc.1.6430...
Workload installation failed. Rolling back installed packs...
Rolling back pack Microsoft.Maui.Core.Ref.android installation...
Workload installation failed: microsoft.maui.core.ref.android::7.0.0-rc.1.6430 is not found in NuGet feeds https://api.nuget.org/v3/index.json;https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-eng/nuget/v3/index.json".

☹️

mattjohnsonpint avatar Sep 16 '22 02:09 mattjohnsonpint

However, if I strip away all the platform-specific targets and just built for .net7.0, all our tests pass (with one verify test update). SO at least we know things will work, but we can't actually commit this yet until the platform-specific targets work.

See the dotnet-7 branch for work in progress.

mattjohnsonpint avatar Sep 16 '22 02:09 mattjohnsonpint

Ok, we can install the .NET 7 RC 1 workloads for MAUI using:

sudo dotnet workload install maui \
  --from-rollback-file https://aka.ms/dotnet/maui/7.0.0-rc.1.json \
  --source https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet7/nuget/v3/index.json \
  --source https://aka.ms/dotnet6/nuget/index.json \
  --source https://api.nuget.org/v3/index.json

But we still can't target .NET 7 and any platform-specific .NET 6 TFMs at the same time.

  • <TargetFrameworks>net7.0;net6.0;net7.0-android</TargetFrameworks> works
  • <TargetFrameworks>net7.0;net6.0;net6.0-android</TargetFrameworks> doesn't

We'll have to wait for tooling improvements from Microsoft before proceeding.

mattjohnsonpint avatar Sep 16 '22 17:09 mattjohnsonpint

The above presumes .NET 7 RC1 installed, and a global.json with:

{
  "sdk": {
    "version": "7.0.100-rc.1.22431.12",
    "rollForward": "latestMinor",
    "allowPrerelease": true
  }
}

Or no global.json at all.

mattjohnsonpint avatar Sep 16 '22 17:09 mattjohnsonpint

The problems stated above with the target frameworks appears to have been resolved with the current workloads (6.0.540 / 7.0.0-rc.1.6683)

mattjohnsonpint avatar Sep 27 '22 01:09 mattjohnsonpint

We seem to be doing fine with .NET 7 in our test projects. Thus far there has been no need to create a .NET 7 target for the source projects. The .NET 6 targeted libraries are working just fine in .NET 7 - as they should.

We can consider adding .NET 7 targets where needed if and when we want to take advantage of any .NET 7 specific features.

mattjohnsonpint avatar Dec 16 '22 19:12 mattjohnsonpint

Can we please stop with this "it works for me" kinda stuff. It's clearly broken. image It may just be temporary, but it's been multiple days, and there's no reporting mechanism.

robomac avatar Apr 02 '23 21:04 robomac

@robomac - If you're referring to the global.json mentioned above, that comment was made when .NET 7 was still in RC1 status. It's been quite some time since then. There's no need to use preview releases. Our current global.json we is is here and references the released version of .NET 7.

Also, this issue was specifically about how the Sentry .NET SDKs would be using .NET 7 - not about anything that should affect how you use MAUI or Sentry in your own projects. If you're having issues with your own projects that are specific to Sentry, or if you are wanting to contribute to this repo and having difficulties, please file a new issue and explain in more detail. Thanks.

mattjohnsonpint avatar Apr 03 '23 02:04 mattjohnsonpint

Also , just FYI - it's generally not a good idea to put a global.json directly into your home directory, as it will affect every project from that level and lower. If you have a ~/global.json, I suggest you delete it. They belong only with the specific solution you're working with, usually right next to the .sln file, and committed to source code.

mattjohnsonpint avatar Apr 03 '23 02:04 mattjohnsonpint