Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add code for linker to generate xml warning suppressions when builds fail #44264

Closed

Conversation

layomia
Copy link
Contributor

@layomia layomia commented Nov 4, 2020

Makes it a bit easier to suppress linker warnings in PRs such as https://github.com/dotnet/runtime/pull/43965/checks?check_run_id=1355002347. With this change the following steps can be taken:

  1. Build locally using the command failing in CI, adding /p:GenerateLinkerWarningSuppressions=true,
    • e.g. build.cmd -subset libs -configuration Debug -ci -allConfigurations /p:GenerateLinkerWarningSuppressions=true from this failure log.
  2. Find suppression XML file(s) generated to the local/config-dependent equivalent of this location: runtime\artifacts\bin\ILLinkTrimAssembly\net6.0-windows-Release-x64\trimmed-runtimepack
  3. Move the contents of the generated XML files to the corresponding XML linker suppression file location for the erring assembly (or create one if necessary). e.g, suppression data for warnings in System.Net.Security would go to a file named runtime\src\libraries\System.Net.Security\src\ILLink\ILLink.Suppressions.xml.

TODO:

  • Document these steps somewhere appropriate
  • Investigate a way to automatically make such changes (e.g. an MSBuild target). We should be sure not to lower the bar for adding new linker-unfriendly code as a result.

@layomia layomia added this to the 6.0.0 milestone Nov 4, 2020
@layomia layomia self-assigned this Nov 4, 2020
@ghost
Copy link

ghost commented Nov 4, 2020

Tagging subscribers to this area: @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

@layomia
Copy link
Contributor Author

layomia commented Nov 4, 2020

FWIW - At the moment, I'm unsure why #43965 causes new linker warnings but the steps above should help resolve them.

@ericstj
Copy link
Member

ericstj commented Jan 22, 2021

@layomia @eerhardt is this still needed?

@layomia
Copy link
Contributor Author

layomia commented Jan 29, 2021

@ericstj - yes, it just needs some doc additions. Will circle back to this.

Base automatically changed from master to main March 1, 2021 09:07
@ericstj
Copy link
Member

ericstj commented Mar 8, 2021

I'm going to go ahead and close this one-line PR. Please feel free to open a new PR when you're ready to resume this work if it's still necessary.

@ericstj ericstj closed this Mar 8, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants