Improve handling of caveats for API installs

[carlocab]

Verification

• 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.

Provide a detailed description of the proposed feature

Currently, you can see different caveats depending on whether HOMEBREW_NO_INSTALL_FROM_API is set or not. For example, as I mentioned in #14925,

❯ diff -u <(HOMEBREW_NO_INSTALL_FROM_API= brew info socket_vmnet) <(HOMEBREW_NO_INSTALL_FROM_API=1 brew info socket_vmnet)
--- /dev/fd/13  2023-03-17 15:39:33.183107616 +0800
+++ /dev/fd/14  2023-03-17 15:39:33.183970272 +0800
@@ -16,6 +16,11 @@
 socket_vmnet is keg-only, which means it was not symlinked into /usr/local,
 because /usr/local/bin is often writable by a non-admin user.

+
+To restart socket_vmnet after an upgrade:
+  sudo brew services restart socket_vmnet
+Or, if you don't want/need a background service you can just run:
+  /usr/local/opt/socket_vmnet/bin/socket_vmnet --vmnet-gateway=192.168.105.1 /usr/local/var/run/socket_vmnet
 ==> Analytics
 install: 116 (30 days), 664 (90 days), 773 (365 days)
 install-on-request: 116 (30 days), 662 (90 days), 771 (365 days)

What is the motivation for the feature?

Users should see caveats as intended regardless of whether they install using the API or from a Git clone.

How will the feature be relevant to at least 90% of Homebrew users?

90% of Homebrew users are installing, or will eventually install, from the API. They should see the same caveats as non-API installs.

What alternatives to the feature have been considered?

Removing caveats? Doing nothing?

[carlocab]

Quoting @apainintheneck (#14925 (comment)):

It looks like caveats are hardcoded when loaded from the API right now and that's what's probably causing the problem. Probably worth making a separate issue about that.

brew/Library/Homebrew/formulary.rb

Lines 266 to 270 in 5550aa2

	@caveats_string = json_formula["caveats"]
	def caveats
	self.class.instance_variable_get(:@caveats_string)
	&.gsub(HOMEBREW_PREFIX_PLACEHOLDER, HOMEBREW_PREFIX)
	end

[apainintheneck]

It looks like there is also at least one formula that uses Dir.home in caveats as well.

• redshift.rb

Edit: This would be a one line change but do we want people to use Dir.home in caveats?

[apainintheneck]

Did this get fixed by adding the service block to the API? It seems like in this specific case it wasn't showing because it didn't know it was a service but now it should work fine.

[MikeMcQuaid]

This would be a one line change but do we want people to use Dir.home in caveats?

Seems reasonable to do so.

Did this get fixed by adding the service block to the API?

No idea, maybe!

[apainintheneck]

I just double-checked and this works fine now because the service block was added to the API. Will close this out once the one line Dir.home substitution mentioned above is added for caveats.

[reitermarkus]

want people to use Dir.home in caveats?

Why would caveats not just use ~?

[apainintheneck]

Why would caveats not just use ~?

I think most of them do. It should be equivalent so I don't know why it can't be used instead.

[apainintheneck]

@reitermarkus and I just found out that the caveats with on_* blocks and branching logic aren't getting serialized correctly with the API. This affects 57 different formulae.

• Script: Prints out all caveats functions with branching logic or on_* blocks.
• Results: All offending caveats functions
• Affected Formulae List

[MikeMcQuaid]

@carlocab @apainintheneck is this now done?

[Bo98]

on_* should certainly be working correctly, and it looks like it does... sometimes? e.g. gawk has a correct JSON but apcupsd does not.

Will take a look along with a few other API issues tomorrow. Been having a look through them tonight seeing where the issues are.

[carlocab]

I count ~70 affected formulae, but my script is a lot simpler so maybe there are a bunch of false positives.

#!/usr/bin/env brew ruby

require "formula"
require "method_source"

CoreTap.instance.formula_names.each do |name|
  formula = Formula[name]
  caveats_source = formula.method(:caveats).source

  first_method_line = caveats_source.lines.second.strip
  next if first_method_line == "nil"
  next if first_method_line.start_with?("<<~")

  puts name
end

Formulae

apcupsd
cdpr
cntlm
coreutils
csound
ed
emscripten
findutils
firefoxpwa
fontforge
freerdp
gawk
gdb
gnu-indent
gnu-sed
gnu-tar
gnu-time
gnu-units
gnu-which
golo
grep
haskell-language-server
inetutils
liboping
libtool
llnode
llvm
llvm@13
llvm@14
llvm@15
make
mercurial
mmseqs2
moc
mono
mysql
mysql@5.7
neko
neovim
nu
ocl-icd
open-tyrian
opencl-icd-loader
openconnect
openj9
openjdk
openjdk@11
openjdk@17
openjdk@8
percona-server
podman
postgresql@12
postgresql@13
postgresql@14
pymol
python-typing-extensions
pyyaml
ruby
ruby@2.7
ruby@3.0
ruby@3.1
samba
six
swift
synergy-core
util-linux
uutils-coreutils
vis
ykdl
you-get

[Bo98]

Formulae with if branches or of other dynamic nature will never be supported and a homebrew-core issue should be opened for those.

I do have a fix for on_os blocks not working when the only on_os block in the file is in caveats. Formulae which already use on_os blocks elsewhere should already be working for caveats.

[MikeMcQuaid]

Formulae with if branches or of other dynamic nature will never be supported and a homebrew-core issue should be opened for those.

Yeh, and we should probably have an audit etc. too.