FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[release/9.0-staging] Fix inadvertently upgrading compiler warnings to errors by akoeplinger · Pull Request #114331 · dotnet/runtime · GitHub

Repository navigation

[release/9.0-staging] Fix inadvertently upgrading compiler warnings to errors - #114331

Merged
akoeplinger merged 1 commit into
dotnet:release/9.0-stagingfrom
akoeplinger:9.0-compiler-warning
Apr 8, 2025
Merged

akoeplinger merged 1 commit into
dotnet:release/9.0-stagingfrom
akoeplinger:9.0-compiler-warning

Conversation

Copy link
Copy Markdown
Member

Backport of #114323 to release/9.0-staging.

Don't inadvertently upgrade compiler warnings to errors in libs.native.

Customer Impact

  • Customer reported
  • Found internally

Building with a newer compilers can introduce new warnings, breaking the build unintentionally even though we explicitly tried to avoid this by disabling this behavior in release branches. This could especially affect source-build partners which don't use our build infrastructure.

Regression

  • Yes
  • No

No, we just forgot to apply this fix to the libs.native subset. It was there for coreclr and mono runtime builds already.

Testing

CI testing.

Risk

Low, this is a build-only change.

I noticed this while looking at an issue in the 9.0 branch with latest clang which runs into a couple warnings fixed in main with dotnet#108888.

We intentionally don't set `-Werror` in release branches to avoid breaking the build when newer compilers add warnings via https://github.com/dotnet/runtime/blob/37e4d45236e68946db9d264593aa31a9c00534bc/eng/native/configurecompiler.cmake#L564-L567

However this didn't work because msbuild's WarnAsError was still reading the console output of the native build and upgrading any line with "warning: " to an msbuild error and breaking the build.

Suppress this by setting IgnoreStandardErrorWarningFormat="true" on the Exec call. We already did this in the coreclr and mono runtime builds, we just missed it in libs.native.
Copilot AI review requested due to automatic review settings April 7, 2025 14:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)
  • src/native/libs/build-native.proj: Language not supported

Copy link
Copy Markdown
Member Author

Applying servicing-approved, infrastructure-only change required to build with newer compilers, tell mode.

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

akoeplinger added the Servicing-approved Approved for servicing release label Apr 7, 2025

This comment was marked as outdated.

This comment was marked as outdated.

akoeplinger changed the title [release/9.0-staging] Fix inadvertently upgrading compiler warnings to errors (#114323) [release/9.0-staging] Fix inadvertently upgrading compiler warnings to errors Apr 7, 2025

This comment was marked as outdated.

Copy link
Copy Markdown
Member Author

/ba-g wasm failures are due to using a not-yet-published .net version, unrelated

akoeplinger merged commit 5407686 into dotnet:release/9.0-staging Apr 8, 2025
akoeplinger deleted the 9.0-compiler-warning branch April 8, 2025 07:06
github-actions Bot locked and limited conversation to collaborators May 8, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL