-
Notifications
You must be signed in to change notification settings - Fork 841
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
Add cabal-hash revision info when displaying recommended packages when failing to construct buildplan #4068
Add cabal-hash revision info when displaying recommended packages when failing to construct buildplan #4068
Conversation
@@ -22,7 +22,9 @@ import Stack.Prelude hiding (Display (..)) | |||
import Control.Monad.RWS.Strict hiding ((<>)) |
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 add a note to the ChangeLog about the change and which issue it resolves?
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 have updated the changelog 👍
src/Stack/Build/ConstructPlan.hs
Outdated
@@ -135,7 +137,7 @@ data Ctx = Ctx | |||
, ctxEnvConfig :: !EnvConfig | |||
, callStack :: ![PackageName] | |||
, extraToBuild :: !(Set PackageName) | |||
, getVersions :: !(PackageName -> IO (Set Version)) | |||
, getVersions :: !(PackageName -> IO (HashMap Version (NE.NonEmpty CabalHash))) |
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.
Regarding our discussion: can you replace the NonEmpty CabalHash
with either [CabalHash]
or Maybe CabalHash
and remove the usage of partial functions below? Alternatively, you could modify the original [CabalHash]
to be a NonEmpty CabalHash
.
Unfortunately, any modification to the
Since I don't really have the time to dig into the template-haskell code right now, I will remove the use of the partial function |
This PR should be ready to go barring no other requested changes 🎉 |
case latestApplicableVersion range vs of | ||
Nothing -> pure Nothing | ||
Just lappVer -> do | ||
let mlappRev = join (HashMap.lookup lappVer vsAndRevs) |
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.
Nice join!
This PR addresses the following issue #3925
TODO:
Tested by removing the
rio
extra-dependency in the stack.yaml file and then trying to build the package with the newly generated stack executable.