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

Use brew path from which in mac_brew_pkg module #57946

Merged
merged 8 commits into from
Aug 9, 2023

Conversation

arsa-dev
Copy link
Contributor

Call brew directly to fetch brew full path will throw command not found
error. Fetching with which as in other parts of module code works fine.

What does this PR do?

I checked a bit deeply and I see that, as @Ch3LL explain at PR #48804, after this fix #47212 we aren't passing the PATH via the env anymore so we need to run which brew to get the entire path or else we see this error. So call brew directly to fetch brew full path will throw previous error, and I think that a valid solution can be fetch full path as @Ch3LL proposes in the past.

What issues does this PR fix or reference?

Related with previously merged PR #48804. See saltstack/salt-ci-images#1033 conversation for details.

Previous Behavior

I was getting the following error trying to run 'salt pkg.install wget':

ERROR: Unable to run command '['brew', '--prefix']' with the context '{'env': {'LC_NAME': 'C', 'XPC_FLAGS': '0x0', 'LC_MONETARY': 'C', 'LC_MESSAGES': 'C', 'LC_IDENTIFICATION': 'C', 'LC_MEASUREMENT': 'C', 'LC_COLLATE': 'C', 'SHLVL': '0', 'LC_PAPER': 'C', 'LC_NUMERIC': 'C', 'LC_CTYPE': 'C', 'LC_TELEPHONE': 'C', 'LC_ADDRESS': 'C', 'PATH': '/usr/bin:/bin:/usr/sbin:/sbin', 'LANGUAGE': 'C', 'PWD': '/', 'XPC_SERVICE_NAME': 'com.saltstack.salt.minion', 'LC_TIME': 'C'}, 'close_fds': True, 'bg': False, 'with_communicate': True, 'shell': False, 'stderr': -2, 'timeout': None, 'cwd': '/var/root', 'stdout': -1, 'stdin': None}', reason: command not found
ERROR: Minions returned with non-zero exit code

New Behavior

Command 'salt pkg.install wget' runs well and do his work.

Merge requirements satisfied?

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

Commits signed with GPG?

Yes

@arsa-dev arsa-dev requested a review from a team as a code owner July 15, 2020 09:12
@ghost ghost requested review from dwoz and removed request for a team July 15, 2020 09:13
@arsa-dev arsa-dev marked this pull request as draft July 16, 2020 08:41
@arsa-dev arsa-dev changed the title Use brew path from which in mac_brew_pkg module WIP: Use brew path from which in mac_brew_pkg module Jul 16, 2020
@dwoz dwoz added Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases has-failing-test labels Jul 27, 2020
@arsa-dev arsa-dev closed this Jun 17, 2023
@arsa-dev arsa-dev force-pushed the fix-module-mac_brew_pkg branch 2 times, most recently from ad35e5a to db228b3 Compare June 17, 2023 15:42
@s0undt3ch s0undt3ch temporarily deployed to nightly June 17, 2023 15:42 — with GitHub Actions Inactive
@s0undt3ch s0undt3ch temporarily deployed to nightly June 17, 2023 15:42 — with GitHub Actions Inactive
@s0undt3ch s0undt3ch temporarily deployed to nightly June 17, 2023 15:42 — with GitHub Actions Inactive
@s0undt3ch s0undt3ch temporarily deployed to nightly June 17, 2023 15:42 — with GitHub Actions Inactive
@s0undt3ch s0undt3ch temporarily deployed to nightly June 17, 2023 15:42 — with GitHub Actions Inactive
@s0undt3ch s0undt3ch temporarily deployed to nightly June 17, 2023 15:42 — with GitHub Actions Inactive
@s0undt3ch s0undt3ch temporarily deployed to nightly June 17, 2023 15:42 — with GitHub Actions Inactive
@s0undt3ch s0undt3ch temporarily deployed to nightly June 17, 2023 15:42 — with GitHub Actions Inactive
@s0undt3ch s0undt3ch temporarily deployed to nightly June 17, 2023 15:44 — with GitHub Actions Inactive
@arsa-dev arsa-dev reopened this Jun 17, 2023
@arsa-dev
Copy link
Contributor Author

Hi, I've reopened this PR since a long time ago without using salt with macos, I've realised that I'm experiencing the same error behaviour when trying to pkg.install something with brew on macos:

ERROR: Unable to run command '['brew', '--prefix']' with the context '{'cwd': '/var/root', 'shell': False, 'env': {'OLDPWD': '/', 'PATH': '/usr/bin:/bin:/usr/sbin:/sbin', 'PWD': '/opt/salt', 'XPC_FLAGS': '0x0', 'XPC_SERVICE_NAME': '0', 'SHLVL': '0', 'LC_CTYPE': 'C', 'SSL_CERT_DIR': '/private/etc/ssl/certs', 'SSL_CERT_FILE': '/private/etc/ssl/cert.pem', 'LC_NUMERIC': 'C', 'LC_TIME': 'C', 'LC_COLLATE': 'C', 'LC_MONETARY': 'C', 'LC_MESSAGES': 'C', 'LC_PAPER': 'C', 'LC_NAME': 'C', 'LC_ADDRESS': 'C', 'LC_TELEPHONE': 'C', 'LC_MEASUREMENT': 'C', 'LC_IDENTIFICATION': 'C', 'LANGUAGE': 'C'}, 'stdin': None, 'stdout': -1, 'stderr': -2, 'with_communicate': True, 'timeout': None, 'bg': False, 'close_fds': True}', reason: [Errno 2] No such file or directory: 'brew'

This is reproduced with latest released (3006.1) salt-master & salt-minion on macOS Ventura (also latest) with a just installed homebrew.

@arsa-dev arsa-dev marked this pull request as ready for review June 17, 2023 16:26
@arsa-dev arsa-dev changed the title WIP: Use brew path from which in mac_brew_pkg module Use brew path from which in mac_brew_pkg module Jun 17, 2023
s0undt3ch
s0undt3ch previously approved these changes Jun 17, 2023
@arsa-dev arsa-dev temporarily deployed to ci June 17, 2023 20:02 — with GitHub Actions Inactive
@arsa-dev arsa-dev temporarily deployed to ci June 17, 2023 20:02 — with GitHub Actions Inactive
@arsa-dev arsa-dev temporarily deployed to ci June 17, 2023 20:02 — with GitHub Actions Inactive
@arsa-dev arsa-dev temporarily deployed to ci June 17, 2023 20:02 — with GitHub Actions Inactive
@arsa-dev arsa-dev temporarily deployed to ci June 17, 2023 20:19 — with GitHub Actions Inactive
@arsa-dev arsa-dev temporarily deployed to ci June 17, 2023 20:26 — with GitHub Actions Inactive
@arsa-dev arsa-dev temporarily deployed to ci August 2, 2023 00:48 — with GitHub Actions Inactive
@arsa-dev arsa-dev temporarily deployed to ci August 2, 2023 00:48 — with GitHub Actions Inactive
@arsa-dev arsa-dev temporarily deployed to ci August 2, 2023 00:48 — with GitHub Actions Inactive
@arsa-dev arsa-dev temporarily deployed to ci August 2, 2023 00:48 — with GitHub Actions Inactive
@arsa-dev arsa-dev temporarily deployed to ci August 2, 2023 00:48 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 7, 2023 19:22 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 7, 2023 19:23 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 7, 2023 19:23 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 7, 2023 19:35 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 7, 2023 19:37 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 7, 2023 19:44 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 8, 2023 00:33 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 8, 2023 00:33 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 8, 2023 00:33 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 8, 2023 00:33 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 8, 2023 00:34 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 8, 2023 00:34 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 8, 2023 01:00 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 8, 2023 01:00 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 8, 2023 01:00 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 8, 2023 01:00 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 8, 2023 01:01 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 8, 2023 01:01 — with GitHub Actions Inactive
@Ch3LL Ch3LL merged commit 4ac4da9 into saltstack:master Aug 9, 2023
@welcome
Copy link

welcome bot commented Aug 9, 2023

Congratulations on your first PR being merged! 🎉

@github-actions
Copy link

github-actions bot commented Aug 9, 2023

💚 All backports created successfully

Status Branch Result
3006.x

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@github-actions github-actions bot added the backport:complete PR Backported label Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:complete PR Backported backport:3006.x Backport PR to 3006.x branch Chlorine v3007.0 has-failing-test Needs-Testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases Sulfur v3006.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants