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 at 0.1.0.0 #91279

Merged

Conversation

@GuillaumeDesforges
Copy link
Contributor

GuillaumeDesforges commented Jun 22, 2020

Motivation for this change

Add haskell-language-server to nixpkgs
https://github.com/haskell/haskell-language-server

Things done (after WIP)
  • 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

maralorn commented Jun 22, 2020

Hey @GuillaumeDesforges,

cool that you are working on this.

I am a bit apprehensive about deploying a software for which the developers intentionally decided not to do a release yet. Especially since we might expected additional maintenance overhead. But that‘s not on me to decide. I‘d actually love to test hls, so why not.^^

I will just give you a bit of feedback inline. I hope that‘s okay even if this is still WIP. Feel free to ignore.

@maralorn
Copy link
Contributor

maralorn commented Jun 22, 2020

btw. can you please make haskell PRs against the haskell-updates branch? It will get merged into master once a week, if we are sure that everything works fine.

@GuillaumeDesforges GuillaumeDesforges changed the base branch from master to haskell-updates Jun 23, 2020
@GuillaumeDesforges
Copy link
Contributor Author

GuillaumeDesforges commented Jun 23, 2020

Thanks for the review @maralorn , it is always appreciated !

As it is a WIP PR I might not apply everything right now (since it's still changing/moving around a lot) but once it stabilizes I will properly do all the above. Let's keep things clean!

@GuillaumeDesforges GuillaumeDesforges force-pushed the GuillaumeDesforges:haskell-language-server branch from 5b1f99d to f6fe619 Jun 23, 2020
@GuillaumeDesforges
Copy link
Contributor Author

GuillaumeDesforges commented Jun 24, 2020

@maralorn Sorry to bother you, but it seems like ./Setup build from buildPhase makes a dist folder... while ./Setup test expects a dist-newstyle folder. Does it ring a bell to you ?

https://gist.github.com/GuillaumeDesforges/7da3253decd1e5b3ac8776cd0f184747

running tests
Running 1 test suites...
Test suite tests: RUNNING...
tests: dist-newstyle: getDirectoryContents:openDirStream: does not exist (No such file or directory)
Test suite tests: FAIL
@maralorn
Copy link
Contributor

maralorn commented Jun 24, 2020

Can‘t say that I have encountered this one before.

I mean the general issue is the difference between cabal v1-build (which uses dist) and cabal v2-build (which uses dist-newstyle). So maybe you can fix this by using ./Setup v2-build or something?

@GuillaumeDesforges
Copy link
Contributor Author

GuillaumeDesforges commented Jun 25, 2020

I'm not sure if it is a Nixpkgs issue (for instance related to a cabal version/haskell wrapping voodoo) or an issue from lsp-test itself.
As of now, I'll put dontCheck on this package. Note that it is the lsp-test_0_11_0_2, not lsp-test, so it has a narrow scope of impact.

It seems to work, let's squash and go

@GuillaumeDesforges GuillaumeDesforges force-pushed the GuillaumeDesforges:haskell-language-server branch from 58be0bc to 53cf823 Jun 25, 2020
@GuillaumeDesforges GuillaumeDesforges marked this pull request as ready for review Jun 25, 2020
@GuillaumeDesforges GuillaumeDesforges force-pushed the GuillaumeDesforges:haskell-language-server branch from 53cf823 to 90e9a40 Jun 25, 2020
@GuillaumeDesforges
Copy link
Contributor Author

GuillaumeDesforges commented Jun 25, 2020

Rebased

@GuillaumeDesforges GuillaumeDesforges changed the title [WIP] haskell-language-server: init at 0.1.0.0 haskell-language-server: init at 0.1.0.0 Jun 25, 2020
Copy link
Member

cdepillabout left a comment

@GuillaumeDesforges Thanks for working on this!

I think this is looking pretty good. I left a few comments on things to fix up, but in general this is looking good.

pkgs/development/haskell-modules/hackage-packages.nix Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
@GuillaumeDesforges
Copy link
Contributor Author

GuillaumeDesforges commented Jun 25, 2020

Thanks @cdepillabout for the feedback, will do it right away 👍

@GuillaumeDesforges
Copy link
Contributor Author

GuillaumeDesforges commented Jun 26, 2020

TODO

  • use a update.sh script to generate pkgs/development/tools/haskell/haskell-language-server/default.nix and pkgs/development/tools/haskell/haskell-language-server/hls-ghcide.nix
  • make override in pkgs/development/haskell-modules/configuration-common.nix
  • regenerating pkgs/development/haskell-modules/hackage-packages.nix with the matching all-cabal-hashes version
@maralorn
Copy link
Contributor

maralorn commented Jun 26, 2020

* [ ]  regenerating `pkgs/development/haskell-modules/hackage-packages.nix` with the matching all-cabal-hashes version

Let me know if you have problems with that. We introduced ./regenerate-nixpkgs.sh to cabal2nix that should help a lot with this job and I would be really interested if it is helpful for other people.

@peti peti force-pushed the NixOS:haskell-updates branch 3 times, most recently from 2afb81a to c7cadae Jun 26, 2020
@GuillaumeDesforges GuillaumeDesforges force-pushed the GuillaumeDesforges:haskell-language-server branch from 808f974 to d36627d Jun 29, 2020
@GuillaumeDesforges
Copy link
Contributor Author

GuillaumeDesforges commented Jun 29, 2020

Lost a lot of time when trying to automate the update process since cabal2nix was not fetching the same revision of the ghcide fork as the one used by hls, but now this looks pretty good!

@GuillaumeDesforges GuillaumeDesforges requested a review from cdepillabout Jun 29, 2020
@maralorn
Copy link
Contributor

maralorn commented Jun 29, 2020

@GuillaumeDesforges Thank you for putting so much work into this!

I have compiled and tested this on a project of mine with neovim+coc.nvim. Works like a charm.

We should probably do something like #89450 for haskell-language-server, too.

I have also tried running the ./update.sh script. It reproduced exactly the same to nix expressions. Seems to work.

@GuillaumeDesforges
Copy link
Contributor Author

GuillaumeDesforges commented Jun 29, 2020

We should probably do something like #89450 for haskell-language-server, too.

I'm open to suggestions, but I think this can be done in another PR so that this "step" is closed and I can move on in my life ;)

@maralorn
Copy link
Contributor

maralorn commented Jun 29, 2020

We should probably do something like #89450 for haskell-language-server, too.

I'm open to suggestions, but I think this can be done in another PR so that this "step" is closed and I can move on in my life ;)

By all means. Let‘s do that in a later PR.

@maralorn
Copy link
Contributor

maralorn commented Jun 29, 2020

@GrahamcOfBorg build haskellPackages.haskell-language-server

@cdepillabout
Copy link
Member

cdepillabout commented Jun 30, 2020

@GuillaumeDesforges This looks good! Thanks for going back and forth with us so much on this.

@maralorn Thanks for your reviews as well.

@cdepillabout cdepillabout merged commit bc6776a into NixOS:haskell-updates Jun 30, 2020
14 checks passed
14 checks passed
Evaluation Performance Report Evaluator Performance Report
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="2998bc1"; rev="2998bc1f779637fa03d259de2e0f573786b67e1b"; } ./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="2998bc1"; rev="2998bc1f779637fa03d259de2e0f573786b67e1b"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="2998bc1"; rev="2998bc1f779637fa03d259de2e0f573786b67e1b"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="2998bc1"; rev="2998bc1f779637fa03d259de2e0f573786b67e1b"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="2998bc1"; rev="2998bc1f779637fa03d259de2e0f573786b67e1b"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="2998bc1"; rev="2998bc1f779637fa03d259de2e0f573786b67e1b"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="2998bc1"; rev="2998bc1f779637fa03d259de2e0f573786b67e1b"; } ./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
@GuillaumeDesforges
Copy link
Contributor Author

