-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
Vendored Ruby tweaks #494
Vendored Ruby tweaks #494
Conversation
Revealed this on first install:
|
# shellcheck source=/dev/null | ||
source "$HOMEBREW_LIBRARY/Homebrew/utils/ruby.sh" | ||
setup-ruby-path | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xu-cheng Without this if there's a bug in vendor-install
we cannot get people to brew update
to fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is incorrect because ENV/scm/git
which is used by update
requiring Ruby path set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The long term solution is to rewrite scm script in bash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we can avoid using ENV/scm/git
in update
again (which feels like a better solution for now)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we want to avoid calling any Ruby code until we get to update-report
(which should call vendor-install
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind to wait for a moment? As I think porting to bash is not too difficult and much better solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Any idea how long?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can inline this block into below if [[ -n "$HOMEBREW_BASH_COMMAND" ]] ...
instead of adding an extra condition check.
@xu-cheng Are you 👍 on this now? |
One comment. Otherwise 👍 |
brew tests
with your changes locally?CC @xu-cheng