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

dotnet fsi can't be started on Windows #1087

Closed
KillyMXI opened this issue Apr 30, 2019 · 16 comments
Closed

dotnet fsi can't be started on Windows #1087

KillyMXI opened this issue Apr 30, 2019 · 16 comments
Labels
enhancement Accepted suggestions that makes existing features better

Comments

@KillyMXI
Copy link

Describe the bug
With VS2019 and with .NET Core SDK 2.2 comes an fsi.exe that has to be run like this:
dotnet fsi
(Microsoft (R) F# Interactive version 10.4.0 for F# 4.6)
That means there is no fsiFilePath that is called directly.
This extension settings don't take it into account yet.
There is one suggested way to make it work:

"FSharp.fsacRuntime": "netcore",
"FSharp.fsiFilePath": "C:\\Program Files\\dotnet\\dotnet.exe",
"FSharp.fsiExtraParameters": ["fsi"]

It works on some platforms, but not Windows. It also hacky since the parameter names doesn't match their usage ("fsiFilePath" is not fsi, "fsiExtraParameters" is not extra).

Something has to be done to support the new way of calling FSI.

To Reproduce
Steps to reproduce the behaviour:

  1. Configure settings.json as shown above;
  2. Command Palette -> FSI: Start

Expected behaviour
FSI terminal opened.

Observed behavior
Following error message shown:

The terminal process command 'C:\Program Files\dotnet\dotnet.exe --fsi-server-input-codepage:65001 fsi' failed to launch (exit code: 1)

On Windows, there is an extra parameter added to the head of the parameters list, making it impossible to construct valid command for launching FSI.
Corresponding line in the code:
https://github.com/ionide/ionide-vscode-fsharp/blob/master/src/Components/Fsi.fs#L60

https://i.imgur.com/c7OvSbv.png

Environment (please complete the following information):

  • OS: Windows 10
  • Ionide version: 3.35.0
  • VSCode version: 1.33.1
  • dotnet SDK version: 2.2.202
  • mono / .Net Framework version: (4.6.1)

Known workaround
Make an fsi.cmd file with following content:

@echo off
dotnet.exe fsi %*

Set it as "fsiFilePath", set "fsiExtraParameters" empty.

@vbfox
Copy link
Contributor

vbfox commented May 6, 2019

While I agree that supporting the dotnet core fsi binary would be interesting to try it out, it has nothing much to do with windows itself and should absolutely not be the default as it's not officialy released.

The version of dotnet fsi shipped in VS 2019 isn't final and users should not be driven toward it. It will be officialy released in .Net Core 3.0 ( See F# 4.6 announcement for more details https://devblogs.microsoft.com/dotnet/announcing-f-4-6/ )

Also once it's here Ionide would need a way for users to choose the version of fsi being used (.Net FW and .Net Core) as they are very incompatible (And there is no fsproj here to drive what should be used).

@KillyMXI
Copy link
Author

KillyMXI commented May 10, 2019

@vbfox the link you've provided has nothing to back your statement about anything being "not final".

.Net Core version of FSI was considered "good enough" by developers to be shipped in generally available versions of .Net Core (2.1, 2.2) and VS2019 with F# 4.6.

I know there are some issues with it though - I'm myself waiting for the next Core SDK update (it should be 2.2.300) to get some stuff fixed.

If there are blocking issues upstream - they are better to be stated clearly and tracked somewhere.
And if it's not something dependant on underlying runtime - it will be included with all currently supported Core versions updates.

I'm not convinced you have to wait until 3.0 release to start thinking about Core support. Previews are coming one after another already. And it is quite disappointing to see the lack of fleshed-out responses here. It has to be implemented sooner or later, so I would've expected rough roadmap at least already.

(This issue is just my perspective at the current state. For the grand task of .Net Core support tracking, a different issue can be opened. Or I can rename this one with a more appropriate name if it will get traction instead.)

@KillyMXI KillyMXI changed the title FSI can't be started on Windows (dotnet core) FSI can't be started on Windows May 11, 2019
@cartermp
Copy link
Contributor

Although the latest previews of 2.1.x, 2.2.x, and 3.0-preview5 all have dotnet-fsi working just as good as it would work with desktop FSI, the decision to use that as the default isn't a bug but a policy question. If the policy is that things are not on by default unless the tech is out of preview, then I recommend no action be taken until when .NET Core 3.0 ships.

@KillyMXI
Copy link
Author

You might not ship the thing until 3.0 ships. Just let the users know why (no guesses please).
This question keep surfacing again and again and the only answer is a half-working workaround in someones blog post.
This is a communication failure.

Honestly, it feels like Ionide can't keep up the pace, not being a Microsoft project and with limited maintainers time. I hope Microsoft can consider it important enough to pick up as they did with Python VSCode extension.

@voronoipotato
Copy link
Contributor

It's in preview is the reason. See hyper's issue log if you want to know why people do this. Hyper released immediately to make people happy and now everyone is miserable. Good software takes time.

@KillyMXI
Copy link
Author

Links please.
Where is it defined as being in preview while being a part of generally available distributions? I honestly can't see.

And again, silence serves no good.
I'd be happy enough to know from maintainers that things are under control.
So far I only got non-answers.

@vbfox
Copy link
Contributor

vbfox commented May 13, 2019

Where is it defined as being in preview while being a part of generally available distributions? I honestly can't see.

FSI status in .Net Core distributions of F# has been long and complicated (partially because a version was erroneously included in .Net Core some times ago before it was even working) but if Phillip Carter (The program manager for F# at Microsoft) say it's in preview it is. He is the one calling the shots after all.

I'd be happy enough to know from maintainers that things are under control.

Ionide is a community project, if something in it isn't to your taste (Missing feature, bug, the color of the logo is annoying) you have 2 choices, either you change the code or you convince someone to do the change for you.

Follows that .Net core FSI will be supported out of the box when someone PR that change and the maintainer (Krzysztof) accept it.

Essentially thanks for volunteering to do the change, everyone is awaiting your PR 😉

@KillyMXI
Copy link
Author

You're asking someone who haven't been involved much in development of any VSCode extension yet. And quite new to F# too (Touched it only a bit before, decided to give it a try for a small project after I found tools for Haskell in inferior shape).

Sure, why not.
But make it look like a "good first issue" then.
It would increase the chance I decide to dig into it if I could see the amount of things to be changed.

Looks like it's just the usual lack of working hands. Looks like .Net Core 3.0 release has not much to do with it after all. Almost an arbitrary point in time before which everything FSI is considered lowest priority, since there is no better idea when it becomes "good enough".

Trying to extract useful bits:

... supporting the dotnet core fsi binary would be interesting to try it out, [but] ... should absolutely not be the default as it's not officialy released.

Also once it's here Ionide would need a way for users to choose the version of fsi being used (.Net FW and .Net Core) as they are very incompatible (And there is no fsproj here to drive what should be used).

  • need FSharp.fsiCoreFilePath in addition to FSharp.fsiFilePath
  • might need to ask user which one to use and save that in workspace settings FSharp.fsiRuntime
  • fsproj files might be within the workspace - might be used to provide the default choice?
  • knowing which version(s) installed in the system and available through PATH env - would help too?
  • relation with FSharp.fsacRuntime - can they be different in any realistic scenario?
  • maybe provide separate commands to run either version of FSI

No idea about incompatibilities between two versions and what implications they have for this issue. I only have core version and will have to look up where to get the classic one.

Even if my understanding is correct and that covers all the work to be done - there is no guarantee I will follow it further. But I hope we end up with something more approachable to anyone who also need this feature working.

@baronfel
Copy link
Contributor

The good news is that with the dotnet SDK you don't need to worry about points 3, 4, 5 in your list. This is because there's already a mechanism with the dotnet sdk to pin specific versions (which effectively acts as a pin on the version of dotnet fsi), which is the global.json file. Ionide already implicitly uses this file when spawning any tool that needs the dotnet command, as would be the case here.

So at a high level we need:

  • a config option to choose between .net/mono FSI and .net core FSI
  • in the existing Core/LanguageService.fs:fsi and/or Components/Fsi.fs:start functions, these functions need to pivot off of the value of that FSI-choosing config. In the case of .Net/mono FSI the existing logic would be kept. In the case of .Net Core FSI, we would need to spawn dotnet fsi instead.

It's not a gigantic change, just a little finicky.

@voronoipotato
Copy link
Contributor

As @baronfel demonstrated we can help you contribute and give you advice along the way, or perhaps you can put a bounty on it. In F/OSS things come from each according to their need and ability, to all. If you can't do it and you can't afford to support someone doing it, it may not get done right away. This doesn't mean what you want to do isn't valuable, I think it is quite valuable. Without contributions valuable things may not get done in a timely manner. It's important that it stay that way if we want our community to have a say in the growth of the language and tooling.

@KillyMXI
Copy link
Author

All the usual talk about contribution process - that's obvious, needless to repeat.
Partly my failure that it took so long to get to baronfel's comment.
The comment actually explains what lies behind the issue. No fluff. I know where to dive and what skills needed. The rest is the matter of time and motivation - how much I need this feature to put it before other things I have on the table. At this point I would rather accepted bounty myself, or focus on getting other stuff out...
But as I said, since it is fleshed out now - there will be less friction for anyone who will approach it.

@voronoipotato
Copy link
Contributor

Apologies, your responses led me to believe it was non obvious. Best of luck!

@enricosada enricosada changed the title (dotnet core) FSI can't be started on Windows dotnet fsi can't be started on Windows May 16, 2019
@Krzysztof-Cieslak Krzysztof-Cieslak added the enhancement Accepted suggestions that makes existing features better label Jun 24, 2019
@9072997
Copy link

9072997 commented Dec 8, 2019

as a workaround I threw

dotnet fsi %*

in a bat file and am using that as my FSI file path. It seems to be working.

@Krzysztof-Cieslak
Copy link
Member

This is fixed in latest releases of Ionide. To use dotnet fsi with Ionide set useSdkScripts to true in settings.

@eiriktsarpalis
Copy link

@Krzysztof-Cieslak Do you think it might make sense to make this the default configuration?

@Krzysztof-Cieslak
Copy link
Member

@eiriktsarpalis, yes it's the plan. I think we'll give people a few more weeks to move to SDK 3.x and then change it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Accepted suggestions that makes existing features better
Development

No branches or pull requests

8 participants