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

Some net472 compatible packages are needed during source build #30375

Closed
nguerrera opened this issue Jul 25, 2019 · 13 comments · Fixed by dotnet/corefx#40061
Closed

Some net472 compatible packages are needed during source build #30375

nguerrera opened this issue Jul 25, 2019 · 13 comments · Fixed by dotnet/corefx#40061
Assignees
Milestone

Comments

@nguerrera
Copy link
Contributor

Filing this on corefx as discussed with @ericstj, but it will spin-off issues for other repos that will need to meet the same constraint.

cc @JunTaoLuo @crummel @dseefeld

Background: For the most part, we produce a Linux/netcoreapp only output in source-build. However, the source built MSBuild SDKs (the things you get from the Sdk attribute in csproj) actually need to be compatible with mono msbuild too. This is because mono msbuild resolves them to the .NET Core SDK just like VS does, and mono msbuild uses the netfx assets from there just like VS too.

See dotnet/source-build#125, which was resolved to make VS Code on Linux work with source built SDK, but without being rigorous about eliminating the prebuilts that pulled in.

For corefx, the netfx DLLs in this closure are:

  • System.Reflection.Metadata
  • System.Collections.Immutable

This is opportunistically small for now since PresentationBuildTasks.dll is not currently supported x-plat for cross-targeting to Windows. Should that change, we get quite a few more netfx corefx dlls in its closure.

The source build must produce packages that have netfx implementations of these. The version number could be different and injected into source build, and the contents could be a subset of the real package. The package will not ship, it only has to be enough for the downstream repos to produce correct output and run properly.

Here is my analysis of all the netfx DLLs in the SDK that will be pulled in by Mono. This will be used to spin off more issues, but I'd like to discuss this and be sure we have a plan before filing a bunch more requests like this.

mono/linker

  • Sdks\ILLink.Tasks\tools\net472\ILLink.Tasks.dll
  • Sdks\ILLink.Tasks\tools\net472\Mono.Cecil.dll

Dependencies from dotnet/corefx

  • Sdks\ILLink.Tasks\tools\net472\System.Collections.Immutable.dll
  • Sdks\ILLink.Tasks\tools\net472\System.Reflection.Metadata.dll

dotnet/sdk

  • Sdks\Microsoft.NET.Sdk\tools\net472\Microsoft.NET.Build.Tasks.dll

Dependencies from dotnet/core-setup

  • Sdks\Microsoft.NET.Sdk\tools\net472\Microsoft.DotNet.PlatformAbstractions.dll
  • Sdks\Microsoft.NET.Sdk\tools\net472\Microsoft.Extensions.DependencyModel.dll
  • Sdks\Microsoft.NET.Sdk\tools\net472\Microsoft.NET.HostModel.dll

Dependencies from JamesNK/Newtonsoft.Json

  • Sdks\Microsoft.NET.Sdk\tools\net472\Newtonsoft.Json.dll

Dependencies from NuGet/NuGet.Client

  • Sdks\Microsoft.NET.Sdk\tools\net472\NuGet.Common.dll
  • Sdks\Microsoft.NET.Sdk\tools\net472\NuGet.Configuration.dll
  • Sdks\Microsoft.NET.Sdk\tools\net472\NuGet.DependencyResolver.Core.dll
  • Sdks\Microsoft.NET.Sdk\tools\net472\NuGet.Frameworks.dll
  • Sdks\Microsoft.NET.Sdk\tools\net472\NuGet.LibraryModel.dll
  • Sdks\Microsoft.NET.Sdk\tools\net472\NuGet.Packaging.dll
  • Sdks\Microsoft.NET.Sdk\tools\net472\NuGet.ProjectModel.dll
  • Sdks\Microsoft.NET.Sdk\tools\net472\NuGet.Protocol.dll
  • Sdks\Microsoft.NET.Sdk\tools\net472\NuGet.Versioning.dl

Dependencies from dotnet/corefx

  • Sdks\Microsoft.NET.Sdk\tools\net472\System.Collections.Immutable.dll
  • Sdks\Microsoft.NET.Sdk\tools\net472\System.Reflection.Metadata.dll

dotnet/websdk

  • Sdks\Microsoft.NET.Sdk.Publish\tools\net46\Microsoft.NET.Sdk.Publish.Tasks.dll
  • Sdks\Microsoft.NET.Sdk.Publish\tools\net46\Microsoft.Web.Delegation.dll
  • Sdks\Microsoft.NET.Sdk.Publish\tools\net46\Microsoft.Web.Deployment.dll
  • Sdks\Microsoft.NET.Sdk.Publish\tools\net46\Microsoft.Web.XmlTransform.dll

aspnetcore/tooling

  • Sdks\Microsoft.NET.Sdk.Razor\tasks\net46\Microsoft.NET.Sdk.Razor.Tasks.dll

Dependencies from dotnet/corefx

  • Sdks\Microsoft.NET.Sdk.Razor\tasks\net46\System.Collections.Immutable.dll
  • Sdks\Microsoft.NET.Sdk.Razor\tasks\net46\System.Reflection.Metadata.dll

dotnet/wpf

NOTE: We can ignore this one for now as it is not currently supported x-plat

We may need to build a stub version though to allow the not supported message to come up correctly.

  • Sdks\Microsoft.NET.Sdk.WindowsDesktop\tools\net472\PresentationBuildTasks.dll

