Be more careful in handling unconfigured programs in the program database #9967
Conversation
This file contains 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
10 tasks
4 tasks
We should configure unconfigured programs before serialising them (using the Binary ProgramDb instance) in Distribution.Client.ProjectPlanning.configureCompiler, as otherwise we can accidentally discard information. This isn't currently a live bug, because in practice we end up re-running configuration steps when interfacing with the Cabal library. However, in the bright future in which we avoid re-configuring things when building a Cabal package that we already determined when invoking cabal-install, this starts causing problems.
In configureRequiredProgram, we would unconditionally configure the
program, even if it was configured already. To explain why this is
problematic:
- The programs we try to configure in this function are those that
arise from a "build-tool-depends" field of a package.
- If the program is not in the "unconfigured" state, we would
unconditionally treat it as a simpleProgram.
This means that if such a program is builtin but is not a
simpleProgram, we might fail to configure it properly.
This problem arises when we configure programs more eagerly: when, in
the past, we might have gone looking up an unconfigured program and
successfully configured it, now we might find the program is already
configured.
One example in which this would cause an issue is when we have
build-tool-depends: hsc2hs
If we end up re-configuring hsc2hs in configureRequiredProgram, we would
treat it as a simple program, which would cause us to be unable to
determine its version.
This commit configures the unconfigured programs in the program database when we are building a custom Setup script. This ensures that we can find the "strip" program which might otherwise still be unconfigured.
There was some hacky logic in place to give a "haskell-suite" compiler a dummy location so that we would attempt to configure it. This logic is no longer necessary since 'configureRequiredProgram' was changed to accomodate the situation in which 'lookupKnownProgram' fails. In fact, it is downright harmful, as this dummy location will trigger errors when we attempt to configure all known programs, which is a necessary step before attempting to serialise a program database.
Mikolaj
added
attention: needs-review
re: --program-suffix
|
I've been trying to construct a test for these hard to trigger bugs, but I was unable to. I think it would be reasonable to move forward with this even if there are no tests given the difficulty in defining a test case for these properties. |
geekosaur
approved these changes
May 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 PR contains several commits that address problems with how we were handling unconfigured programs in the program database:
Binary ProgramDbinstance) inDistribution.Client.ProjectPlanning.configureCompiler, as otherwise we can accidentally discard information.Not doing so can lead to situations in which we can build a package just fine, but if we re-start a build we will fail,
because the program database on disk stored after configuring doesn't match the program database in memory.
configureRequiredProgramneeds to gracefully handle an already-configured program.Not doing so means we would fall back to the
simpleProgramtreatment, which can causeCabalto fail to configure a program such ashsc2hswhich has custom logic for parsing its version number.Setupexecutable, we would pass a program database for use of thestripExecommand. However,the
stripprogram might not be configured in this program database; so we make sure to configure it (in the correctenvironment).
Template Α: This PR modifies behaviour or interface.
TODO: