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

If no RUBY specified for chruby-exec, fall back to .ruby-version file #126

Conversation

ravigadad
Copy link

Great project; thank you so much for maintaining it. Our dev team drove ops insane (well, more insane) by sledgehammering the environment first with RVM, then rbenv. We'd finally caught a breath of uncluttered air with rbfu when it was discontinued in favor of chruby - but that was a welcome change, since we were about to fork rbfu to fix some issues which you've handled even more cleanly.

One thing we do miss about rbfu, however, is the ability for the execution wrapper (chruby-exec, in this case) to use a .ruby-version file when no RUBY is specified on the command line. Our thin init scripts did something like this:

cd ${APPDIR} && rbfu bundle exec thin -C ${CONF}

...and with chruby, that became a bit of a quandary. Sourcing the auto.sh script doesn't work for the exact command above even in a login shell, since the PROMPT_COMMAND obviously won't re-execute after the && - that said, we'd like to avoid the requirement for auto-switching for our deployment user accounts anyway.

With chruby-exec, we'd have to do this:

cd ${APPDIR} && chruby-exec ${RUBY_VERSION} -- bundle exec thin -C ${CONF}

...which, while doable, requires us to source the .ruby-version file in the deployment script, or have duplication of version specification. Therefore, I forked so we could follow our current convention, and figured you might at least want to take a look. Now we can do the following, as long as there's a .ruby-version file in $APPDIR:

cd ${APPDIR} && chruby-exec -- bundle exec thin -C ${CONF}

I don't know if this goes against your design principles, or if there should at least be a configurable option to allow/disallow hunting for a .ruby-version file using chruby-exec, but I figured since it was something we were using extensively with rbfu (a similarly barebones approach to the problem), it might come in handy for other chruby users as well.

I'm not an experienced shell programmer, so I may have made some obvious mistakes, but hopefully it's at least close. I thought about DRYing up the hunt for a .ruby-version file so it's not duplicated in auto.sh and the new function I added to chruby-exec, but the refactoring would probably introduce more complexity than it's worth (since auto.sh's is so dependent on the RUBY_VERSION_FILE env variable). Maybe there's a relatively clean way of doing so, but again - shell scripting isn't a forte.

I did also make a small tweak to the chruby-exec test file to reduce false positives depending on developer setup, but possibly a resolution such as that proposed in #95 would render that tweak unnecessary.

Again, thanks for this remarkably sane piece of software, from our dev team and especially our inexplicably-not-completely-bald-yet ops team.

Ravi Gadad added 4 commits April 6, 2013 13:47
Testing ruby's RUBY_VERSION makes for more likely false positives;
testing RUBY_ROOT (set by chruby) makes it more likely we're actually
detecting the correct Ruby.
If chruby-exec is called with a command but no RUBY specified (before
the "--" delimiter), it will attempt to locate a .ruby-version file
in the current working directory or its ancestors (just as chruby_auto
does).
"argv" is misleading, since the total argument vector includes the
delimiter and the command, but also even more so now that chruby-exec
can set the Ruby from a .ruby-version file if it's not present in args.
@DavidEGrayson
Copy link
Contributor

Hey ravigadad. I am considering switching from RVM to chruby and like you I would like to avoid specifying the ruby version in lots of different places (e.g. cron jobs), even though the chruby wiki page for cron says to do it.

Wouldn't this work ok?

cd ${APPDIR} && chruby-exec `cat .ruby-version` -- bundle exec thin -C ${CONF}

This doesn't "source" the .ruby-version file, it just reads it with cat. You have clearly thought about this a lot more than I have, so is there something wrong with using a line of code like the one above?

@ravigadad
Copy link
Author

David,

That would work fine -- chruby is cating the file when it reads a .ruby-version file anyway. However, the one disadvantage to this approach is that you won't get the benefit of walking up the directory hierarchy.. so if you chruby-exec in a subdirectory, the .ruby-version file won't be found. In the specific case we've both called out, this wouldn't be an issue, if you're following the convention of putting the .ruby-version file at your app root. So it's up to you whether this is enough of an issue to make this an undesirable approach.

Our devops has just been using my fork for now, since this pull request appears to have been "silently rejected," though I think if the enhancement discussed in issue #127 (changing chruby-exec to a function) gets developed, it can reuse the ruby-version lookup from auto.sh, making for a cleaner adoption of this functionality (whether or not that ends up being pulled upstream).

@postmodern
Copy link
Owner

Sorry work has been dominating my free-time. In order to preserve the semantics of chruby-exec while preventing duplication of code, we should add a new function in auto.sh.

Pseudo Code

function chruby_auto_exec() {
  cd $1
  shift
  chruby_auto
  $*
}

This is similar to @DavidEGrayson's suggestion, but uses chruby_auto.

@postmodern
Copy link
Owner

@ravigadad do you want to try implementing this chruby_auto_exec function or should I implement that in the 0.4.0 branch?

@postmodern postmodern closed this May 28, 2013
@postmodern
Copy link
Owner

Closing in favour of a chruby-auto-exec function.

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.

None yet

3 participants