-
Notifications
You must be signed in to change notification settings - Fork 712
configureCompiler: separate compiler vs ProgramDb #10993
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
-- Now, **outside** of the caching logic of 'rerunIfChanged', add on | ||
-- auxiliary unconfigured programs to the ProgramDb (e.g. hc-pkg, haddock, ar, ld...). | ||
-- | ||
-- See Note [Caching the result of configuring the compiler] | ||
finalProgDb <- liftIO $ Cabal.configCompilerProgDb verbosity hc hcProgDb hcPkg | ||
return (hc, plat, finalProgDb) |
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 is the main change: we move out the logic for finding ar
, ld
etc outside of the recompilation logic. This is because they will typically remain as unconfigured programs until they are needed, which means they would be silently dropped when reloading from cache due to the lossiness of the Binary ProgramDb
instance when unconfigured programs are involved.
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 change makes a lot of sense to me.
This commit splits up the logic in configureCompiler into two parts: 1. Configuring the compiler proper, e.g. finding the location and version of GHC. 2. Creating a program database of attendant programs such as ghc-pkg, haddock, and build tools such as ar, ld. This is done using information about the compiler, such as its location on the filesystem and toolchain information from its settings file.
d68cb50
to
ecf8eb7
Compare
I will not be providing a test for this change, as I think the prior hacky solution was "good enough" to solve the issue with recompilation-checking. I discussed the design with @mpickering and I think the new approach makes a lot more sense than what was there before. |
This commit splits up the logic in configureCompiler into two parts:
ghc-pkg
,haddock
, and build tools such asar
,ld
. This is done using information about the compiler, such as its location on the filesystem and toolchain information from its settings file.The main goal is to avoid an issue in which we get the wrong program database from the cache we get from recompilation checking. However, it's not straightforward to trigger the bug, so I don't have a test at the moment. In practice, I expect this change to be invisible to users.
See Note [Caching the result of configuring the compiler] for an explanation of the problem and the design of the solution.
Template B: This PR does not modify behaviour or interface
E.g. the PR only touches documentation or tests, does refactorings, etc.
Include the following checklist in your PR: