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

Handle case in _configure_openmp() for MacOS system without Homebrew #482

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

isobelhooper
Copy link

On a MacOS system that's not using Homebrew, running brew --prefix llvm with subprocess.check_output doesn't produce a CalledProcessError; instead, when the shell produces a FileNotFoundError it passes it back and check_output raises that instead.

If we check for OSError (the parent of FileNotFoundError, which should handle other OS-related errors that could get passed back) as well, then this should correctly handle this case.

The MacPorts check doesn't have the same issue, because the which command is built in and so which port will raise a CalledProcessError if MacPorts isn't installed.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

…mebrew

On a MacOS system that's not using Homebrew, running `brew --prefix llvm`
with `subprocess.check_output` doesn't produce a CalledProcessError; instead,
when the shell produces a FileNotFoundError it passes it back and
`check_output` raises that instead.

If we check for OSError (the parent of FileNotFoundError, which should handle
other OS-related errors that could get passed back) as well, then this should
correctly handle this case.

The MacPorts check doesn't have the same issue, because the `which` command
is built in and so `which port` will raise a CalledProcessError if MacPorts
isn't installed.
@isobelhooper isobelhooper force-pushed the handle-macos-without-brew-in-setup-py branch from 61334fd to a71f3d1 Compare November 19, 2024 14:00
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.

2 participants