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.

Sign up for GitHub

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

Jump to bottom

[FST 1003 discussion] #229

Closed
dsyme opened this issue Nov 8, 2017 · 5 comments
Closed

[FST 1003 discussion] #229

dsyme opened this issue Nov 8, 2017 · 5 comments

Comments

@dsyme
Copy link
Contributor

dsyme commented Nov 8, 2017

Discussion thread for https://github.com/fsharp/fslang-design/blob/master/tooling/FST-1003-loading-type-provider-design-time-components.md
@KevinRansom
Copy link
Contributor

@dsyme

The proposal has a layout and probing rules for the compiler which incorporates, arch, execution platform and fsharp version, and ... weirdly not the target platform. I assume and I may have overlooked it, that you expect a TP to be able to create for all future target platforms.

My concern is that having the probing logic inside the compiler is just more of that helpful resolution which invariably bites us when we try to extend it.

The actual resolution sounds an awful lot like what nuget and paket already do. It seems like it would be preferable to ensure that nuget and paket understand how to run select compiler extensions such as F# TypeProviders and Roslyn Analyzers.

And we should ensure that FSI and FCS have good mechanisms for finding that out. Actually, I think they probably will be fine anyway if we have the means to make a TP reference a -r:.

Anyway ... I am going to get a discussion going with the nuget team and sdk team to see if we can figure something out, so that Compiler Extensions are treated as first class items within the dotnet ecco-system.

Kevin

@dsyme
Copy link
Contributor Author

dsyme commented Nov 9, 2017 •
edited
Loading

The proposal has a layout and probing rules for the compiler which incorporates, arch, execution platform and fsharp version, and ... weirdly not the target platform.

The target platform is selected via the primary reference (-r:lib\net45\FSharp.Data.dll). This gives the name of the TPDTC in an attribute, e.g. TypeProviderAssembly("FSharp.Data.DesignTime.dll"). This is then combined with information about the host to find the actual TPDTC DLL. Every TPDTC made using the recently updated TPSDK is able to provide types and generate code for all its target platforms.

This means it would be incorrect to add a target qualification: - we are specifically encouraging people to make TPDTCs that can cross-target, just as the F# compiler itself can cross-target.

The It seems like it would be preferable to ensure that nuget and paket understand how to run select compiler extensions such as F# TypeProviders and Roslyn Analyzers.

I think it's great to have a discussion about compiler extensions in general. There are also similarities with MSBuild extensions.

That said, my starting position is that we should not be gating on additions to nuget and paket here, despite the superficial similarities to other load-a-dynamic-extension problems. Neither NuGet nor Paket provide a "load-a-package-dynamcially" operation today, so we are gating on something that doesn't yet exist. This is covered in the Drawbacks and Alternatives sections of the RFC, but here are the basic reasons:

  1. We are under a tight timeline – we need to get type providers supported in .NET Core-based tooling in a reasonable timeframe, preferably without taking dependencies on other technologies.

  2. We need an approach that ensures consistency across F# tooling providers, including all the tools listed in the RFC and any other tools that use FSharp.Compiler.Service.dll (including F# Interactive, Visual Studio, F# compiler, FsAutoComplete.exe/Ionide, FSharp.Formatting, the JetBrains Host.exe and so on).

  3. Realistically our only way of delivering a consistent, simple implementation of the Load-A-Type-Provider-Into-Tooling (LATPIT) operation is by implementing the operation in FSharp.Compiler.Service.dll. Specifically we don’t want any other external tool involved in "configuring" FSharp.Compiler.Service.dll “from the outside”, as that would mean every piece of F# tooling would have to participate in this configuration process. That’s a nightmare to roll out and impractical for some other settings.

  4. The proposal in the RFC is that

    LATPIT = “Find an associated design-time assembly“ + “Call Assembly.LoadFrom (or 
AssemblyLoadContext.Default.LoadFromAssemblyPath on .NET Core)”.

    This is what we do today for F# type providers for .NET Framework tooling. It has problems, but if we want to deliver something to customers then it has the advantages of being

    a. Simple (just 50 lines of code)
    b. Well known
    c. Gives us parity with .NET framework-based tooling
    d. Gives us a strong backwards-compatibility story
    e. Dead-easy to deliver (we just update FSharp.Compiler.Service.dll)
    f. Adds no dependencies
    g. Stable (let's face it, NuGet revs frequently....)

  5. Now, in the ideal world, .NET Standard 2.0 or NuGet.XYZ.dll might in theory contain a polished, tested and standardized “LoadPackage(…)” dynamic package resolution/composition/load capability which we could use to implement the second half of the LATPIT operation. However, this doesn’t exist, is very hard to get right, is really a fundamental addition to .NET and in practice would probably use LoadFromAssemblyPath under the hood anyway.

  6. If a LoadPackage existed operation today in the heart of .NET then we would almost certainly use it I suppose. But let’s be realistic: my educated guess is that NuGet won't deliver such a capability soon, and .NET can’t interpret package formats and dependencies without NuGet.

  7. Even if we were to have a “LoadPackage(…)” operation, e.g. in NuGet.XYZ.dll, I am a little skeptical about whether we should make all F# editor/scripting/compiler tooling dependent on NuGet.XYZ.dll. For example, many in the F# community would have a heart attack if FSharp.Compiler.Service.dll takes a dependency on NuGet.XYZ.dll to help load type providers into the compiler. But I think that would be inevitable if we push down that path.

  8. The proposed approach used for #r references, e.g. #r “nuget: …” or #r “paket: …” does not appear to be an adequate replacement here, because type providers must work with normal F# projects when analyzed and interpreted by FSharp.Compiler.Service.dll, not just scripts and F# Interactive.

  9. If LoadPackage eventually exists then we could iterate to that architecture,

The RFC and PR navigate these issues and give us something we can deliver. It’s a tricky area, but IMHO that’s because .NET lacks a very fundamental dynamic operation (LoadPackage, giving us only LoadAssembly).

@sergey-tihon
Copy link

sergey-tihon commented Nov 18, 2017 •
edited
Loading

Can we make design-time assembly resolution logic a part of TypeProvider.SDK instead of part of the compiler? Seems like logic may change in the future, so it is likely to allow TP authors fix/adjust resolution strategy.

Right now we have similar situation with NuGet dependencies of design-time assembly. If we do not want to pack all dlls inside TP package and restore them from NuGet we need to register AssemblyResolve handler on AppDomain and inject resolution strategy for 3rd party dependencies. (at least SwaggerProvider and RProvider do so)

 AppDomain.CurrentDomain.add_AssemblyResolve(fun source args ->
        SwaggerProvider.Internal.Configuration.resolveReferencedAssembly args.Name)

See here for full resolution logic. Seems this code in the future should be also adjusted to take into account the design-time runtime that hosts TP.

Can we unify dependency resolution logic between TP.DesignTime assembly and NuGet dependencies?

  • Include resolution strategy in a new file inside SDK.
  • TP authors add new file in their project and register AssemblyResolve handler in TP.dll
  • Compiler/host process ask AppDomain.CurrentDomain to resolve TP.DesignTime.dll
  • We can fix/patch assembly resolution logic and time and ship new version with TP.
  • The compiler should know that resolution strategy may be pluggable - we can control it using new param in TypeProviderAssembly

@KevinRansom
Copy link
Contributor

KevinRansom commented Nov 18, 2017 via email

@cartermp
Copy link
Member

cartermp commented Jul 9, 2018

Closing old discussion, as this is implemented and shipped

@cartermp cartermp closed this as completed Jul 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@sergey-tihon @KevinRansom @cartermp @dsyme