Skip to content

Add analyzer bootstrap phase - #2540

Merged
agocke merged 4 commits into
dotnet:mainfrom
agocke:analyzer-bootstrap
Jan 25, 2022
Merged

Add analyzer bootstrap phase#2540
agocke merged 4 commits into
dotnet:mainfrom
agocke:analyzer-bootstrap

Conversation

@agocke

@agocke agocke commented Jan 23, 2022

Copy link
Copy Markdown
Member

Should run by default in CI and catch analyzer crashes. Adding this already found one -- the fix is in the second commit.

The existing integration test build will now also use the live analyzer
built first.
Comment thread build.sh Outdated
Comment thread eng/Analyzers.props
<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'">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why just Mono.Cecil.Pdb and not the other cecil projects (mainly the Mono.Cecil itself)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread eng/common/build.ps1 Outdated
@marek-safar

Copy link
Copy Markdown
Contributor

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?

@agocke

agocke commented Jan 25, 2022

Copy link
Copy Markdown
Member Author

@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.

@agocke
agocke merged commit 5fca8b8 into dotnet:main Jan 25, 2022
@agocke
agocke deleted the analyzer-bootstrap branch January 25, 2022 16:59
agocke added a commit to dotnet/runtime that referenced this pull request Nov 16, 2022
The existing integration test build will now also use the live analyzer built first.

Commit migrated from dotnet/linker@5fca8b8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants