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.

Sign up for GitHub

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

Don't ignore ghc compile bootstrap error, fix --remote-source-dist #1172

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Don't ignore ghc compile bootstrap error, fix --remote-source-dist #1172

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
170f0f4
Fix use of 'boot' file to determine workdir; apply patches in 'compil…
dfordivam Dec 8, 2024
7757dd2
Separate boot step from getGHCVer
dfordivam Dec 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 37 additions & 13 deletions lib/GHCup/GHC.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -868,20 +868,26 @@ compileGHC targetGhc crossTarget vps bstrap hghc jobs mbuildConfig patches aargs
tmpDownload <- lift withGHCupTmpDir
tmpUnpack <- lift mkGhcupTmpDir
tar <- liftE $ download uri Nothing Nothing Nothing (fromGHCupPath tmpDownload) Nothing False
(bf, tver) <- liftE $ cleanUpOnError @'[UnknownArchive, ArchiveResult, ProcessError] tmpUnpack $ do
(workdir, tver) <- liftE $ cleanUpOnError @'[UnknownArchive, ArchiveResult, ProcessError, PatchFailed, DownloadFailed, DigestError, ContentLengthError, GPGError, NotFoundInPATH] tmpUnpack $ do
liftE $ unpackToDir (fromGHCupPath tmpUnpack) tar
let regex = [s|^(.*/)*boot$|] :: B.ByteString
[bootFile] <- liftIO $ findFilesDeep

-- bootstrapped ghc renames boot to boot.source
let regex = [s|^(.*/)*boot(.source)*$|] :: B.ByteString
(boot:_) <- liftIO $ findFilesDeep
tmpUnpack
(makeRegexOpts compExtended
execBlank
regex
)
tver <- liftE $ catchAllE @_ @'[ProcessError, ParseError, NotFoundInPATH] @'[] (\_ -> pure Nothing) $ fmap Just $ getGHCVer
(appendGHCupPath tmpUnpack (takeDirectory bootFile))
pure (bootFile, tver)
(makeRegexOpts compExtended execBlank regex)

let workdir = appendGHCupPath tmpUnpack (takeDirectory boot)

lift $ logDebug $ "GHC compile workdir: " <> T.pack (fromGHCupPath workdir)

liftE $ applyAnyPatch patches (fromGHCupPath workdir)

-- bootstrap, if necessary
liftE $ bootAndGenVersion workdir

let workdir = appendGHCupPath tmpUnpack (takeDirectory bf)
tver <- liftE $ catchAllE @_ @'[ProcessError, ParseError, NotFoundInPATH] @'[] (\_ -> pure Nothing) $ fmap Just $ getGHCVer workdir
pure (workdir, tver)

ov <- case vps of
Just vps' -> fmap Just $ expandVersionPattern tver "" "" "" "" vps'
Expand Down Expand Up @@ -934,6 +940,8 @@ compileGHC targetGhc crossTarget vps bstrap hghc jobs mbuildConfig patches aargs
liftE $ applyAnyPatch patches (fromGHCupPath tmpUnpack)

-- bootstrap
liftE $ bootAndGenVersion tmpUnpack

tver <- liftE $ catchAllE @_ @'[ProcessError, ParseError, NotFoundInPATH] @'[] (\_ -> pure Nothing) $ fmap Just $ getGHCVer
tmpUnpack
liftE $ catchWarn $ lEM @_ @'[ProcessError] $ darwinNotarization _rPlatform (fromGHCupPath tmpUnpack)
Expand Down Expand Up @@ -1035,6 +1043,24 @@ compileGHC targetGhc crossTarget vps bstrap hghc jobs mbuildConfig patches aargs
pure installVer

where
bootAndGenVersion :: ( MonadReader env m
, HasSettings env
, HasDirs env
, HasLog env
, MonadIO m
, MonadThrow m
)
=> GHCupPath
-> Excepts '[ProcessError, NotFoundInPATH] m ()
bootAndGenVersion tmpUnpack = do
let bootFile = fromGHCupPath tmpUnpack </> "boot"
hasBootFile <- liftIO $ doesFileExist bootFile
when hasBootFile $ do
Copy link
Member

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.

@bgamari @AndreasPK

Copy link
Collaborator Author

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

lift $ logDebug "Doing ghc-bootstrap"
lEM $ execLogged "python3" ["./boot"] (Just $ fromGHCupPath tmpUnpack) "ghc-bootstrap" Nothing
-- This configure is to generate VERSION file
liftE $ configureWithGhcBoot Nothing [] (Just $ fromGHCupPath tmpUnpack) "ghc-bootstrap"
Copy link
Member

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

Copy link
Collaborator Author

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.


getGHCVer :: ( MonadReader env m
, HasSettings env
, HasDirs env
Expand All @@ -1045,8 +1071,6 @@ compileGHC targetGhc crossTarget vps bstrap hghc jobs mbuildConfig patches aargs
=> GHCupPath
-> Excepts '[ProcessError, ParseError, NotFoundInPATH] m Version
getGHCVer tmpUnpack = do
lEM $ execLogged "python3" ["./boot"] (Just $ fromGHCupPath tmpUnpack) "ghc-bootstrap" Nothing
liftE $ configureWithGhcBoot Nothing [] (Just $ fromGHCupPath tmpUnpack) "ghc-bootstrap"
let versionFile = fromGHCupPath tmpUnpack </> "VERSION"
hasVersionFile <- liftIO $ doesFileExist versionFile
if hasVersionFile
Expand Down
Loading