-
-
Notifications
You must be signed in to change notification settings - Fork 10k
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
pour_bottle?: Pour local bottles without sha256 #3105
Conversation
@@ -85,7 +85,7 @@ def pour_bottle?(install_bottle_options = { warn: false }) | |||
return false if @pour_failed | |||
|
|||
bottle = formula.bottle | |||
return false unless bottle | |||
return false unless bottle || formula.local_bottle_path |
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.
return true if formula.local_bottle_path
is on line 94 already so it feels weird to duplicate it here. Can this logic be moved around instead so it's only checked once?
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've removed return true if formula.local_bottle_path
. Pouring a local bottle should still check that that bottle is compatible. --force-bottle
may be used to pour a local bottle that is incompatible, as it is for remote bottles.
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, cool. Can you make this an if
instead; unless
with multiple conditions are hard to parse. After that should be 👍 to 🚢
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.
Done. Thanks for the feedback, Mike.
96821e5
to
77a59ba
Compare
Don't ignore f.pour_bottle? and compatible_cellar? when pouring a local bottle. --force-bottle may be used to pour a local bottle that is incompatible, as it is for remote bottles.
Pouring a local bottle for a formula without a bottle sha256 in the formula for that OS would unexpectedly install from source instead.
77a59ba
to
4cfd333
Compare
Thanks again @sjackman! |
My pleasure. Thanks for the feedback, and thanks for merging, Mike. |
brew tests
with your changes locally?Pouring a local bottle for a formula without a bottle sha256
in the formula for that OS would unexpectedly install from
source instead.