search

fsharp / emacs-fsharp-mode

F# Emacs mode
Apache License 2.0
206 stars 62 forks source link

Draft version of using .NET tool rather than zip file for .NET core #288

Closed amcguier closed 2 years ago

amcguier commented 2 years ago

This PR would solve FSAutoComplete #883 by installing FsAutoComplete as a .NET tool using .NET itself rather than downloading and expanding binary zip, which wasn't included on the last release of FsAutoComplete.

Issuing as a draft PR to see if folk are interested in this approach before I fix the tests, etc. I have confirmed that this will work for FsAutoComplete releases >= 0.49.0.

Previous versions this could be made to work back to around FsAutoComplete ~ 0.42 but the name of the .NET tool changed so if that's desirable I'd need to conditionally check the version string and do a bit more testing.

juergenhoetzel commented 2 years ago

Issuing as a draft PR to see if folk are interested in this approach before I fix the tests, etc. I have confirmed that this will work for FsAutoComplete releases >= 0.49.0.

Yes :+1: this is definitely the way to go.

For some unknown reason the install hangs on my "Emacs 29.0.50" setup (zombie process):

  28109 ?        Ssl    0:00 /usr/bin/dotnet tool install fsautocomplete --tool-path /home/juergen/.emacs.d/FsAutoComplete/netcore/ --version 0.50.0
  28126 ?        Z      0:01 [dotnet] <defunct>
amcguier commented 2 years ago

Hmmm, I haven't fully tested under linux. I'm curious to know what happens if you navigate to that directory and run the command directly in a non emacs shell?

Also, using the same tool-path maybe try running dotent tool install dotnetsay ..... (with the same args) just to make sure it's not an issue with the tool install for fsautocomplete

juergenhoetzel commented 2 years ago

Hmmm, I haven't fully tested under linux. I'm curious to know what happens if you navigate to that directory and run the command directly in a non emacs shell?

I have narrowed down the problem to my Emacs 29.0.50 installation.

It works without problems using the latest Emacs stable release 27.2.

amcguier commented 2 years ago

I haven't abandoned this PR :-)

I have all the integration tests passing locally under linux, but they're failing for me on my CI builds on github. I'm suspicious it's a concurrency issue, I've noticed the 'publishDiagnostic' event comes back very, very quickly under new builds of fsautocomplete and I suspect what's happening is that event is coming back before the tests are looking for it.

Once I get that sorted I'll try installing 29.0.5 build and see if I can replicate (I'm testing under ElementaryOS if the distro matters)

juergenhoetzel commented 2 years ago

On Wed, 2022-02-16 at 09:37 -0800, Andrew McGuier wrote:

I haven't abandoned this PR :-)

Me too 😁️

I have all the integration tests passing locally under linux, but they're failing for me on my CI builds on github. I'm suspicious it's a concurrency issue, I've noticed the 'publishDiagnostic' event comes back very, very quickly under new builds of fsautocomplete and I suspect what's happening is that event is coming back before the tests are looking for it. Once I get that sorted I'll try installing 29.0.5 build and see if I can replicate (I'm testing under ElementaryOS if the distro matters) I bisected the root cause of this issue. It's the switch too posix_spawn in the Emacs master branch:

commit a60053f8368e058229721f1bf1567c2b1676b239 (HEAD, refs/bisect/bad)
Author: Philipp Stephani ***@***.***>
Date: Wed Dec 30 14:42:01 2020 +0100

Use posix_spawn if possible.
posix_spawn is less error-prone than vfork + execve, and can make
better use of system-specific enhancements like 'clone' on Linux. Use
it if we don't need to configure a pseudoterminal.
* configure.ac (HAVE_SPAWN_H, HAVE_POSIX_SPAWN)
(HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR)
(HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP)
(HAVE_POSIX_SPAWNATTR_SETFLAGS, HAVE_DECL_POSIX_SPAWN_SETSID): New
configuration variables.
* src/callproc.c (USABLE_POSIX_SPAWN): New configuration macro.
(emacs_posix_spawn_init_actions)
(emacs_posix_spawn_init_attributes, emacs_posix_spawn_init): New
helper functions.
(emacs_spawn): Use posix_spawn if possible.
razzmatazz commented 2 years ago

I bisected the root cause of this issue. It's the switch too posix_spawn in the Emacs master branch: commit a60053f8368e058229721f1bf1567c2b1676b239 (HEAD, refs/bisect/bad) Author: Philipp Stephani ***@***.***> Date: Wed Dec 30 14:42:01 2020 +0100 Use posix_spawn if possible. posix_spawn is less error-prone than vfork + execve, and can make better use of system-specific enhancements like 'clone' on Linux. Use it if we don't need to configure a pseudoterminal. * configure.ac (HAVE_SPAWN_H, HAVE_POSIX_SPAWN) (HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR) (HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP) (HAVE_POSIX_SPAWNATTR_SETFLAGS, HAVE_DECL_POSIX_SPAWN_SETSID): New configuration variables. * src/callproc.c (USABLE_POSIX_SPAWN): New configuration macro. (emacs_posix_spawn_init_actions) (emacs_posix_spawn_init_attributes, emacs_posix_spawn_init): New helper functions. (emacs_spawn): Use posix_spawn if possible.

Hey. I bisected the same thing -- "they" broke make-process by setting non-default sigmask for subprocesses (SIGCHLD is apparently masked), -- the fix is to:

lsp mode fix:

And actually this fixes fsautocomplete on macos emacs 28.0 too (as they switched to posix_spawn on 28.0 macos) -- linux switched only with emacs-29

There is a thread about this on emacs-devel:

They have elected not to add this patch/fix to master and/or emacs-28 (yet)

juergenhoetzel commented 2 years ago

Update:

razzmatazz commented 2 years ago

As I understand, at least on a mac, 28.1 (and maybe later versions) will still need the /bin/ksh workaround, right?

amcguier commented 2 years ago

Thanks for pulling the changes in. I've been getting crushed by client work and haven't been able to drive it home.

Once we get this in and things cool off, I'd love to take a crack at getting the type signature highlights a-la-Ionide in (and possibly some of the missing code fix actions)

juergenhoetzel commented 1 year ago

Saulius Menkevičius @.***> writes:

As I understand, at least on a mac, 28.1 (and maybe later versions) will still need the /bin/ksh workaround, right?

I have just added macos to the build matrix: https://github.com/juergenhoetzel/emacs-fsharp-mode/commit/2a8ec2b4ee69b96389adbe28902298ca4c8a8476 (not merged yes)