localPkgDescr is frequently out-of-date
- Dominant language
- Haskell
- Stars
- 1.7k
- Forks
- 750
- Avg merge
- 4d 3h
- Merged PRs (30d)
- 28
Description
`LocalBuildInfo` has a `localPkgDescr`, so you might think that it would be OK to read out the `PackageDescription` from this rather than a `PackageDescription` that is passed into all of the main actions (e.g., `build` takes a `PackageDescription` as WELL as a `LocalBuildInfo`.) Well, that would be poorly assumed; a `buildinfo` hook can _update_ the `PackageDescription` with extra information after the `LocalBuildInfo` has been built... meaning that the stored `localPkgDescr` may very well be out of date.
So I wrote https://github.com/haskell/cabal/commit/30836c4623135758987e3fb01b74aebc15810e55 to make sure that we actually update `localPkgDescr` ourselves. BUT LITTLE DID I REALIZE that there are scads of code in the ecosystem which also `updatePackageDescription`; GHC is one of them! And in fact I spent most of this morning tracking down a bug whereby GHC's build system was silently dropping an extra library, because Cabal had been using the wrong `PackageDescription`.
What's the moral of the story?
1. DO NOT, I repeat, DO NOT use `localPkgDescr`. You CANNOT assume that it is up-to-date because client code may not have updated it while modifying the package description. I suppose something we could do which would make uses of `localPkgDescr` more likely to work is, wherever we have a function that takes both a `PackageDescription` and a `LocalBuildInfo`, override the `localPkgDescr` right there.
2. It really was a mistake to not apply hooked build info only once, at configure step. If we had done this, then it would not be necessary to repeatedly reapply the updates. Can we do this? I bet it will break client code too.
So there are three paths we can take.
1. We can paper over this bug by updating `LocalBuildInfo` with an explicitly passed in `PackageDescription` so that the two are consistent.
2. Going further, expunge all references to `localPkgDescr` in Cabal, such that it is ONLY used to reconstitute a `PackageDescription` at the very beginning. This could be done in combination with (1).
3. Apply hooked build info at configure time, drop post-facto updates. Will probably break Setup code which is written to reapply the hooked build info at build/whatever time.
Contributor guide
Assessment
This issue has not been assessed yet.