Formula dependency incorrectly marked as `installed_on_request` upon upgrade/migration

kidonng commented 1 day ago

`brew doctor` output

Your system is ready to brew.

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.5
ORIGIN: https://github.com/Homebrew/brew
HEAD: 254bf3fe9d8fa2e1b2fb55dbcf535b2d870180c4
Last commit: 23 hours ago
Core tap JSON: 12 Nov 09:38 UTC
Core cask tap JSON: 12 Nov 09:38 UTC
HOMEBREW_PREFIX: /opt/homebrew
HOMEBREW_CASK_OPTS: []
HOMEBREW_EDITOR: code
HOMEBREW_MAKE_JOBS: 8
Homebrew Ruby: 3.3.6 => /opt/homebrew/Library/Homebrew/vendor/portable-ruby/3.3.6/bin/ruby
CPU: octa-core 64-bit arm_firestorm_icestorm
Clang: 16.0.0 build 1600
Git: 2.47.0 => /opt/homebrew/bin/git
Curl: 8.7.1 => /usr/bin/curl
macOS: 15.1-arm64
CLT: 16.1.0.0.1.1729049160
Xcode: N/A
Rosetta 2: false

What were you trying to do (and why)?

pkg-config was installed as a dependency of rust on my machine. https://github.com/Homebrew/homebrew-core/pull/194885 replaced pkg-config with pkgconf, and after upgrading and doing brew bundle dump, I noticed pkgconf now becomes part of the generated Brewfile, which it probably shouldn't:

$ chezmoi diff --reverse
diff --git a/.Brewfile b/.Brewfile
index ea055bf1f44199bf23b351bd130cc91929545b74..2cf5aaa3d33c8d6b55eed2bbf4447aa89ebad047 100644
--- a/.Brewfile
+++ b/.Brewfile
@@ -8,6 +8,7 @@ brew "fzf"
 brew "git"
 brew "mpv"
 brew "node"
+brew "pkgconf"
 brew "rust"

What happened (include all command output)?

After investigation it turns out pkgconf is somehow marked as installed_on_request after upgrading:

$ jq .installed_as_dependency /opt/homebrew/Cellar/pkgconf/2.3.0_1/INSTALL_RECEIPT.json
false
$ jq .installed_on_request /opt/homebrew/Cellar/pkgconf/2.3.0_1/INSTALL_RECEIPT.json
true

What did you expect to happen?

Compared to a normal dependency:

$ jq .installed_as_dependency /opt/homebrew/Cellar/brotli/1.1.0/INSTALL_RECEIPT.json
true

$ jq .installed_on_request /opt/homebrew/Cellar/brotli/1.1.0/INSTALL_RECEIPT.json
false

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

1. Make sure on a older Core tap JSON before `pkg-config` migration
2. Have `pkg-config` installed as a dependency (e.g. by `brew install rust`)
3. `brew upgrade` to upgrade `pkg-config` to `pkgconf`
4. `brew bundle dump`
5. See `pkgconf` is in the generated `Brewfile`

gromgit commented 1 day ago

Did you do a targeted upgrade, i.e. brew upgrade pkg-config or brew upgrade pkgconf instead of just brew upgrade? If so, that marks the package as installed-on-request. The same goes for brew reinstall <formula>.

If all you did was brew upgrade, then that begs the question: should any install operation change the installed-on-request status of already-installed formulae?

Anyway, the workaround: brew tab --no-installed-on-request pkgconf.

ZhongRuoyu commented 1 day ago

It seems that a plain brew upgrade did cause pkgconf to be marked as installed on request although pkg-config wasn't previously. I reproduced this locally.

installed_as_dependency in the tab, on the other hand, changed from true to false.

gromgit commented 1 day ago

That seems like a bug to me. In fact, as I mentioned elsewhere, it's puzzling that anything but the original formula install (requested or dependency) should automatically change those flags. Surely the fact that XYZ needs to be reinstalled doesn't change the reason it was installed in the first place?

MikeMcQuaid commented 1 day ago

It seems that a plain brew upgrade did cause pkgconf to be marked as installed on request although pkg-config wasn't previously. I reproduced this locally.

@ZhongRuoyu seems like a bug with the alias migration stuff for png-config.

If all you did was brew upgrade, then that begs the question: should any install operation change the installed-on-request status of already-installed formulae?

brew install foo should at least change this status for foo. This is consistent with what apt-get does.

Increasingly I wonder if brew upgrade foo should do so, that seems a bit more ill advised.

brew upgrade should not/never.

kidonng commented 1 day ago

Did you do a targeted upgrade, i.e. brew upgrade pkg-config or brew upgrade pkgconf instead of just brew upgrade?

Just brew upgrade, same as the result @ZhongRuoyu has reproduced.

Anyway, the workaround: brew tab --no-installed-on-request pkgconf.

I don't think that is enough to avoid it in Brewfile since brew bundle checks both installed_on_request and installed_as_dependency:

https://github.com/Homebrew/homebrew-bundle/blob/055eb6e7c6fc2e2d5fce43c6531a23ded4cb5d7a/lib/bundle/brew_dumper.rb#L55-L57

(As for my case I corrected the fields in INSTALL_RECEIPT.json manually)

cho-m commented 1 day ago

I guess trying to find old Keg fails as new directory doesn't exist on filesystem for formula-to-alias as opposed to rename migrator https://github.com/Homebrew/brew/blob/798136976623b0a865f67babe0e1007390556729/Library/Homebrew/upgrade.rb#L135-L137

So false || nil is nil which gets compacted and the default installed_on_request: true is used. https://github.com/Homebrew/brew/blob/798136976623b0a865f67babe0e1007390556729/Library/Homebrew/upgrade.rb#L151

Homebrew / brew