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

Ghc 9.2.5 #1869

Merged
merged 12 commits into from
Mar 2, 2023
Merged

Ghc 9.2.5 #1869

merged 12 commits into from
Mar 2, 2023

Conversation

ylecornec
Copy link
Member

@ylecornec ylecornec commented Feb 16, 2023

Depends on #1868

This PR updates the version of GHC used for tests to 9.2.5.

@ylecornec ylecornec force-pushed the ghc-9.2.5 branch 4 times, most recently from d4918bf to bda3e5f Compare February 17, 2023 15:39
@ylecornec ylecornec marked this pull request as ready for review February 17, 2023 16:03
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.

Good work, thank you for pushing this through!

@@ -132,6 +203,15 @@ stack_snapshot(
"hspec",
"package1",
],
setup_deps = {
"HUnit": ["@Cabal//:Cabal"],
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a comment like the one above here, too, so that it's not forgotten when we can update to the new Cabal version. (Best also open a GH issue on rules_haskell to track it.)

@@ -1,4 +1,5 @@
package(default_visibility = ["//:__pkg__"])

Copy link
Member

Choose a reason for hiding this comment

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

Not opposed to including this here, but, out of curiosity, how is bazel_json connected to the GHC update?
Also, for a separate PR, but, could we get rid of this and replace it by the builtin Bazel JSON module?

Copy link
Member Author

Choose a reason for hiding this comment

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

After double-checking, the modifications to bazel_json were not due to an update but to buildifier. And they seem unnecessary since CI still passes after removing this commit (so I am merging without them).

About replacing bazel_json with bazel json module. Looking at the documentation of https://bazel.build/rules/lib/json, I do not see a way to test if a json string is valid before trying to parse it, in order to customize the error message. And we use the fail_on_invalid = False functionnality of bazel_json for this once (here).

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for looking into this! Makes sense!

I do not see a way to test if a json string is valid before trying to parse it, in order to customize the error message. And we use the fail_on_invalid = False functionnality of bazel_json for this once (here).

I see. Well, given that that code fails immediately itself after observing an error, it doesn't look like a big loss. So, I think we could still remove bazel_json and replace it with the builtin JSON parser.

@dpulls
Copy link

dpulls bot commented Feb 27, 2023

🎉 All dependencies have been resolved !

ylecornec and others added 10 commits February 27, 2023 09:42
Co-Authored-By: GuillaumeGen <[email protected]>
Co-Authored-By: GuillaumeGen <[email protected]>
Co-Authored-By: GuillaumeGen <[email protected]>
Co-Authored-By: GuillaumeGen <[email protected]>
Co-Authored-By: GuillaumeGen <[email protected]>
Paths_ modules generated by Cabal 3.6.3.0 contain paths with some duplicated backslashes (but they are still valid).
This update tests to be more permissive when comparing them to the paths we generate.
haskell_libraries without versions used as dependencies of haskell_cabal_{library/binary} rules are not found anymore when using ghc 9.2.5

Co-Authored-By: GuillaumeGen <[email protected]>
@ylecornec ylecornec added the merge-queue merge on green CI label Feb 27, 2023
@ylecornec ylecornec removed the merge-queue merge on green CI label Mar 2, 2023
@ylecornec
Copy link
Member Author

Commit 3b2c2e4 makes use of the execroot prefix in hie-bios flags for source files (that may be in the workspace directory) instead of relying on the runfiles library (this makes a difference because of symlinks).

This way, paths are consistent with the ones used for the import_dirs and we work around the following issue:
haskell/haskell-language-server#3510

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!

@ylecornec ylecornec added the merge-queue merge on green CI label Mar 2, 2023
@mergify mergify bot merged commit 9ef1f76 into master Mar 2, 2023
@mergify mergify bot deleted the ghc-9.2.5 branch March 2, 2023 09:54
@mergify mergify bot removed the merge-queue merge on green CI label Mar 2, 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