Bootstrap Ruby version of buildpack #886
Conversation
schneems
force-pushed
the
schneems/no-ruby-for-bootstrap
branch
2 times, most recently
from
cffe2b3 to
ab47ae5
Compare
Previously the ruby buildpack used itself to bootstrap a version of Ruby to run on. This technique works fine, but means that it must be able to load the HerokuRubyInstaller which requires shell helpers and lots of other code to be able to bootstrap. This means that these classes cannot use any syntax unless it is valid in all versions of system Ruby present. Currently that includes 1.9.3. By moving the bootstrap download code to bash we can target our syntax to use any version of Ruby that the buildpack bootstraps. In an attempt to keep things dry-ish the bash bootstrap code pulls the ruby version from the `buildpack.toml` file. This file also contains the vendor declarations that are used when the buildpack is published. Having all the versions in the same file should encourage keeping all versions up to date.
|
|
||
| # Pull ruby version out of buildpack.toml to be used with bootstrapping | ||
| regex=".*ruby_version = [\'\"]([0-9]+\.[0-9]+\.[0-9]+)[\'\"].*" | ||
| if [[ $(cat "$BIN_DIR/../buildpack.toml") =~ $regex ]] |
There was a problem hiding this comment.
This is fine, but the regex is slightly incorrect for the toml spec from what I can tell, as that allows various whitespace around the = and of course requires matching quotes. Here's a grep version, which is a bit more powerful than Bash's regex:
| if [[ $(cat "$BIN_DIR/../buildpack.toml") =~ $regex ]] | |
| ruby_version=$(grep -E "ruby_version([^\S\r\n]*)=\1" buildpack.toml | grep -owE "[0-9]+.[0-9]+.[0-9]+") |
If you wanted, you could use partial ruby versions like chruby / .ruby-version / etc. do, in which case the last regex changes to "[0-9]+.[0-9]+(?:.[0-9]+)?" and you could default that to the highest patch version.
Speaking of which, do you want this to handle -p253 style versions?
There was a problem hiding this comment.
Speaking of which, do you want this to handle -p253 style versions?
I don't think we'll ever need to, if the need arises we can update the match, it would fail in CI so we would catch it pretty early.
This is fine, but the regex is slightly incorrect for the toml spec
We have a test that will check to see if the file compiles via toml spec and to ensure that key exists. I'm not super concerned there, more worried about the overall idea of grepping for a version in the buildpack.toml and also my bash-fu is pretty weak so i'm concerned there's something sub-optimal i'm doing.
There was a problem hiding this comment.
We have a test that will check to see if the file compiles via toml spec
Right, but toml would allow:
ruby_version = "1.2.3"
and your regex wouldn't match that, but mine does.
Yours also matches this:
ruby_version = '1.2.3"
Previously the ruby buildpack used itself to bootstrap a version of Ruby to run on. This technique works fine, but means that it must be able to load the HerokuRubyInstaller which requires shell helpers and lots of other code to be able to bootstrap. This means that these classes cannot use any syntax unless it is valid in all versions of system Ruby present. Currently that includes 1.9.3.
By moving the bootstrap download code to bash we can target our syntax to use any version of Ruby that the buildpack bootstraps.
In an attempt to keep things dry-ish the bash bootstrap code pulls the ruby version from the
buildpack.tomlfile. This file also contains the vendor declarations that are used when the buildpack is published. Having all the versions in the same file should encourage keeping all versions up to date.