Consolidate all cabal.project files by pepeiborra · Pull Request #2866 · haskell/haskell-language-server

pepeiborra · 2022-04-29T08:40:23Z

Consolidate all the various project files into a single one that aggregates all the allow-newer and constraints from the existing project files.

cabal.project
cabal-ghc90.project
cabal-ghc92.project

This is possible now that our dependencies (stylish-haskell, hlint, etc.) have moved forward to ghc-lib 9.2 and that all the per-ghc-version cabal flags have been eliminated (#2867).

michaelpj · 2022-05-03T09:39:29Z

          os:  ${{ runner.os }}

-      # repeating builds to workaround segfaults in windows and ghc-8.8.4
-      - name: Build


Previously we could have had plugins which built on GHC X (and hence weren't commented out in the special cabal.project), but whose tests didn't pass (and hence weren't included in the testing workflow. These plugins would have been built by this step, which would have ensured that they kept compiling, even if the tests didn't pass.

Now I think we will only build stuff that's necessary for the tests that we try and run, so if we don't test a plugin we also won't ensure that it keeps building.

Also I think we won't check compilation of the main HLS executable either?

This is a good observation, thanks!

I deleted this step before figuring out that we can use the buildable field to selectively disable projects per ghc version. With that trick, hopefully this step can stay and we can avoid all the pitfalls that you discovered

I have a feeling that that won't work. I think I suggested this to @jneira in the past and he convinced me that it wouldn't work. I can't remember why though >:(

I can't see the reason why it wouldn't work. CI is failing because the Cabal plan it computes is wrong for 9.0.1, but I get the right plan locally so it might be a CI bug

I think maybe it had something to do with Cabal not taking buildable into account when making a plan, so it won't be clever enough to try setting the splice flag to false if the splice plugin isn't buildable, it'll just fail. Worth checking though.

It doesn't need to be:

haskell-language-server/haskell-language-server.cabal

Lines 237 to 239 in 4b475b0

if flag(haddockComments) && (impl(ghc < 9.2.1) || flag(ignore-plugins-ghc-bounds))

build-depends: hls-haddock-comments-plugin ^>= 1.0

cpp-options: -DhaddockComments

Ah, so you're going to keep the existing machinery? Maybe the thing that wouldn't work was a more ambitious thing: I'd though that maybe we could automatically have plugins be enabled or disabled depending on whether the plugin package was buildable given the rest of the configuration or not. So then we'd only have to put the logic for deciding buildability in the plugin package, which would be nice.

One step at a time..

pepeiborra · 2022-05-04T22:20:31Z

@michaelpj the Nix CI is failing, but I am going to leave it to you (after this PR has landed). Deal?

fendor · 2022-05-05T07:48:19Z

+  if impl(ghc >= 9.2)
+    buildable: False
+  else
+    buildable: True


This is not ideal, afaict, this spreads disabling plugins over multiple files, but it looks like we have a hard time doing better than this.
Is haskell/cabal#7783 good enough to allow us to centralise this again?

I don't know whether it's good enough, but I agree that buildabilty with ghc x.y would sit better in the project file

I actually think it's better in the plugin files! "Does this plugin work on GHC X" feels like a concern of the plugin.

fendor

LGTM, awesome!

michaelpj · 2022-05-05T08:34:53Z

the Nix CI is failing

It's already broken, you're not making it any worse!

michaelpj · 2022-05-05T08:37:14Z

-    -haddockComments
-    -retrie
-    -splice
-    -tactic,


Shouldn't the retrie, splice, and tactics plugins also get buildable conditionals?

In fact, how is the CI passing?? Did they just start working without us noticing?

Oh, it's because we still have the GHC conditionals for inclusion in haskell-language-server.cabal

Yeah you are right - I'm not sure how cabal build decides what to build, so far I'm just adding the buildable conditionals on demand

Support was broken by #2866 where the configuration-ghc-921 file was removed. #2892 will indeed restore these file and fix the build with GHC 922, but I was in need a GHC 921 support at work.

pepeiborra mentioned this pull request Apr 29, 2022

simplify hlint plugin Cabal descriptor #2867

Merged

michaelpj reviewed Apr 29, 2022

View reviewed changes

Comment thread cabal.project Outdated

pepeiborra force-pushed the cabal-ghc90-no-more branch 3 times, most recently from f11c688 to 99f0217 Compare May 1, 2022 20:28

pepeiborra changed the title ~~Get rid of cabal-ghc90.project~~ Consolidate all cabal.project files May 1, 2022

pepeiborra force-pushed the cabal-ghc90-no-more branch 2 times, most recently from b22ade9 to d6cdd0c Compare May 1, 2022 21:54

pepeiborra marked this pull request as ready for review May 2, 2022 06:58

pepeiborra requested review from Ailrun, Anton-Latukha, berberman, guibou and jneira as code owners May 2, 2022 06:58

pepeiborra force-pushed the cabal-ghc90-no-more branch from 439c689 to cf05dee Compare May 2, 2022 10:42

michaelpj reviewed May 3, 2022

View reviewed changes

pepeiborra force-pushed the cabal-ghc90-no-more branch from ee5d6d8 to 77cca7b Compare May 3, 2022 15:03

pepeiborra added 8 commits May 4, 2022 19:50

Get rid of cabal-ghc90.project

53108d3

fix trailing comma

6361a45

hie-compat does not need ghc-lib on GHC 9.x

2e57e16

remove unnecessary allow-newer entries

af27b0c

Get rid of cabal-ghc92.project

d599529

hls-haddock-comments-plugin is not buildable with ghc 9.2

912b1cf

clean up references to cabal-ghc92.project and other stale docs

13783b9

rm configuration-ghc.nix files

ee71a89

pepeiborra force-pushed the cabal-ghc90-no-more branch from 152307f to a65466a Compare May 4, 2022 18:50

pepeiborra requested a review from fendor as a code owner May 4, 2022 18:50

Fix wrong allow-newer brittany entries

acf3603

pepeiborra force-pushed the cabal-ghc90-no-more branch from a65466a to acf3603 Compare May 4, 2022 19:05

we should not store cabal freeze files in the CI cache

9195eb6

fendor reviewed May 5, 2022

View reviewed changes

fendor approved these changes May 5, 2022

View reviewed changes

Merge branch 'master' into cabal-ghc90-no-more

5f4c2e0

michaelpj reviewed May 5, 2022

View reviewed changes

Merge branch 'master' into cabal-ghc90-no-more

3eae75e

pepeiborra enabled auto-merge (squash) May 5, 2022 11:56

pepeiborra merged commit 8cbefb7 into master May 5, 2022

guibou mentioned this pull request May 13, 2022

WIP: restore support for nix for GHC 921 #2905

Closed

sloorush pushed a commit to sloorush/haskell-language-server that referenced this pull request May 21, 2022

Consolidate all cabal.project files (haskell#2866)

7766cb0

georgefst mentioned this pull request May 23, 2022

chore: Upgrade to GHC 9.2 hackworthltd/primer#244

Merged

hololeap pushed a commit to hololeap/haskell-language-server that referenced this pull request Aug 26, 2022

Consolidate all cabal.project files (haskell#2866)

37d7737

	if flag(haddockComments) && (impl(ghc < 9.2.1) \|\| flag(ignore-plugins-ghc-bounds))
	build-depends: hls-haddock-comments-plugin ^>= 1.0
	cpp-options: -DhaddockComments

Uh oh!

Conversation

pepeiborra commented Apr 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pepeiborra commented May 4, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fendor left a comment

Choose a reason for hiding this comment

Uh oh!

michaelpj commented May 5, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pepeiborra commented Apr 29, 2022 •

edited

Loading