Skip to content

Fix chruby-exec sourcing chruby.sh - #250

Closed
artempyanykh wants to merge 4 commits into
postmodern:masterfrom
artempyanykh:chruby-exec-sourcing
Closed

artempyanykh wants to merge 4 commits into
postmodern:masterfrom
artempyanykh:chruby-exec-sourcing

Conversation

@artempyanykh

Copy link
Copy Markdown
Contributor

On commit

If I got it right, chruby-exec works as follows:

  1. source chruby.sh
  2. check arguments
  3. execute chruby {target_ruby} && $command in a subshell

The problem is that sourced functions unlike environmental variables
are not 'inherited' by subshells.
(It is true at least in Ubuntu 12.04 LTS, where I tested this)

With "basic setup" you create /etc/profile.d/chruby.sh which sources /usr/share/local/chruby/chruby.sh
Then, when you invoke chruby_exec it works just fine, because
/usr/share/local/chruby/chruby.sh is sourced automaticaly by bash
through /etc/profile initialization script.

It would be great if you consider moving the process of sourcing
/usr/share/local/chruby/chruby.sh from the top of chruby_exec to a
command executed by a subshell. It would allow to use chruby_exec
without needing to source /usr/local/chruby/chruby.sh with shell init
scripts (/etc/profile.d/chruby.sh)

About sourced functions in subshells

You could check it with the following setup:

set_func.sh:

function test_func()
{
    0
}

get_func.sh:

source ./set_func.sh
type test_func | head -1
exec "$SHELL" -i -l -c "type test_func | head -1"

and then:

$ ./get_func.sh
test_func is a function
bash: type: test_func: not found

chruby-exec works as follows:
1. source chruby.sh
2. check arguments
3. execute 'chruby {target_ruby} && $command' in a subshell

The problem is that sourced functions unlike environmental variables
are not 'inherited' by subshells.
(It is true at least in Ubuntu 12.04 LTS, where I tested this)

With "basic setup" you copy chruby.sh to /etc/profile.d, and this
chruby.sh sources /usr/share/local/chruby/chruby.sh
Then, when you invoke chruby_exec it works just fine, because
/usr/share/local/chruby/chruby.sh is sourced automaticaly by bash
through /etc/profile initialization script (which sources
/etc/profile.d/chruby.sh)

It would be great if you consider moving the process of sourcing
/usr/share/local/chruby/chruby.sh from the top of `chruby_exec` to a
command executed by a subshell. It would allow to use `chruby_exec`
without needing to source /usr/local/chruby/chruby.sh with shell init
scripts (/etc/profile.d/chruby.sh)

About function inheritance in a subshells

You could check it with the following setup:

- file set_func.sh:
====================
function test_func()
{
    0
}
====================

- file get_func.sh:
===================
source ./set_func.sh

type test_func | head -1

exec "$SHELL" -i -l -c "type test_func | head -1"
==================

and then:
$ ./get_func.sh
test_func is a function
bash: type: test_func: not found
@postmodern

Copy link
Copy Markdown
Owner

There is one problem here. What if the user has custom configuration for $RUBIES. If we re-source chruby in the sub-shell, it will re-populate $RUBIES and override any custom configuration.

@artempyanykh

Copy link
Copy Markdown
Contributor Author

Oh, I see.

Then, the original source .../chruby.sh at the top of chruby-exec doesn't make any effect. Am I correct?

What would you say, if I add a check before sourcing chruby.sh?
In that case, no custom configuration will brake, and we'll be able to use chruby-exec without the need of sourcing chruby.sh in shell init scripts.

I could check if chruby function exists, and if it is, I won't source chruby.sh.
Another suggestion is to use env var (like CHRUBY_SOURCED) to check if chruby.sh was sourced instead of checking if chruby function exists.

We don't want to mess with user custom chruby config, so we'll source
chruby.sh only if it wasn't sourced before.
@artempyanykh

Copy link
Copy Markdown
Contributor Author

@postmodern could you, please, provide some feedback on the proposed solution?

@schisamo

Copy link
Copy Markdown

👍 I have been seeing the same issue if /etc/profile.d/chruby.sh does not exist a machine.

@mxhold

mxhold commented Dec 19, 2014

Copy link
Copy Markdown

👍

If you don't have /etc/profile.d/chruby.sh and run chruby-exec from a non-interactive shell then chruby-exec never actually sources /usr/local/share/chruby/chruby.sh and chruby never gets defined. This breaks capistrano, see capistrano/chruby#7.

@postmodern can you take another look at this?

@postmodern

Copy link
Copy Markdown
Owner

@mxhold that makes sense for non-login non-interactive shells.

@artempyanykh update the command to use command -v and I'll merge this.

@artempyanykh

Copy link
Copy Markdown
Contributor Author

@postmodern OK, I'll do it.

@artempyanykh

Copy link
Copy Markdown
Contributor Author

I updated the PR. Is it OK?

@postmodern

Copy link
Copy Markdown
Owner

I manually rebased and then squashed (git rebase -i HEAD~3) the three commits into one. Everything should be in 6cdaefd.

@postmodern postmodern closed this Dec 30, 2014
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.

4 participants