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

chruby-exec seems to be broken #10

Closed
dalizard opened this Issue Jun 2, 2014 · 9 comments

Comments

5 participants
@dalizard

dalizard commented Jun 2, 2014

I am running OSX 10.9.3, along with the following:

~ $ chruby --version
chruby: 0.3.8
chruby-fish: 0.6.0
~ $ fish --version
fish, version 2.1.0
~ $

However, when trying to execute chruby-exec, I get the following error:

~ $ chruby
   jruby-1.7.12
 * ruby-2.1.2
~ $ chruby-exec jruby-1.7.12 -- gem env
fish: Expected a command name, got token of type 'Run job in background'. Did you mean 'COMMAND; and COMMAND'? See the help section for the 'and' builtin command by typing 'help and'.
Standard input: chruby jruby-1.7.12 && gem env
                                     ^
~ $

Any ideas?

@JeanMertz

This comment has been minimized.

Show comment
Hide comment
@JeanMertz

JeanMertz Jun 3, 2014

Owner

chruby-exec is not implemented in this fork (yet). It did exist in the pre-0.5.0 version though, so we might dust that one off and commit it.

The error you get is because you are executing the bash version of chruby-exec, which gets properly executed in a bash environment but then at the last line, executes the command x && y in the current $SHELL shell, which in your case is Fish, and Fish uses x; and y instead. That's the error you are seeing.

Owner

JeanMertz commented Jun 3, 2014

chruby-exec is not implemented in this fork (yet). It did exist in the pre-0.5.0 version though, so we might dust that one off and commit it.

The error you get is because you are executing the bash version of chruby-exec, which gets properly executed in a bash environment but then at the last line, executes the command x && y in the current $SHELL shell, which in your case is Fish, and Fish uses x; and y instead. That's the error you are seeing.

@dalizard

This comment has been minimized.

Show comment
Hide comment
@dalizard

dalizard Jun 3, 2014

Thank you for the update. Should I look into it, and send you a pull request or you are already working on it?

dalizard commented Jun 3, 2014

Thank you for the update. Should I look into it, and send you a pull request or you are already working on it?

@JeanMertz

This comment has been minimized.

Show comment
Hide comment
@JeanMertz

JeanMertz Jun 3, 2014

Owner

Go ahead, at the earliest I can take a look at it this weekend.

Owner

JeanMertz commented Jun 3, 2014

Go ahead, at the earliest I can take a look at it this weekend.

@JoshCheek

This comment has been minimized.

Show comment
Hide comment
@JoshCheek

JoshCheek Aug 31, 2014

Oh, I hit the same thing, but reported it on chruby side postmodern/chruby#284

JoshCheek commented Aug 31, 2014

Oh, I hit the same thing, but reported it on chruby side postmodern/chruby#284

@JeanMertz

This comment has been minimized.

Show comment
Hide comment
@JeanMertz

JeanMertz Aug 31, 2014

Owner

Yes, this is clearly a shortcoming of the current version. I will publish an update sometime next week to have a custom fish version of chruby-exec. It's not ideal, but it will have to do for now.

Owner

JeanMertz commented Aug 31, 2014

Yes, this is clearly a shortcoming of the current version. I will publish an update sometime next week to have a custom fish version of chruby-exec. It's not ideal, but it will have to do for now.

@britishtea

This comment has been minimized.

Show comment
Hide comment
@britishtea

britishtea Nov 21, 2014

I had a stab at implementing this today, but ran into a problem.

The way the bash version of chruby-exec works is by passing a properly escaped (shellquoted) string to exec. That is done with the builtin printf, which fish does not seem to have a (built-in) equivalent for. Consequently, the command has to be escaped manually by the user when calling chruby-exec.

Without proper escaping, the only way to make chruby-exec 2.1.5 -- ruby -e "print RUBY_VERSION" work, for example, is to invoke it as chruby-exec 2.1.5 -- ruby -e '\"print RUBY_VERSION\"'. This is pretty impractical.