GuillaumeDesforges commented Jun 30, 2020

Thank you very much for your time and comments, I learned a lot doing this PR :)

@GuillaumeDesforges GuillaumeDesforges deleted the GuillaumeDesforges:haskell-language-server branch Jun 30, 2020
@poscat0x04
Copy link
Contributor

poscat0x04 commented Jul 3, 2020

this package doesn't build for ghc 8.10.1 btw:

nix-repl> :b nixpkgs.haskell.packages.ghc8101.haskell-language-server
builder for '/nix/store/csy7bipjf6hcqkvzyfa3mksz6gaphq09-data-tree-print-0.1.0.2.drv' failed with exit code 1; last 10 log lines:
    $, called at libraries/Cabal/Cabal/Distribution/Simple/Configure.hs:1024:20 in Cabal-3.2.0.0:Distribution.Simple.Configure
    configureFinalizedPackage, called at libraries/Cabal/Cabal/Distribution/Simple/Configure.hs:477:12 in Cabal-3.2.0.0:Distribution.Simple.Configure
    configure, called at libraries/Cabal/Cabal/Distribution/Simple.hs:625:20 in Cabal-3.2.0.0:Distribution.Simple
    confHook, called at libraries/Cabal/Cabal/Distribution/Simple/UserHooks.hs:65:5 in Cabal-3.2.0.0:Distribution.Simple.UserHooks
    configureAction, called at libraries/Cabal/Cabal/Distribution/Simple.hs:180:19 in Cabal-3.2.0.0:Distribution.Simple
    defaultMainHelper, called at libraries/Cabal/Cabal/Distribution/Simple.hs:116:27 in Cabal-3.2.0.0:Distribution.Simple
    defaultMain, called at Setup.hs:2:8 in main:Main
  Setup: Encountered missing or private dependencies:
  base >=4.8 && <4.14
cannot build derivation '/nix/store/4dc9dn4h0pqqwm854sz56nkfksf5b2d3-brittany-0.12.1.1.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/9x573ymz2j3387gvhbdrkibhxj0cra1j-haskell-language-server-0.1.0.0.drv': 1 dependencies couldn't be built
[1 built (1 failed), 305 copied (2342.7 MiB), 181.3 MiB DL]
error: build of '/nix/store/9x573ymz2j3387gvhbdrkibhxj0cra1j-haskell-language-server-0.1.0.0.drv' failed
@GuillaumeDesforges
Copy link
Contributor Author

GuillaumeDesforges commented Jul 3, 2020

this package doesn't build for ghc 8.10.1

It would be needed to make overrides in all the different GHC configurations.

It is true I only tested 8.8.3 (current "main" GHC on nixpkgs)

@maralorn
Copy link
Contributor

maralorn commented Jul 3, 2020

Well that concrete issue seems likely to be solvable with one simple jailbreak and is an easy upstream issue.

@GuillaumeDesforges
Copy link
Contributor Author

GuillaumeDesforges commented Jul 3, 2020

@poscat0x04 If it's an upper bound issue, could you please report to upstream?

@poscat0x04
Copy link
Contributor

poscat0x04 commented Jul 3, 2020

this is the corresponding upstream issue: lspitzner/brittany#269

@GuillaumeDesforges
Copy link
Contributor Author

GuillaumeDesforges commented Jul 3, 2020

If it is bound to be fixed, we can just wait for upstream to fix.

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

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