Updated NuGet packages #801
Conversation
| @@ -11,7 +11,11 @@ | |||
| using Xunit; | |||
| using Xunit.Abstractions; | |||
| using static StructuredLogger.Tests.TestUtilities; | |||
| using ArchiveFileEventArgs = Microsoft.Build.Logging.StructuredLogger.ArchiveFileEventArgs; | |||
There was a problem hiding this comment.
Without it compiler gets lost because ArchiveFileEventArgs is present in two different namespaces
| @@ -7,6 +8,12 @@ namespace StructuredLogger.Tests | |||
| { | |||
| public class ConditionParserTests | |||
| { | |||
| public ConditionParserTests() | |||
| { | |||
| CultureInfo.DefaultThreadCurrentCulture = new("en-US"); | |||
There was a problem hiding this comment.
Could you explain what are these for?
There was a problem hiding this comment.
Some tests fail When unit tests run on a computer with a different culture than English - which involves different decimal separator.
| EndProject | ||
| Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "StructuredLogger.Utils", "src\StructuredLogger.Utils\StructuredLogger.Utils.csproj", "{AC634B46-D57C-44C5-BF56-480843182F21}" | ||
| EndProject | ||
| Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "StructuredLogViewer", "src\StructuredLogViewer\StructuredLogViewer.csproj", "{C2E67DD4-F4F7-4CDE-A51A-6D46049A0CCA}" |
There was a problem hiding this comment.
StructuredLogViewer is intentionally the first project such that it gets selected as startup by default when you first open the solution without a .vs folder.
There was a problem hiding this comment.
Sure, I'll fix that
There was a problem hiding this comment.
Don't worry about it, I can fix later myself
|
Appreciate the PR! Curious what motivated it. |
|
I appreciate your work and have used MSBuildStructuredLog almost from the beginning. It has been very helpful in my day-to-day tasks. In the meantime, I've also tried to resolve some issues I've encountered by upgrading Avalonia and related packages, but couldn't finish it. And now you've done it yourself. So, I've just merged the rest of my changes. There are however some breaking changes in newer versions of MSBuild related packages - so I didn't upgrade them. |
|
Updating NuGet packages is never as easy as just bumping versions. We also need to check binding redirects and whether we ship all the necessary dlls with the app. So far I had to make the following changes: The most disastrous update was NuGet.ProjectModel. I'm not blaming you because you couldn't have known this, but I had some fears about this and decided to just yolo this. Now I'm afraid whoever updated to 2.2.317 will be stuck as the auto-update to 2.2.320 doesn't seem to be working (likely because Squirrel requires Cecil 0.9.6.1 but we updated to 0.11.5.0). I do have a tool https://www.nuget.org/packages/checkbinarycompat that you can run on the directory where StructuredLogViewer.exe is located, but it's not integrated into the build so we didn't notice the breaks. I'm not too happy that I have to spend my Sunday trying to feverishly fix all of this stuff, but again, not your fault. Just making you aware of the unintended consequences and the invisible tax that project maintainers have to pay after random drive-by PRs. |
|
I added a comment to never ever bump Cecil again. |
|
Wow, I wasted my whole Sunday on this. |
|
I added checkbinarycompat to the CI here: I also added a first-chance exception indicator which will light up on first-chance exceptions. Hopefully next time there's an exception we'll see it better. |
|
Indeed, this PR fails as expected: https://ci.appveyor.com/project/KirillOsenkov/msbuildstructuredlog/builds/50516603
OK at least we did some safeguarding so that this kind of stuff doesn't happen next time. |
|
Next time we'll also see an indicator like this: |
|
And you can click the indicator to show LoggerExceptions.txt which has the full call stack. |
|
I'm very sorry that ruined your Sunday. Didn't intend to do so. Great, that you've added fail-safe now. |
|
No problem, it's not your fault, you wanted to help. It's more of my fault, I tried updating Cecil before many years ago, broke the updater the same way, got people stuck on a broken version, rolled back, and didn't do anything to prevent it in the future, not even a comment. And when I reviewed your PR I should have remembered, but I forgot. This time we added redundant safety checks so at least it was good in a way, and will help us catch other stuff in the future. Long term I'll find time to update to the latest Squirrel but this is an insane amount of work and testing so we'll keep limping along on a super old version that works fine if you don't touch it. |
I've updated all NuGet packages that weren't making breaking changes