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

Fix stack.yaml (aeson-1.5.2.0 dependency). #14

Merged
merged 1 commit into from
Sep 17, 2020

Conversation

kukimik
Copy link
Contributor

@kukimik kukimik commented Jul 6, 2020

As reported in #12, because of the aeson-1.5.2.0 dependency, the project didn't build with using neither the stack.yaml file in the repo, nor the one described in the README. I am able to build it using the stack.yaml as in this PR.

However, this boosts the required GHC version to 8.8.3. Probably lts-15.3 (GHC 8.8.2) would also work. I'm not sure what the best practice is.

I didn't manage to build it using stack and GHC 8.6.5.

@georgefst
Copy link
Collaborator

Is it easy to see where the conflict is with GHC 8.6.5?

@parsonsmatt what's our policy on GHC versions? I usually aim to support the last three majors, and I don't think 8.12 is due for a few more months. Though I can't see anyone being too upset right now that they couldn't use the latest Fourmolu with an older GHC.

Perhaps I jumped the gun on requiring such a recent aeson. It wouldn't cost us much to widen the bound (we'd have to lose warnings for unrecognised fields in config files, due to a now-fixed bug). And it's also what's holding up haskell/haskell-language-server#161.

@parsonsmatt
Copy link
Collaborator

Hmmm. I'd like to support 8.6 - the "three release" rule was more appropriate before the new release cadence, and now I think a target of 2-3 years of GHCs make more sense.

However, that rule is more for libraries IMO. The executable can be built with 8.10 and then used on an 8.6.5 codebase, right? Since it's a binary, I think I'm in favor of requiring a newer build of GHC than would be prudent for a library.

@georgefst
Copy link
Collaborator

The executable can be built with 8.10 and then used on an 8.6.5 codebase, right?

I believe so. I'm not sure precisely what the guarantees are from ghc-lib-parser, but it's supposed to give some flexibility there.

@georgefst
Copy link
Collaborator

Of course, thinking about it, Fourmolu is also a library, and HLS, for one, is using it as such...

@kukimik
Copy link
Contributor Author

kukimik commented Jul 13, 2020

Is it easy to see where the conflict is with GHC 8.6.5?

I'm not sure what is the nature of the problem. Perhaps it is not really the compiler version that matters. To be honest, I have little idea how stack's dependency resolving works.

Trying to build with resolver lts-14.27 (which is the newest one available for ghc 8.6.5) or nightly-2020-01-21 (ghc 8.8.1) with aeson-1.5.2.0 included in extra-deps gives:

Dependency cycle detected in packages:
    [aeson,these,aeson,fourmolu]

In the dependencies for these-1.0.1:
    aeson dependency cycle detected: aeson, these, aeson, fourmolu
needed due to fourmolu-0.1.0.0 -> these-1.0.1

In the dependencies for yaml-0.11.2.0:
    aeson dependency cycle detected: aeson, these, aeson, fourmolu
needed due to fourmolu-0.1.0.0 -> yaml-0.11.2.0

@georgefst
Copy link
Collaborator

Finally looked into this. Being as ignorant of Stack as I am, I didn't even realise the resolver actually specifies the version of GHC.

We can build with 8.6.5 with Cabal (after 1de072e), so it could presumably be done with Stack after adding enough extra-deps. The only issue then is what we actually check in to the repo. Having just the one stack.yaml, tied to GHC 8.8.3, seems reasonable right now, so I'm happy to merge this.

@parsonsmatt Am I missing anything?

@georgefst georgefst requested a review from parsonsmatt August 7, 2020 13:19
@georgefst georgefst merged commit a09ec7e into fourmolu:master Sep 17, 2020
@kukimik kukimik deleted the fix_stack_yaml branch October 4, 2020 16:17
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