Homebrew / homebrew-core

🍻 Default formulae for the missing package manager for macOS (or Linux)
https://brew.sh
BSD 2-Clause "Simplified" License
13.74k stars 12.43k forks source link

Re-evaluate OpenSSL's keg-only status #104784

Closed carlocab closed 1 year ago

carlocab commented 2 years ago

OpenSSL is keg-only on macOS because Apple provides LibreSSL:

https://github.com/Homebrew/homebrew-core/blob/d8c7d1338d7a4e16836fa0f209fa00e150efc9a4/Formula/openssl%401.1.rb#L28

I think we should discuss whether this is still necessary today. It was previously made non-keg-only [1], but that was reverted [2].

The short version for why we might wish to reconsider this is that the reasons why we reverted [1] are no longer as significant for the versions of macOS that we support. (See [3] for a discussion of those reasons.)

On Big Sur and newer, it is, as best I can tell, impossible to link with the Apple-supplied LibreSSL. This is because the macOS SDKs do not provide the *.tbd stubs required to link with LibreSSL. [4]

On Catalina, it is still possible, but difficult. It requires, at minimum, a third-party toolchain: Apple's clang refuses to tell the linker to look in /usr/lib, while Apple's ld will flat-out refuse to link LibreSSL. If you manage to get around that (using a Homebrew compiler will also require you to clear the sysroot), you'll find that the OS quickly kills your executable for trying to load LibreSSL.

[1] https://github.com/Homebrew/legacy-homebrew/commit/9ca3c054c070cc825870a6fc0fc5c5a22f7d8559 [2] https://github.com/Homebrew/legacy-homebrew/pull/41615 [3] https://github.com/Homebrew/legacy-homebrew/issues/41613 [4] I suppose, in theory, you can create your own *.tbd files that tell the linker how to link to LibreSSL. What is true about Catalina probably applies here if you manage to make that work.

MikeMcQuaid commented 2 years ago

I guess my main question would be: what's the advantage of changing this and have any users been asking for this?

carlocab commented 2 years ago

I guess my main question would be: what's the advantage of changing this and have any users been asking for this?

Well, I think formulae being keg-only is generally a worse user experience than them not being so, so I think it's a good idea to avoid it wherever we can.

More to your point: there's probably no shortage of examples online recommending brew link --force openssl (even if that doesn't work), or even something similar to ln -s "$(brew --prefix openssl)/lib/* /usr/local/lib (which does work).

That said, I am sympathetic to the implied "if it's not broken...", which is why I wanted an open discussion about this, at least in part to gauge interest.

MikeMcQuaid commented 2 years ago

That said, I am sympathetic to the implied "if it's not broken...", which is why I wanted an open discussion about this, at least in part to gauge interest.

Yeh, I lean towards "if it's not broken..." but also agree that keg-only is nicer to avoid when possible.

If you're (which you normally are!) signing up to do this and deal with any issues that crop up: go for it, as far as I'm concerned.

carlocab commented 2 years ago

If you're (which you normally are!) signing up to do this and deal with any issues that crop up: go for it, as far as I'm concerned.

Yes, I'm 👍 to work on this.

I'll probably wait until we complete/get further along some bigger currently ongoing work (e.g. Python migration, Boost/ICU version bump) to avoid hitting CI too hard. Or, potentially, do it at an OpenSSL version bump, but I'm a little wary of shipping non-atomic updates like this, particularly for formulae like OpenSSL. Another possibility is to do it as part of the OpenSSL 3 migration, but we're probably a long way from that.

Ping @Homebrew/maintainers -- please let me know if there are any subtle issues associated from doing this that I'm missing, and if you have feedback regarding the timelines suggested above, etc.

Bo98 commented 2 years ago

Another possibility is to do it as part of the OpenSSL 3 migration, but we're probably a long way from that.

I agree this makes sense. I think we can make OpenSSL 3 not keg-only and avoid the hassle of later migrating that keg-only status. The general migration of formulae to OpenSSL 3 should happen very soon I reckon.

The steps that need to be done first before OpenSSL 3 migration:

carlocab commented 2 years ago
  • Make OpenSSL 1.1 keg-only on Linux

This may require rebuilding everything that links with OpenSSL on Linux.

We don't add opt directories to RPATH on Linux when a formula is not keg-only, and instead rely on formulae being linked. (We should really change this.)

Bo98 commented 2 years ago
  • Make OpenSSL 1.1 keg-only on Linux

This may require rebuilding everything that links with OpenSSL on Linux.

We don't add opt directories to RPATH on Linux when a formula is not keg-only, and instead rely on formulae being linked. (We should really change this.)

I thought we did add everything, but older bottles prioritised HOMEBREW_PREFIX/lib over them (which shouldn't be an issue for OpenSSL 99% of the time)

carlocab commented 2 years ago

Hmm, maybe I'm mistaken then. I'll look into it later.

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

carlocab commented 2 years ago

Now that we've started the migration to OpenSSL 3, this seems like a fine place to continue discussion about how we plan to do this.

We need to identify groups of formulae that need to be migrated together. One place to start is looking at openssl@1.1's dependent graph: openssl.dot.txt

This was generated with

brew uses --recursive --eval-all --formula openssl@1.1 | \
  xargs brew deps --union --graph --dot --eval-all --formula > openssl.dot

I think we can figure out which formulae need to be handled together by removing all the openssl@1.1 nodes (openssl_removed.dot.txt) and then running a DFS on the resulting graph.

Edit: Ah, but the graph(s) above contain too many formulae, since they will also contain (many) formulae that don't depend on OpenSSL. Fixed in openssl_removed_only_deps.dot.txt.

carlocab commented 2 years ago

I think these are the leaves which can be migrated individually:

alpine
amqp-cpp
arangodb
authoscope
bibtexconv
bitchx
btpd
bulk_extractor
cargo-audit
cargo-instruments
conserver
crackpkcs
cvsync
dmg2img
dovecot
duo_unix
dura
elinks
eureka
git-crypt
git-trim
gocryptfs
gsoap
gtmess
hcxtools
http_load
httrack
imap-uw
iperf3
jj
kore
libcapn
libcoap
libmowgli
liboqs
libtins
links
lychee
lynx
md5sha1sum
minimal-racket
mit-scheme
mongoose
monit
monitoring-plugins
mysql-client@5.7
mysql@5.6
nopoll
ntp
nushell
nzbget
openrtsp
opensc
osslsigncode
pev
pjproject
pkcs11-tools
pulledpork
rover
s3-backer
sblim-sfcc
scamper
sccache
scrypt
selene
shairport
shellinabox
siege
sipp
sipsak
slowhttptest
snobol4
softhsm
spiped
squid
ssh-permit-a38
stuntman
tcpflow
testssl
u-boot-tools
uftp
unshield
virtuoso
webfs
wrk
xaric
xidel
equal-l2 commented 1 year ago

Tracking list for the leaves

WIP

Failures

Done

- [x] `alpine` - #115272 - [x] `amqp-cpp` - #113514 - [x] `authoscope` - #113079 - [x] `bibtexconv` - #116143 - [x] `bitchx` - #113560 - [x] `btpd` - #113561 - [x] `bulk_extractor` - #113568 - [x] `cargo-audit` - #113398 - [x] `cargo-instruments` - #113400 - [x] `conserver` - #113410 - [x] `cvsync` - #113570 - [x] `dmg2img` - #116145 - [x] `dovecot` - #116146 - [x] `duo_unix` - #116147 - [x] `dura` - #113572 - [x] `elinks` - #116253 - [x] `eureka` - #113575 - [x] `git-crypt` - #116148 - [x] `git-trim` - #116149 - [x] `gocryptfs` - #113599 - [x] `gsoap` - #113076 - [x] `gtmess` - #113590 - [x] `hcxtools` - #116151 - [x] `http_load` - #116152 - [x] `httrack` - #116154 - [x] `imap-uw` - #116155 - [x] `iperf3` - #113602 - [x] `jj` - #113648 - [x] `kore` - #116157 - [x] `libcapn` - #116158 - [x] `libcoap` #116159 - [x] `libmowgli` #116160 - [x] `liboqs` - #116161 - [x] `libtins` - #113662 - [x] `links` - #113504 - [x] `lychee` - #113503 - [x] `lynx` - #116163 - [x] `md5sha1sum` - #116164 - [x] `mit-scheme` - #119887 - [x] `mongoose` - #116166 - [x] `monit` - #116167 - [x] `monitoring-plugins` - #116168 - [x] `nopoll` - #116169 - [x] `ntp` - #119175 - [x] `nushell` - #116172 - [x] `nzbget` - #116239 - [x] `openrtsp` - #116240 - [x] `opensc` - #116241 - [x] `osslsigncode` - #116242 - [x] `pev` - #116243 - [x] `pjproject` - #116244 - [x] `pulledpork` - #116246 - [x] `rover` - #116247 - [x] `sblim-sfcc` - #116249 - [x] `scamper` - #116250 - [x] `sccache` - #113217 - [x] `scrypt` - #116252 - [x] `selene` - #113220 - [x] `shairport` - #116153 - [x] `shellinabox` - #116150 - [x] `siege` - #116122 - [x] `sipp` - #116119 - [x] `sipsak` - #116113 - [x] `slowhttptest` - #105942 - [x] `snobol4` - #116112 - [x] `softhsm` - #116055 - [x] `spiped` - #116054 - [x] `squid` - #113684 - [x] `stuntman` - #116051 - [x] `tcpflow` - #113500 - [x] `testssl` - #113499 - [x] `u-boot-tools` - #115914 - [x] `uftp` - #112996 - [x] `unshield` - #113154 - [x] `virtuoso` - #113152 - [x] `webfs` - #115412 - [x] `wrk` - #113104 - [x] `xaric` - #115340 - [x] `xidel` - #114470
carlocab commented 1 year ago

Thanks for your work on getting this going here, @equal-l2, @chenrui333, and @cho-m.

We still need to discuss how to handle the rest, which are going to be harder.

Some ideas:

  1. Use a staging branch for the migration. We can try to migrate formulae incrementally on a staging branch. This staging branch will fail the audit, but it should not once it is ready to be merged into master.
  2. Try to leverage actions parallelism better. I'm not 100% clear on how this might work, though.

Any other suggestions, @Homebrew/core?

MikeMcQuaid commented 1 year ago

@carlocab My thinking is that stuff like https://github.com/Homebrew/homebrew-core/pull/125556 and https://github.com/Homebrew/homebrew-core/pull/125595 will help us here. I think rather than a staging branch ideally the PR itself should be the staging branch and reruns are better at not repeatedly rebuilding the same things.

carlocab commented 1 year ago

reruns are better at not repeatedly rebuilding the same things.

Actually, I think this is key. If we figure this out then I think we'll be fine, and I agree that #125556 and #125595 will help. It would be good to know what other pieces we need to get to a state where we can safely re-run CI, with re-runs skipping all the work that's previously been completed.

MikeMcQuaid commented 1 year ago

@carlocab Agreed. I think basically it is: we need to publish artefacts of everything built and reuse rather than rebuilding these unless the formula (or maybe ideally the actual output of the formula somehow) has changed.

carlocab commented 1 year ago

My thinking is that stuff like #125556 and #125595 will help us here.

We've done those -- what are our next steps here?

One thing that would be nice to have is to be able to selectively re-run failing matrix jobs in CI. Currently, if you re-run, say, only 13-arm64 through the GitHub UI, that results in a rerun for all other ephemeral runners too (because GITHUB_RUN_ATTEMPT is incremented). I think @Bo98 had ideas for how to handle this, but not sure.

Bo98 commented 1 year ago

I'll take a look.

In theory it should be impossible for two attempts to overlap so should be safe for two things to have the same name. I mostly use it to prevent double deploys (webhooks can be sent twice). If the workflow_job hook as an attempt number I can fetch it from that.

I might need to modify the job completion script on the VM machines themselves which will mean a short maintenance period but we'll see. Maybe not - will have a think.

carlocab commented 1 year ago

If the workflow_job hook as an attempt number I can fetch it from that.

The docs include it, so it should be there.

Bo98 commented 1 year ago

Ok I can probably implement this quite simply then where it keeps the same naming system but changes the label (or add rather for a transition period). Will take a look tomorrow.

MikeMcQuaid commented 1 year ago

Sounds great, thanks @bo98 and @carlocab

carlocab commented 1 year ago

Avoiding rebuilds implemented at Homebrew/homebrew-test-bot#922.

Another thing that would be helpful would be to avoid re-testing a dependent if we tested it earlier and it passed (and nothing relevant has changed since then).

MikeMcQuaid commented 1 year ago

Another thing that would be helpful would be to avoid re-testing a dependent if we tested it earlier and it passed (and nothing relevant has changed since then).

@carlocab Agreed, this is a great idea 👍🏻

carlocab commented 1 year ago

Here's the list of the 333 direct dependents of openssl@1.1, sorted by number of direct dependents (indicated in the second column):

``` python@3.11 720 node 156 libevent 57 python@3.10 45 libpq 42 systemd 28 qt 28 libzip 24 curl 21 python@3.9 20 php 20 ruby 16 tcl-tk 15 pulseaudio 15 mysql-client 15 erlang 15 apr-util 14 krb5 13 postgresql@14 12 groonga 12 libssh2 11 node@18 10 gstreamer 10 berkeley-db 9 opusfile 8 openldap 8 httpd 8 libssh 7 libfido2 7 rtmpdump 6 freetds 6 folly 6 unbound 5 redis 5 neon 5 mono 5 grpc@1.54 5 w3m 4 srt 4 python@3.8 4 node@16 4 node@14 4 mongo-c-driver 4 libshout 4 librdkafka 4 git 4 dotnet 4 crystal 4 apache-arrow 4 zookeeper 3 xml-security-c 3 mysql 3 libxmlsec1 3 libwebsockets 3 libimobiledevice 3 ldns 3 fizz 3 dotnet@6 3 cargo-c 3 awscli 3 yara 2 xml-tooling-c 2 wangle 2 ttyd 2 tor 2 texlive 2 tarsnap 2 subversion 2 sane-backends 2 ruby@2.7 2 qpdf 2 pytorch 2 python@3.7 2 nmap 2 net-snmp 2 mysql@5.7 2 libtrace 2 libfixbuf 2 libewf 2 grpc 2 gpac 2 davix 2 cpprestsdk 2 breezy 2 ansible 2 afflib 2 xrootd 1 thrift 1 srtp 1 spice-gtk 1 sofia-sip 1 qpid-proton 1 pypy 1 postgresql@15 1 postgresql@13 1 php@8.1 1 php@8.0 1 php@7.4 1 percona-server 1 opensaml 1 nginx 1 neko 1 mupdf 1 mariadb-connector-c 1 mariadb 1 libtorrent-rasterbar 1 libstrophe 1 libslax 1 libsignal-protocol-c 1 liboauth 1 keyring 1 hatch 1 h2o 1 gwenhywfar 1 getdns 1 gdcm 1 gambit-scheme 1 fbthrift 1 fb303 1 fabric 1 etcd-cpp-apiv3 1 duplicity 1 couchdb 1 condure 1 aria2 1 znc 0 zeek 0 ykman 0 xmount 0 wownero 0 web100clt 0 wdc 0 watchman 0 vineyard 0 uwsgi 0 upscaledb 0 tremor-runtime 0 transmission-cli 0 tiny-fugue 0 thrift@0.9 0 termius 0 telegram-cli 0 tectonic 0 syslog-ng 0 sysdig 0 sysbench 0 sylpheed 0 svtplay-dl 0 subversion@1.8 0 sstp-client 0 sslyze 0 sslsplit 0 ssh-permit-a38 0 sproxy 0 spotify_player 0 spotify-tui 0 sphinx 0 spdylay 0 solana 0 snownews 0 sile 0 shibboleth-sp 0 sheldon 0 shairport-sync 0 sapling 0 samba 0 salt 0 s3ql 0 s3-backer 0 ruby@3.0 0 ruby@2.6 0 ruby@2.5 0 ruby@2.4 0 rtags 0 root 0 robot-framework 0 rethinkdb 0 retdec 0 pypy3 0 pwntools 0 pure-ftpd 0 psqlodbc 0 prowler 0 profanity 0 postgresql@9.5 0 postgresql@9.4 0 postgresql@12 0 postgresql@11 0 postgresql@10 0 poac 0 pgloader 0 pgcli 0 pgbouncer 0 percona-xtrabackup 0 percona-toolkit 0 pdns 0 passenger 0 overdrive 0 osc 0 openstackclient 0 openssh 0 openrct2 0 openiked 0 onlykey-agent 0 nut 0 nsd 0 nmh 0 nghttp2 0 neomutt 0 mytop 0 mysql@5.6 0 mysql-connector-c++ 0 mysql-client@5.7 0 mydumper 0 mycli 0 mutt 0 musikcube 0 moto 0 mosquitto 0 monkeysphere 0 monero 0 molecule 0 mitmproxy 0 minimal-racket 0 midnight-commander 0 micromamba 0 mfterm 0 megatools 0 mavsdk 0 mathlibtools 0 mariadb@10.9 0 mariadb@10.8 0 mariadb@10.7 0 mariadb@10.6 0 mariadb@10.5 0 mariadb@10.4 0 mariadb@10.3 0 mariadb@10.2 0 mariadb@10.11 0 mariadb@10.10 0 mariadb-connector-odbc 0 manticoresearch 0 makepkg 0 magic-wormhole 0 luvit 0 localstack 0 linode-cli 0 lighttpd 0 libzdb 0 libvnc 0 libpulsar 0 libfreefare 0 libevhtp 0 libdap 0 libcouchbase@2 0 libcouchbase 0 ldapvi 0 lastpass-cli 0 lasso 0 lanraragi 0 irssi 0 ios-webkit-debug-proxy 0 innotop 0 icecast 0 hydra 0 hurl 0 howdoi 0 heimdal 0 hashpump 0 got 0 glib-openssl 0 gkrellm 0 git-series 0 gimme-aws-creds 0 gerbil-scheme 0 gammu 0 freeswitch 0 freeradius-server 0 flintrock 0 fdroidserver 0 fastnetmon 0 ettercap 0 esptool 0 erlang@23 0 erlang@22 0 erlang@21 0 eralchemy 0 emqx 0 ejabberd 0 efl 0 ecflow-ui 0 dzr 0 dvc 0 dstack 0 dog 0 dnsviz 0 dnsperf 0 dnsdist 0 cyrus-sasl 0 cyral-gimme-db-token 0 crystal-icr 0 cryfs 0 credstash 0 crackpkcs 0 cppcms 0 coturn 0 conan@1 0 conan 0 cnats 0 clamav 0 charm-tools 0 certbot 0 center-im 0 cargo-outdated 0 cargo-edit 0 cadaver 0 burp 0 buku 0 borgbackup 0 bigloo 0 biber 0 azure-storage-cpp 0 azure-cli 0 awsume 0 awslogs 0 aws-elasticbeanstalk 0 arangodb 0 appscale-tools 0 ansible@7 0 ansible@6 0 ansible@2.9 0 ansible@2.8 0 abricate 0 ```

I guess it makes sense to start building formulae at the top.

Bo98 commented 1 year ago

Python might be tricky but the rest is definitely possible in single PRs - we've done more than all those sizes before.

If you can split the Python dependents into subtrees, there may be a couple we can add exceptions for. Ones that use the Python interpreter but don't link to Python are candidates for things that I think are safe to split without needing to investigate much further.

carlocab commented 1 year ago

It's not that simple, unfortunately. Even trying to migrate libevent, for example, drags in a large part (if not all) of the rest of the dependency graph: #129500

carlocab commented 1 year ago

Formulae with linkage to openssl@1.1 (on Linux)

Details

``` afflib amber ansible ansible@7 apr-util arangodb aria2 aws-elasticbeanstalk aws-google-auth aws-sam-cli aws-sdk-cpp awscli awscurl azure-cli azure-storage-cpp berkeley-db biber bigloo borgbackup btfs buku cargo-c cargo-edit cargo-outdated center-im certbot charm-tools charmcraft citus clamav cmusfm cnats condure coturn couchdb cpprestsdk crackpkcs credstash crystal-icr cups curl cyral-gimme-db-token cyrus-sasl davix ddclient dnsdist dnsperf dnsviz dog dotnet dotnet@6 dstack duplicity dvc dxpy ecflow-ui efl ejabberd emqx erlang erlang@23 esphome esptool ettercap fabric fastnetmon fb-client fbthrift fdroidserver fizz flintrock folly freeradius-server freeswitch freetds gambit-scheme gdal gdcm gerbil-scheme get_iplayer getdns gimme-aws-creds git git-series gkrellm glib-openssl gpac groonga grpc gstreamer gwenhywfar h2o hadoop hashpump hatch heimdal howdoi htslib httpd hydra icecast imapsync irssi jrnl keepkey-agent keyring krb5 lanraragi lasso lastpass-cli ldapvi ldns lexicon libcouchbase libevent libewf libfido2 libfixbuf libfreefare libgit2@1.5 libimobiledevice liboauth libpq librdkafka libshout libssh libssh2 libstrophe libtorrent-rasterbar libxmlsec1 libzip licensed lighttpd localstack luvit magic-wormhole makepkg manticoresearch mariadb mariadb-connector-c mariadb@10.10 mariadb@10.11 mariadb@10.4 mariadb@10.5 mariadb@10.6 mariadb@10.9 mathlibtools megatools memcached metview mfterm micromamba mitmproxy molecule monero mongo-c-driver monkeysphere mosquitto mupdf mutt mycli mysql mysql-client mysql-client@5.7 mysql-connector-c++ mysql@5.7 neomutt net-snmp nmap nmh node node@16 node@18 nsd ntopng nut nvchecker oci-cli ocrmypdf onlykey-agent openiked openldap openrct2 openssh opusfile osc pam-u2f passenger pdal percona-server pgbouncer php php@8.1 poac pocsuite3 postgresql@11 postgresql@12 postgresql@13 postgresql@14 postgresql@15 prowler pulseaudio pure-ftpd pwntools pypy pypy3 python@3.10 python@3.11 python@3.8 python@3.9 qpdf qpid-proton qt redis retdec rethinkdb rizin robot-framework rtl_433 rtmpdump ruby rustup-init s3-backer sapling sgr shairport-sync sheldon sile snapcraft snownews sofia-sip spice-gtk spotify-tui spotify_player sproxy srt srtp ssh-permit-a38 sslsplit sslyze sstp-client subversion svtplay-dl sylpheed sysdig syslog-ng systemd tarsnap tcl-tk tectonic texlive thrift tor transmission-cli trezor-agent ttyd unbound upscaledb uwsgi volatility w3m wangle watchman wownero x3270 xml-security-c xml-tooling-c xrootd yafc yara ykman zeek znc zookeeper zurl ```

Generated using this workflow.

Would be nice to get a topological sort of this list.

carlocab commented 1 year ago

Closing this in favour of #134234 and #134251.