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] Allow Homebrew package manager in all systems but Windows #66609

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/66609.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow mac_brew_pkg to be used in systems different from macOS
33 changes: 24 additions & 9 deletions salt/modules/mac_brew_pkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
Typically, this is set to ``/usr/local`` for Intel Macs and ``/opt/homebrew``
for Apple Silicon Macs.

For Linux systems, the default prefix is: ``/home/linuxbrew/.linuxbrew``.

.. important::
If you feel that Salt should be using this module to manage packages on a
minion, and it is using a different module (or gives an error similar to
Expand All @@ -25,6 +27,7 @@
import salt.utils.json
import salt.utils.path
import salt.utils.pkg
import salt.utils.platform
import salt.utils.versions
from salt.exceptions import CommandExecutionError, MinionError, SaltInvocationError

Expand All @@ -36,11 +39,11 @@

def __virtual__():
"""
Confine this module to macOS with Homebrew.
Confine this module to systems with Homebrew.
"""
if __grains__["os"] != "MacOS":
return False, "brew module is macos specific"
if not _homebrew_bin():
if salt.utils.platform.is_windows():
return False, "Homebrew does not support Windows"
if not _homebrew_bin(quiet=True):
return False, "The 'brew' binary was not found"
return __virtualname__

Expand Down Expand Up @@ -109,10 +112,10 @@ def _homebrew_os_bin():

original_path = os.environ.get("PATH")
try:
# Add "/opt/homebrew" temporary to the PATH for Apple Silicon if
# the PATH does not include "/opt/homebrew"
# Temporary add the default Homebrew's prefix to PATH
# if prefix is not contained yet.
current_path = original_path or ""
homebrew_path = "/opt/homebrew/bin"
homebrew_path = _homebrew_default_prefix() + "/bin"
if homebrew_path not in current_path.split(os.path.pathsep):
extended_path = os.path.pathsep.join([current_path, homebrew_path])
os.environ["PATH"] = extended_path.lstrip(os.path.pathsep)
Expand All @@ -129,14 +132,14 @@ def _homebrew_os_bin():
return brew


def _homebrew_bin():
def _homebrew_bin(quiet=False):
"""
Returns the full path to the homebrew binary in the homebrew installation folder
"""
ret = homebrew_prefix()
if ret is not None:
ret += "/bin/brew"
else:
elif not quiet:
log.warning("Failed to find homebrew prefix")

return ret
Expand Down Expand Up @@ -180,6 +183,18 @@ def _list_pkgs_from_context(versions_as_list):
return ret


def _homebrew_default_prefix():
"""
Returns the default homebrew prefix based on the OS and CPU architecture.
"""
if salt.utils.platform.is_darwin():
if salt.utils.platform.is_arm64():
Comment on lines +190 to +191
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried using grains, but rsax931.py uses this method to load openssl library. However, it fails since, apparently, grains are not loaded yet or they are not defined.

return "/opt/homebrew"
return "/usr/local"

return "/home/linuxbrew/.linuxbrew"


def homebrew_prefix():
"""
Returns the full path to the homebrew prefix.
Expand Down
8 changes: 8 additions & 0 deletions salt/utils/platform.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,14 @@ def is_aarch64():
return platform.machine().startswith("aarch64")


@real_memoize
def is_arm64():
Copy link
Contributor Author

@cdalvaro cdalvaro Jun 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really sure about adding this new method instead of modifying the existing aarch64 with something like this:

return re.search(r'^(aarch|arm)64', platform.machine()) is not None

I think my initial approach is more conservative, since I'm not modifying the current behavior of the is_aarch64 method. But essentially, Aarch64 and ARM64 are two different ways to refer the same thing.

TL;DR: Aarch64 is how the LLVM community refers to this architecture while Apple calls it ARM64.

Copy link
Contributor Author

@cdalvaro cdalvaro Jun 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to ask the opinion of @s0undt3ch and @krionbsd. Since Kirill implemented the original is_aarch64 method and Pedro is one of the main developers for macOS support.

"""
Simple function to return if host is ARM64 or not
"""
return platform.machine().startswith("arm64")


def spawning_platform():
"""
Returns True if multiprocessing.get_start_method(allow_none=False) returns "spawn"
Expand Down
27 changes: 25 additions & 2 deletions tests/pytests/unit/modules/test_mac_brew_pkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ def test_homebrew_prefix_returns_none_even_with_execution_errors():
# '_homebrew_os_bin' function tests: 1


def test_homebrew_os_bin_fallback_apple_silicon():
def test_homebrew_os_bin_appends_prefix_to_path():
"""
Test the path to the homebrew executable for Apple Silicon.

Expand All @@ -543,10 +543,33 @@ def mock_utils_path_which(*args):
return apple_silicon_homebrew_bin
return None

with patch("salt.utils.path.which", mock_utils_path_which):
with patch("salt.utils.platform.is_darwin", return_value=True), patch(
"salt.utils.platform.is_arm64", return_value=True
), patch("salt.utils.path.which", mock_utils_path_which):
assert mac_brew._homebrew_os_bin() == apple_silicon_homebrew_bin


# '_homebrew_default_prefix' function tests: 1


def test_homebrew_default_prefix():
"""
Tests homebrew's default prefix
"""
with patch("salt.utils.platform.is_darwin", return_value=True), patch(
"salt.utils.platform.is_arm64", return_value=True
):
assert mac_brew._homebrew_default_prefix() == "/opt/homebrew"

with patch("salt.utils.platform.is_darwin", return_value=True), patch(
"salt.utils.platform.is_arm64", return_value=False
):
assert mac_brew._homebrew_default_prefix() == "/usr/local"

with patch("salt.utils.platform.is_darwin", return_value=False):
assert mac_brew._homebrew_default_prefix() == "/home/linuxbrew/.linuxbrew"


# '_homebrew_bin' function tests: 1


Expand Down
Loading