upgrading project and msbuild deps

[colombod]

Addressing issue #139

[daveaglick]

It looks like binlog version 9 was introduced in a commit on January 14:
dotnet/msbuild@6426e79#diff-44a058faa22f024c71a08263069158bcR39

That makes me wonder if 15.8.166 is even new enough to read version 9 logs. There's a balance here - the further forward we move the MSBuild dependency, the more recent our target has to be. The 16.x releases of MSBuild don't target .NET Standard anymore either so if we have to go to them then it's into multi-targeting which I'd really like to avoid. Need to do a little more research on binlog version compat.

[daveaglick]

I was having trouble reproducing any errors at all with binlog reading so I started manually peeping the version flag on binlogs created from the CLI. It turns out I'm only getting version 7 binlogs on .NET Core SDK 3.1.202. I updated to 3.1.300 and sure enough I'm now seeing version 9. So it looks like version 9 is only produced by the latest SDK for .NET Core 3.1.

Reproducing inside a unit test is also tough because the integration tests that read binlog files produce them by triggering binlog production using Buildalyzer, which adds the BinaryLogger from the referenced MSBuild to the post-streamed logging events (as opposed to instrumenting inside MSBuild directly with something like /bl). That guarantees Buildalyzer can always read the binlogs it produces, but might not be able to read other binlogs. I'll need to create a new integration test that generates a binlog from MSBuild and tries to read that in order to verify a fix.

[colombod]

How can I help you on this? That is exactly the repro. How do we make sure that we can keep on taking advantage of Buildalyzer in botnet try? Do you want to have a look over to botnet try integration as well?

[daveaglick]

Not sure, but getting closer. I can now produce a version 9 binlog using the /bl switch inside an integration test. As expected, it fails even with the dependency bump to MSBuild 15.8.166:

I think I've got three options:

• Update to the latest MSBuild at the expense of higher targets and multi-targeting for Buildalyzer (I don't love this option - the higher targets means all consumers will be forced to update, and I try to avoid multi-targeting whenever I can).
• Change the version 9 binlog into a version 7 one at analyze time so that the older MSBuild binlog APIs can read it.
• Decouple binlog reading and see if just the binlog portion can be updated without updating all of MSBuild (maybe the separate binlog reader from MSBuild.StructuredLogger can be used).

I'm leaning towards the last approach and doing some experiments. We'll figure out one way or another - I'll let you know when/if I could use an assist.

[colombod]

I love the 3rd option. I'd also propose to bump Buildalyzer up a major version number, this is quite a strong change and it is important to get it as visible as possible to the users.

Let me know how can I help or if you want to pair on this.

[daveaglick]

Success!

This is kind of a versioning mess. Turns out the third option won't work - I was actually already using the separate MSBuild.StructuredLogger package to read binlogs, but looking at the release history the latest version it'll go to before bumping it's own deps to the 16.x versions of MSBuild is binlog version 8.

I ended up bumping everything up to the latest but then continuing to target netstandard2.0 and turning off warnings (which I treat as errors) for NU1701 ("this package may not be compatible..."). Buildalyzer builds and runs fine since it doesn't use anything past .NET Standard 2.0 APIs internally and as long as the calling application targets .NET Core 2.1 or .NET Framework 4.7.2 or higher everything will match up and I can avoid multi-targeting Buildalyzer. Of course that's a pretty big target bump for consumers so I'll increment the major version and document it.

I merged your changes in manually and then iterated, so I'll close the PR once the release is out. Just need to do a little more verification, but assuming other integration tests all run I'll get a release out and you can take it for a spin. Let me know if it still doesn't work or something else breaks and we'll keep at it.

[colombod]

Wow amazing, let em know as soon as it is ready osi can take it in and test. Thank you so much

[daveaglick]

It looks good - even the OSS projects pass (at least locally):

Publishing 3.0.0 to NuGet right now.

[colombod]

Thank you. I got the package and works! Now onto some more upgrade pain in other areas but then I log issue is gone!

[daveaglick]

Excellent, now I just need to update my own stuff 🤣