macOS: Fix yt-dlp launch through mpv - #791
Conversation
Removed deprecated handling for youtube-dl on macOS and updated environment variable management for mpv subprocesses.
| env.pop('PYTHONPATH', None) | ||
|
|
||
| # Add common executable locations while preserving inherited PATH entries. | ||
| macPaths = [ |
There was a problem hiding this comment.
There was a problem hiding this comment.
I know what you're trying to accomplish, I authored that PR :)
my question is about the addition in this PR relative to mine (extending PATH with 'common' paths) - what makes you think PATH is not sufficiently populated therefore needs to be extended?
There was a problem hiding this comment.
Fair question. The PATH change isn't needed for the "PYTHONHOME"/"PYTHONPATH" fix itself. I added it because removing the old workaround also removes its explicit "/opt/homebrew/bin:/usr/local/bin:/usr/bin" PATH handling.
My concern is the normal DMG/Finder-launched use case: py2app documents that applications launched normally get a minimal environment and don't pick up changes from the user's shell profile. This was also discussed in #276, where it was noted that macOS GUI apps may not have "/usr/local/bin" in PATH and the suggestion was to append it rather than replace the existing PATH.
So the intention was to preserve the old code's ability to find a Homebrew-installed downloader, while avoiding overwriting any PATH that is already present. "/opt/homebrew/bin" covers current Apple Silicon Homebrew and "/usr/local/bin" covers Intel Homebrew.
On reflection, I think the patch can be narrower here: rather than adding all of the standard system paths, it could just preserve the inherited PATH and append "/opt/homebrew/bin" and "/usr/local/bin" if missing.
What's your thoughts on it being tweaked to:
if isMacOS():
env.pop('PYTHONHOME', None)
env.pop('PYTHONPATH', None)
# Add common Homebrew locations while preserving inherited PATH entries.
pathEntries = [path for path in env.get('PATH', '').split(os.pathsep) if path]
for path in ['/opt/homebrew/bin', '/usr/local/bin']:
if path not in pathEntries:
pathEntries.append(path)
env['PATH'] = os.pathsep.join(pathEntries)
2 participants
This is a fix inspired by #785 to address #585 and is hopefully more reliable.
This PR removes deprecated handling for youtube-dl on macOS and updates environment variable management for mpv subprocesses.