support .NET Standard 2.0 in F# Interactive and type providers #3307
Conversation
|
cc @terrajobst - perhaps you could chime in about this?
|
|
@cartermp @terrajobst It turns out that #3309 is basically the same problem - support loading .NET Standard 2.0 components into F# Interactive on .NET Framework Some of these facade DLLs (e.g. System.IO.dll) are included with .NET Framework 4.6.1 (our minimal assumption with this PR), so it's a bit confusing if we need to include them or not. Do we only need to include |
|
I augmented the PR with enough to include |
src/FSharpSource.targets
Outdated
| @@ -246,6 +247,19 @@ | |||
| <PackageTags Condition="'$(PackageTags)' == ''" >Visual F# Compiler FSharp coreclr functional programming</PackageTags> | |||
| </PropertyGroup> | |||
|
|
|||
| <Import Project="$(NugetLocalPackagesDir)\Microsoft.Packaging.Tools.1.0.0-preview2-25401-01\build\Microsoft.Packaging.Tools.targets" Condition="Exists('$(NugetLocalPackagesDir)\Microsoft.Packaging.Tools.1.0.0-preview2-25401-01\build\Microsoft.Packaging.Tools.targets')" /> | |||
There was a problem hiding this comment.
It could be we don't need all this and would be better off with a simple direct reference to the appropriate netstandard.dll
|
Investigating CI failure: Ubuntu build (still currently using "xbuild") doesn't like those new Import declarations We should move to "msbuild" and/or simplify what we're adding here |
|
Additional changes were needed to
|
dsyme
changed the title
Thanks for looping me in.
That is the plan: .NET Framework 4.7.1 will ship the type forwarding facades for .NET Standard 1.x and 2.0 in-box. The reason we couldn't do this .NET Framework 4.6.1 is because it shipped way before .NET Standard 2.0 was even conceived. However, since it basically supports the API surface we thought it's best to enable the .NET Standard 2.0 consumption by requiring the application to ship the necessary facades.
Strictly speaking loading .NET Standard 2.0 components only requires loading one extra forwarding DLL, namely |
|
@dotnet-bot Test Windows_NT Release_ci_part3 Build please |
OK, thanks. That makes sense
Thanks, that makes sense too. I think there are scenarios today where F# Interactive (Fsi.exe) will not correctly load all .NET Standard 1.x components. I'll try to repro this. |
|
A snippet of mine from a conversation with @KevinRansom about whether we need this, i.e. whether F# Interactive should support loading .NET Standard 1.x and 2.0 components in Visual Studio 2017 (i.e. on .NET 4.6.1) @dsyme says:
The other alternative being discussed was basically whether we should punt on support for loading .NET Standard 2.0 into F# Interactive and for type providers until .NET 4.7.1 can be assumed, i.e. VS vNext. That feels wrong to me
|
src/fsharp/CompileOps.fs
Outdated
| Some asm.Location | ||
| else None | ||
| with _ -> None | ||
| let path = Path.GetDirectoryName(GetDefaultFSharpCoreReference()) |
There was a problem hiding this comment.
This is not really right and the existing code is wrong too ...
This forces us to co-locate System.ValueTuple.dll with FSharp.Core.dll which is not right. Especially since dotnet cli hosting can locate assemblies in a central shared cache rather than app local.
The existing code assumes that ValueTuple was loaded from an assembly named System.ValueTuple.dll which is also not right. Since from dotnet 4.7 it exists in mscorlib, probably System.Private.Corlib.dll on coreclr.
I guess I will have to revisit this code at some point, but we need to do better long term.
There was a problem hiding this comment.
OK, yes, this is wrong when running the compiler or F# Interactive in .NET Core without --noframework etc.
There was a problem hiding this comment.
And it won't work on the dotnet cli. Since we don't co-locate system.valuetuple.dll
@terrajobst You might want to add some documentation explaining that applications running on .NET 4.6.1/4.7 which do Assembly.Load/LoadFrom on .NET Standard 1.x DLLs or .NET Standard 2.0 DLLs should ship the entire set of facades. It's not at all obvious :) |
|
@KevinRansom I checked the PR comment (1) that code you mentioned is protected by (2) The old code (before the last FCS integration) just used So I think the code here is no worse. However the code we really need to adjust in order to get sane default references for That code that was added came from @ncave who wanted a functioning default set of references for the F# scripting model implemented by FCS. We have that under test in the FCS repository but not yet here. Adjust it as needed This also opens the question of whether the .NET Core version of the compiler (and the compiler service DLL) is fully behaviour-compatible with the .NET Framework version, especially w.r.t. what assumptions are made about the compilation if incomplete arguments (without I'll open an issue to track known subtle behaviour differences between the .NET Core and NET Framework versions of the compiler. |
"This theorem can easily be proven and is thus left as an exercise for the reader" 😄 Agreed. We've recently changed how the applications acquire the necessary facades. We've switched from a NuGet package to global machine state. I'll ensure that reflection scenarios will work. Let me come back to you on this. |
For some other project, I'm getting around that by compiling against netcoreapp2.0 (even for libraries). It works, but it may (or may not) be a good solution for distributed libraries, so YMMV. |
Is the final consumption of your library only .NET Core App 2.0 right? In that case yes, that's a workaround. I'm not entirely sure how this issue plays out for your .NET Core F# scripting model using the .NET Core version of FSharp.Compiler.Service.dll. It could be that having the compiler service add a default reference to the the right netstandard.dll to match all the rest of the default references when doing .NET Core compilation will work. We should probably add an issue in the FCS repository about this ("make sure .NET Standard 2.0 libraries can be referenced in the F# scripting model implemented by the .NET Core version of FSharp.Compiler.Service.dll"). Could you do that? And maybe add a failing test case? |
|
@dsyme I just tried it, you can still consume such libraries from other targets (netcoreapp1.0 etc.) if they are multi-target built for them: And the reference trimming seems pretty good, only what is actually used. But yes, it's a work-around. |
Yes, that's correct. But why not build for |
|
I have added a basic smoke test for .NET Framework fsi.exe referencing a .NET Standard 2.0 type provider |
|
@KevinRansom Just to mention that I removed the change to System.ValueTuple resolution, it was not relevant to the PR |
I assume a nuget/MSBuild15 As I understand it from this doc, MSBuild15/.NET SDK support for Paket is actually mediated via msbuild, so perhaps Paket should actually use a similar technique? This could be a very satisfying direction as it would get F# Interactive out of the business of resolving anything. |
|
The problem is: there is no MSBuild context. ;-) I mean we could probably run MSBuild in a temp folder and give it all the files it needs and reference whatever it came up with after restore. But is that really needed? |
|
Side note: It feels like part of the long-term solution here would be to have a |
There must be some kind of technique for doing this - it's much the same as what the devenv.exe project system does when you add a package reference using the GUI, isn't it? At some level At the high level I'm just wondering if we can use .targets/.props in the nuget package to compute the relevant type provider references, and incorporate this into the |
|
Vs has it's own implementation of MSBuild. It's not the MSBuild.exe we know and love. Their background build is not 100% compatible with MSBuild. It even uses its own targets. Just saying... |
OK, then Xamarin Studio. In both cases my understanding is that they use the MSBuild API to maintain a resolved state through incremental changes to a project specification. That's basically what the MSBild API is for (I've only experience of the MSBuild 14 API for doing this , not MSBuild 15 - though they may be basically the same).
Yeah, I know. I get the impression that much of this is due to incrementality, e.g. you can get subtle differences where |
Just to step back: do you see way to get BTW we should probably move this to the discussion thread of https://github.com/fsharp/fslang-design/blob/master/RFCs/FS-1027-fsi-references.md#unresolved-questions, or at least summarize there after we've done his thread to it's death :) |
|
Props are probably pretty easy. Targets are way more complex and allow you to program.
The question is: what do you really need to express? Why can't be expressed in the file structure? Do we really need everything in the beginning to make something useful?
|
|
To be honest this all reads like running into the wrong direction (or you could say against a wall).
I think we need to embed what we need here properly in the package metadata/contents:
|
For most type providers we probably just need especially once .NET 4.7.1 is widely assumed. A more complex type provider could use etc. where these platforms refer to the compile-time context.. (not sure of the directory names - where possible, the conventions should be identical to those used by Roslyn analyzers, MSBuild tasks etc. - any other design-time tooling.) We could indeed hack F# tooling (e.g. the F# compiler type provider loader) to probe around looking through packages for type provider DLLs to load. But this is really going against the grain: we would like the implementation of the F# scripting model (i.e. in the F# compiler) to get out of the business of assembly resolution and package interpretation altogether. As @KevinRansom has pointed out, it's a continual problem for F# engineering. It's OK to have plugins that do this, but we shouldn't have these decisions being made right inside the compiler. For assembly references, #2483 and its corresponding RFC goes a long way to achieving this, but leaves some things unaddressed:
|
|
Adding targets for resolution is really a bad idea. It's same category as the ps1 hack. This should all be done in meta data in path name and nuspec. It's sad that all teams like asp.net at first then BCL and now we are hacking around that just because nuspec is not extended fast enough. Runtime packages belong into the same category. Really bad hack around the meta data model. |
|
@matthid Thanks for the link to the nuget spec for analyzers - for reference the general spec is though I'm not sure that includes framework restrictions.
No one is suggesting we take an MSBuild dependency in the F# compiler. However, for the implementation of the resolver plugin that implements For the implementation of
Yes, we should make sure the type providers use fully declarative paths, of course, and re-use existing norms
@matthid let's just keep iterating ok? |
|
I think @matthid and myself still don't understand why targets files would be needed at all.
How would it look like? As a concrete sample for a type provider?
|
|
I think we need to differentiate a bit:
Also we need to differentiate about the package support in F#:
@forki is correct that it's the same design decision we made for paket when supporting |
I get that. That said, I don't think that there's any need for us to make things any worse for Paket here - we can use nice declarative paths under
There are separate issues:
My answers:
My answer: Yes, I think they probably should. From the F# perspective I would like ``#r "nuget: FunkyAspNetPackageThatUsesTargets" to work
My answer:: It's up to you
My answer: Let's assume "no". After all, yesterday I was an advocate of type providers just being single netstandard2.0 DLLs :) If forced I could contrive something, e.g.
But you could put that kind of logic in the implementation of the type provider itself, or just have different type provider packages. |
|
@forki BTW is there a definition of the exact technical difference between a "props" and "targets" file? Thanks |
I think technically they are both msbuild build files. The difference is the time they are imported.
|
|
My main takeaways from all this recent discussion is
Together with some other changes, this would basically get the compiler out of the business of doing assembly resolution and type provider resolution. |
|
Yes exactly. The compiler should not do dll resolution. That belongs into
the addin by providing a proper load script.
The mechanism for that may change over time. I personally don't think a
#typeprovider Is needed since that would not know which package resolution
way to go. Should it go nuget, or file system or paket? It can't know.
|
|
Perhaps we should draft an RFC for FSI behavior in this new world of netcore and netstandard, and discuss there. It feels to me like what we ultimately want is a bit transformative for FSI. I'd personally love for us to move away from the concept of |
|
Yes we don't want to reference dlls individually. But the issue is that there is no definition of how to reference nuget packages properly. Is just a pile of fixes on top of undocumented behavior and many many workarounds. It will be extremely hard to formalize that. So we should really not do
that in the compiler. Putting this into an addin that can be developed independently is imho much
saner.
What we need to do is to define how the addin should communicate. And for
this we have already a RFC that can be extended if needed
|
(just edited your "before" to "netcore" to keep the conversation clean :) ) |
|
@dsyme as a note regarding MSbuild background and XS: https://bugzilla.xamarin.com/show_bug.cgi?id=58038 - this shows it's really hard and most tools are not there yet. |
|
@dsyme : We need to investigate how nuget and roslyn handles analyzers for multiple platforms. |
Just checked. With the updated preview tooling this is how it works today:
Would this design work for F#? |
I will try adjusting the PR, thanks. It certainly makes it simpler, though perhaps the changes to setup are still required sadly However both @KevinRansom and @cartermp are opposed to accepting this PR, primarily because
I understand the reasoning. That said, I'm still very concerned that we have an awkward story for .NET Standard 2.0 DLLs in the interim. |
Our plan in the long run is to have type provider design time components (the ones hosted in the F# compiler and IDEs and FsAutoComplete.exe and FSI.exe) be implemented as .NET Standard 2.0 components. These components can then be used and hosted whether the F# compiler is running on .NET Core, .NET Framework or Mono.
As a first step in this direction, I've done an experiment to see what is required for the .NET Framework/Mono version of the compiler to host a type provider implemented as a .NET Standard 2.0 component.
I have tested this with a private version of FSharp.Data where both FSharp.Data.dll and FSharp.Data.DesignTime.dll are .NET Standard 2.0 components. It works, which is good.
The good news is that it isn't too bad to implement. We basically have to
release\net40\bin\netstandard.dlletc.The bad news is
as far as I can see these facade assemblies are not shipped on .NET 4.6.1 or .NET 4.7 and must be part of the F# compiler when shipped as a .NET 4.6.1 application. This is a consequence of how the .NET team have tricked .NET 4.6.1 into supporting .NET Standard 2.0: i.e. by requiring .NET 4.6.1 applications to ship versions of netstandard.dll and friends that simply redirect to mscorlib.dll.
The list of added facade assemblies is quite long - there are 96 of them. We already ship a handful of these. But because type providers are loaded by reflection into the compiler, then in principle we need to ship all of these facade DLLs as part of our .NET Framework compiler, because any .NET Standard 2.0 type provider could end up using any of these (the DLLs can't be included with the type provider because they are .NET 4.6.1. forwarding DLLs). I presume any .NET Framework 4.6.1 application which is going to reflection-load arbitrary NET Standard 2.0 components will need all of these facade assemblies. Without
release\net40\bin\netstandard.dllin place we get an error that netstandard.dll 2.0.0.0 is missing whenever we try to reflection-load a .NET Standard 2.0 component into the F# compiler running on 4.6.1To see that these components are not included in the .NET Framework itself:
I very much hope that there is a plan to eventually ship these "in-application forwarding DLLs" as part of the .NET Framework itself, because it really doesn't feel that .NET 4.6.1 properly supports reflection-loading of arbitrary .NET Standard 2.0 components if that means adding ~96 small forwarding DLLs to your application. It's not the end of the world, it just feels odd that one call to Reflection.Load should necessitate adding so many DLLs. Note that this will also affect all applications that host FSharp.Compiler.Service as well.