Building the most straightforward tests with GHC Bindist compiled with Hadrian#1842
Draft
GuillaumeGen wants to merge 41 commits intomasterfrom
Draft
Building the most straightforward tests with GHC Bindist compiled with Hadrian#1842GuillaumeGen wants to merge 41 commits intomasterfrom
GuillaumeGen wants to merge 41 commits intomasterfrom
Conversation
GuillaumeGen
changed the title
Contributor
Author
|
Might be worth putting the generated bindist somewhere on the internet, to avoid reference to my local file system. |
aherrmann
reviewed
Dec 7, 2022
| url = "file:///home/guillaume/ExternalPrograms/ghc/_build/bindist/ghc-9.2.5-x86_64-unknown-linux.tar.xz", | ||
| sha256 = "e27724de38998dd6c3fb46ac5df4cf6818d779c1b749ea1afcd0b64e55b8217e", | ||
| strip_prefix = "ghc-9.2.5-x86_64-unknown-linux", | ||
| version = "9.2.5", |
Contributor
There was a problem hiding this comment.
Some of the issues are probably unrelated to the hadrian toolchain and instead just due to the version bump. It might make sense to tackle those in a separate PR and bump the GHC version under test.
Contributor
Author
There was a problem hiding this comment.
You are completely right. Separating issues is always beneficial and it will help both review and understanding the cause of the failing tests.
| if local_python_repo_name not in native.existing_rules(): | ||
| _configure_python3_toolchain(name = local_python_repo_name) | ||
|
|
||
| def _configure_python3_toolchain_impl(repository_ctx): |
Contributor
There was a problem hiding this comment.
Fine for development, but we'll want to clean this up and deduplicate with haskell/ghc_bindist.bzl.
| "%{TOOL}": hs.tools.ghci.path, | ||
| "%{ARGS}": "(" + " ".join( | ||
| "%{TOOL}": hs.tools.ghc.path, | ||
| "%{ARGS}": "--interactive (" + " ".join( |
Contributor
There was a problem hiding this comment.
Does this work for other GHC versions as well? If so, great! If not, we'll have to gate this to not break the other GHC versions.
Contributor
Author
There was a problem hiding this comment.
Good question. To investigate.
Comment on lines
+95
to
+117
| generated_bin_filegroup = define_rule( | ||
| "copy_filegroups_to_this_package", | ||
| name = "generated_bin_filegroup", | ||
| srcs = [":bin"], | ||
| ) | ||
|
|
||
| generated_lib_filegroup = define_rule( | ||
| "copy_filegroups_to_this_package", | ||
| name = "generated_lib_filegroup", | ||
| srcs = [":lib"], | ||
| ) | ||
|
|
||
| generated_docdir_filegroup = define_rule( | ||
| "copy_filegroups_to_this_package", | ||
| name = "generated_docdir_filegroup", | ||
| srcs = [":{}".format(docdir)], | ||
| ) | ||
|
|
||
| generated_include_filegroup = define_rule( | ||
| "copy_filegroups_to_this_package", | ||
| name = "generated_include_filegroup", | ||
| srcs = [":{}".format(docdir)], | ||
| ) |
Contributor
There was a problem hiding this comment.
You can move these into haskell/toolchain.bzl next to _hadrian_bindist_settings as
copy_filegroups_to_this_package(
name = "generated_bin_filegroup",
srcs = [":bin"],
)
...
and the definition of copy_filegroups_to_this_package as well.
Reasons why that may be preferable:
- The
ghc.BUILD.tplis also used by the other bindist toolchains and needs to remain valid for those use cases until we can supersede them with the Hadrian one. But, the%{generated_*_filegroup}placeholders are only used for the Hadrian case. Invoking the copy-files rules withinhaskell_toolchainmacro means that these placeholders are no longer needed. - It's preferable to not mix repository rule and regular rule definitions in the same file, so that changes to the rule definition don't also invalidate the repository rule and cause unnecessary refetching. If the copy-file rule is moved into
haskell_toolchainthen the repository rules and regular rules are separated and the import ofghc_bindist_hadrian.bzlinghc.BUILD.tplis also no longer needed. - It puts both concerns into the same place: generating the settings file and placing the bindist files next to it.
Contributor
Author
There was a problem hiding this comment.
Convinced. Will do it.
avdv
force-pushed
the
gg/hadrian-bindist
branch
from
1c574d7 to
171af7b
Compare
avdv
force-pushed
the
gg/hadrian-bindist
branch
2 times, most recently
from
3a71f45 to
fd50666
Compare
Member
|
Made some progress:
|
avdv
force-pushed
the
gg/hadrian-bindist
branch
3 times, most recently
from
78ddc14 to
7d4f967
Compare
avdv
force-pushed
the
gg/hadrian-bindist
branch
3 times, most recently
from
49346e7 to
0f69a7f
Compare
This is needed to support GHC >= 9.4
We skip the `make install` step which would wrap executables with custom scripts. For runghc it passes the GHC executable using the `-f` flag. By default `runghc` would try to execute `ghc-stage2` instead.
avdv
force-pushed
the
gg/hadrian-bindist
branch
from
0313d1d to
c1e1916
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is using a locally built bindist using Hadrian.
This is currently passing 153 of the 206 tests when running
bazel test //... --keep_going.The main identified issues for failing tests are:
splitFileNameandminusFileNameas described in splitFileName and minusFileName Variable not in scope causes builds to fail #1832ghcibinary. As far as I understood, this should be moved toghc --interactiveto pass tests related to REPL.