search

dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.82k stars 772 forks source link

Add dependencies for System libraries that need to be lifted in the source-build / VMR context #17344

Closed ViktorHofer closed 4 days ago

ViktorHofer commented 5 days ago

Fixes https://github.com/dotnet/source-build/issues/4477

Source-build lifts the MSBuild package versions to the N-1 (previously produced assets) but fsharp pins the System.Reflection.Metadata (and similar System libs) version which is a transitive dependency to 8.0.0. That results in a package downgrade error. Define a dependency for System.Reflection.Metadata (and other System libs) so that source-build can lift that version as well.

github-actions[bot] commented 5 days ago

:white_check_mark: No release notes required

MichaelSimons commented 5 days ago

@ViktorHofer - What are you thinking for Preview 6? Should we patch this since we are getting late in the release cycle?

tmds commented 5 days ago

@ViktorHofer - What are you thinking for Preview 6? Should we patch this since we are getting late in the release cycle?

I am passively following this as I ran into it in our CI.

If you don't need to go through hoops to get this in preview 6 that would be nice. This affects when we're trying to do a native build on ppc64le/s390x using assets from a cross-build.

ViktorHofer commented 5 days ago

Downgrading and relying on SBRP doesn't work in this case as msbuild comes in transitively and as a preview version. I will instead add a Version.Details entry for SRM which should fix the downgrade issue.

Yes, we should Patch this for P6. @MichaelSimons do we still have time for this?

vzarytovskii commented 5 days ago

Hm. The issue with that is it'll likely start failing all of our VS CI, since we rely on newer msbuild there.

Update: it is. What would be the guidance here?

MichaelSimons commented 5 days ago

Yes, we should Patch this for P6. @MichaelSimons do we still have time for this?

We can't release without it.

ViktorHofer commented 4 days ago

Just successfully validated that this fixes the stage 2 build error described in the linked issue. @vzarytovskii can you please merge the PR?