Cannot upgrade bottled formula when pinning build/test dependency

muneebmahmed commented 1 week ago

`brew doctor` output

Please note that these warnings are just used to help the Homebrew maintainers
with debugging if you file an issue. If everything you use Homebrew for is
working fine: please don't worry or file an issue; just ignore this. Thanks!

Warning: Some installed formulae are deprecated or disabled.
You should find replacements for the following formulae:
  cowsay
  neofetch
  openssl@1.1
  youtube-dl

Verification

[X] My "brew doctor output" above says Your system is ready to brew. and am still able to reproduce my issue.
[X] I ran brew update twice and am still able to reproduce my issue.
[X] This issue's title and/or description do not reference a single formula e.g. brew install wget. If they do, open an issue at https://github.com/Homebrew/homebrew-core/issues/new/choose instead.

`brew config` output

HOMEBREW_VERSION: 4.4.0-127-g3291ad4
ORIGIN: https://github.com/Homebrew/brew
HEAD: 3291ad4fc78f33f8e9bb7ce37745d2a0e697f5fb
Last commit: 17 minutes ago
Core tap HEAD: 3fa17a6c4d41427a92f8e4d976db4301ce1b79eb
Core tap last commit: 15 minutes ago
Core tap JSON: 12 Oct 01:42 UTC
Core cask tap HEAD: e8f346d9cc4a8a99e923b870b4efc3d8928f1ce8
Core cask tap last commit: 29 minutes ago
Core cask tap JSON: 12 Oct 01:42 UTC
HOMEBREW_PREFIX: /usr/local
HOMEBREW_BAT: set
HOMEBREW_CASK_OPTS: []
HOMEBREW_GITHUB_API_TOKEN: set
HOMEBREW_MAKE_JOBS: 12
HOMEBREW_NO_AUTO_UPDATE: set
HOMEBREW_SORBET_RUNTIME: set
Homebrew Ruby: 3.3.5 => /usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/3.3.5/bin/ruby
CPU: dodeca-core 64-bit kabylake
Clang: 16.0.0 build 1600
Git: 2.39.5 => /Applications/Xcode.app/Contents/Developer/usr/bin/git
Curl: 8.7.1 => /usr/bin/curl
macOS: 15.0.1-x86_64
CLT: 16.0.0.0.1.1724870825
Xcode: 16.0

What were you trying to do (and why)?

After upgrading my Intel MacBook to Sequoia (from Monterey), I ran brew update && brew outdated, decided to brew pin llvm as it's 2GB and I didn't want to download all of it, then ran brew upgrade --formula. A number of formulae were unable to be upgraded due to a dependency on llvm; however, I confirmed with brew uses llvm --installed --include-build --include-test that no formula on my system depends on llvm, and it is in fact included in the output of brew leaves

What happened (include all command output)?

muneeb@Muneebs-MacBook-Pro-2 /u/l/H/L/Homebrew (master)> brew upgrade --formula -v
Warning: Not upgrading 1 pinned package:
llvm 19.1.1
==> Upgrading 4 outdated packages:
libraw 0.21.2 -> 0.21.3
libomp 18.1.8 -> 19.1.1
imagemagick 7.1.1-38 -> 7.1.1-39
fastfetch 2.23.0 -> 2.27.1
==> Downloading https://ghcr.io/v2/homebrew/core/libomp/manifests/19.1.1
Already downloaded: /Users/muneeb/Library/Caches/Homebrew/downloads/b34f89c5148ff15ebf37b882437f32f883a8f174ed50c7b91c004273186c8a0e--libomp-19.1.1.bottle_manifest.json
Error: You must `brew unpin llvm` as installing libomp requires the latest version of pinned dependencies
==> Downloading https://ghcr.io/v2/homebrew/core/libraw/manifests/0.21.3
Already downloaded: /Users/muneeb/Library/Caches/Homebrew/downloads/c3f2d55bb029b267a1ff362e85d1e8fa2377ff34c6480c1c97a9011dcc7a301f--libraw-0.21.3.bottle_manifest.json
Error: You must `brew unpin llvm` as installing libraw requires the latest version of pinned dependencies
==> Downloading https://ghcr.io/v2/homebrew/core/imagemagick/manifests/7.1.1-39
Already downloaded: /Users/muneeb/Library/Caches/Homebrew/downloads/43c8e424a0486139b1c221b857d81a758e457c5c7261e9f5f6666b40ac1e935c--imagemagick-7.1.1-39.bottle_manifest.json
Error: You must `brew unpin llvm` as installing imagemagick requires the latest version of pinned dependencies
==> Downloading https://ghcr.io/v2/homebrew/core/fastfetch/manifests/2.27.1
Already downloaded: /Users/muneeb/Library/Caches/Homebrew/downloads/a66138766a73103375f4e584e3436f13e535a000d6ac0d89cf3d6585bd3cb7a5--fastfetch-2.27.1.bottle_manifest.json
Error: You must `brew unpin llvm` as installing fastfetch requires the latest version of pinned dependencies

