Skip to content

Added HLS support via nix#3977

Closed
reactormonk wants to merge 8 commits intopurescript:masterfrom
reactormonk:no-hpack
Closed

Added HLS support via nix#3977
reactormonk wants to merge 8 commits intopurescript:masterfrom
reactormonk:no-hpack

Conversation

@reactormonk
Copy link
Copy Markdown

@reactormonk reactormonk commented Dec 30, 2020

This PR is built on top of #3933 to go the cabal route. It also enables straightforward access to the haskell-language-server via nix (see the note in CONTRIBUTING).

Haskell.nix uses the stack snapshot as base, so the cabal freeze file isn't really required if you go this route.

hdgarrood and others added 7 commits September 13, 2020 18:19
Stack offers a relatively poor developer experience on this repository
right now. The main issue is that build products are invalidated far
more often than they should be. cabal-install is better at this, but
using cabal-install together with hpack is a bit awkward.

Additionally, hpack isn't really pulling its weight these days. Current
versions of stack recommend that you check your generated cabal file in,
which is a huge pain as you have to explain to contributors to please
leave the cabal file alone and edit package.yaml instead (the comment
saying the file is auto-generated is quite easy to miss).

Current versions of Cabal also solve the issues which made hpack
appealing in the first place, namely:

- common stanzas mean you don't have to repeat yourself for things like
  -Wall or dependencies
- tests are run from inside a source distribution by default, which
  means that if you forget to include something in extra-source-files
  you find out when you run the tests locally, rather than having to
  wait for CI to fail
- the globbing syntax is slightly more powerful (admittedly not quite as
  powerful as hpack's, but you can use globs like tests/**/*.purs now,
  which gets us close enough to hpack that the difference is basically
  negligible).

We do still need to manually maintain exposed-modules lists, but I am
happy to take that in exchange for the build tool not invalidating our
build products all the time.

This PR drops hpack in favour of manually-maintained Cabal files. It
also introduces a cabal.project file and a cabal.project.freeze file
which make it easier to use cabal-install as a build tool when working
on this repo. Stack still works, and the CI, contributing, and
installation docs still use Stack.

Since the npm installer uses Stack when building from source, it would
be difficult for us to drop support for Stack entirely, and so I don't
intend to work towards dropping Stack support. If this PR is merged, I
will probably start using cabal-install as my main build tool when
working on the compiler locally, and I might also update the
installation and contributing docs to do the same.

Stack works a little better now than it used to, because I think one of
the causes of unnecessary rebuilds was us specifying optimization flags
in the Cabal file. (Newer versions of Cabal warn you not to do this, so
I think this might be a known issue). To ensure that release builds are
built with -O2, I've updated the CI script to specify that advanced
optimizations should be used via the CLI (see ci/build.sh).
@hdgarrood
Copy link
Copy Markdown
Contributor

Sorry, but I’m not comfortable with having a bunch of extra configuration in this repo which we need to learn how to keep working when it inevitably stops working. (I know what Nix is and that it’s supposed to stop this sort of thing from happening but I do not have that level of confidence in it.)

@reactormonk
Copy link
Copy Markdown
Author

Sorry, but I’m not comfortable with having a bunch of extra configuration in this repo which we need to learn how to keep working when it inevitably stops working. (I know what Nix is and that it’s supposed to stop this sort of thing from happening but I do not have that level of confidence in it.)

Nix is reasonably decent at staying stable. It might break on updating the cabal / stack configuration. Also the haskell.nix code could be pinned, haven't done that yet.

I could probably push the Haskell language server configuration upstream, so that wouldn't have to be flying around here.

@hdgarrood
Copy link
Copy Markdown
Contributor

The Nix stuff is more objectionable to me than the HLS stuff. I think it’s extremely unlikely that you’ll persuade me to merge this; the only way I might be on board is if there’s strong support from other core team members.

@hdgarrood
Copy link
Copy Markdown
Contributor

That’s still too much, sorry. The furthest I’m willing to go is a note pointing to your fork in the CONTRIBUTING.md file; I don’t want these files in this repo.

@reactormonk
Copy link
Copy Markdown
Author

For quick setup, I considered a shell.nix file here which would link to the settings in an own repo. If that's still too much, then just close the PR.

@rhendric
Copy link
Copy Markdown
Member

@reactormonk, what are the minimal changes that would need to happen to PureScript to enable someone to develop using HLS? Is all the Nix stuff necessary? Is the extra HIE configuration necessary? It's unclear to me whether this PR is primarily motivated to help enable people who aren't currently hacking on PureScript to do so, or to offer a fancier way to do things that is more to your taste.

I don't really know anything about HLS; my limited experience with Nix makes me think you should be able to keep Nix code in a separate repo and install PureScript with that without any changes here, but if I'm wrong, I would be motivated to help you find and merge a minimal fix. What I don't think anyone wants is to see the main PureScript repo loaded up with files specific to a particular dev environment, especially if that dev environment isn't one used by any of the maintainers.

@reactormonk
Copy link
Copy Markdown
Author

The HIE config isn't 100% necessary, HLS can sometimes generate it itself just fine. Having the config at hand just makes it more stable. It requires regeneration when you create new internal packages though.

If you have a shell.nix file around, it's easier on the convention (and e.g. the nix environment picker from vscode, which picks from the current directory) and for people to find it. It reduces friction - digging through a contributing.md file takes mental capacity, if you just wanna browse around. Otherwise you'd have to follow the HLS setup guides - which will be automatic at some point in the future I assume, in which case the config could be discarded again.

@hdgarrood
Copy link
Copy Markdown
Contributor

The HIE config is probably going to break quite quickly when any of the referenced modules are removed or renamed, or new modules are added, so if it can sometimes be generated then I think leaving it out is probably the best bet for now.

@hdgarrood hdgarrood closed this Jan 13, 2021
@rhendric rhendric mentioned this pull request Dec 28, 2021
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants