This is the pattern we've been adopting for a while and it's a bit
cleaner. Let's remove all of the existing usage of the existing pattern
to avoid confusion when adopting the new one.
`dlopen`ing a library executes potentially untrusted code (e.g. if the
library has initialisers). We can avoid the `dlopen` call by asking
`libSystem` directly about whether a library can be found in the shared
cache.
Of course, the `dlopen` happens after a `ENOENT`, so the attack surface here
is relatively small. But relying on this still exposes us to a potential
TOCTOU[^1] bug. Let's avoid it entirely by skipping `dlopen` altogether.
Also: add RBI for `Fiddle` constants used in `linkage_checker`
Upstream don't have these definitions yet, so I've added an RBI for them
in the meantime.
[^1]: https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use
Some formulae include these files, and they can't always be removed.
However, they can cause spurious linkage failures, so let's skip them
when checking for linkage. See, for example, faust at
Homebrew/homebrew-core#191308.
- Previously I thought that comments were fine to discourage people from
wasting their time trying to bump things that used `undef` that Sorbet
didn't support. But RuboCop is better at this since it'll complain if
the comments are unnecessary.
- Suggested in https://github.com/Homebrew/brew/pull/18018#issuecomment-2283369501.
- I've gone for a mixture of `rubocop:disable` for the files that can't
be `typed: strict` (use of undef, required before everything else, etc)
and `rubocop:todo` for everything else that should be tried to make
strictly typed. There's no functional difference between the two as
`rubocop:todo` is `rubocop:disable` with a different name.
- And I entirely disabled the cop for the docs/ directory since
`typed: strict` isn't going to gain us anything for some Markdown
linting config files.
- This means that now it's easier to track what needs to be done rather
than relying on checklists of files in our big Sorbet issue:
```shell
$ git grep 'typed: true # rubocop:todo Sorbet/StrictSigil' | wc -l
268
```
- And this is confirmed working for new files:
```shell
$ git status
On branch use-rubocop-for-sorbet-strict-sigils
Untracked files:
(use "git add <file>..." to include in what will be committed)
Library/Homebrew/bad.rb
Library/Homebrew/good.rb
nothing added to commit but untracked files present (use "git add" to track)
$ brew style
Offenses:
bad.rb:1:1: C: Sorbet/StrictSigil: Sorbet sigil should be at least strict got true.
^^^^^^^^^^^^^
1340 files inspected, 1 offense detected
```
Check for indirect dependencies with linkage with linkage in strict
test mode.
This should be done to ensure we accurately declare dependencies in
homebrew/core.
Currently, `brew linkage` reports linkage with system frameworks only if
they can be found on the file system. This results in this linkage not
being reported on Big Sur and newer, where system libraries are stored
in the dyld cache instead.
Let's fix that by avoiding silently ignoring system frameworks by moving
them out of `#harmless_broken_link?`. We retain the behaviour desired
from 7228e60da51392b20d55e0c087b2082b86fb3bbf by deferring checking if a
broken library is actually a system framework to just before we add it
to `@broken_dylibs`.
To see how this changes the behaviour of `brew linkage`, here's an
example with this change:
❯ brew linkage neovim
System libraries:
/System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices
/usr/lib/libSystem.B.dylib
/usr/lib/libiconv.2.dylib
/usr/lib/libutil.dylib
Homebrew libraries:
/usr/local/opt/gettext/lib/libintl.8.dylib (gettext)
/usr/local/opt/libtermkey/lib/libtermkey.1.dylib (libtermkey)
/usr/local/opt/libuv/lib/libuv.1.dylib (libuv)
/usr/local/opt/luajit/lib/libluajit-5.1.2.dylib (luajit)
/usr/local/opt/luv/lib/libluv.1.dylib (luv)
/usr/local/opt/msgpack/lib/libmsgpackc.2.dylib (msgpack)
/usr/local/opt/tree-sitter/lib/libtree-sitter.0.dylib (tree-sitter)
/usr/local/opt/unibilium/lib/libunibilium.4.dylib (unibilium)
and without this change:
❯ brew linkage neovim
System libraries:
/usr/lib/libSystem.B.dylib
/usr/lib/libiconv.2.dylib
/usr/lib/libutil.dylib
Homebrew libraries:
/usr/local/opt/gettext/lib/libintl.8.dylib (gettext)
/usr/local/opt/libtermkey/lib/libtermkey.1.dylib (libtermkey)
/usr/local/opt/libuv/lib/libuv.1.dylib (libuv)
/usr/local/opt/luajit/lib/libluajit-5.1.2.dylib (luajit)
/usr/local/opt/luv/lib/libluv.1.dylib (luv)
/usr/local/opt/msgpack/lib/libmsgpackc.2.dylib (msgpack)
/usr/local/opt/tree-sitter/lib/libtree-sitter.0.dylib (tree-sitter)
/usr/local/opt/unibilium/lib/libunibilium.4.dylib (unibilium)
This test is causing some rebuilds due to failed linkage upon upgrade.
That's a problem because rebuilds won't fix the problem that the `RPATH`
check identifies.
An executable that links against @rpath-prefixed dylibs must include at least
one runtime path. This will prevent issues like the one resolved in #91485.
Caveats:
1. This won't find executables that have only recursive dylib dependencies with
@rpath prefixes.
2. This makes no attempt to resolve @rpath, @executable_path or @loader_path
dylibs, or to verify the correctness of any LC_RPATH entries, or to
otherwise simulate dlopen logic.
3. Weakly-linked dylibs are still excluded from the search for broken linkage.
The scope is narrow in order to focus on this particular problem. It is meant
only as a sanity check.
There was a previous discussion about making `brew linkage --test` fail
for unrequested dependencies (#9172). I'm not sure what the outcome of
that was, but it still seems like a good idea to try to help us find
cases of opportunistic linkage as they happen rather than when they
cause CI failures in another PR sometime later.
Let's do this by adding a `--strict` flag to `brew linkage --test`. My
intention is for `brew linkage --test --strict` failures to be warnings
rather than errors in CI, which should mitigate some of the concerns
about doing this that were raised in #9172.
The linkage check currently does nothing to check the validity of
variable-referenced libraries (prefixed with an `@`).
We could rectify that by mimicking the dynamic linker in looking up the
variable-referenced library, but this could get quite complicated.
Instead, let's let the linker do the hard work by checking if we can
`dlopen` libraries and bundles that contain variable linkage. The
`dlopen` will fail if the linker cannot resolve the variable reference.
There are at least two disadvantages to this approach relative to the
alternative suggested above:
1. This doesn't work for binary executables.
2. This doesn't identify which variable references are broken.
It's still better than not checking them at all, which is what we do
currently, and saves us from having to carry around code that parses and
verifies variable references directly.
- manually `raise Errno::ENOENT` to ensure that a keg that doesn't exist
isn't flagged as a system dependency.
- remove the inconsistent and incorrect summary messaging.
This makes use of both the existing interfaces and could use the
existing cache file but we'll create a new one and cleanup the old one
to avoid issues and use a more consistent name.
