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

Upgrade default GHC to 9.2.8 #1994

Merged
merged 11 commits into from
Nov 9, 2023
Merged

Upgrade default GHC to 9.2.8 #1994

merged 11 commits into from
Nov 9, 2023

Conversation

avdv
Copy link
Member

@avdv avdv commented Oct 20, 2023

depends on #1988

@avdv avdv force-pushed the upgrade-ghc-9.2.8 branch 6 times, most recently from 06062c4 to e977937 Compare October 24, 2023 16:26
@@ -159,6 +159,7 @@ stack_snapshot(
"_" + str(GHC_VERSION) if GHC_VERSION else "",
),
packages = [
"Cabal",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, this is used here

setup_deps = {
"alex": ["@stackage//:Cabal"],
"c2hs": ["@stackage//:Cabal"],
"happy": ["@stackage//:Cabal"],
},

What this means is that it uses the Cabal from the git commit given in the stackage yaml config.

So instead of adding a http_archive for Cabal like this

http_archive(
name = "Cabal",
build_file_content = """
load("@rules_haskell//haskell:cabal.bzl", "haskell_cabal_library")
haskell_cabal_library(
name = "Cabal",
srcs = glob(["Cabal/**"]),
verbose = False,
version = "3.8.1.0",
visibility = ["//visibility:public"],
)
""",
sha256 = "b697b558558f351d2704e520e7dcb1f300cd77fea5677d4b2ee71d0b965a4fe9",
strip_prefix = "cabal-ghc-9.4-paths-module-relocatable",
urls = ["https://github.com/tweag/cabal/archive/refs/heads/ghc-9.4-paths-module-relocatable.zip"],
)

and then using @Cabal//:Cabal in setup_deps we might just use the cabal from the stackage snapshot directly as above. This would probably mean that every stack_snapshot would build it's own Cabal package instead of re-using the one from the Cabal repo. WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would probably mean that every stack_snapshot would build it's own Cabal package instead of re-using the one from the Cabal repo. WDYT?

What do you mean by every stack_snapshot? It would only affect those that use this custom snapshot, correct? Or do you mean that this sets it as the default, and users would have to opt out by explicitly setting their own snapshot? I think in this case this is okay since it fixes a critical bug, i.e. without this most builds just won't work at all. And users can opt out with an explicit snapshot.

Copy link
Member Author

@avdv avdv Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, I'll try to remove the redundant @Cabal repositories and use the Cabal from stackage directly.

@avdv avdv marked this pull request as ready for review October 25, 2023 08:37
Copy link

dpulls bot commented Nov 1, 2023

🎉 All dependencies have been resolved !

Copy link
Member

@aherrmann aherrmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! LGTM, only a small comment.

@@ -159,6 +159,7 @@ stack_snapshot(
"_" + str(GHC_VERSION) if GHC_VERSION else "",
),
packages = [
"Cabal",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would probably mean that every stack_snapshot would build it's own Cabal package instead of re-using the one from the Cabal repo. WDYT?

What do you mean by every stack_snapshot? It would only affect those that use this custom snapshot, correct? Or do you mean that this sets it as the default, and users would have to opt out by explicitly setting their own snapshot? I think in this case this is okay since it fixes a critical bug, i.e. without this most builds just won't work at all. And users can opt out with an explicit snapshot.

haskell/cabal.bzl Outdated Show resolved Hide resolved
When there are no executable components the repository rule would fail with a
IOException, e.g.:
```
ERROR: /home/runner/.cache/bazel/_bazel_runner/8c1e0159f4edbec0d500d51744201c1f/bazel_testing/bazel_6/WORKSPACE:111:15: fetching _stack_executables rule //external:stackage-exe: java.io.IOException: _stack_executables rule //external:stackage-exe must create a directory
ERROR: /home/runner/.cache/bazel/_bazel_runner/8c1e0159f4edbec0d500d51744201c1f/bazel_testing/bazel_6/BUILD.bazel:32:15: //:c2hs-toolchain-impl depends on @stackage-exe//c2hs:c2hs in repository @stackage-exe which failed to fetch. no such package '@stackage-exe//c2hs': _stack_executables rule //external:stackage-exe must create a directory
```
This is needed by the rules_haskell_test lib-repl test which uses the stackage snapshot json file.
Newer versions depend on Win32 >= 2.13 on Windows, but the stack snapshot still just includes Win32 2.12.
@avdv avdv added the merge-queue merge on green CI label Nov 9, 2023
@mergify mergify bot merged commit ce7957d into master Nov 9, 2023
41 checks passed
@mergify mergify bot deleted the upgrade-ghc-9.2.8 branch November 9, 2023 09:04
@mergify mergify bot removed the merge-queue merge on green CI label Nov 9, 2023
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.

2 participants