rails / cssbundling-rails

Bundle and process CSS in Rails with Tailwind, PostCSS, and Sass via Node.js.
MIT License
579 stars 83 forks source link

rbenv breaks an assumption in `bin/dev`. #87

Closed MrJoy closed 2 years ago

MrJoy commented 2 years ago

Because rbenv uses shims, this line in the generated bin/dev script will always return true in an environment in which foreman has been installed in any gemset at any point, even if it's not in the current gemset:

if ! command -v foreman &> /dev/null

I've found this to be a viable alternative that I believe should work everywhere:

if [ $(gem list --local --no-details --quiet --exact foreman | grep foreman | wc -l) -eq 0 ]

(The use of grep is because when foreman isn't installed, gem list will produce an empty line of output.)

dhh commented 2 years ago

Are there any considerations here re: Windows / WSL we need to account for if we use grep + wc?

MrJoy commented 2 years ago

That's an excellent question. I've got an engineer who's using WSL that I can ask. I will update the ticket when I have an answer.

Another option might be something like:

if ! type rbenv &> /dev/null
then
  if ! command -v foreman &> /dev/null
  then
    # ...
  fi
else
  if ! foreman version >/dev/null
  then
    # ...
  fi
fi

It's a little more cumbersome to be sure, but it seems like it would be viable. I think it would neatly compartmentalize any potential quirks / changes of semantics to just us rbenv users.

MrJoy commented 2 years ago

I'm informed by my colleague that when run from a Linux terminal in WSL, grep works fine.

MrJoy commented 2 years ago

@dhh What can I do to move this forward?

t27duck commented 2 years ago

Could if ! foreman version &> /dev/null be used across the board regardless if the developer is using rbenv or not?

MrJoy commented 2 years ago

@t27duck That solution is compatible with rbenv, and certainly seems lower risk. Honestly, I feel silly that it didn't occur to me. Might be worth tossing in a > /dev/null 2>&1 to avoid cluttering output, since you'll get some output to STDOUT/STDERR regardless of which environment you're on. (Edit: Sorry, my shell-fu is weak. Didn't notice the use of &> instead of >.)

dhh commented 2 years ago

Sounds like @t27duck's solution would do. Feel free to PR that.

MrJoy commented 2 years ago

Done.