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.

Sign up for GitHub

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

Jump to bottom

haskellPackages.haskell-language-server: 027fq67 -> 15hyscf #106705

Closed
wants to merge 1 commit into from
Closed

haskellPackages.haskell-language-server: 027fq67 -> 15hyscf #106705

wants to merge 1 commit into from

Conversation

expipiplus1
Copy link
Contributor

@expipiplus1 expipiplus1 commented Dec 12, 2020 •
edited

Fix the update script to include hls-explicit-imports-plugin and hls-retrie-plugin

Motivation for this change

Not sure if this is an important update or not, but the change to
update.sh here might save the next person doing it some small amount of
work.

It should also be possible to drop the brittany fork too as the latest brittany release supports ghc 8.10.2.

update.sh could be improved too, it currently fetches the hls repo 5 times! (otoh, this is computer time and perhaps not worth valuable human time fixing)

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.

@cdepillabout
Copy link
Member

cc @maralorn and @GuillaumeDesforges (I think?)

@maralorn
Copy link
Member

First of all thank you very much for your work. I really appreciate it.

The problem here is this:

It is my understanding, that it is the aim of nixpkgs to primarily support released packages. In special cases we can also bump versions to unreleased versions, but for that we need a good reason. Such as

  • upstream does not do releases
  • upstream takes to long for a release which
    • contains a critical bug fix,
    • is necessary to keep the program compatible with the rest of the internet or nixpkgs.
  • etc.

hls has a healthy monthly release cycle and the current version of hls seems fine. So unless you can give me a good reason I don‘t think we should bump it randomly in between.

That being said,

  1. your changes to the update script are most welcome and we should definitely keep them.
  2. the last hls release was on 2020-11-11, our next merge into master will be at the earliest next friday, so it's quite likely we'll see a release before then.

So my suggestion is:

  • Let's keep this PR around.
  • I‘d be in favor of removing the brittany override
  • Prepare this for the release
  • Update it once the release lands and then merge it.

Alternatively I wouldn’t mind merging this earlier and have it tested by hydra for good measure. But for that I‘d wait at least for some live sign in the #haskell-ide-engine channel, that they intend to do an update very soon.

@maralorn
Copy link
Member

Code wise I can‘t find any fault in this.

@expipiplus1
Copy link
Contributor Author

Agree with all of that! I should have been clearer I didn't have an intention of having this merged asap. It was more that I made the change and though it might be useful for whoever bumps to the next HLS release.

Happy for any of the proposed actions.

@maralorn
Copy link
Member

One more thing of course … hls right now doesn‘t build on hydra, because there is a build problem with ghcide. Nothing very complicated to fix but something we need to do before we can test this.

@poscat0x04 poscat0x04 mentioned this pull request Dec 14, 2020
Merged
10 tasks
@maralorn
Copy link
Member

Maybe you can rebase this, so we can test if it build?

@expipiplus1
haskellPackages.haskell-language-server: 027fq67 -> 15hyscf
cd28068 
Fix the update script to include hls-explicit-imports-plugin and
hls-retrhls-retrie-plugin
@expipiplus1
Copy link
Contributor Author

rebased!

@maralorn
Copy link
Member 

Building library for haskell-language-server-0.6.0.0..
[1 of 4] Compiling Paths_haskell_language_server ( dist/build/autogen/Paths_haskell_language_server.hs, dist/build/Paths_haskell_language_server.o, dist/build/Paths_haskell_language_server.dyn_o )
[2 of 4] Compiling Ide.Version      ( src/Ide/Version.hs, dist/build/Ide/Version.o, dist/build/Ide/Version.dyn_o )
[3 of 4] Compiling Ide.Arguments    ( src/Ide/Arguments.hs, dist/build/Ide/Arguments.o, dist/build/Ide/Arguments.dyn_o )
[4 of 4] Compiling Ide.Main         ( src/Ide/Main.hs, dist/build/Ide/Main.o, dist/build/Ide/Main.dyn_o )

src/Ide/Main.hs:161:25: error:
    Ambiguous occurrence ‘findCradle’
    It could refer to
       either ‘Development.IDE.Session.findCradle’,
              imported from ‘Development.IDE.Session’ at src/Ide/Main.hs:37:1-30
           or ‘HIE.Bios.Cradle.findCradle’,
              imported from ‘HIE.Bios.Cradle’ at src/Ide/Main.hs:42:1-22
    |
161 |         cradles <- mapM findCradle files
    |                         ^^^^^^^^^^

builder for '/nix/store/dws54bzzjpbn5vk1g5njkpdk0ymsvwqj-haskell-language-server-0.6.0.0.drv' failed with exit code 1

@expipiplus1
Copy link
Contributor Author

expipiplus1 commented Dec 16, 2020 via email

@maralorn
Copy link
Member

Also:

hls-0.7.0 just has been released :-)

@expipiplus1 expipiplus1 mentioned this pull request Dec 17, 2020
Merged
10 tasks
@expipiplus1
Copy link
Contributor Author

Closed in favor of #107078

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdepillabout cdepillabout Awaiting requested review from cdepillabout

Assignees
No one assigned
Labels
6.topic: haskell 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@expipiplus1 @cdepillabout @maralorn