Add analyzer bootstrap phase #2540
Conversation
The existing integration test build will now also use the live analyzer built first.
| <Import Project="$(BootstrapBuildPath)/Microsoft.NET.ILLink.Analyzers.props" Condition="'$(BootstrapBuildPath)' != ''" /> | ||
|
|
||
| <!-- Don't enable for Cecil, as they can't be suppressed --> | ||
| <PropertyGroup Condition="'$(BootstrapBuildPath)' != '' and '$(MSBuildProjectName)' != 'Mono.Cecil.Pdb'"> |
There was a problem hiding this comment.
Why just Mono.Cecil.Pdb and not the other cecil projects (mainly the Mono.Cecil itself)?
There was a problem hiding this comment.
Mono.Cecil.PDB has a warning (COM) that I need to file a bug for.
We also can't disable it because it sets <NoWarn> without using $(NoWarn) inside, so it overrides the higher warnings. None of the other projects do, so we can temporarily disable warnings for them.
There was a problem hiding this comment.
We could push a change to the project to correctly pick up NoWarn from the outside.
Once we have XML support it should be possible to get rid of the warning that way.
|
I'm not sure how much value this adds. Wouldn't be better to write integration tests with e.g. SPC where different code patterns actually exist? |
|
@marek-safar /runtime is definitely a good test suite, but our changes take longer to flow there. Since this already found a crash, I think it's worth bringing in. |
The existing integration test build will now also use the live analyzer built first. Commit migrated from dotnet/linker@5fca8b8
Should run by default in CI and catch analyzer crashes. Adding this already found one -- the fix is in the second commit.