Skip to content

Add cabal-hash revision info when displaying recommended packages when failing to construct buildplan#4068

Merged
snoyberg merged 3 commits intocommercialhaskell:masterfrom
tdietert:recommended-packages-with-rev-info
Jun 7, 2018
Merged

Add cabal-hash revision info when displaying recommended packages when failing to construct buildplan#4068
snoyberg merged 3 commits intocommercialhaskell:masterfrom
tdietert:recommended-packages-with-rev-info

Conversation

@tdietert
Copy link

@tdietert tdietert commented Jun 6, 2018 •
edited
Loading

This PR addresses the following issue #3925

TODO:

  • Add changes to ChangeLog.md
  • Test the output of failed build plan package recommendations

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.

. . .

  * Recommended action: try adding the following to your extra-deps in /home/thomas/github/stack/stack.yaml:

rio-0.1.2.0@sha256:cb2a65cee1c0450815002ca890633215e4544b5c1c9f7091a950142c6efe8f3f

snoyberg
snoyberg requested changes Jun 6, 2018
View reviewed changes
src/Stack/Build/ConstructPlan.hs
@@ -22,7 +22,9 @@ import Stack.Prelude hiding (Display (..))
import Control.Monad.RWS.Strict hiding ((<>))
Copy link
Contributor

@snoyberg snoyberg Jun 6, 2018

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?

Copy link
Author

@tdietert tdietert Jun 7, 2018

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
, callStack :: ![PackageName]
, extraToBuild :: !(Set PackageName)
, getVersions :: !(PackageName -> IO (Set Version))
, getVersions :: !(PackageName -> IO (HashMap Version (NE.NonEmpty CabalHash)))
Copy link
Contributor

@snoyberg snoyberg Jun 6, 2018

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.

@tdietert
Copy link
Author

tdietert commented Jun 7, 2018 •
edited
Loading

Unfortunately, any modification to the [CabalHash] type that represents one or two cabal hashes (the latter encoding both the hash of the cabal file with and without carriage returns) returns the error:

/home/thomas/github/stack/src/Stack/PackageIndex.hs:416:21: error:
    • Exception when trying to run compile-time code:
        For Stack.Types.PackageIndex.PackageCache GHC.Tuple.()

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 Data.List.NonEmpty.fromList and instead use Data.List.NonEmpty.nonEmpty to capture the notion that the version may not have a properly parsed tarball file neither/nor correctly parshes/hashes the cabal revisions.

@tdietert
Copy link
Author

tdietert commented Jun 7, 2018

This PR should be ready to go barring no other requested changes 🎉

snoyberg
snoyberg approved these changes Jun 7, 2018
View reviewed changes
src/Stack/Build/ConstructPlan.hs
case latestApplicableVersion range vs of
Nothing -> pure Nothing
Just lappVer -> do
let mlappRev = join (HashMap.lookup lappVer vsAndRevs)
Copy link
Contributor

@snoyberg snoyberg Jun 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice join!

@snoyberg snoyberg merged commit 33d6edd into commercialhaskell:master Jun 7, 2018
@snoyberg snoyberg mentioned this pull request Jun 19, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@snoyberg snoyberg snoyberg approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tdietert @snoyberg