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

Install GHC from file #2232

Merged
merged 3 commits into from
Aug 6, 2016
Merged

Install GHC from file #2232

merged 3 commits into from
Aug 6, 2016

Conversation

cblp
Copy link
Contributor

@cblp cblp commented Jun 3, 2016

Implements #2224.

Should I write tests?

@mgsloan
Copy link
Contributor

mgsloan commented Jun 4, 2016

Thanks for working on this!

@mgsloan
Copy link
Contributor

mgsloan commented Jun 4, 2016

Why not allow setting content-length and sha1 for local files? This way you could potentially test the stack.yaml configuration before uploading, and not need to test downloading.

Also, other places that use chattyDownload could benefit from this as well. While it's an unlikely case, you could specify your own 7zip version for example.

I guess the resolution of this would be to have chattyDownload and verifiedDownload take a Either (Path Abs File) DownloadRequest, and return a Path Abs File for the local location of the file.

I realize that's a fair bit of refactoring atop this. I can go ahead and merge if you don't want to do that (and probably get around to it myself at some point)

@cblp
Copy link
Contributor Author

cblp commented Jun 4, 2016

Why not allow setting content-length and sha1 for local files? This way you could potentially test the stack.yaml configuration before uploading, and not need to test downloading.

OK, I'm removing warnings about them.

@cblp
Copy link
Contributor Author

cblp commented Jun 4, 2016

I didn't change chattyDownload, because downloading to specified path is strange — it is actually copying and it's always better to use file from original location on disk than copying it to another directory. Or not always?

@mgsloan
Copy link
Contributor

mgsloan commented Jun 6, 2016

Right, we'd want to call it something different. I understand why you didn't make the change that way, it would be larger.

I don't think that this verification stuff should be coupled with downloading and should be split up. I can worry about that decoupling, though.

@mgsloan mgsloan merged commit 74d7cd9 into commercialhaskell:master Aug 6, 2016
mgsloan added a commit that referenced this pull request Aug 6, 2016
@mgsloan
Copy link
Contributor

mgsloan commented Aug 6, 2016

Merged! Sorry for letting this languish.

@cblp
Copy link
Contributor Author

cblp commented Aug 6, 2016

Thanks!

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