brew icon indicating copy to clipboard operation
brew copied to clipboard

Next steps for `HOMEBREW_INSTALL_FROM_API`

Open Rylan12 opened this issue 3 years ago • 5 comments

Provide a detailed description of the proposed feature

Here are the tasks that need to be completed in order to consider HOMEBREW_INSTALL_FROM_API ready for official testing by maintainers and interested users:

  • [x] https://github.com/Homebrew/brew/pull/13795: brew update should still update core and cask taps when HOMEBREW_INSTALL_FROM_API and HOMEBREW_DEVELOPER are both set (allowing maintainers to test this and still maintain homebrew/core and homebrew/cask)
  • [x] https://github.com/Homebrew/brew/pull/13815: Allow HOMEBREW_DEVELOPER users to use all dev commands even with HOMEBREW_INSTALL_FROM_API set
  • [ ] Migrate all casks to use on_{macos_version} blocks instead of if MacOS.version conditionals
  • [ ] Remove the small number of Hardware::CPU methods that slipped through in formulae and casks (and improve the rubocops to catch more cases)
  • [ ] Update CaskLoader to load casks from their JSON API like we do with formulae
  • [ ] A warning or some sort of message should be shown when installing a formula with a post_install block or a cask with a {pre,post}flight block so that users know that some things may not work at the moment (or, in the case of formulae, they need to run brew postinstall <formula>)
  • [ ] https://github.com/Homebrew/brew/pull/13842: I think it might be a good idea to mark in the INSTALL_RECEIPT.json or .metadata directory whether the formula/cask was installed from the API in case we run into issues and it's helpful to know how a formula was installed. But, this is a lower priority and might not even be needed at all
  • [ ] We need to handle download errors of the formula.json and cask.json files

What is the motivation for the feature?

I'm back at school so my availability over the next several months will be much more limited and less consistent. I'm opening this tracking issue to help monitor progress over time.

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

Eventually, HOMEBREW_INSTALL_FROM_API will be the default for everyone.

What alternatives to the feature have been considered?

None

Rylan12 avatar Sep 02 '22 05:09 Rylan12

[ ] A warning or some sort of message should be shown when installing a formula with a post_install block or a cask with a {pre,post}flight block so that users know that some things may not work at the moment (or, in the case of formulae, they need to run brew postinstall <formula>)

Is this something we expect to be fixed but just hasn't been yet? For a bottle installation, it should be able to use the post_install from within the bottle, right?

  • I think it might be a good idea to mark in the INSTALL_RECEIPT.json or .metadata directory whether the formula/cask was installed from the API in case we run into issues and it's helpful to know how a formula was installed. But, this is a lower priority and might not even be needed at all

This is a good idea 👍🏻

I'm back at school so my availability over the next several months will be much more limited and less consistent. I'm opening this tracking issue to help monitor progress over time.

No worries, thanks for opening this tracking issue!

MikeMcQuaid avatar Sep 02 '22 08:09 MikeMcQuaid

Is this something we expect to be fixed but just hasn't been yet? For a bottle installation, it should be able to use the post_install from within the bottle, right?

This requires us to build new bottles every time the postinstall method changes. And it's hard to tell whether existing bottles have outdated postinstall methods, so this might require rebottling everything. Also, style changes in the future may require rebottling all formulae that have postinstall blocks (currently 239 formulae) for the same reason. There's been some discussion in various places about the pros and cons of this as well as some other potential ways to deal with this, but I'll have to track those down

Rylan12 avatar Sep 02 '22 19:09 Rylan12

This requires us to build new bottles every time the postinstall method changes.

Yeh, I think this is fine for changes we need to push to users. Don't think this needs a warning as long as maintainers are made aware with docs/Slack communication.

And it's hard to tell whether existing bottles have outdated postinstall methods, so this might require rebottling everything.

This seems overkill.

Also, style changes in the future may require rebottling all formulae that have postinstall blocks (currently 239 formulae) for the same reason.

Why would a style change need rebottling?

but I'll have to track those down

That'd be great, thanks ❤️!

MikeMcQuaid avatar Sep 05 '22 11:09 MikeMcQuaid

Why would a style change need rebottling?

I used the wrong words here. Most style changes that are truly only style changes should be fine as long as the old style isn't invalid ruby or anything like that. Really the issue is deprecations and stuff that, if not updated, might not be able to be understood by a brew version newer than the one that created the bottle.

but I'll have to track those down

We discussed this a bit on June 18 in Slack. I'll send a link over in slack to the messages. Here on GitHub, we discussed this in https://github.com/Homebrew/brew/pull/12350, but there probably isn't anything in that discussion that didn't make it into the slack one.

Rylan12 avatar Sep 05 '22 18:09 Rylan12

Really the issue is deprecations and stuff that, if not updated, might not be able to be understood by a brew version newer than the one that created the bottle.

Gotcha 👍🏻

MikeMcQuaid avatar Sep 06 '22 10:09 MikeMcQuaid

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

github-actions[bot] avatar Oct 03 '22 00:10 github-actions[bot]

@Rylan12

Migrate all casks to use on_{macos_version} blocks instead of if MacOS.version conditionals

I guess we need a new issue on https://github.com/Homebrew/homebrew-cask to get the word out to people and have them modify the casks. How about it?

hyuraku avatar Oct 05 '22 13:10 hyuraku

@hyuraku I am game to start working on these.

p-linnane avatar Oct 05 '22 23:10 p-linnane

