Skip to content
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.

Sign up for GitHub

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

Jump to bottom

utils/ruby.sh: search PATH for Ruby on Linux. update.sh: keep HOMEBREW_RUBY_PATH set. #7545

Merged
merged 14 commits into from
May 27, 2020
Merged

utils/ruby.sh: search PATH for Ruby on Linux. update.sh: keep HOMEBREW_RUBY_PATH set. #7545

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
9124e07
cmd/update.sh: keep HOMEBREW_RUBY_PATH set when updating
maxim-belkin May 11, 2020
1cba582
cmd/update.sh: define conditions for unsetting HOMEBREW_RUBY_PATH
maxim-belkin May 12, 2020
328c75f
Move logic related to HOMEBREW_RUBY_PATH to utils/ruby.sh
maxim-belkin May 15, 2020
cbc266b
utils/ruby.sh: split long line in test-ruby() function
maxim-belkin May 15, 2020
d21ecff
utils/ruby.sh: break down long ruby code in test-ruby
maxim-belkin May 15, 2020
ec6e449
utils/ruby.sh: unset HOMEBREW_RUBY_PATH unconditionally
maxim-belkin May 15, 2020
44ef1ab
utils/ruby.sh: suppress standard error in test-ruby
maxim-belkin May 15, 2020
f7e0705
utils/ruby.sh: try installing portable Ruby ...
maxim-belkin May 15, 2020
083f56f
utils/ruby.sh: quote HOMEBREW_RUBY_PATH when calling test-ruby
maxim-belkin May 15, 2020
b4267d8
utils/ruby.sh: avoid nested ifs
maxim-belkin May 17, 2020
dcc486f
utils/ruby.sh: find usable Ruby from PATH only
maxim-belkin May 19, 2020
8dcb163
utils/ruby.sh: add explanatory comments
maxim-belkin May 20, 2020
db561eb
utils/ruby.sh: setup-ruby: handle 'vendor-install' cmd early
maxim-belkin May 21, 2020
5d3456f
utils/ruby.sh: make code more readable
maxim-belkin May 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion Library/Homebrew/cmd/update.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,6 @@ EOS
-d "$HOMEBREW_LIBRARY/LinkedKegs" ||
(-n "$HOMEBREW_DEVELOPER" && -z "$HOMEBREW_UPDATE_PREINSTALL") ]]
then
unset HOMEBREW_RUBY_PATH
brew update-report "$@"
return $?
elif [[ -z "$HOMEBREW_UPDATE_PREINSTALL" ]]
Expand Down
35 changes: 33 additions & 2 deletions Library/Homebrew/utils/ruby.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
test-ruby () {
"$1" --enable-frozen-string-literal --disable=gems,did_you_mean,rubyopt -rrubygems -e "puts Gem::Version.new(RUBY_VERSION.to_s.dup).to_s.split('.').first(2) == Gem::Version.new('$required_ruby_version').to_s.split('.').first(2)"
maxim-belkin marked this conversation as resolved.
Show resolved Hide resolved
}

setup-ruby-path() {
local vendor_dir
local vendor_ruby_current_version
Expand All @@ -6,11 +10,19 @@ setup-ruby-path() {
# When bumping check if HOMEBREW_MACOS_SYSTEM_RUBY_NEW_ENOUGH (in brew.sh)
# also needs to be changed.
local required_ruby_version="2.6"
local old_ruby_path
local old_ruby_usable

vendor_dir="$HOMEBREW_LIBRARY/Homebrew/vendor"
vendor_ruby_current_version="$vendor_dir/portable-ruby/current"
vendor_ruby_path="$vendor_ruby_current_version/bin/ruby"

if [[ -n $HOMEBREW_RUBY_PATH ]]
then
old_ruby_path="$HOMEBREW_RUBY_PATH"
old_ruby_usable=$(test-ruby "$HOMEBREW_RUBY_PATH")
fi

if [[ -z "$HOMEBREW_DEVELOPER" ]]
then
unset HOMEBREW_RUBY_PATH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about just unsetting this unconditionally? It would allow simplifying the logic above and below

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, It should be OK to do it now.

Expand Down Expand Up @@ -45,15 +57,34 @@ EOS
then
HOMEBREW_RUBY_PATH="/System/Library/Frameworks/Ruby.framework/Versions/Current/usr/bin/ruby"
else
HOMEBREW_RUBY_PATH="$(type -P ruby)"
HOMEBREW_RUBY_PATH=$(type -P ruby)
sjackman marked this conversation as resolved.
Show resolved Hide resolved
if [[ $(test-ruby $HOMEBREW_RUBY_PATH) != "true" ]]
Copy link
Contributor

@rmNULL rmNULL May 15, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maxim-belkin L#60,
the test-ruby fn considers $1.
i suspect this will be problematic.
can i know the trick that handles spaced ruby path here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right -- here quotes are necessary. Will fix! Thanks again!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 083f56f.
I was considering using "$@" but then decided to proceed with $1. Thanks for catching it.

then
HOMEBREW_RUBY_PATH=$(PATH="$HOMEBREW_PATH" type -P ruby)
if [[ $(test-ruby "$HOMEBREW_RUBY_PATH") != "true" ]]
then
if [[ $old_ruby_usable != true ]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to avoid quite so much nesting here and instead use an intermediate variable(s) and multiple ifs at the same level?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converted nested ifs to a single if/elif/else conditional block in b4267d8.
Had to add a line to test-ruby that checks that tested file exists and is executable.

then
odie <<-EOS
Failed to find usable Ruby $required_ruby_version!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to move this new logic below so the messaging can be shared rather than repeated.

If there's no Homebrew Portable Ruby available for your processor:
- install $required_ruby_version with your system package manager (or rbenv/ruby-build)
- make it first in your PATH
- try again
EOS
else
HOMEBREW_RUBY_PATH="$old_ruby_path"
fi
fi
fi
fi

if [[ -n "$HOMEBREW_MACOS_SYSTEM_RUBY_NEW_ENOUGH" ]]
then
usable_ruby_version="true"
elif [[ -n "$HOMEBREW_RUBY_PATH" && -z "$HOMEBREW_FORCE_VENDOR_RUBY" ]]
then
usable_ruby_version="$("$HOMEBREW_RUBY_PATH" --enable-frozen-string-literal --disable=gems,did_you_mean,rubyopt -rrubygems -e "puts Gem::Version.new(RUBY_VERSION.to_s.dup).to_s.split('.').first(2) == Gem::Version.new('$required_ruby_version').to_s.split('.').first(2)")"
usable_ruby_version=$(test-ruby "$HOMEBREW_RUBY_PATH")
fi

if [[ -z "$HOMEBREW_RUBY_PATH" || -n "$HOMEBREW_FORCE_VENDOR_RUBY" || "$usable_ruby_version" != "true" ]]
Expand Down