Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

brew.sh: don't set HOMEBREW_NO_INSTALL_FROM_API automatically. #15765

Merged
merged 1 commit into from
Jul 26, 2023
Merged

brew.sh: don't set HOMEBREW_NO_INSTALL_FROM_API automatically. #15765

merged 1 commit into from
Jul 26, 2023

Conversation

MikeMcQuaid
Copy link
Member

My understanding from conversations with Bo98 is that this should now work fine on both older macOS versions and non-default prefixes.

Similarly, it's super confusing when this is set automatically and you can't figure out why...

@MikeMcQuaid MikeMcQuaid requested a review from Bo98 July 26, 2023 13:22
@MikeMcQuaid MikeMcQuaid changed the title brew.sh: don't set HOMEBREW_NO_INSTALL_FROM_API` automatically. brew.sh: don't set HOMEBREW_NO_INSTALL_FROM_API automatically. Jul 26, 2023
@EricFromCanada
Copy link
Member

In Installation.md, drop the "Note that this will automatically be set" paragraph as well.

My understanding from conversations with Bo98 is that this should now
work fine on both older macOS versions and non-default prefixes.

Similarly, it's super confusing when this is set automatically and you
can't figure out why...
@MikeMcQuaid
Copy link
Member Author

@EricFromCanada good catch, done, thanks.

@MikeMcQuaid MikeMcQuaid added the critical Critical change which should be shipped as soon as possible. label Jul 26, 2023
@MikeMcQuaid
Copy link
Member Author

Pretty sure @Bo98 is out for a bit so: gonna merge this as-is for now.

@MikeMcQuaid MikeMcQuaid merged commit 818ede5 into Homebrew:master Jul 26, 2023
25 checks passed
@MikeMcQuaid MikeMcQuaid deleted the more_install_from_api branch July 26, 2023 14:59
@gerlero
Copy link
Contributor

gerlero commented Jul 27, 2023

FWIW, this change seems to have broken a bunch of formula installs on a non-default prefix, e.g.:

==> Fetching fftw
==> Downloading https://ghcr.io/v2/homebrew/core/fftw/manifests/3.3.10_1
==> Downloading https://ghcr.io/v2/homebrew/core/fftw/blobs/sha256:bd3ae1b553913b3b627bd1af592d84da4c6a93e45dde5af4df7c393564b0f174
==> Installing dependencies for fftw: gcc, hwloc, ca-certificates, openssl@3, libevent and open-mpi
==> Installing fftw dependency: gcc
Error: An exception occurred within a child process:
  FormulaUnavailableError: No available formula with the name "/Volumes/OpenFOAM-v2306/homebrew/Library/Taps/homebrew/homebrew-core/Formula/gcc.rb".
Installing fftw has failed!

Looks like attempting to install a formula that depends on gcc or openssl@3 on a custom prefix fails with this error. Curiously, installing gcc and/or openssl@3 directly works; and doing that first appears to be a valid workaround for the above error. I have no clue on what could be causing this behavior, but a common factor between gcc and openssl@3 is that both need to be built from source when using non-default prefixes...

Setting HOMEBREW_NO_INSTALL_FROM_API=1 again fixes all problems.

@carlocab
Copy link
Member

Reverting in #15768.

@Bo98
Copy link
Member

Bo98 commented Jul 27, 2023

Ah forgot we were still doing this on older macOS.

The breakage however wasn't expected - I'm guessing there's an edge case in dependency installs as that's the one area that probably hasn't been fully tested. Will take a look as soon as I can.

@MikeMcQuaid
Copy link
Member Author

Ah forgot we were still doing this on older macOS.

Older macOS and non-default prefixes too.

@gerlero
Copy link
Contributor

gerlero commented Jul 28, 2023

Thanks for the quick response and revert. I can confirm that everything went back to working after #15768 was merged.

(This issue notwithstanding, eventually having API installs enabled on non-default prefixes would of course be great).

@osalbahr
Copy link
Contributor

osalbahr commented Jul 30, 2023

(This issue notwithstanding, eventually having API installs enabled on non-default prefixes would of course be great).

I agree. Do we know why the API installs don’t work on non-default prefixes? Homebrew technically doesn’t support having a non-default prefix, but I’m interested to look into it to see if a non-intrusive fix is possible.

@gerlero
Copy link
Contributor

gerlero commented Jul 30, 2023

@osalbahr A fix is being proposed at #15778

@github-actions github-actions bot added the outdated PR was locked due to age label Aug 30, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
critical Critical change which should be shipped as soon as possible. outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants