-
Notifications
You must be signed in to change notification settings - Fork 104
Don't ignore ghc compile bootstrap error, fix --remote-source-dist #1172
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
base: master
Are you sure you want to change the base?
Conversation
lib/GHCup/GHC.hs
Outdated
| let regex = [s|^(.*/)*boot$|] :: B.ByteString | ||
| [bootFile] <- liftIO $ findFilesDeep | ||
|
|
||
| let regex = [s|^(.*/)*compiler/ghc.cabal.in$|] :: B.ByteString |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you explain this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is to fix the workdir retrieval. The bootstrapped src does not contain boot file.
finding the directory name using ghc.cabal.in seems to be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this file will always exist in that place. If there is no boot file, then we can assume there is a configure file. And the topmost such configure file is what we need. We should depend on the files we actually use.
From what I understand findFilesDeep is breadth first, so we can just take the first find (possibly using the conduit functions to cut off early).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reverted back to boot file, the file gets renamed now in hadrian when making release builds.
-- And move ./boot so we can't accidentally call it in CI
moveFile (dest -/- "boot") (dest -/- "boot.source")
| lift $ logDebug "Doing ghc-bootstrap" | ||
| python3 <- liftE $ makeAbsolute "python3" | ||
| lEM $ execLogged python3 ["./boot"] (Just $ fromGHCupPath tmpUnpack) "ghc-bootstrap" Nothing | ||
| liftE $ configureWithGhcBoot Nothing [] (Just $ fromGHCupPath tmpUnpack) "ghc-bootstrap" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this belong outside of getGHCVer. The reason we run configure here (without any meaningful arguments) is because it creates the VERSION file, which we then use. So this is not the "proper" configure. The only reason we do this is to get the GHC version.
See GHCs configure.ac:
dnl Create the VERSION file, satisfying #22322.
printf "$ProjectVersion" > VERSION
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, in that case it does belong here, as the bootstrapped ghc will contain the VERSION file.
…e ghc --remote-source-dist'
25d0798 to
7757dd2
Compare
| bootAndGenVersion tmpUnpack = do | ||
| let bootFile = fromGHCupPath tmpUnpack </> "boot" | ||
| hasBootFile <- liftIO $ doesFileExist bootFile | ||
| when hasBootFile $ do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure. As in: if we have booted, does it mean there is always a VERSION file? From what I understand one has to run configure too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The boot file is being renamed to boot.source when doing hadrian build source-dist, so this is a scenario different from simply bootstrapped ghc. And as part of this build source-dist the VERSION is always being copied.
ref: https://gitlab.haskell.org/ghc/ghc/-/blob/master/hadrian/src/Rules/SourceDist.hs#L123-143
fixes #1163
Also included in this
This does not contain boot file, and currently it results in following