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

DependencyProvider: Add a stable path which is tooling independent #8880

Open
smoothdeveloper opened this issue Apr 4, 2020 · 6 comments
Open

Comments

@smoothdeveloper
Copy link
Contributor

smoothdeveloper commented Apr 4, 2020

@smoothdeveloper The spec for dependency manager resolution seems not to be implemented - it references things like "fsx-extensions" but that string doesn't occur anywhere in the F# codebase.

So it's kind of hard to discern how things are intended to work, or how things are actually resolved today, or how things could be adjusted.

I'd say the first thing is to update the spec to clarify what is there today. I'll adjust this issue to track that, also close #12895 as a duplicate since everything else depends on that.


Problem

The dependency provider feature has extensibility points to allow third party providers to hook into the resolution.

The current search paths are tooling dependent (VS has its own, Ionide its own, Rider its own, dotnet fsi its own, etc.) as they are based on location of compiler assemblies used by the tooling.

This makes impossible a one step system wide or user account wide installation, working across all the toolchains. It will hurt adoption of third party providers and yield fair amount of support requests.

Location of the search paths AFAIK:

let assemblySearchPaths = lazy (
[
let assemblyLocation = typeof<IDependencyManagerProvider>.GetTypeInfo().Assembly.Location
yield Path.GetDirectoryName assemblyLocation
yield AppDomain.CurrentDomain.BaseDirectory
])

Describe the solution you'd like

A stable search path under user account may be added to the list. Maybe a system wide can be added as well.

Another aproach would be for referenced assemblies to be probed as well, it would enable using existing #r syntax to load the extension from a known path or an already installed handler. Albeit this has an extra runtime cost, maybe a baked-in extension should have this role:

#r @"load-extension: path/to/extension.dll"

Although those incur extra code in the scripts.

Describe alternatives you've considered

Looking for some, please share your insight, provide feedback on this feature request.

Additional context

The RFC had hints on such location: https://github.com/fsharp/fslang-design/blob/master/tooling/FST-1027-fsi-references.md#handler-resolution

@smoothdeveloper
Copy link
Contributor Author

Also, I think this solution would be easy to implement but incur a minor cost of checking the assembly attribute:

#r "nuget: FSharp.DependencyManager.Paket"
#r "paket: ..." // should work

@smoothdeveloper
Copy link
Contributor Author

I'm getting to the idea that an environment variable containing list of paths for extensions would be a good first approach, and which doesn't imply significant change either.

That environment variable will have the same effect as passing --compilertool flags for each path defined in it.

The compiler and FCS would rely on it which allows the extensions to be used implicitly system wide so long user has ability to get the environment variable set.

@dsyme
Copy link
Contributor

dsyme commented Apr 21, 2022

@smoothdeveloper The spec for dependency manager resolution seems not to be implemented - it references things like "fsx-extensions" but that string doesn't occur anywhere in the F# codebase.

So it's kind of hard to discern how things are intended to work, or how things are actually resolved today, or how things could be adjusted.

I'd say the first thing is to update the spec to clarify what is there today. I'll adjust this issue to track that, also close #12895 as a duplicate since everything else depends on that.

@dsyme dsyme changed the title DependencyProvider: Add a stable path which is tooling independent DependencyProvider: Spec needs clarification before feature is usable Apr 21, 2022
@dsyme dsyme added the Area-FSI label Apr 21, 2022
@dsyme
Copy link
Contributor

dsyme commented Apr 21, 2022

@smoothdeveloper If you could take a pass at updating the spec to match what is actually there today that would be great

@smoothdeveloper
Copy link
Contributor Author

@dsyme, I confirm that what went in the early stage of the RFC, didn't make it through the "let's ship #r "nuget:"" story, it also didn't make it to do back & forths on the design proposed in the RFC, the whole thing turned into "let's ship #r "nuget:"" (understandable on some respects, and helped tons of people since V5 release).

There are also, maybe things that got done in nuget one, that I have no idea exist, and would be of use, for other extensions.

Technically, the truth is in the code, and the head of @KevinRansom, pretty much.

I don't agree with closing #12895 on basis that the RFC isn't clear, it is more important that extension works the way they do, it just doesn't work in VS, for some (AppDomain.AssemblyResolve?) reasons, and for me it is a pragmatic priority, this blocks me even using extensions (outside of nuget), in places where it would be most useful to me.

From my perspective, it is "as clear as we can", how to use an extension like the paket one (we use it like the nuget one), from the documentation: https://fsprojects.github.io/Paket/fsi-integration.html or for FSC, there is some information about the --compilertool.

I sadly can't put energy, as things stand, into shifting things, so the extension become super easy to use (the same as installing vscode extension), pragmatically speaking, if we'd like to setup stuff for "next release will improve on this", maybe I should hear what Microsoft wants to do, and then see where I can contribute efforts, once it is clear.

Right now, there is more urgency, to me, to fix what has been a "competive advantage" for the nuget extension and hinders any extension to exist (#12895 is precisely it), even if this is not "documented"; while we figure out how it will evolve and be documented in the future.

I feel the issues I've open, and the efforts that went into bringing into attention should be processed altogether on Microsoft end, so there is a clear picture, and meantime.

For a person like me, it is not the RFC that hinders the usefulness of extension mechanism, but more to hear what Microsoft or the community interested going beyond "nuget:" decided to do to improve the situation (beyond VS, once #12895 can be taken care), as a consensus.

@smoothdeveloper
Copy link
Contributor Author

TLDR: please don't close #12895 please, and it should be fixed first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: New
Development

No branches or pull requests

3 participants