The best solution would be to use a custom escape function for fish. I haven't been able to find one and don't trust my fish skills enough to write one. Bash' printf unfortunately escapes specifically for bourne shells, which differs from the way fish' escaping works, so altering the last line in the chruby bash script is out as well :(

Here's the script and (failing) test-file that requires the manual escaping.

bin/chruby-exec

#!/usr/bin/env fish

source (dirname (status -f))/../share/chruby/chruby.fish

switch $argv[1];
    case "-h" "--help"
        echo "usage: chruby-exec RUBY [RUBYOPTS] -- COMMAND [ARGS...]"
        exit
    case "-V" "--version"
        echo "chruby version      " $CHRUBY_VERSION
        echo "chruby-fish version " $CHRUBY_FISH_VERSION
        exit
end

if test (count $argv) -eq 0;
    echo "chruby-exec: RUBY and COMMAND required"
    exit 1
end

for arg in $argv;
    set -e argv[1]
    set chruby_args $chruby_args $arg

    if test "$arg" = "--";
        set -e chruby_args[-1]
        break
    end
end

if test (count $argv) -eq 0;
    echo "chruby-exec: COMMAND required"
    exit 1
end

# TODO: Properly escape $argv here
set cmd "\"chruby $chruby_args; and $argv\""

if isatty 0;
    eval exec "$SHELL -i -l -c $cmd"
else
    eval exec "$SHELL       -c $cmd"
end

test/chruby_exec_text.fish

function suite_chruby_exec
  function setup
    set PATH (dirname (status -f))../bin $PATH
    set RUBIES (dirname (status -f))/opt/rubies
  end

  function test_chruby_exec_no_arguments
    bin/chruby-exec 2>/dev/null

    assert_equal $status 1
  end

  function test_chruby_exec_no_command
    bin/chruby-exec "$test_ruby_version" 2>/dev/null

    assert_equal $status 1
  end

  function test_chruby_exec
    set -l ruby_version (eval "bin/chruby-exec $test_ruby_version -- ruby -e \"print RUBY_VERSION\"")

    echo $ruby_version

    assert_equal "$test_ruby_version" "$ruby_version"
  end
end

if not set -q tank_running
  . (dirname (status -f))/helper.fish
  tank_run
  reset_system_defaults
end

britishtea commented Nov 21, 2014

I had a stab at implementing this today, but ran into a problem.

The way the bash version of chruby-exec works is by passing a properly escaped (shellquoted) string to exec. That is done with the builtin printf, which fish does not seem to have a (built-in) equivalent for. Consequently, the command has to be escaped manually by the user when calling chruby-exec.

Without proper escaping, the only way to make chruby-exec 2.1.5 -- ruby -e "print RUBY_VERSION" work, for example, is to invoke it as chruby-exec 2.1.5 -- ruby -e '\"print RUBY_VERSION\"'. This is pretty impractical.

The best solution would be to use a custom escape function for fish. I haven't been able to find one and don't trust my fish skills enough to write one. Bash' printf unfortunately escapes specifically for bourne shells, which differs from the way fish' escaping works, so altering the last line in the chruby bash script is out as well :(

Here's the script and (failing) test-file that requires the manual escaping.

bin/chruby-exec

#!/usr/bin/env fish

source (dirname (status -f))/../share/chruby/chruby.fish

switch $argv[1];
    case "-h" "--help"
        echo "usage: chruby-exec RUBY [RUBYOPTS] -- COMMAND [ARGS...]"
        exit
    case "-V" "--version"
        echo "chruby version      " $CHRUBY_VERSION
        echo "chruby-fish version " $CHRUBY_FISH_VERSION
        exit
end

if test (count $argv) -eq 0;
    echo "chruby-exec: RUBY and COMMAND required"
    exit 1
end

for arg in $argv;
    set -e argv[1]
    set chruby_args $chruby_args $arg

    if test "$arg" = "--";
        set -e chruby_args[-1]
        break
    end
end

if test (count $argv) -eq 0;
    echo "chruby-exec: COMMAND required"
    exit 1
end

# TODO: Properly escape $argv here
set cmd "\"chruby $chruby_args; and $argv\""

if isatty 0;
    eval exec "$SHELL -i -l -c $cmd"
else
    eval exec "$SHELL       -c $cmd"
end

test/chruby_exec_text.fish

function suite_chruby_exec
  function setup
    set PATH (dirname (status -f))../bin $PATH
    set RUBIES (dirname (status -f))/opt/rubies
  end

  function test_chruby_exec_no_arguments
    bin/chruby-exec 2>/dev/null

    assert_equal $status 1
  end

  function test_chruby_exec_no_command
    bin/chruby-exec "$test_ruby_version" 2>/dev/null

    assert_equal $status 1
  end

  function test_chruby_exec
    set -l ruby_version (eval "bin/chruby-exec $test_ruby_version -- ruby -e \"print RUBY_VERSION\"")

    echo $ruby_version

    assert_equal "$test_ruby_version" "$ruby_version"
  end
end

if not set -q tank_running
  . (dirname (status -f))/helper.fish
  tank_run
  reset_system_defaults
end
@JeanMertz

This comment has been minimized.

Show comment
Hide comment
@JeanMertz

JeanMertz May 18, 2016

Owner

I've been doing some testing, and have a semi-working prototype up in the chruby-exec branch. Some scenarios worked, others didn't, and I'm not too fond of the hacky solution, but we'll see where this ends up.

See 41f0e85 for the implementation itself.

Owner

JeanMertz commented May 18, 2016

I've been doing some testing, and have a semi-working prototype up in the chruby-exec branch. Some scenarios worked, others didn't, and I'm not too fond of the hacky solution, but we'll see where this ends up.

See 41f0e85 for the implementation itself.

@JeanMertz

This comment has been minimized.

Show comment
Hide comment
@JeanMertz

JeanMertz Jul 1, 2017

Owner

I've never really had a need for chruby-exec in the last year or more, so I'm not going to try and fix this at this point in time, but PRs are always welcome.

Owner

JeanMertz commented Jul 1, 2017

I've never really had a need for chruby-exec in the last year or more, so I'm not going to try and fix this at this point in time, but PRs are always welcome.

@JeanMertz JeanMertz closed this Jul 1, 2017

@der-flo

This comment has been minimized.

Show comment
Hide comment
@der-flo

der-flo Jul 7, 2017

I'm also very interested in a solution.

der-flo commented Jul 7, 2017

I'm also very interested in a solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment