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

Builds not reporting source-build prebuilts as errors #69846

Closed
mthalman opened this issue Sep 7, 2023 · 7 comments
Closed

Builds not reporting source-build prebuilts as errors #69846

mthalman opened this issue Sep 7, 2023 · 7 comments
Assignees

Comments

@mthalman
Copy link
Member

mthalman commented Sep 7, 2023

The Roslyn CI pipeline includes a Source-Build (Managed) build leg which builds the repo in the context of source-build. When source-build prebuilts are detected, they are reported in that build leg. However, they are being reported as a warning and not an error. This seems to be unique to the roslyn repo since other repos are correctly reporting prebuilts as errors. It's not clear why this is happening. This is a problem since it lets changes get merged which cause prebuilts to be introduced. This will then fail later on in dependency flow (e.g. dotnet/installer#17319) which does detect the prebuilts and reports them as an error.

cc @MichaelSimons @mmitche

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 7, 2023
@genlu genlu removed the untriaged Issues and PRs which have not yet been triaged by a lead label Sep 8, 2023
@genlu
Copy link
Member

genlu commented Sep 8, 2023

@333fred I think you are the expert here? :)

@mthalman
Copy link
Member Author

mthalman commented Sep 8, 2023

Here are some binlogs to compare between roslyn, which doesn't reported prebuilts as errors, and sdk, which does: binlogs.zip. Search for the ValidateUsageAgainstBaseline task in the binlog viewer. I don't see any difference in the parameters being passed. I see that TreatWarningsAsErrors is set to true.

The weird thing is that FailOnPrebuiltBaselineError is always set to false. So I don't know how this actually works in all the other repos.

@mmitche
Copy link
Member

mmitche commented Sep 11, 2023

What appears to be happening is that TreatWarningsAsErrors is set to false for the SB leg. This is the default in the roslyn repo https://github.com/dotnet/roslyn/blob/main/eng/build.sh#L76 unless --warnaserror is explicitly passed.

The other issue, as @mthalman has found, is that the baseline comparison task is not automatically failing on a failed baseline comparison. We were essentially relying on the typical repo setting of treating warnings as errors to get an error out.

I think the right thing to do here is to change the arcade infra to default to error on a baseline comparison failure, rather than changing roslyn's behavior. Roslyn only enables warnaserror for its code check leg, and primarily we just want to be as close to the default repo behavior for the source-build leg.

@mmitche
Copy link
Member

mmitche commented Sep 11, 2023

dotnet/arcade#14022

@333fred
Copy link
Member

333fred commented Jun 28, 2024

@mthalman @mmitche is there anything for Roslyn to do here? It seems like the linked issue is closed?

@mmitche
Copy link
Member

mmitche commented Jun 28, 2024

Nope, I think this can be closed.

@333fred
Copy link
Member

333fred commented Jun 28, 2024

Thanks!

@333fred 333fred closed this as not planned Won't fix, can't repro, duplicate, stale Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants