Fix `brew formulae` and `brew casks` when the API is used #15471

Omoeba · 2023-05-20T23:38:30Z

I'm not sure if this is an elegant solution but it gives an identical output to when HOMEBREW_NO_INSTALL_FROM_API is set, regardless of whether homebrew/core or homebrew/cask are tapped. Feel free to make any improvements.

Closes #15229

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same change?
Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes? Here's an example.
Have you successfully run brew style with your changes locally?
Have you successfully run brew typecheck with your changes locally?
Have you successfully run brew tests with your changes locally?

apainintheneck

Nice work so far. I left some comments about excluding the default cask and formula taps if they exist locally because they might be out-of-date.

It would also be nice to see some benchmarks here comparing what we have now with after this change since we're kind of combining the two current methods. I don't expect it to be slow but we might as well double check.

Library/Homebrew/cmd/casks.sh

MikeMcQuaid · 2023-05-22T12:33:08Z

Library/Homebrew/cmd/casks.sh

+      cat "${HOMEBREW_CACHE}/api/cask_names.txt"
+      echo
+      homebrew-items '*/Casks/*\.rb' '' 's|/Casks/|/|' '^homebrew/cask'
+    } | sort -u


If we're repeating this sort -u in both places: perhaps it should be part of homebrew-items or homebrew-items-unique or something?

The sort -u is used here to sort the names again because combining the 2 lists messes up the sorting. The -u can probably be removed once the core taps are filtered out though because there should be no more duplicates.

There can still be duplicates with the short names, right? It should be possible for two formulas to exist with the same name as long as they're in different taps. Not sure how big of a deal any duplication would be though.

It should be possible for two formulas to exist with the same name as long as they're in different taps.

Yes, this is definitely a thing that happens and is OK.

Omoeba · 2023-05-23T01:49:09Z

Switching to regex needed a few more changes than I expected due to the different find on macOS and Linux having different ways to turn on extended regex. sort -u is also replaced by sort -f because-u is now unnecessary thanks to the core taps being filtered out and -f is added to maintain a consistent order with sort -uf in items.sh. One interesting thing to note though is brew formulae returns an extra newline with the API enabled but brew casks doesn't. I'm not sure what causes it, whether it is dependent on the specific taps I have, or what negative effects it may cause for things like tab completion (if any). As always, feel free to make any edits.

MikeMcQuaid

Makes sense to me so far but interested in thoughts from @apainintheneck!

apainintheneck

I think this looks really good. The blank line seems to be a part of the formula_names.txt file as far as I can tell so that probably needs to get fixed somewhere else. Other than that I left a nit but I think this ready to merge as is.

apainintheneck · 2023-05-24T06:27:15Z

Library/Homebrew/cmd/formulae.sh

    echo
  else
-    homebrew-items '*\.rb' 'Casks' 's|/Formula/|/|' '^homebrew/core'
+    homebrew-items '*\.rb' '.*Casks.*' 's|/Formula/|/|' '^homebrew/core'


I'm not sure why but adding slashes breaks the filtering and no longer excludes casks.

It seems to be because we're not filtering by file but directory instead and the final slash is omitted. The following works for me locally. If I remember correctly, we don't currently use nesting but it will probably get added to the core repos in the future for git performance reasons.

Suggested change

homebrew-items '*\.rb' '.*Casks.*' 's|/Formula/|/|' '^homebrew/core'

homebrew-items '*\.rb' '.*/Casks(/.*|$)' 's|/Formula/|/|' '^homebrew/core'

I've implemented the changes and removed an unnecessary echo in formulae.sh that caused the extra blank line which I somehow missed.

Omoeba · 2023-05-24T16:21:06Z

sort -f was changed to sort -uf to maintain the existing behavior of removing duplicate short names when there are formulae with the same name in homebrew/core and a custom tap.

… when the API is used Fixes Homebrew#15229

MikeMcQuaid · 2023-05-25T11:18:19Z

Thanks so much for your first contribution (hopefully of many)! Without people like you submitting PRs we couldn't run this project. You rock, @Omoeba!

apainintheneck reviewed May 21, 2023

View reviewed changes

Library/Homebrew/cmd/casks.sh Show resolved Hide resolved

Library/Homebrew/cmd/casks.sh Outdated Show resolved Hide resolved

MikeMcQuaid reviewed May 22, 2023

View reviewed changes

Omoeba force-pushed the fix-noncore branch from 20f1628 to 94b425a Compare May 23, 2023 01:21

MikeMcQuaid requested a review from apainintheneck May 23, 2023 08:39

MikeMcQuaid approved these changes May 23, 2023

View reviewed changes

apainintheneck approved these changes May 24, 2023

View reviewed changes

Omoeba force-pushed the fix-noncore branch from 94b425a to d680986 Compare May 24, 2023 16:06

Omoeba added 2 commits May 25, 2023 01:49

cmd/formulae,cmd/casks: include casks/formulae in local non-core taps…

b43ed6e

… when the API is used Fixes Homebrew#15229

items.sh: use regex for find_exclude_filter

76120ad

Omoeba force-pushed the fix-noncore branch from d680986 to 76120ad Compare May 25, 2023 08:52

MikeMcQuaid merged commit 2f63d26 into Homebrew:master May 25, 2023
24 checks passed

Omoeba deleted the fix-noncore branch May 25, 2023 11:20

github-actions bot added the outdated PR was locked due to age label Jun 25, 2023

github-actions bot locked as resolved and limited conversation to collaborators Jun 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `brew formulae` and `brew casks` when the API is used #15471

Fix `brew formulae` and `brew casks` when the API is used #15471

Omoeba commented May 20, 2023 •

edited

apainintheneck left a comment

MikeMcQuaid May 22, 2023

Omoeba May 22, 2023

apainintheneck May 24, 2023

MikeMcQuaid May 24, 2023

Omoeba commented May 23, 2023

MikeMcQuaid left a comment

apainintheneck left a comment

apainintheneck May 24, 2023

MikeMcQuaid May 24, 2023

Omoeba May 24, 2023

apainintheneck May 25, 2023

Omoeba May 25, 2023

Omoeba commented May 24, 2023

MikeMcQuaid commented May 25, 2023

	homebrew-items '\.rb' '.Casks.*' 's\|/Formula/\|/\|' '^homebrew/core'
	homebrew-items '\.rb' './Casks/.*' 's\|/Formula/\|/\|' '^homebrew/core'

	homebrew-items '\.rb' '.Casks.*' 's\|/Formula/\|/\|' '^homebrew/core'
	homebrew-items '\.rb' './Casks(/.*\|$)' 's\|/Formula/\|/\|' '^homebrew/core'

Fix brew formulae and brew casks when the API is used #15471

Fix brew formulae and brew casks when the API is used #15471

Conversation

Omoeba commented May 20, 2023 • edited

apainintheneck left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Omoeba commented May 23, 2023

MikeMcQuaid left a comment

Choose a reason for hiding this comment

apainintheneck left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Omoeba commented May 24, 2023

MikeMcQuaid commented May 25, 2023

Fix `brew formulae` and `brew casks` when the API is used #15471

Fix `brew formulae` and `brew casks` when the API is used #15471

Omoeba commented May 20, 2023 •

edited