postmodern / chruby

Changes the current Ruby
MIT License
2.85k stars 190 forks source link

Fix Issue #367: Value of $_ overwritten in bash #381

Open HaleTom opened 7 years ago

HaleTom commented 7 years ago

Fix Issue #367 based on this answer

postmodern commented 7 years ago

I don't think how we hook in chruby_auto works within the unit-test functions. I had to explicitly invoke chruby_auto to simulate it running after other commands.

HaleTom commented 7 years ago

@postmodern how should I deal with that in this PR?

HaleTom commented 7 years ago

@postmodern Merry Festivus. Where to from here?

postmodern commented 7 years ago

@HaleTom so trap '...' DEBUG does not capture commands invoked from inside of functions, only at the top level. This means chruby_auto is automatically invoked in your unit-test. We need to device a test that actually exercises this somehow.

#!/bin/sh

trap 'echo "Command: $BASH_COMMAND"' DEBUG

func() {
    echo 'inside of a function'
}

echo 'outside of a function'
func
HaleTom commented 7 years ago

set -o functrace is what you're after.

Adding this, the output is:

Command: echo 'outside of a function'
outside of a function
Command: func
Command: func
Command: echo 'inside of a function'
inside of a function

I'm not sure why Command: func is duplicated... let me know if it's an issue. This may help.

I'm having a brain-fart: how do I run the tests again? (I couldn't see anything in CONTRIBUTING about it).

Let me know if I can help out with anything from here.

HaleTom commented 7 years ago

@postmodern Here is the code you want:

#!/bin/bash
set +o functrace
trap 'trap_func' DEBUG

trap_func() {
  [[ $BASH_COMMAND != "${FUNCNAME:0}" ]] && echo "Command: $BASH_COMMAND"
}

func() {
  echo 'inside of a function'
}

echo 'outside of a function'
func

Please let me know what the next step is.

HaleTom commented 6 years ago

@postmodern, @steakknife, Any blockers to merge this?

HaleTom commented 6 years ago

Just checking in on this as it's been one human gestation since the last comment.

What is the next step here? @postmodern @steakknife

HaleTom commented 2 years ago

I see that Travis on Apple didn't seem to run correctly - it has null output.

Is this expected? Any other blockers to merging?

postmodern commented 2 years ago

@HaleTom Apple (still) ships with Bash 3, so unfortunately all code has to be Bash 3 compatible. I have since switched to GitHub Actions, however I still need to fix how the test suite downloads/installs the macOS test ruby, which must be compatible with GitHub Action version of macOS.