Skip to content
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

Update on missing SHA #2916

Merged
merged 4 commits into from
Jan 17, 2017
Merged

Update on missing SHA #2916

merged 4 commits into from
Jan 17, 2017

Conversation

snoyberg
Copy link
Contributor

Note: Documentation fixes for https://docs.haskellstack.org/en/stable/ should target the "stable" branch, not master.

Please include the following checklist in your PR:

  • Any changes that could be relevant to users have been recorded in the ChangeLog.md
  • The documentation has been updated, if necessary.

OK, this is a bit involved. Ignoring cabal file revisions for a moment, consider you run stack build in a project using lts-7.15, and the project uses foldl-1.2.2. Suppose you had last updated your package index when foldl-1.2.2 wasn't available. Stack will automatically update your index before trying to build so that it can get the .cabal file for foldl-1.2.2.

Now consider a similar situation: your project uses yesod-core, which for nightly-2017-01-10 is version 1.4.30, with cabal file revision #1. If your index is missing that revision, previously you would get an error message about a missing revision. With this patch, you'll instead get the behavior that the index will automatically update.

In addition, there are commits here to improve the error messages delivered (to indicate whether the lookup occurred in a Git repo or in the tarball), and to standardize logic around choosing Git vs HTTP index methods.

Testing

In order to test, I first created a 00-index.tar file with the most recent content (by running stack update), and then pruned all recent .cabal files (including the first revision of yesod-core-1.4.30) with the following program run from ~/.stack/indices/Hackage:

#!/usr/bin/env stack
{- stack --resolver lts-7.14 --install-ghc runghc
    --package tar
    --package classy-prelude-conduit
-}
{-# LANGUAGE NoImplicitPrelude, OverloadedStrings #-}
import ClassyPrelude.Conduit
import qualified Codec.Archive.Tar as Tar
import qualified Codec.Archive.Tar.Entry as Tar
import Codec.Compression.GZip (decompress)
import System.Directory

main :: IO ()
main = do
    void $ tryIO $ removeFile "01-index.cache"
    void $ tryIO $ removeFile "01-index.tar"
    lbs <- readFile "01-index.tar.gz"
    let entries = Tar.read $ decompress lbs
    writeFile "01-index.tar" $ Tar.write $ go entries
  where
    go (Tar.Next e es)
        | toKeep e = e : go es
        | otherwise = go es
    go Tar.Done = []
    go (Tar.Fail e) = impureThrow e

    toKeep e = Tar.entryTime e < 1483747200

Then, I ran the following:

$ rm -rf yesod-core-1.4.30/ && stack --resolver nightly-2017-01-10 unpack yesod-core

Before this change: I would get an error message about a missing Git SHA. After this change, I get the following:

$ rm -rf yesod-core-1.4.30/ && stack --resolver nightly-2017-01-10 unpack yesod-core
Missing some cabal revision files, updating indices
Downloading package index from https://s3.amazonaws.com/hackage.fpcomplete.com/01-index.tar.gz                  
Populated index cache.                                                                                          
Unpacked yesod-core-1.4.30 to /Users/michael/Desktop/yesod-core-1.4.30/

I also confirmed that the output yesod-core-1.4.30/yesod-core.cabal file contains:

x-revision: 1

This was not a high priority issue by itself, but I needed this
functionality in order to test cabal file revision logic in general more
easily.
This ensures a better error message when the Git SHA isn't found,
warning that it isn't in the tarball instead of not being in the Git
repo.
Copy link
Contributor

@borsboom borsboom left a comment

Choose a reason for hiding this comment

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

This looks good to me.

-- snapshot, we may arguably want to ensure
-- that we're grabbing the version of Hoogle
-- present in the snapshot currently being
-- used.
Copy link
Contributor

Choose a reason for hiding this comment

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

The behaviour described in the FIXME does seem more expected to me. I wonder how many people rely on the current behaviour.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll open up a new Github issue about this then, but changing the behavior of this doesn't seem like it should block the current PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants