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

Script command #2805 #2992

Merged
merged 8 commits into from
Feb 16, 2017
Merged

Conversation

snoyberg
Copy link
Contributor

@snoyberg snoyberg commented Feb 9, 2017

This adds a basic script command. Most of the work involved has to do
with preventing config files from being loaded. Originally we planned on
creating an alternative set of config types for with and without a local
config. That turned out to be prohibitively invasive. Instead, we now
just create a dummy config file instead ~/.stack/script/lts-x.y (or
/nightly-...).

While this addresses reproducibility, it doesn't help with the speed
concerns: script is now about 100ms faster than runghc on my system for
the case where --package is provided, but it's still over a second for
Hello World. The slowdown is inherent right now in checking if the
relevant packages are installed. It would be nice to figure out a way to
optimize the package check.

Also, this should include some integration tests. It should be a simple
matter of a test that includes a bogus stack.yaml and proving that stack
script ignores it.

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.

Please also shortly describe how you tested your change. Bonus points for added tests!

Manual testing so far. We should definitely add some test cases. I used the same example as from the GUIDE.md update:

#!/usr/bin/env stack
-- stack --resolver lts-6.25 script --package turtle
{-# LANGUAGE OverloadedStrings #-}
import Turtle
main = echo "Hello World!"

@snoyberg snoyberg force-pushed the 2805-new-script-command-no-config-change branch 4 times, most recently from 65238e2 to 2019a56 Compare February 10, 2017 12:25
@mgsloan
Copy link
Contributor

mgsloan commented Feb 11, 2017

Seems good to me 👍

@snoyberg snoyberg force-pushed the 2805-new-script-command-no-config-change branch 4 times, most recently from 205eac1 to 38473e7 Compare February 15, 2017 16:49
This adds a basic script command. Most of the work involved has to do
with preventing config files from being loaded. Originally we planned on
creating an alternative set of config types for with and without a local
config. That turned out to be prohibitively invasive. Instead, we now
just create a dummy config file instead ~/.stack/script/lts-x.y (or
/nightly-...).

While this addresses reproducibility, it doesn't help with the speed
concerns: script is now about 100ms faster than runghc on my system for
the case where --package is provided, but it's still over a second for
Hello World. The slowdown is inherent right now in checking if the
relevant packages are installed. It would be nice to figure out a way to
optimize the package check.

Also, this should include some integration tests. It should be a simple
matter of a test that includes a bogus stack.yaml and proving that stack
script ignores it.
This was originally done to help implementation of #2805, but we ended
up going a different route.
@snoyberg snoyberg force-pushed the 2805-new-script-command-no-config-change branch from 65720ab to 011e4da Compare February 16, 2017 06:05
@snoyberg snoyberg merged commit cd6542c into master Feb 16, 2017
@snoyberg snoyberg deleted the 2805-new-script-command-no-config-change branch February 16, 2017 06:36
@decentral1se
Copy link
Member

🍷

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.

3 participants