Dependencies from dotnet/corefx

  • Sdks\Microsoft.NET.Sdk.WindowsDesktop\tools\net472\System.Collections.Immutable.dll
  • Sdks\Microsoft.NET.Sdk.WindowsDesktop\tools\net472\System.Memory.dll
  • Sdks\Microsoft.NET.Sdk.WindowsDesktop\tools\net472\System.Numerics.Vectors.dll
  • Sdks\Microsoft.NET.Sdk.WindowsDesktop\tools\net472\System.Reflection.Metadata.dll
  • Sdks\Microsoft.NET.Sdk.WindowsDesktop\tools\net472\System.Reflection.MetadataLoadContext.dll
  • Sdks\Microsoft.NET.Sdk.WindowsDesktop\tools\net472\System.Runtime.CompilerServices.Unsafe.dll

NuGet/NuGet.Client

NOTE: This one is IL merged in the closed source build, so I can't see its dependencies. :(

  • Sdks\NuGet.Build.Tasks.Pack\Desktop\NuGet.Build.Tasks.Pack.dll
@nguerrera
Copy link
Contributor Author

There is dotnet/source-build#210 tracking building allConfigurations to get packages, but I'm not sure that it captures the need for netfx compatible assets in the packages as this does.

@JunTaoLuo
Copy link
Contributor

From the list, I see Sdks\Microsoft.NET.Sdk.Razor\tasks\net46\Microsoft.NET.Sdk.Razor.Tasks.dll. If we need to produce that, we'll need System.Reflection.Metadata see dotnet/source-build#1150.

@ericstj
Copy link
Member

ericstj commented Jul 26, 2019

System.Memory and System.Numerics.Vectors don't even build out of master any more, so that's going to be challenging. Need to think about how to best satisfy these dependencies.

@nguerrera
Copy link
Contributor Author

As of right now, we can exclude them because we don't need WindowsDesktop SDK. But I suspect either we'll pull them in elsewhere before long or support cross-building for wpf/winforms eventually. NuGet was just looking at taking s.t.json which would pull them in and I asked them to hold off for 3.0. So yeah, we need to be prepared to build them somehow at some point not too far away.

@ericstj
Copy link
Member

ericstj commented Aug 5, 2019

@nguerrera if we can exclude the desktop assets entirely we should. There is not a good way to build these that doesn't imply forking code that's only meant to exist in 2.1 servicing.

@ericstj ericstj self-assigned this Aug 5, 2019
@nguerrera
Copy link
Contributor Author

We can exclude them right at this moment, but I don't think it's necessarily sustainable. We would have to tell nuget that they can never use System.Memory/System.Text.Json. We can do it for 3.0, but I expect there will be pushback when dependencies of the things that need to build net4x are told they cannot use these packages.

@nguerrera
Copy link
Contributor Author

nguerrera commented Aug 5, 2019

if we can exclude the desktop assets

Which desktop assets do you mean here? All net4x assets in my list? Only those needed by WindowsDesktop SDK. The latter is possible, but the risk is that something else than WindowsDesktop needs these or we support cross-compiling for WindowsDesktop.

@ericstj
Copy link
Member

ericstj commented Aug 6, 2019

I'd actually question the requirement on whether the source-built product needs to support desktop scenarios at all. I don't think that's a reasonable requirement. Regardless I've addressed the two packages you've called out above, but I think you might want to consider blocking that scenario as I can imagine it becomes hard to support at the limit.

@nguerrera
Copy link
Contributor Author

nguerrera commented Aug 6, 2019

It is not supporting "desktop scenarios". It is supporting using mono msbuild working with your .NET Core SDK. We can't just take that away from SDK because it was source built.

The only other alternatives to just building them I've seen.

  1. load netcoreapp task dlls in a mono process, which people tried to hack and hit runtime errors, which is to be expected. I think this is a dead end.

  2. Say that mono msbuild is not supported with .NET Core SDK msbuild SDKs. We can't do that until VS 4 Mac and VS Code can move off of it at least. This is a big conversation. It is probably the long term direction, but I have no power to make this call nor do I think it is reasonable for source build to impose an earlier deadline on this.

@nguerrera
Copy link
Contributor Author

nguerrera commented Aug 6, 2019

VS 4 Mac can maybe get away with it longer than VS Code if we don't source build for Mac. But I think that supporting mono/netfx mwbuild in VS and VS 4 Mac, but not VS Code on Linux due to source build is not a coherent customer-driven story.

@ericstj
Copy link
Member

ericstj commented Aug 6, 2019

load netcoreapp task dlls in a mono process, which people tried to hack and hit runtime errors, which is to be expected. I think this is a dead end.

What if it was a sufficiently new Mono that had all the facades inbox and was NETStandard2.0 compatible? What's missing?

@nguerrera
Copy link
Contributor Author

The tasks are multi-targeted netfx + netcoreapp, not netstandard.

Building netstandard msbuild tasks doesn't work because tasks have to carry their non-framework dependencies and we can't work those out without a concrete TFM.

Building against netcoreapp and running on mono is not supported, right? It could be that nothing is missing at any point in time, but it also does not prevent a task from taking a dependency that will not work. Also, do mono facades stay up to date with netcoreapp assembly versions?

@ericstj
Copy link
Member

ericstj commented Aug 6, 2019

I suppose you're right. I was thinking that mono codebase is technically satisfying all the Xamarin frameworks which can load .NETCore versioned assemblies. Also anticipating some of the future mono changes that unify stuff so I'm imagining that these ought to be the same. Perhaps that's more future perspective vs present. It'd certainly be nice if Mono MSBuild could behave more like core MSBuild rather than desktop.

Regardless, I think the non-corefx dependencies here are for another discussion. The corefx dependencies should be addressed in my PR.

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 3.0 milestone Feb 1, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants