Skip to content

Conversation

@akoeplinger
Copy link
Member

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 #108888.

We intentionally don't set -Werror in release branches to avoid breaking the build when newer compilers add warnings via

# Suppress warnings-as-errors in release branches to reduce servicing churn
if (PRERELEASE)
add_compile_options(-Werror)
endif(PRERELEASE)

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 build-native.proj in libs.native.

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 09:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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

Files not reviewed (3)
  • src/coreclr/runtime.proj: Language not supported
  • src/native/corehost/corehost.proj: Language not supported
  • src/native/libs/build-native.proj: Language not supported

@dotnet-policy-service
Copy link
Contributor

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

@akoeplinger akoeplinger changed the title Fix inadvertently upgrading compiler warnings to errors Fix inadvertently upgrading compiler warnings to errors in libs.native Apr 7, 2025
@am11
Copy link
Member

am11 commented Apr 7, 2025

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.

Since this suppression never worked and we are managing just fine without it, we can delete that instead? The compiler toolchain upgrades should be deterministic with new plan.

@akoeplinger
Copy link
Member Author

@am11 well I hit this when my Xcode upgraded to the latest version and I tried to compile the 9.0 branch locally :)

it's also important for source-build partners who don't use our CI setup, we've just been lucky they haven't hit a new warning in the libs.native code so far (the recent ones were Apple-specific)

@am11
Copy link
Member

am11 commented Apr 7, 2025

@am11 well I hit this when my Xcode upgraded to the latest version and I tried to compile the 9.0 branch locally :)

When build break is discovered, we have to fix them (either in code or by suppressing new warning) as several "fix build with new-msvc/clang/gcc" type of PRs merged previously. They are then backported to last release branch. Official builds aren't usually the first ones which discover these issues, so hiding them on official builds alone isn't saving much.

@akoeplinger
Copy link
Member Author

akoeplinger commented Apr 7, 2025

When build break is discovered [...]

The point is to not have warnings break the build in release branches, or needing to backport (harmless) warning fixes.

@akoeplinger akoeplinger merged commit 6331d44 into dotnet:main Apr 7, 2025
158 of 164 checks passed
@akoeplinger akoeplinger deleted the fix-warning-prerelease branch April 7, 2025 14:12
akoeplinger added a commit to akoeplinger/runtime that referenced this pull request Apr 7, 2025
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.
akoeplinger added a commit that referenced this pull request Apr 8, 2025
…14331)

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 #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.
@github-actions github-actions bot locked and limited conversation to collaborators May 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants