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

[master] fix(mac_brew_pkg): Improve search for Homebrew's prefix #64924

Merged
merged 7 commits into from
Dec 22, 2023

Conversation

cdalvaro
Copy link
Contributor

@cdalvaro cdalvaro commented Aug 4, 2023

What does this PR do?

This PR improves the way salt tries to figure out the brew macOS command.

What issues does this PR fix or reference?

After latest changes in brew command: Homebrew/brew#15787, Homebrew/brew#15818 and finally Homebrew/brew#15827, salt fails executing brew commands when no HOME env variable is set with the following error:

[salt.state       :325 ][ERROR   ][9799] An error was encountered while removing package(s): Path not found: Error: $HOME must be set to run brew./bin/brew
[salt.loaded.int.module.cmdmod:920 ][ERROR   ][13361] Command 'brew' failed with return code: 1

This is due because salt-minion is launched as a Launch Daemon in macOS with a very reduced environment:

LC_PAPER=C
LC_ADDRESS=C
LC_MONETARY=C
LC_NUMERIC=C
LC_TELEPHONE=C
PATH=/opt/homebrew/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
LC_MESSAGES=C
LC_IDENTIFICATION=C
LC_COLLATE=C
PWD=/private/var/root
LC_MEASUREMENT=C
XPC_FLAGS=0x0
XPC_SERVICE_NAME=0
SHLVL=1
LANGUAGE=C
HOMEBREW_PREFIX=/opt/homebrew
LC_CTYPE=C
LC_TIME=C
LC_NAME=C
_=/usr/bin/env

(I have added PATH and HOMEBREW_PREFIX manually to my daemon's plist file)

When salt tries to recover the brew --prefix with the following code:

def _homebrew_bin():
"""
Returns the full path to the homebrew binary in the PATH
"""
ret = __salt__["cmd.run"]("brew --prefix", output_loglevel="trace")
ret += "/bin/brew"
return ret

There is no $HOME, neither $USER, neither $LOGNAME in the environment.

This PR tries to figure out the Homebrew's prefix, by using the HOMEBREW_PREFIX env variable. As it is already used in other salt modules, like in rsax931.py:

brew_prefix = os.getenv("HOMEBREW_PREFIX", "/usr/local")

If the environment variable is not set, then it tries the most common paths for brew regarding the system architecture. /opt/homebrew for Apple Silicon (a.k.a.: arm64), /usr/local for Intel Macs (a.k.a.: x86_64).

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

Closes #61340

@cdalvaro cdalvaro requested a review from a team as a code owner August 4, 2023 19:33
@cdalvaro cdalvaro requested review from twangboy and removed request for a team August 4, 2023 19:33
@salt-project-bot-prod-environment salt-project-bot-prod-environment bot changed the title fix(mac_brew_pkg): Guess homebrew prefix [master] fix(mac_brew_pkg): Guess homebrew prefix Aug 4, 2023
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 19:54 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 19:54 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 19:54 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 19:54 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 20:09 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 20:56 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 20:56 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 20:56 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 20:56 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 21:12 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 21:15 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 22:05 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 22:05 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 22:05 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 22:05 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 22:05 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 22:05 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 22:12 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 22:12 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 22:12 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 22:12 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 22:12 — with GitHub Actions Inactive
@cdalvaro cdalvaro temporarily deployed to ci August 4, 2023 22:12 — with GitHub Actions Inactive
cdalvaro added a commit to cdalvaro/salt that referenced this pull request Aug 5, 2023
@cdalvaro cdalvaro force-pushed the bugfix_macOS_homebrew_bin branch from 4332a2a to 28d7818 Compare August 5, 2023 07:35
@dwoz dwoz modified the milestones: Argon v3008.0, Chlorine v3007.0 Dec 22, 2023
@dwoz dwoz merged commit 0a88399 into saltstack:master Dec 22, 2023
622 checks passed
dwoz pushed a commit that referenced this pull request Dec 22, 2023
@cdalvaro cdalvaro deleted the bugfix_macOS_homebrew_bin branch December 22, 2023 05:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] OSError: Cannot locate OpenSSL libcrypto on MacOS + M1 Mac
4 participants