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
Use snap for Crystal latest & add cache support #1834
Conversation
Effectively drop support for precise and trusty
cache: shards cache: shards: true
297fbf0
to
ef8dfee
Compare
Hi @BanzaiMan I would like to check if there is anything I can do on my side regarding this PR. |
@bcardiff Hello! I believe we invited you to our slack so we can arrange testing. Have you seen the invitation? |
Now that you mentioned it, I find it and accepted it \o/. Thanks |
Use snap for Crystal latest & add cache support
Advertise better snap in the logs
Use snap for Crystal latest & add cache support
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.
Looks mostly good now, but do take a look at my comments.
lib/travis/build/script/crystal.rb
Outdated
end | ||
|
||
def cache_slug | ||
super << '-crystal' |
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.
You'd probably want to add a piece about the crystal version in cache_slug
, in order to avoid name collision.
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.
The crystal version, from the configuration, is nightly or latest. I could interpolate the configuration value in the slug. But if possible I would rather extract the value of crystal --version
and use that in part of the slugh. Is there a way to do that? All other build scripts seems to emit the configuration value directly.
lib/travis/build/script/crystal.rb
Outdated
end | ||
|
||
def snap_install_crystal(options) | ||
sh.cmd %Q(sudo apt-get install -y gcc pkg-config git tzdata libpcre3-dev libevent-dev libyaml-dev libgmp-dev libssl-dev libxml2-dev), echo: true |
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.
Might want to consider sending this command's STDOUT and STDERR to /dev/null
(or File::NULL
), in order to avoid excess output. It's also OK if you choose to keep it.
The location of the shards cache when installed via snap is fixed. It is /home/travis/snap/crystal/common/.cache/shards The directories for both installation methods is added always in linux
Refactor method to extract the crystal config version and add a default.
af42eeb
to
522675e
Compare
Use snap for Crystal latest & add cache support
As a continuation of #1721, this PR make use of snap for latest Crystal release.
There was already a condition to avoid running snap in Trusty and Precise. (Although Trusty seems to be supported by snap https://ubuntu.com/blog/snaps-are-now-available-for-ubuntu-14-04-lts-desktop-and-server)The condition is left so this is effectively droppinglanguage: crystal
support for trusty and precise.[update: if
snap
is not available crystal latest is installed via apt]Caching support was also added by:
I am not sure how to test this outside the codebase to double-check everything is in good shape.
I would've wanted to include the crystal/shards version in the cache_slug, and not only the configured version (latest/nightly). But I am not sure if it's possible to capture the output of a command and use that in the cache_slug.