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

Match SDK versions of MSBuild framework #3187

Merged
merged 3 commits into from
Jan 26, 2023
Merged

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Jan 20, 2023

The version of Microsoft.Build.Framework that we depend on has a transitive dependency on System.Drawing.Common that is being flagged due to a security vulnerablility in dotnet/sdk#29927. The dependency is probably unused, but we would like to fix it to prevent further false positive reports.

This is being referenced as a transitive dependency of Microsoft.Build.Framework: Microsoft.Build.Framework/17.0.0-preview-21267-01 -> System.Security.Permissions/4.7.0 -> System.Windows.Extensions/4.7.0 -> System.Drawing.Common/4.7.0.

dotnet/sdk uses a lower version (15.4.8) of Microsoft.Build.Framework, which doesn't have a dependency on System.Drawing.Common. dotnet/linker used to reference the same version but we bumped it in #1840 and #2047. @marek-safar do you know why we made the upgrade? My inclination is to change back to 15.4.8, since I don't think we can actually be depending on any API surface newer than what ships with the SDK. If there's a requirement to reference a higher version, please let me know.

@sbomer sbomer requested a review from marek-safar as a code owner January 20, 2023 01:23
@marek-safar
Copy link
Contributor

I think it was to sync with dotnet/runtime https://github.com/dotnet/runtime/blob/43b8dcf48e26019ce2aca5ae2f249001693b42c2/eng/Versions.props#L175

If this dependency versioning should probably be moved to arcade though to avoid further problems in source build.

@sbomer
Copy link
Member Author

sbomer commented Jan 20, 2023

Got it, thanks! Another approach would be to upgrade to the version used by dotnet/runtime, which was also recently bumped to fix component governance warnings: dotnet/runtime#77678.

However, my preference is to match the SDK version. I think using a later version creates the possibility for us to depend on APIs that don't exist at runtime. I think the SDK version should be OK from a component governance perspective, since they presumably aren't hitting warnings either - and if they do, we can upgrade to match when they do.

@ViktorHofer
Copy link
Member

ViktorHofer commented Jan 23, 2023

Got it, thanks! Another approach would be to upgrade to the version used by dotnet/runtime, which was also recently bumped to fix component governance warnings: dotnet/runtime#77678.

Yes that's the preferred path. FWIW, we do have an open PR that removes the System.Drawing.Common dependency: dotnet/runtime#80347 (comment)

@sbomer
Copy link
Member Author

sbomer commented Jan 26, 2023

Thanks - does dotnet/sdk need to be updated too?

@ViktorHofer
Copy link
Member

dotnet/sdk#30141

Unsure. I think what they use works for source-build so it's probably fine to keep it pinned to the lower version. cc @mmitche @marcpopMSFT

@ViktorHofer ViktorHofer merged commit 053438e into dotnet:main Jan 26, 2023
tlakollo pushed a commit to tlakollo/runtime that referenced this pull request Jan 31, 2023
* Match SDK versions of MSBuild framework

* Bump to match runtime versions

Commit migrated from dotnet/linker@053438e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants