search

haskell / haskell-language-server

Official haskell ide support via language server (LSP). Successor of ghcide & haskell-ide-engine.
Apache License 2.0
2.71k stars 368 forks source link

Improve template haskell support (upstream fix and workaround for compiler crashes) #1431

Closed jneira closed 2 years ago

jneira commented 3 years ago

Description of the problem (from https://github.com/haskell/haskell-language-server/issues/469#issuecomment-726287701):

GHC has its own dynamic linker, in the rts, and it gets used in certain cases:

  • when GHC the binary is linked statically, by GHCI and TH
  • when ghc the library is linked statically, by TH and the ghc interactive api Implementing a dynamic linker with support for >=3 platforms is a risky and complex exercise. Fortunately there is an easy opt-out (-dynamic) but unfortunately that opt-out makes distribution much harder, and the precompiled HLS binaries are statically linked.

Hot to check if you hls binary is using the ghc dynamic linker (from https://github.com/haskell/haskell-language-server/issues/1982#issuecomment-873396136):

/path/to/haskell-language-server  +RTS --info | grep "RTS way" | grep dyn && echo "Dynamic" || echo "Static

@pepeiborra @wz1000 could we sketch what could be the needed changes (mainly in ghcide i suppose) to improve it? is it even a good idea?

pepeiborra commented 3 years ago

I don't use TH so I cannot really help much. My understanding is that the main issue is a bug in the GHC static linker for Mac OS, which causes segfaults. The best way to address this would be to enable using the system linker even when the ghc package is statically linked, lifting the restriction described here. But this is a GHC change and it would need some speccing out and a qualified mentor.

jneira commented 3 years ago

So maybe @wz1000?, what do you think about this possible line of work?

jneira commented 3 years ago

I am gonna remove the label for now, as this dont have an actionable outcome

jneira commented 3 years ago

All issues seems to be related with the ghc dynamic linker so i would label this as blocked upstream on https://gitlab.haskell.org/ghc/ghc/-/issues/19021 (opened in #469) and with the generic workaround of compile hls from source with this config in cabal.project or cabal.project.local

package haskell-language-server
  ghc-options: -dynamic
jneira commented 2 years ago

It seems a way to fix template haskell handling in ghc itself has been opened: see https://github.com/haskell/haskell-language-server/issues/2000#issuecomment-962160778 The link in the ghc issue tracker would be https://gitlab.haskell.org/ghc/ghc/-/issues/20628

GavinRay97 commented 2 years ago

All issues seems to be related with the ghc dynamic linker so i would label this as blocked upstream on gitlab.haskell.org/ghc/ghc/-/issues/19021 (opened in #469) and with the generic workaround of compile hls from source with this config in cabal.project or cabal.project.local

package haskell-language-server
  ghc-options: -dynamic

@jneira I asked @jkachmar and this should also be an easy option if you use ghcup right?

$ ghcup compile hls --patchdir $DYNAMIC_PATCHES -v $HLS_VERSION $GHC_VERSION

Where in the patch dir would be this (or something)?

hls-compile-dynamic-cabal.patch

index d1d06d9..e029c9d 100644
--- a/cabal.project
+++ b/cabal.project
@@ -36,6 +36,9 @@ package *
   ghc-options: -haddock
   test-show-details: direct

+package haskell-language-server
+  ghc-options: -dynamic

 write-ghc-environment-files: never

 index-state: 2021-10-04T02:41:06Z
GavinRay97 commented 2 years ago

(Just to confirm, because I know nothing of Haskell: I am not sure if this is correct -- does the thumbs-up mean "LGTM"? 😅)

jneira commented 2 years ago

yeah, thanks for noting it, another option is create a my.cabal.project with those contents 

package haskell-language-server
  ghc-options: -dynamic

and then use the ghcup compile hls --cabal-project-local my.cabal.project option

GavinRay97 commented 2 years ago

another option is create a my.cabal.project with those contents and use the ghcup compile hls --cabal-project-local my.cabal.project option

Oh this is super useful and probably easier than working with a .patch file since it's a regular cabal file. Thank you!

jneira commented 2 years ago

For completeness, there is an issue in the ghcup about just that workaround: https://gitlab.haskell.org/haskell/ghcup-hs/-/issues/245 Dont think it will have direct support, as it can be done as we commented above relatively easy with --patch or --cabal-project-local

jneira commented 2 years ago

About make hls prebuilt executables dinamically linked:

It is true that hls built dynamically linked provides a good workaround for a bad hls/ghc interaction, but it is a workaround after all. The proper fix should be done in ghc, to make possible evaluate th code in a reliable way independently on how the executable requesting that evaluation (hls in our case) is linked.

The relevant upstream issues where imho the definitive fix should be done are:

So i am a little bit reluctant to change our ci to make it produce dynamically linked binaries, having those ci changes and the executables built that way its own problems and issues and being the goal provide a temporary workaround.

Related with documentation we have added recently an entry in the troubleshooting section: https://haskell-language-server.readthedocs.io/en/latest/troubleshooting.html#problems-with-template-haskell and https://haskell-language-server.readthedocs.io/en/latest/troubleshooting.html#problems-with-dynamic-linking

Feel free to suggest more docs changes to make easier indentify the problem and find the workaround.

jkachmar commented 2 years ago

It appears that when haskell-language-server is compiled with ghcup [1] and a patchfile updating either cabal.project.local[2] or haskell-language-server.cabal[3] with the appropriate -dynamic flag, it fails to install the dylibs for any of the libraries that are included with this project:

ldd ~/.ghcup/bin/haskell-language-server-8.10.7 | grep "not found" ``` root@b80e05598361:/tmp/haskell-language-server# ldd /root/.ghcup/bin/haskell-language-server-8.10.7 | grep "not found" libHShls-tactics-plugin-1.4.0.0-inplace-ghc8.10.7.so => not found libHShls-stylish-haskell-plugin-1.0.0.2-inplace-ghc8.10.7.so => not found libHShls-splice-plugin-1.0.0.4-inplace-ghc8.10.7.so => not found libHShls-retrie-plugin-1.0.1.2-inplace-ghc8.10.7.so => not found libHShls-refine-imports-plugin-1.0.0.1-inplace-ghc8.10.7.so => not found libHShls-pragmas-plugin-1.0.1.0-inplace-ghc8.10.7.so => not found libHShls-ormolu-plugin-1.0.1.0-inplace-ghc8.10.7.so => not found libHShls-module-name-plugin-1.0.0.1-inplace-ghc8.10.7.so => not found libHShls-hlint-plugin-1.0.1.1-inplace-ghc8.10.7.so => not found libHShls-haddock-comments-plugin-1.0.0.3-inplace-ghc8.10.7.so => not found libHShls-fourmolu-plugin-1.0.0.2-inplace-ghc8.10.7.so => not found libHShls-floskell-plugin-1.0.0.1-inplace-ghc8.10.7.so => not found libHShls-explicit-imports-plugin-1.0.1.0-inplace-ghc8.10.7.so => not found libHShls-eval-plugin-1.1.2.0-inplace-ghc8.10.7.so => not found libHShls-class-plugin-1.0.1.0-inplace-ghc8.10.7.so => not found libHShls-call-hierarchy-plugin-1.0.1.0-inplace-ghc8.10.7.so => not found libHShls-brittany-plugin-1.0.1.0-inplace-ghc8.10.7.so => not found libHShaskell-language-server-1.4.0.0-inplace-ghc8.10.7.so => not found libHSghcide-1.4.2.0-inplace-ghc8.10.7.so => not found libHShls-plugin-api-1.2.0.1-inplace-ghc8.10.7.so => not found libHShls-graph-1.4.0.0-inplace-ghc8.10.7.so => not found libHShiedb-0.4.0.0-inplace-ghc8.10.7.so => not found libHShie-compat-0.2.1.0-inplace-ghc8.10.7.so => not found ```

However, when haskell-language-server is cloned, checked out to the equivalent tag (1.4.0), patched with the appropriate flags [2], and compiled via the normal manual method (./cabal-hls-install hls-8.10.7) all of the dylibs seem to be installed as-expected.

EDIT: Patching cabal.project to add ghc-options -dynamic for the HLS package works as well, so it's squarely a problem with however ghcup is building this I guess.

[1]:

ghcup compile hls \
  --version 1.4.0 \
  --jobs 4 \
  --patchdir /tmp/hls_patches \
  8.10.7

...where /tmp/hls_patches contains the appropriate patches (see below).

[2]:

cabal.project.local.patch 

diff --git a/cabal.project.local b/cabal.project.local
new file mode 100644
index 00000000..9ea1c780
--- /dev/null
+++ b/cabal.project.local
@@ -0,0 +1,2 @@
+package haskell-language-server
+  ghc-options: -dynamic

[3]:

haskell-language-server.cabal.patch 

diff --git a/haskell-language-server.cabal b/haskell-language-server.cabal
index 9ed034eb..ffff63e2 100644
--- a/haskell-language-server.cabal
+++ b/haskell-language-server.cabal
@@ -77,7 +77,7 @@ library
     , unordered-containers
     , aeson-pretty

-  ghc-options:      -Wall -Wredundant-constraints -Wno-name-shadowing -Wno-unticked-promoted-constructors
+  ghc-options:      -Wall -Wredundant-constraints -Wno-name-shadowing -Wno-unticked-promoted-constructors -dynamic

   if flag(pedantic)
     ghc-options: -Werror
@@ -318,7 +318,7 @@ executable haskell-language-server
   other-modules:    Plugins

   ghc-options:
-    -threaded -Wall -Wno-name-shadowing -Wredundant-constraints
+    -threaded -Wall -Wno-name-shadowing -Wredundant-constraints -dynamic
     -- allow user RTS overrides
     -rtsopts
     -- disable idle GC
@@ -386,7 +386,7 @@ executable haskell-language-server-wrapper
   other-modules:    Paths_haskell_language_server
   autogen-modules:  Paths_haskell_language_server
   ghc-options:
-    -threaded -Wall -Wno-name-shadowing -Wredundant-constraints
+    -threaded -Wall -Wno-name-shadowing -Wredundant-constraints -dynamic
     -- allow user RTS overrides
     -rtsopts
     -- disable idle GC

jneira commented 2 years ago

Afaik ghcup downloads hls from hackage or checking out the project with git, apply the patches and install it using cabal so dont know how that differ from doing it manually using the install script :thinking:

Have you tried the variant ghcup compile hls --cabal-project-local my.cabal.project being my.cabal.project just the entry to make the build dynamic?

package haskell-language-server
  ghc-options: -dynamic

//cc @maerwald just in case you have any clue about what could be happen (and if you can reproduce it)

hasufell commented 2 years ago

Yes, ghcup does not do cabal install, see https://gitlab.haskell.org/haskell/ghcup-hs/-/blob/c05876cc605b26c8e8b13b9f4bdf06ec388e1e09/lib/GHCup.hs#L856-879

The reason is, because cabal install is semi-broken when you install from git, since it caches based on PVP versions and will not always pick up changes from dependencies via source-repository-package. That's why we switched to cabal build. To make both use cases work, there are only these options:

  1. Use cabal install, but with an isolated store-dir, which means every time you compile HLS from ghcup, you compile the entire world
  2. Use cabal build, but keep the build directory somewhere... not sure if we also need a wrapper script then, so the dynlibs are discovered

Both options are rather annoying.

jneira commented 2 years ago

The reason is, because cabal install is semi-broken when you install from git, since it caches based on PVP versions and will not always pick up changes from dependencies via source-repository-package

We have a ticket about that?, it is sad to have to use ugly workarounds downstream The code branch which downloads hls from hackage could use cabal install safely i think. For now we dont have source-repository-package in the main cabal.project so coincidentally it will not trigger the problem. Dont work for the general case of course.

Maybe we could invalidate artificially the cache only for source-repository-package's, if there is any?

hasufell commented 2 years ago

For now we dont have source-repository-package in the main cabal.project so coincidentally it will not trigger the problem.

I think the problem also exists for anything listed in packages, which is the case for e.g. ghcide.

Maybe we could invalidate artificially the cache only for source-repository-package's, if there is any?

I don't think we should. Messing with existing cache can wreak havoc.

jneira commented 2 years ago

hi, thanks to @hasufell we have a new version of ghcup with some relevant changes in ghcup compile hls:

So we could build dinamically hls with:

ghcup compile hls -g master --ghc 8.10.7 -- --ghc-options='-dynamic'

or

ghcup compile hls -g master --ghc 8.10.7 --cabal-project-local https://gist.githubusercontent.com/jneira/c0b0dc1297907a5a139c4ecf4eb6ccd3/raw/c97c97181182b15455cb3969abac2dc164d74b91/cabal-dynhls.project.local

@jkachmar would be great if you could test the new commands (the first is more straightforward and i suppose they are equivalent)

hasufell commented 2 years ago

(the first is more straightforward and i suppose they are equivalent)

They are not. The first applies -dynamic to all packages (and will likely rebuild the entire world), the second only to haskell-language-server.

jneira commented 2 years ago

Another option could be

ghcup compile hls -g master --ghc 8.10.7 -- --enable-executable-dynamic

In theory it should have the same effect than the version with cabal-dynhls.project.local but never i am sure about those cabal things without checking it

EDIT: I've tried cabal install exe:haskell-language-server --enable-executable-dynamic and the behavior is the same than using cabal-dynhls.project.local

jneira commented 2 years ago

@adamse is working in a ghc version which can improve th support.

I've collected some reproducer cases for th issues, posting them here as well:

Revising the issues labeled with template haskell related i've found some:

There will be some others in others th related issues. Sorry i cant verify all those reproducers are up to date and being correct.

nh2 commented 2 years ago

Also:

nh2 commented 2 years ago

I tried the workaround https://github.com/haskell/haskell-language-server/issues/1431#issuecomment-971811045 in my nix environment setting this line:

https://github.com/NixOS/nixpkgs/blob/f366af7a1b3891d9370091ab03150d3a6ee138fa/pkgs/development/tools/haskell/haskell-language-server/withWrapper.nix#L14

to haskell.lib.enableSharedExecutables.

However, in my nix-shell, I got this error:

Consulting the cradle to get project GHC version...
Project GHC version: 8.10.7
haskell-language-server exe candidates: ["haskell-language-server-8.10.7","haskell-language-server"]
Launching haskell-language-server exe at:/nix/store/ri354r8xy2xrlagrj5aljrgawg23dcyb-haskell-language-server-1.4.0.0/bin/haskell-language-server-8.10.7
/nix/store/ri354r8xy2xrlagrj5aljrgawg23dcyb-haskell-language-server-1.4.0.0/bin/haskell-language-server-8.10.7: error while loading shared libraries: libHSCabal-3.2.1.0-ghc8.10.7.so: cannot open shared object file: No such file or directory
haskell-language-server-wrapper: callProcess: /nix/store/ri354r8xy2xrlagrj5aljrgawg23dcyb-haskell-language-server-1.4.0.0/bin/haskell-language-server-8.10.7 (exit 127): failed

So the error is libHSCabal-3.2.1.0-ghc8.10.7.so: cannot open shared object file: No such file or directory.

Reason it's in ldd twice, and the first reference is not found:

% ldd /nix/store/ri354r8xy2xrlagrj5aljrgawg23dcyb-haskell-language-server-1.4.0.0/bin/haskell-language-server-8.10.7 | grep HSCabal
    libHSCabal-3.2.1.0-ghc8.10.7.so => not found
    libHSCabal-3.2.1.0-ghc8.10.7.so => /nix/store/673ic4dsw35wmqwq59z821l67fjfix8w-ghc-8.10.7/lib/ghc-8.10.7/Cabal-3.2.1.0/libHSCabal-3.2.1.0-ghc8.10.7.so (0x00007f1c98094000)

Anybody happen to know what's up here?

Very ugly workaround:

Given that all these .so are there twice (once not found, once found), I just grepped for all of the not found one, took the paths from their found equivalents, and put them in an LD_LIBRARY_PATH in my shell.

This, together with https://github.com/haskell/haskell-language-server/issues/365#issuecomment-981607251, apparently makes HLS work completely for my larger project that uses a lot of inline-c.

jneira commented 2 years ago

I am gonna close all th issues about compiler crashes but this and #2000 as all of them seems to have the same root cause

jfraudeau commented 2 years ago

It looks like this was fixed in release 1.7.0.0 by #2675 and #2431