-
-
Notifications
You must be signed in to change notification settings - Fork 9.9k
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
link: don't allow more openssl/libressl linkage. #612
link: don't allow more openssl/libressl linkage. #612
Conversation
EOS | ||
next | ||
if HOMEBREW_PREFIX.to_s == "/usr/local" | ||
if keg.name.start_with?("openssl") || keg.name.start_with?("libressl") |
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 nested if
will mess up the logic in the following elsif keg.linked?
and what follows, i.e. you'll have to keep the condition in one piece. Note you can use multiple arguments to express “starts with any of?”, so this might be short enough to stay on a single line:
if HOMEBREW_PREFIX.to_s == "/usr/local" && keg.name.start_with?("openssl", "libressl")
(My personal belief is that the prefix check could be completely dropped so that users get a more uniform behavior independently of where they chose to install Homebrew, though I do understand that there isn't any issue with non-/usr/local
prefixes. Just a thought—I'm happy either way.)
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.
(My personal belief is that the prefix check could be completely dropped so that users get a more uniform behavior independently of where they chose to install Homebrew, though I do understand that there isn't any issue with non-/usr/local prefixes. Just a thought—I'm happy either way.)
I'd prefer we kept exempting non-/usr/local
prefixes since there's not actually a problem there. I'd argue openssl
shouldn't even really be keg_only
outside of /usr/local
, but there we go.
👍 on the rest of Martin's comment though.
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'd prefer we kept exempting non-
/usr/local
prefixes since there's not actually a problem there. I'd argueopenssl
shouldn't even really bekeg_only
outside of/usr/local
, but there we go.
Maybe to provide a rationale for my thinking: Since those users have to supply custom paths to their compiler and linker invocations anyway, they could as well supply the PREFIX/opt/openssl/{include,lib}
paths. The upside would be that their scripts/etc. will continue to work even if they decide to switch from a non-/usr/local
to a /usr/local
installation or share their work with other Homebrew users.
But as I said, it's just a thought. I'm fine with limiting this to /usr/local
installations.
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.
Part of me wants to get Travis building in /opt/brew
as well so we know how much breaks in that directory. I'd generally like us to stop vilifying non-/usr/local
installations & think it'd be reasonably easy to support, as well as opening up the option of moving in future if Apple clamps down even more or we decide it's worthwhile.
But that's not strictly related to this discussion, so I'll leave that be.
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'd generally like us to stop vilifying non-/usr/local installations & think it'd be reasonably easy to support, as well as opening up the option of moving in future if Apple clamps down even more or we decide it's worthwhile.
My main issue is bottles. They massively improve the user experience and reduce support burden for us.
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.
We're making progress on that front though.
2356 formulae currently have some form of cellar :any
, another 451 are bottle :unneeded
, another 5 are bottle :disable
. So that leaves... 783 non-portable bottles. Rough numbers, obviously, but we're making solid progress here.
We'll get a lot more wrapped up once we work out how to relocate NLS stuff as well.
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.
If we got everything cellar :any
my objection goes away. There's things like git
that'll require upstream changes, though. The best middle ground I've heard is leaving the cellar as-is but just moving the default prefix/repo checkout to /usr/local/homebrew
, /opt/homebrew
or whatever using @chdiza's method.
Addressed feedback. |
This extends the approach in #597 to further prevent linkage of formulae that conflict with the system OpenSSL and can cause the issues described in that issue.
👍 |
brew tests
with your changes locally?This extends the approach in #597 to further prevent linkage of formulae that conflict with the system OpenSSL and can cause the issues described in that issue. CC @DomT4 for thoughts.