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

Update MicrosoftBuildVersion to latest #98006

Merged

Conversation

LoopedBard3
Copy link
Member

Update MicrosoftBuildVersion to latest to fix a component governance alert.

@ghost
Copy link

ghost commented Feb 5, 2024

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

Issue Details

Update MicrosoftBuildVersion to latest to fix a component governance alert.

Author: LoopedBard3
Assignees: LoopedBard3
Labels:

area-Infrastructure

Milestone: -

@ViktorHofer
Copy link
Member

ViktorHofer commented Feb 6, 2024

ViktorHofer added a commit to dotnet/source-build-reference-packages that referenced this pull request Feb 7, 2024
* Add MSBuild 17.8.3 + dependencies

... necessary for dotnet/runtime#98006

* Add manual pragma warning disable as in dotnet/runtime
@ViktorHofer
Copy link
Member

@ericstj @rainersigwald MSBuild exposes the internal type Microsoft.Build.Framework.IMetadataContainer in the Microsoft.Build.Framework reference assembly (from the NuGet package) but SBRP doesn't expose that because it's internal.

Why is internal type required? Its omission causes the build failures in this PR.

@ericstj
Copy link
Member

ericstj commented Feb 7, 2024

Looks like this introduced the interface: dotnet/msbuild@f69319c

I suspect we're mixing assemblies here. Looks like we have TaskItem that claims it implements internal interface IMetadataContainer, but then we have a reference package for Microsoft.Build.Framework without IMetadataContainer. Looks like the compiler needs to resolve (all) interfaces whenever you cast from contrete type to interface.

Can we fix this by making sure we reference all live or all SBRP packages and not a mix? IIUC The interface lives in Microsoft.Build.Framework and the consumer is in Microsoft.Build.Utilities.

Update: we don't seem to be mixing assemblies. I see the following in the binlog.
image
I'm at a loss for how Microsoft.Build.Utilities.Core got a reference to IMetadataContainer if they were all built from SBRP.

@ericstj
Copy link
Member

ericstj commented Feb 7, 2024

Oh, maybe it's because we don't actually have a 17.8.3 in SBRP so it pulled the real one. That explains it. https://github.com/dotnet/source-build-reference-packages/tree/main/src/referencePackages/src/microsoft.build.utilities.core

Looks like we have a consistent set for 17.4.0, but not 17.8.3

image

@ViktorHofer
Copy link
Member

ViktorHofer commented Feb 7, 2024

Hmm I wonder why the source-build flag doesn't report Microsoft.Build.Utilties.Core and Microsoft.Build.Tasks.Core 17.8.3 as new prebuilts. cc @dotnet/source-build-internal

Maybe because the build failed? I wonder if the prebuilt analysis should always run. It only needs project.assets.json so only the restore needs to succeed, right?

@MichaelSimons
Copy link
Member

Maybe because the build failed? I wonder if the prebuilt analysis should always run. It only needs project.assets.json so only the restore needs to succeed, right?

Yes - prebuilt detection only runs upon a successful build.

@ViktorHofer
Copy link
Member

Filed dotnet/source-build#4103 for prebuilt detection not running for failing builds.

ViktorHofer added a commit to dotnet/source-build-reference-packages that referenced this pull request Feb 8, 2024
MichaelSimons pushed a commit to dotnet/source-build-reference-packages that referenced this pull request Feb 8, 2024
…8.3 (#886)

* Add Microsoft.Build.Tasks.Core and Microsoft.Build.Utilities.Core 17.8.3

Required for dotnet/runtime#98006
Follow-up on #885

* Update unrelated files

* Manual change because of attribute filtering by GenAPI

* Remove dependency from nuspec as well
@ViktorHofer
Copy link
Member

Needs #98178

@ViktorHofer ViktorHofer merged commit 0976796 into dotnet:main Feb 8, 2024
181 of 188 checks passed
@LoopedBard3 LoopedBard3 deleted the UpdateSystemSecurityCryptographyXml branch February 8, 2024 23:49
@steveisok
Copy link
Member

@LoopedBard3 @ViktorHofer looks like we need to backport this to release/8.0, 7.0, and 6.0.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 22, 2024
@ericstj
Copy link
Member

ericstj commented Apr 3, 2024

/backport to release/8.0

Copy link
Contributor

github-actions bot commented Apr 3, 2024

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/8542745559

@github-actions github-actions bot unlocked this conversation Apr 3, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 3, 2024
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.

5 participants