-
Notifications
You must be signed in to change notification settings - Fork 802
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
FSI assembly resolution logic doesn't work for self-contained FCS scenarios. #7701
Comments
I'd be willing to work on a refactor/rewrite of this code, but I'd like to get a get set of requirements before running down that path. It seems that for the .net core side, you need two things in order to get the full set of necessary refs:
If we implemented a function |
cc @KevinRansom |
I will add support for a dotnet_hostpath environment variable. The host app will be responsible for ensuring that path is set to dotnet.exe. This is what the dotnet sdk uses as it's final fallback to find dotnet.exe. Do you think that will work? |
I've got a pretty fully fleshed out implementation in FSAC that was based off of the FCS version, was going to PR it here if you all thought it was reasonable: https://github.com/fsharp/FsAutoComplete/blob/master/src/FsAutoComplete.Core/FSIRefs.fs The main entrypoint is the function where you pass in a dotnet root, sdk version, and runtime version and get the full reference set. This seems to be working quite well for us in tests so far. |
So … DotNetFrameworkDependencies is an implementation detail of fsi. The probing it does and resolutions that it reaches are not intended to be made available to hosts. Those resolutions are certainly not expected to be a contract to which we wish to hold, or even support. We are satisfied with the probing we do because we know that if the dotnet sdk change stuff, we will be broken and know it before we ship - because we ship inside the sdk - which gives us time to fix things, that is not something we can do when the probing ships externally to the S; in fact: I think that we will probably have exactly this problem with the notebook variant of fsi, which will ship at least initially as a global tool. The correct way for a host to determine what assemblies to be considered is for it to collect what it thinks the assemblies for references and execution need to be based on user preferences and the platform it knows it is running on and then passing them to the FSI session, that way the code DotNetFrameworkDependencies shouldn't be relied on to grab dependencies. This is how fsi.exe should have been originally, however, even with several refactors it still isn't. Primarily because of desktop compatability I suppose. Anyway, your PR looks like a fine Api, for you to provide separate from the dotnet/fsharp, in order to collect the dependencies you need for hosts. However, we wouldn't want it to be added in this repo, because it essentially, provides an API that the dotnet SDK should provide and wraps it in fsi specialness. I would note that dotnet.exe can be xcopy deployed so the two well known locations provided in your example |
I suppose that if you embed FSI you take it on yourself to provide a set of reference/impl assemblies. I was speaking primarily from the point of view of editor tooling, which whould naturally want to remain as close to the dotnet sdk deployment strategies as possible. In this way I would think that FSAC and VS are very similar. So I was hoping we could come to an API that both could use and benefit from, irrespective of any other non-editor-tooling consumers of the compiler as a service. |
@baronfel, I get that getting a list of framework assemblies is desirable, however, that is an API that the SDK needs to provide, I am reluctant to provide one. For sure if they had one, I would use it :-) For tests we just run msbuild over a project and dump the referenced paths to a file, and use that. I think an App would want a better API than. I don't think providing that API as a durable commitment is one that the F# team should do, it really depends on the host. |
The thing that strikes me most about this is now the entire |
I managed to make FSharp.Compiler.Services work with FSharp.Literate inside dotnet fsi for the doc generation of the RegexProvider, but I had to force the loading using a nasty hack. By forcing some code execution at the begining of the script, the Assembly is correctly resolved. Once loaded the rest of the script is running as expected. But without it, the Assembly resolution fails miserably.. Here is the script with the hack: fsprojects/FSharp.Text.RegexProvider#31 |
This should be addressed by: #7725. |
Cool, thanks for taking a crack at it. It seemed fine to me when I took a look after the merge, though the logic changed from taking every dll in the pack for to just the dependencies of netstandard.dll. I'll get master merged through FCS and compare. Thanks again! |
@baronfel, those dependencies weren't really the problem. The issue, I think is that it couldn't find the reference assemblies in a wide enough range of scenarios. Whilst the new heuristic is quite horrible. It's pretty easy to reason about and is likely to remain quite stable over time. |
The logic in
DotNetFrameworkDependencies
is written assuming that thegetFSharpCompilerLocation
is inside the dotnet SDK root directory tree. This is fine for the compiler, but for FCS deployed as a component in some other application (ie. FSAC):getFSharpCompilerLocation
is not inside this treegetFrameworkRefsPackDirectoryPath
no longer validInstead, the full assemblies from the
shared
directory are used. As a side effect of thegetImplementationReferences()
code path being used, it becomes necessary to insert a manual reference tomscorlib.dll
, without which no fsx script will typecheck. This wouldn't be necessary if thepacks
codepath was used, because that codepath yields every DLL in thepacks
folder.Repro steps
backgroundCompiler.GetProjectOptionsFromScript()
with a valid file, settinguseSdkRefs
to true andassumeDotNetFramework
to false.OtherOptions
by filtering all options that start with-r:
.Expected behavior
The refs are supplied from the
pack
references, in my case/usr/local/share/dotnet/packs/Microsoft.NETCore.App.Ref/3.0.0/ref/netcoreapp3.0
Actual behavior
The refs are supplied from the
full
references, in my case/usr/local/share/dotnet/shared/Microsoft.NETCore.App/3.0.0/
Known workarounds
Reimplement reference location :(
Related information
In addition, these APIs are very hard to use in a cross-TFM manner. They are tightly bound to the execution TFM of the currently-running process so you can't for example ask for the references set of a
.netcoreapp3.0
script while running FCS in a.netcoreapp2.1
application. This problem doesn't exist when asking fornet4x
references because those are an immutable set and are just magically resolved by the system by name.Provide any related information (optional):
The text was updated successfully, but these errors were encountered: