Skip to content

Conversation

@ericstj
Copy link
Member

@ericstj ericstj commented Oct 30, 2025

Fixes #6986

I discovered that all builds had disabled warning as error except the source-index step, which went through the arcade script instead of the repo script.

A warning was introduced in 5ca1721 that caused official build to fail due to source-index job.

I've left warning as error disabled for the build scripts (local developer scenario) but I've made all the AzDo pipelines enable warnAsError.

These warnings are important, the one here would mean actual runtime problems if it were in a product assembly. microsoft/semantic-kernel#13316 I'm not sure why the test didn't hit it, maybe it happens to not call any API that use async interfaces on netfx.

Microsoft Reviewers: Open in CodeFlow

@ericstj ericstj requested a review from a team as a code owner October 30, 2025 18:35
Copilot AI review requested due to automatic review settings October 30, 2025 18:35
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.

Pull Request Overview

This PR makes build configuration improvements and adds a workaround for .NET Framework builds:

  • Removes the warnAsError: 0 override from CI pipelines to enforce warnings as errors consistently
  • Adds a custom source indexing build command for main branch builds
  • Includes a workaround for Microsoft.Bcl.AsyncInterfaces dependency issue on .NET Framework

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
test/Libraries/Microsoft.Extensions.DataIngestion.Tests/Microsoft.Extensions.DataIngestion.Tests.csproj Adds conditional package reference for Microsoft.Bcl.AsyncInterfaces to work around Semantic Kernel issue #13316 on .NET Framework
azure-pipelines.yml Removes warnAsError override and adds sourceIndexBuildCommand for source indexing on main branch
azure-pipelines-unofficial.yml Removes warnAsError override to enforce warnings as errors
azure-pipelines-public.yml Removes warnAsError override to enforce warnings as errors

@ericstj
Copy link
Member Author

ericstj commented Oct 30, 2025

This is working
image

However I noticed a few other places where warnAsError was not passed and have fixed those.

@ericstj
Copy link
Member Author

ericstj commented Oct 30, 2025

@ericstj
Copy link
Member Author

ericstj commented Oct 30, 2025

@joperezr let me know that this is supposed to be handled by Correctness WarningsCheck job, however that job is building on ubunutu, so it doesn't include the .NETFramework targets for tests --

<TargetFrameworks Condition=" '$(IsWindowsBuild)' == 'true' ">$(TestNetCoreTargetFrameworks)$(ConditionalNet462)</TargetFrameworks>

This allows folks to get test results without being blocked by warnings.

Make sure the job testing for warnings runs on windows to ensure it builds a superset of targets.
@ericstj ericstj enabled auto-merge (squash) October 30, 2025 22:14
@ericstj ericstj merged commit 2670097 into dotnet:main Oct 30, 2025
6 checks passed
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

Big thanks for solving the problem and providing all the explanation @ericstj !

And apologies for introducing the problem.

This was referenced Nov 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extensions build is using warnAsError false

3 participants