phmonte / Buildalyzer

A utility to perform design-time builds of .NET projects without having to think too hard about it.
MIT License
589 stars 92 forks source link

Consider updating Roslyn dependencies #201

Closed brettfo closed 2 years ago

brettfo commented 2 years ago

In commit a3312a95f17a21c9575a077e61706b82b59be6f8 on Feb 16 support for net6.0 was added and the Roslyn packages were updated to version 4.0.1, but a few hours later in commit bbbd23a3a11c24702f7f3e5a751bd12224ce2e9d that support was removed, possibly due to a failed merge.

daveaglick commented 2 years ago

Nope, that was me :). See the discussion in https://github.com/daveaglick/Buildalyzer/pull/197. With some clever use of the StructuredLogger package to polyfill an older MSBuild package version, I was able to leave Buildalyzer at a .NET Core 3.1 target. That doesn't "remove" .NET 6 support or prevent use on .NET 6 (at least that I know!) - it should work just fine on that platform. It just continues to allow older consumers to continue using it too.

daveaglick commented 2 years ago

Let me know if you see any problems with this approach by the way. The adding/removing you noticed was while I was trying to work it all out. I can go back to a .NET 6 target if I really need to, but would prefer not to if it's not required so that the largest number of consumers can continue to use it.

brettfo commented 2 years ago

The explicit net6.0 support isn't too worrysome, but the Roslyn downgrade from 4.0.1 to 3.11.0 is.

daveaglick commented 2 years ago

Ah, gotcha. So AFAIK Roslyn 4.x still targets .NET Core 3.1/.NET Standard 2.0, right? Roslyn is only used in the Buildalyzer.Workspaces package, and upsetting Roslyn won't break the .NET Core 3.1 target anyway, so that shouldn't be a problem to update Roslyn without updating the target.

So now I'm curious what's not working under Roslyn 3.x? Is it just that the Workspace you get back isn't compatible with a more recent Roslyn version in the calling app?

brettfo commented 2 years ago

The Roslyn completion APIs aren't perfectly compatible, e.g., a Buildalyzer workspace (backed by Roslyn 3.11.0) when passed to the 4.0.1 version of this method results in a InvalidOperationException, but if the Buildalyzer (3.11.0) workspace is passed to the Roslyn 3.11.0 version of that function (actually, the 3.11.0 version has the suffix Async) then everything works, but my use case requires Roslyn 4.0.1.

daveaglick commented 2 years ago

Gotcha, so some compat issues across the major version line there. Not a problem - since the targeting still lines up, I don't see why we couldn't bump the Roslyn versions to latest. I'll play around with that tomorrow and ship it quick as long as it doesn't break anything else.

daveaglick commented 2 years ago

New release with updated Roslyn dependencies for Buildalyzer.Buildalyzer.Workspaces is out. I'll close this issue for now, but please let me know if the new release does/doesn't work for you.