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

haskell-language-server: Init wrapper for multiple ghc versions at 0.5.0 #99519

Merged
merged 5 commits into from Oct 10, 2020

Conversation

@maralorn
Copy link
Contributor

@maralorn maralorn commented Oct 4, 2020

Motivation for this change

haskell-language-server needs a binary which is compiled with the ghc the user uses for their project. That's why hls normally get's distributed (e.g. via ghcup) with the haskell-language-server-wrapper that picks the right binary based on the current project and a binary for every supported ghc version.
I want hls support in nixpkgs to be as good as that, so we also need to provide all those binaries.

I am introducing pkgs.haskell-language-server here which has a overridable supportedGhcVersions list.

I have fixed the closure size with reference-to. A glance at the binary made a reasonable impression that those references aren‘t needed. A quick test of the binary seems to do just fine.

\cc @peti @GuillaumeDesforges @cdepillabout

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@maralorn
Copy link
Contributor Author

@maralorn maralorn commented Oct 4, 2020

I have reduced the closure size to a reasonable ~330 MB. One hls binary is about 95 MB.

I am eager for feedback.

Copy link
Member

@cdepillabout cdepillabout left a comment

I don't use haskell-language-server, but this looks pretty good to me.

If I could make one suggestion, it would be to have some documentation in development/tools/haskell/haskell-language-server/withWrapper.nix that gave a short explanation of how this is supposed to be used.

For instance, if I was new to nixpkgs, my first thoughts would be:

  • How do I use haskell-language-server from nixpkgs?
  • Is it sufficient to do nix-shell -p cabal -p ghc -p haskell-language-server and just start using it? Or is there some other sort of setup I need?
  • It appears that haskell-language-server installs different versions, so will it basically just automatically work with different ghc versions? Or do I have to do something so this works? If I want haskell-language-server for a different ghc version, do I have to override the haskellPackages argument to development/tools/haskell/haskell-language-server/withWrapper.nix?

I guess it would be nice to have this in the official documentation as well, but personally I'd be fine with just documenting it here in the code.

@maralorn
Copy link
Contributor Author

@maralorn maralorn commented Oct 5, 2020

Yeah, there is definitely documentation missing. I actually would go all in and add a section to nixpkgs manual. First I need to learn how to do that.

@Cmdv Cmdv mentioned this pull request Oct 6, 2020
6 of 10 tasks complete
@peti peti force-pushed the NixOS:haskell-updates branch 3 times, most recently from be2e4fb to 1436509 Oct 9, 2020
maralorn added 3 commits Oct 4, 2020
@maralorn maralorn force-pushed the maralorn:hls-wrapper branch 2 times, most recently from 4a4d0cb to c26ce5e Oct 9, 2020
@maralorn maralorn force-pushed the maralorn:hls-wrapper branch from c26ce5e to 7052076 Oct 9, 2020
@maralorn
Copy link
Contributor Author

@maralorn maralorn commented Oct 9, 2020

I have added a documentation section. Would love some feedback if it's all clearly understandable.

@maralorn
Copy link
Contributor Author

@maralorn maralorn commented Oct 10, 2020

I could have add 9.0.1 but I want to do that in a subsequent PR, because right know tons of packages are broken on 9.0.1.

Copy link
Member

@cdepillabout cdepillabout left a comment

Easy to understand! Looks good!

maralorn added 2 commits Oct 10, 2020
@maralorn maralorn merged commit 0756b8a into NixOS:haskell-updates Oct 10, 2020
19 checks passed
19 checks passed
tests tests
Details
action
Details
haskell-language-server, haskell-language-server.passthru.tests, haskellPackages.haskell-language-server, haskellPackages.haskell-language-server.passthru.tests on aarch64-linux Failure
Details
haskell-language-server, haskell-language-server.passthru.tests, haskellPackages.haskell-language-server, haskellPackages.haskell-language-server.passthru.tests on x86_64-linux Timed out, unknown build status
Details
Evaluation Performance Report Evaluator Performance Report
Details
Wait for ofborg
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="97d78f8"; rev="97d78f8616d8025ad9de65ed242048f45d35252d"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="97d78f8"; rev="97d78f8616d8025ad9de65ed242048f45d35252d"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="97d78f8"; rev="97d78f8616d8025ad9de65ed242048f45d35252d"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="97d78f8"; rev="97d78f8616d8025ad9de65ed242048f45d35252d"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="97d78f8"; rev="97d78f8616d8025ad9de65ed242048f45d35252d"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="97d78f8"; rev="97d78f8616d8025ad9de65ed242048f45d35252d"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="97d78f8"; rev="97d78f8616d8025ad9de65ed242048f45d35252d"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@maralorn maralorn deleted the maralorn:hls-wrapper branch Oct 10, 2020
siraben added a commit to siraben/nixpkgs that referenced this pull request Oct 17, 2020
…5.0 (NixOS#99519)

* haskell-language-server: Init wrapper for multiple ghc versions at 0.5.0

* Fix closure size

* docs: Add hls section to Haskell part of manual
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.