@hyuraku and @p-linnane, thanks for taking the initiative on this!

I think an issue in homebrew/cask would be great, but only once we've started the process and made sure all of its kinks are worked out (clearly from the past few days, we're not quite there yet 😅)


The immediate next steps that I think we need to take to get this ready are:

  • Make brew readall simulate every OS/arch combination
  • Fix homebrew/cask CI to not rely on checking the cask's contents for if MacOS.version conditionals

I plan on working on both of these soon, but if anyone else is interested feel free to give it a go

Rylan12 avatar Oct 08 '22 06:10 Rylan12

I think an issue in homebrew/cask would be great, but only once we've started the process and made sure all of its kinks are worked out (clearly from the past few days, we're not quite there yet 😅)

Understood, I will try to solve the other task.

hyuraku avatar Oct 09 '22 13:10 hyuraku

@Rylan12 I have a question about We need to handle download errors of the formula.json and cask.json files I couldn't find the code where we download cask.json; should we add the code to download cask.json by any chance?

hyuraku avatar Oct 13 '22 14:10 hyuraku

We will once all casks are migrated to use on_monterey and friends. Only then (and maybe after some more fixes) will we be able to load casks from the JSON API. Right now, the existing cask API doesn't contain enough information to do so.

Rylan12 avatar Oct 13 '22 15:10 Rylan12

Likely related: Homebrew/homebrew-cask#14430

If so, we can track that one here and maybe close the other issue.

carlocab avatar Jan 26 '23 03:01 carlocab

Installing from the API seems to ignore system language settings, or passing --language to brew install.

Reported at https://github.com/Homebrew/homebrew-cask/issues/140264.

carlocab avatar Jan 27 '23 13:01 carlocab

We may want a nicer error message when fetching from the API fails. Maybe a suggestion to set HOMEBREW_NO_INSTALL_FROM_API if trying the same command a few more times still fails?

See Homebrew/homebrew-cask#14451.

carlocab avatar Jan 28 '23 07:01 carlocab

I just realized that service blocks don't work with HOMEBREW_INSTALL_FROM_API just like the different *flight blocks. It'd be good to notify the user about that too.

apainintheneck avatar Jan 30 '23 02:01 apainintheneck

I thinks these issues are also related to this change:

https://github.com/Homebrew/brew/issues/14504

https://github.com/Homebrew/homebrew-cask/issues/140273

miccal avatar Jan 30 '23 05:01 miccal

Also related: Homebrew/homebrew-cask#14465

carlocab avatar Jan 30 '23 06:01 carlocab

Another issue from way out in left field: https://github.com/orgs/Homebrew/discussions/4135

Cask install logs here: https://gist.github.com/gromgit/6a332888b9a77084cf40ae2e8c28d0c6

In a nutshell, the dotnet-sdk cask contains the following stanza:

  binary "/usr/local/share/dotnet/dotnet"

With HOMEBREW_INSTALL FROM API, what seems to be happening is Homebrew replaces /usr/local with $HOMEBREW_PREFIX, causing the symlinking to /opt/homebrew/bin on M1 Macs to fail. HOMEBREW_NO_INSTALL_FROM_API does not seem to do this, and therefore works correctly.

gromgit avatar Feb 01 '23 00:02 gromgit

Uninstalling casks when sudo is required seems to be broken with HOMEBREW_INSTALL_FROM_API. See Homebrew/homebrew-cask#140436.

carlocab avatar Feb 01 '23 05:02 carlocab

I have another probably that is very likely related to this: when brew upgrade builds code from source, I end up with an Empty installation error. Running it multiple times, or running brew update does not change anything. Adding a few prints here and there, some part of Homebrew is using an old version number of the formula, some part the most recent one. As the install path used and expected one do not match, the check if the installation is empty fails.

brew outdated tells me that my copy of some library is outdated, but looking at the local copy of the core tap it was not up to date (even if I run brew update manually). My guess is that due to this change towards using the API the copy of the tap is not updated. And when building from source, part of the Homebrew code uses the version from the API and part of it still uses the version number (or whole formula) from the local copy of the tap so that does not go well.

By the way running HOMEBREW_NO_INSTALL_FROM_API=1 brew upgrade did update the local copy and so fixed the problem so I guess I will have to rely on it until this problem is fixed.

vincentisambart avatar Feb 01 '23 22:02 vincentisambart

when brew upgrade builds code from source

@vincentisambart Can you dump your brew config and brew doctor somewhere (even if there are warnings etc.) as we need to ensure that we never try to use the API when building from source.

MikeMcQuaid avatar Feb 02 '23 10:02 MikeMcQuaid

@MikeMcQuaid Today, a normal brew upgrade did upgrade some formulae that required building but this time everything went well... There might be an additional condition for the problem to appear (or some change in Homebrew itself might have fixed it). I still did a brew config and brew doctor: https://gist.github.com/vincentisambart/61284b0f4cd31ac7a04d5431a7ec1321

I should probably come back with brew config and brew doctor when the bug occurs again (if it does).

Update: Thinking more about it, it might have mainly happened not for patch version changes but only revision version changes (the ones that happen not when the package itself changed but when its dependencies do).

vincentisambart avatar Feb 03 '23 00:02 vincentisambart

@vincentisambart Thanks. I may have fixed the underlying issue here.

MikeMcQuaid avatar Feb 03 '23 08:02 MikeMcQuaid

Possibly related?

https://github.com/Homebrew/brew/issues/14505

miccal avatar Feb 04 '23 23:02 miccal