-
-
Notifications
You must be signed in to change notification settings - Fork 226
Bump .NET SDK to 9.0.300 #4259
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
Bump .NET SDK to 9.0.300 #4259
Conversation
|
I can't get this to work locally. Running It does seem like the issues are mostly related to Maui and Android. I'm not familiar with Maui, so I don't really know what the best approach might be. |
Exactly. Without that we're only pinning the core SDK but all of workloads still roll forward to latests. The workloads are the ones that usually break things - the iOS/Android workloads in particular. Often these require corresponding dependencies to be bumped (e.g. the Xcode version or a new Android SDK version). Sometimes there's other weird stuff going on. I'll try to find some time to take a look. MAUI is a bit fiddly (understatement of the year). |
|
@KnapSac is this what you were seeing when trying to restore the workloads locally?
If so, then I ran into the same issue. I didn't have any issues when bumping to |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4259 +/- ##
==========================================
- Coverage 75.73% 72.81% -2.92%
==========================================
Files 357 458 +101
Lines 13466 16644 +3178
Branches 2671 3318 +647
==========================================
+ Hits 10198 12120 +1922
- Misses 2593 3675 +1082
- Partials 675 849 +174 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Yup, that was at least one of the errors I was getting. Turns out that some process was messing up the Going to |
|
The CI failure doesn't repro locally, but that's probably because I'm on Windows. It might be fixed by bumping the Verify version, but I'm not sure. |
I also can't reproduce it (running on a Mac), which is super annoying. It means that version of Verify can potentially work on macOS - it must be some other difference between the CI runner and my local dev environment 😞 I'll try bumping the version of Verify anyway, but not expecting miracles. @SimonCropp just in case, have you seen any issues with Argon in version 9.0.300 of the .NET SDK? Since bumping in this PR, we're seeing a whole bunch of these in our Verify tests (seemingly only for net4.8 tests running on Mono in CI): Edit: After upgrading to the latest version of Verify we get a different error, which probably reflects the true cause of the error? |
|
Aha... OK I can reproduce this on my local development machine. I had to:
|
| "sdk": { | ||
| "version": "9.0.203", | ||
| "workloadVersion": "9.0.203", | ||
| "version": "9.0.300", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
questions
- Will we run into the same issue again once the next patch version of feature band
3is released? - If so, is this an indicator that we should we keep bumping to the latest .NET SDK in the future?
- Would this be automatable?
- Perhaps partially (
global.json,.github/**/*.ymlandCHANGELOG.md) Xamarin.AndroidX.*may be not as straightforward
- Perhaps partially (
- Would this be automatable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will we run into the same issue again once the next patch version of feature band 3 is released?
We frequently run into issues when new versions of .NET are released. They are, thus far, each unique... some easier than others to fix. This is why we pin the version of .NET and the workloads in the repo (it means we can still make PRs and do other work while we're trying to work out how to upgrade to the latest set of "surprises").
Would this be automatable?
We definitely don't want to turn on roll forward (automatic updates) in the global.json file. Before we were pinning the workload/sdk version in global.json, I was spending 1-2 days per week, consistently, fluffing around with this stuff.
Potentially we could try to automatically create a PR that bumps to the latest version of .NET, like we do with other dependencies. That would give us a way to upgrade regularly but without breaking the main branch and halting all other work. It could be a massive time sink though. It's hard to say whether we're better off making smaller bumps once every 6-8 weeks or having long periods of focus (where we're working on the SDK instead) and then taking all the pain in one hit occasionally (like now).
I'm inclined to stick to what we're doing for the time being - there's just so much other stuff on the backlog without adding what seems to be very meta work around automating something like this.
|
@jamescrosswell nope not aware of any issue with Argon in version 9.0.300 of the .NET SDK. that is what the majority of my repos are running on. if u can supply a repro, i can take a look at it |
Thanks @SimonCropp . After bumping to the latests version of Verify, it doesn't look like the problem stems from Verify or Argon directly. Some of the reflection in Argon doesn't appear to be working on Mono but we have other tests that aren't using Verify which are failing as well. It looks more like the latest version of .NET compiles IL that the current version of Mono isn't handling properly. This test, for example (which isn't using Verify) is failing: sentry-dotnet/test/Sentry.Tests/HttpStatusCodeRangeTests.cs Lines 51 to 64 in 7513600
... with the error: Which I assume indicates mono is falling over on the implicit operator for that class: sentry-dotnet/src/Sentry/HttpStatusCodeRange.cs Lines 48 to 52 in 7513600
I'll see if I can put together a minimal repro with that example and submit a bug report in the mono project. |
|
Replaced by #4272 |
See #4255.