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

obs-scripting: Fix macOS Homebrew Python loading #11590

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

Conversation

jcm93
Copy link
Contributor

@jcm93 jcm93 commented Dec 6, 2024

Description

Use the explicit version number for the directory when dynamically loading Python inside a Framework bundle on macOS.

Motivation and Context

If using Homebrew to manage Python versions (as is commonly done on macOS), no versions of Python supported by OBS seem to currently be loadable on macOS. When currently installed via Homebrew as of December 6th, 2024, supported Python versions (3.10 through 3.12 tested) do not contain a "Current" symlink inside their "Versions" folder, which is what OBS looks for when loading the dynamic library within the framework bundle.

Python 3.13 does contains a "Current" symlink. It also seems clear that at some point, past versions of Python also had this "Current" symlink. I do not know why Homebrew Python is packaged in this way where the "Current" symlink will sometimes be there and other times not. Regardless, the Versions directory does seem to always contain the<major>.<minor> directory that "Current" symlinks to, so it seems safe to just load Python using this path, since we already have the version information explicitly available.

How Has This Been Tested?

macOS 15.1.1, Apple Silicon; Python now loads with current installs of supported Python versions via Homebrew.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@RytoEX RytoEX requested a review from PatTheMav December 6, 2024 19:39
Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

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

The correct prefix would be obs-scripting: .

@jcm93 jcm93 force-pushed the mac-load-python-fix branch from 0de1524 to abeff9f Compare December 6, 2024 19:40
@jcm93 jcm93 changed the title scripting: Fix macOS Python loading obs-scripting: Fix macOS Python loading Dec 6, 2024
@alinsavix
Copy link
Contributor

If I'm reading the homebrew recipes correctly, this seems like a pretty reasonable and reliable way to deal with this, and also should still work with the python install provided by Apple in Xcode and such. 👍

(and the Xcode-bundled python version doesn't have a "Current" link, either)

@WizardCM WizardCM added Bug Fix Non-breaking change which fixes an issue macOS Affects macOS labels Dec 7, 2024
Copy link
Member

@PatTheMav PatTheMav left a comment

Choose a reason for hiding this comment

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

Should work fine. Has this been tested with existing setups? I'd guess because you usually only provided the Frameworks directory that contained the Python framework (and we produced a path from that at runtime) it should lead to no issues?

@jcm93
Copy link
Contributor Author

jcm93 commented Dec 16, 2024

Should work fine. Has this been tested with existing setups? I'd guess because you usually only provided the Frameworks directory that contained the Python framework (and we produced a path from that at runtime) it should lead to no issues?

Correct; the only way OBS will load Python on macOS is by looking inside a Python.framework bundle in the directory the user provides in the Python configuration window. So there shouldn't be any way for this to break anything (that is, unless Python or Homebrew decide to violate the Framework bundle specification in a new and fun way in the future).

@jcm93 jcm93 force-pushed the mac-load-python-fix branch from abeff9f to 604d4a4 Compare December 16, 2024 20:43
@jcm93 jcm93 changed the title obs-scripting: Fix macOS Python loading obs-scripting: Fix macOS Homebrew Python loading Dec 16, 2024
@jcm93
Copy link
Contributor Author

jcm93 commented Dec 16, 2024

As noted out-of-band, Python's official installer will still contain the Current symlink for supported versions. So for anyone happening on this issue thread looking for a fix, the workaround until this PR is merged is to use Python's installer from its website, and then point OBS toward the /Library/Frameworks folder when locating the Python install.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fix Non-breaking change which fixes an issue macOS Affects macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants