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

Update FST-1027-fsi-references.md #659

Merged
merged 3 commits into from
Jun 24, 2022
Merged

Conversation

smoothdeveloper
Copy link
Contributor

Just clarifying the status on "Handler Resolution", for now.

Click “Files changed” → “⋯” → “View file” for the rendered RFC.

Just clarifying the status on "Handler Resolution", for now.

By convention, tools that load extensions, should never fail to do so for binding redirect matters related to FSharp.Core version the extensions must have been compiled against.

Note: additional stable locations were considered by not implemented in earlier drafts of this RFC.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by --> but

Could you check the whole of the rest of the RFC and see if there are incorrect things that should be removed or moved to an "Alternatives" section please? Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dsyme, I've made few more edits on what seems inaccurate; but for whole which remains in Runtime section, I am not sure if it matches the reality and I didn't author those details.

It is likely some of those aspects of the RFC were done before/during "how does this play with dotnet core and how reference assemblies for netstandard stuff work with FSI" period.

For the RFC, I think it is @KevinRansom and @cartermp that can help farther narrow down this particular RFC if needed.

Some general remarks about RFCs, maybe they should only give overview from end user perspective; aside, we could have a postmortem document of RFC that made it to production, focused on summarising key aspects of the implementation details and considerations (here, it would be a documentation on the reflection bits as they are used to make the extension system work).

@KevinRansom
Copy link
Contributor

This requirement is specifically about how fsi finds compiler extensions like the packagemanager handlers.
It should be trivially possible today using the --compilertool: option.

Probably the most straightforward approach since having a developer specify that on the fsi command line is not easy we could probably add a #r command that downloads a package manager from nuget org and uses it.

However, the desktop compiler as shipped with VS needs to be deprecated, I've been trying for years, but it is hard because the desktop compiler is good. So the best solution is probably to allow dotnet global tools to contain package managers somehow. We probably need to think what the heuristic for that would look like.

As for the specific issue that didn't work. When VS went 64-bit we had to move the compiler tools from :

C:\Program Files\Microsoft Visual Studio\2022\Preview\Common7\IDE\CommonExtensions\Microsoft\FSharp

to:
C:\Program Files\Microsoft Visual Studio\2022\Preview\Common7\IDE\CommonExtensions\Microsoft\FSharp\Tools

So if you drop the tool there then it will work again, at least for the desktop compiler,

@smoothdeveloper
Copy link
Contributor Author

@KevinRansom

It should be trivially possible today using the --compilertool: option.

Confirming this works everywhere I tried, FSI, FCS, similarly it works everywhere (but in VS and also Rider) when you copy an arbitrary extension next to the one Microsoft ships for #r "nuget: ...".

However, the desktop compiler as shipped with VS needs to be deprecated

Does it have an impact with the extension system, which interpreter or runtime it is? Maybe the fine details about it could be reprecised with the current status.

In any case, I have been using extension system without issue with fsi.exe/fsiAnyCpu.exe that ships with VS and the one that comes with "dotnet sdk".

we could probably add a #r command that downloads a package manager from nuget org and uses it.

I refer to this in dotnet/fsharp#8880: Another approach would be for referenced assemblies to be probed as well, it would enable using...

So the best solution is probably to allow dotnet global tools to contain package managers somehow.

It would be consistent with efforts within MSFT that aimed to ship the feature for FSI & VS + Dotnet Interactive; agreed it would be a good move and a good first entry to the UX behind those extensions.

@dsyme
Copy link
Contributor

dsyme commented Jun 22, 2022

@smoothdeveloper Are you happy with what's here as suitably clarifying, given Kevin's comments above? Thanks! :)

@smoothdeveloper
Copy link
Contributor Author

@dsyme I am happy with update, it takes the RFC closer to the actual situation for handler resolution, and preserves the suggested approach, which hasn't been mulled over / implemented.

Main concern for me is still dotnet/fsharp#12895

@dsyme dsyme merged commit bc295ae into fsharp:main Jun 24, 2022
@dsyme
Copy link
Contributor

dsyme commented Jun 24, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants