search

dotnet / linker

388 stars 127 forks source link

Change analyzer versions such that the repo can be built with .NET 7 RC2 SDK #3077

Closed vitek-karas closed 1 year ago

vitek-karas commented 1 year ago

Took the versions from dotnet/runtime.

tlakollo commented 1 year ago

I was debugging some of the failures too for my runtime PR, after updating to MicrosoftCodeAnalysisVersion 4.5.* the roslyn analyzers will start generating duplicate warnings on attribute scenarios seems like the compilation now takes into account more stuff and creates callbacks for object creations that it didn't before. I though it was just going to be a change like removing the CheckAttributeInstantiation method but seems like it's more conversome than that

tlakollo commented 1 year ago

Never mind, removing CheckAttributeInstantiation is the solution. I saw some tests failing after removing the method but it was because before CheckAttributeInstantiation was producing double warnings. With the current analysis, only one warning is produced (which is the right outcome).

sbomer commented 1 year ago

@tlakollo would you mind porting the fixes from your branch to this PR?

tlakollo commented 1 year ago

@tlakollo would you mind porting the fixes from your branch to this PR?

Sure, working on it

vitek-karas commented 1 year ago

One observation on a machine with RC2 installed: Clean clone of the repo

Once the repo is built with build.cmd (which will use RC1), trying to build with RC2 may fail (and in VS it does). If the repo is built with RC2 dotnet build illink.sln then building with build.cmd may work (it did for me), but I would suspect things will be weird eventually.

So unfortunately this means that if you need to use VS - don't use build.cmd. And make sure you clean the clone (git clean -xdf) before building for the first time.

I'm guessing that this is due to some kind of breaking change between RC1 and RC2.

The problems should go away once we switch the SDK to RC2 in the global.json (but we have to wait for dotnet/runtime to do that first).

vitek-karas commented 1 year ago

lint is even worse - so far I didn't find a way to run on the RC2 bits. It works when the repo is built with RC1.

Does anybody know where we get the dotnet format tool from? I can't find anything in the repo which says what version of that tool to use.

sbomer commented 1 year ago

It's included in the SDK, and I don't think we try to override the dotnet-format version anywhere.

vitek-karas commented 1 year ago

Unfortunately the RC2 dotnet format fails with:

Could not load file or assembly 'Microsoft.CodeAnalysis, Version=4.5.0.0,

So it seems that the only way to run linter is to clean the enlistment and use the lint.cmd which will download RC1. And then clean it again and rebuild to run VS.

vitek-karas commented 1 year ago

I don't get it - RC1 ships format with Microsoft.CodeAnalysis 4.3.0.0 - and it works RC2 ships format with Microsoft.CodeAnalysis 4.4.0.0 - and it doesn't work I give up - let's merge this - so that at least there's some way to use VS.

tlakollo commented 1 year ago

Notice that runtime has this same behavior, you cannot run dotnet format at all. I've been changing the versions.props file to be able to run the formater for the linker to runtime move PR. In the runtime repo it fails with "Could not load file or assembly 'Microsoft.CodeAnalysis.Workspaces, Version=4.4.0.0" which Im guessing is part of the Microsoft.CodeAnalysis