Shebang processing broken by misbehaving login shell

[jasonm23]

Login shell processes that use >&2 to send warnings etc. will break preprocessing

[jasonm23]

Thinking about this, perhaps we really want to avoid using the login shell.

The likelihood is that preprocessors will live in /usr/local/bin because homebrew. (esp. those using node like... well everything many.)

We can prepend this to the PATH when we run a shell command instead of running the user shell in login mode.

We'd need to have this as an option e.g. Status Icon > Use with homebrew (bool)

I think this solves 99% of issues without introducing new unrelated problems.

Up for discussion.

[jasonm23]

Here's an example where using STDerror + login shell to detect errors in preprocessing gives false positives.

        echo "
Warning: PATH set to RVM ruby but GEM_HOME and/or GEM_PATH not set, see:
    https://github.com/rvm/rvm/issues/3212
" >&2
        if
          [[ -n "${SUDO_USER:-}" ]]
        then
          echo "Hint: To fix PATH errors try using 'rvmsudo' instead of 'sudo', see:
    http://stackoverflow.com/questions/27784961/received-warning-message-path-set-to-rvm-after-updating-ruby-version-using-rvm/28080063#28080063
" >&2
        fi

RVM, is using STDERR to send warnings and hints...

[jasonm23]

I think catching preprocessor errors is more worthwhile than using the login shell, so adding the homebrew bin path will help most people out.

Perhaps others can use a shebang to a shell script as a workaround?

[kasper]

Hah, well isn’t that helpful of RVM! Here’s some quick thoughts:

1. This isn’t likely a widespread issue for most people.
2. Wouldn’t RVM users encounter those “hints” whenever they use Terminal regardless of what they do? I would believe this would irritate people in its own.
3. Preprocessing should work out-of-the-box, this shouldn’t require any workarounds from users.

What we could do though, is first retrieve the user’s $PATH and then set and use that in a loginless shell. That might be the best alternative.

[jasonm23]

I think retrieve the shell to get env, read the path. Then a second shell (with the path injected) to run the preprocessing is the best way.

Well the way which would be most useful to the most users.

Side note, I doubt RVM is the only thing that writes to STDERR which could be in people's login shell, especially those using bash_it, pretzo, oh-my-zsh, antigen... Etc.

On 15 Dec 2015, at 8:19 PM, Kasper Hirvikoski notifications@github.com wrote:

Hah, well isn’t that helpful of RVM! Here’s some quick thoughts:

This isn’t likely a widespread issue for most people.
Wouldn’t RVM users encounter those “hints” whenever they use Terminal regardless of what they do? I would believe this would irritate people in its own.
Preprocessing should work out-of-the-box, this shouldn’t require any workarounds from users.
What we could do though, is first retrieve the user’s $PATH and then set and use that in a loginless shell. That might be the best alternative.

—
Reply to this email directly or view it on GitHub.

[kasper]

Actually, the right solution would be to determine success by the exit code, instead of the length of the standard error. Of course, you would still receive any additional errors to the stream, but I think this might be justifiable. Nevertheless, verifying the exit code should be fixed in any case.

[jasonm23]

I think so too, STDERR from a language preprocessor could easily be a warning, which is non-harmful.

Exit code 👍

However, doing anything with the login exit status is not correct. Running the preprocessor independently of the login shell should be the way we do this.

[kasper]

Indeed.

[kasper]

This should hopefully be now fixed in c4e731d and will be released in 2.0.2. @jasonm23 Can you see any potential issues?

[jasonm23]

Looks fine to me. I will spin up a build in a couple hours to test my config.

On 21 Dec 2015, at 3:55 AM, Kasper Hirvikoski notifications@github.com wrote:

This should hopefully be now fixed in c4e731d and will be released in 2.0.2. @jasonm23 Can you see any potential issues?

—
Reply to this email directly or view it on GitHub.