What did you expect to happen?

All the formulae should have upgraded and been poured from bottle as none of them depend on llvm. I looked through the four that didn't upgrade and:

fastfetch has a build (not runtime!) dependency on imagemagick; its formula doesn't mention llvm
imagemagick has runtime dependencies on libraw and libomp; its formula doesn't mention llvm
libraw has a runtime dependency on libomp; its formula doesn't mention llvm
libomp has no runtime dependencies, but it does have uses_from_macos "llvm" => :build in its formula

I suspect that llvm is not getting filtered out properly in the dependency graph somewhere. This problem is trivial to resolve with a simple brew uninstall llvm on my side, but I figured I ought to make a bug report regardless, as non-dependencies appearing in the dependency tree does appear to be a bug to me and it might affect other formulae.

Step-by-step reproduction instructions (by running `brew` commands)

I don't think homebrew has a way to downgrade formulae, but you could probably either download the older bottles and install them manually, or check out a version of homebrew-core from a month ago and set HOMEBREW_NO_INSTALL_FROM_API=1. For convenience, I've listed the formula versions currently on my system and corresponding bottle hashes:

libomp 18.1.8 - 0f1883c4651a04259281cc0fb4791effe7ab409a0d55d62fdd16435bf4d7e61e
libraw 0.21.2 - 833084fac47f3aae49d8177d56c0e98551b1e24382567bb1fc6111df1b7fcf67
imagemagick 7.1.1-38 - 39e25acf570b9c4bea567bf3b41058c41463b824bc01d7b47bccf71313725b91
fastfetch 2.23.0 - 7559643d626beae9c9c15cd42268a88bfc4dcb368d79bb9e59a67b0af24df43e
llvm 19.1.0 - 7adfafe60764b040a45232002142cc10551cf155ed1ce7d889ba917b2b6716ff or built from source

Run

brew pin llvm
brew update
brew upgrade --formula -v

~~Note that llvm had been built from source on my machine but all other formulae were poured from bottle~~ I reinstalled llvm 19.1.0 using the Ventura bottle and confirmed that the issue is still present

muneebmahmed commented 1 week ago

I did some more investigation with brew deps --tree --include-build --include-test --include-requirements --include-optional libomp to find that libomp has :build dependencies on cmake and lit, and lit has a :test dependency on (Homebrewed) llvm. So, in the full dependency graph, llvm is brought in as a test dependency of a formula that is a build dependency. With that in mind I tried adding

next if dep.test?

to line 382 here: https://github.com/Homebrew/brew/blob/3291ad4fc78f33f8e9bb7ce37745d2a0e697f5fb/Library/Homebrew/formula_installer.rb#L380-L382

and it worked! I'm happy to create a PR, but I don't believe this approach is ideal. We're still generating the full dependency graph and then pruning build + test deps. If, for example, there's a formula that's a required dep of a build dep, it will remain and halt the upgrade. In my opinion, it would be better to not even recurse on build dependencies in the first place when creating the graph.

muneebmahmed commented 1 week ago

Here are the dependency walking functions in question: https://github.com/Homebrew/brew/blob/3291ad4fc78f33f8e9bb7ce37745d2a0e697f5fb/Library/Homebrew/dependency.rb#L128-L179 It would be easy to add something like

elsif dep.build? || dep.test?
    prune

after line 176, but this would probably impact other places where expand() is called and test/build dependencies are expected to remain in the graph. Could this be refactored to conditionally prune build/test dependencies based on parameter flags? That change goes beyond my familiarity with the homebrew codebase however, so I'm not sure I can make that fix

MikeMcQuaid commented 1 week ago

I'm happy to create a PR, but I don't believe this approach is ideal.

Please do, that's much easier to discuss than the issue as-is. This document should help and we're happy to walk you through anything else.

Thanks!

Homebrew / brew