fix(windows): use correct yt-dlp config path and resolve .CMD shims i… - #468
Closed
my1057739543-max wants to merge 1 commit into
Closed
fix(windows): use correct yt-dlp config path and resolve .CMD shims i…#468my1057739543-max wants to merge 1 commit into
my1057739543-max wants to merge 1 commit into
Conversation
…n subprocess Two Windows-install bugs in cli.py: 1. yt-dlp JS runtime config was written to ~/.config/yt-dlp/config (Linux path). On Windows yt-dlp reads %APPDATA%/yt-dlp/config, so install reported 'configured' while doctor (which already uses the cross-platform get_ytdlp_config_path helper) reported it unconfigured. Reuse the existing helper so install and doctor agree. 2. _install_mcporter called subprocess.run(['npm', ...]) / ['mcporter'] with bare command names. On Windows these are .CMD shims and CreateProcess does not append PATHEXT, so subprocess raised FileNotFoundError (WinError 2). Resolve via shutil.which first (mirrors probe_command and the existing undici install path). Verified on Windows 11: install now writes %APPDATA%/yt-dlp/config, doctor reports YouTube as configured, and npm/mcporter subprocess calls succeed instead of raising WinError 2.
Owner
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
yt-dlp config written to wrong path.
_install_system_depswrites--js-runtimes nodeto~/.config/yt-dlp/config(a Linux path). On Windows yt-dlp reads%APPDATA%/yt-dlp/config, so the config is neverapplied. Result:
installprints " yt-dlp configured" butdoctorreports YouTube as[!] 未配置 JS runtime—they contradict because
doctoralready uses the cross-platformget_ytdlp_config_path()helper whileinstallhardcodes the Linux path.
mcporter install fails with WinError 2.
_install_mcportercallssubprocess.run(["npm", ...])with a barecommand name. On Windows
npm/mcporter/npxare.CMDshims, andCreateProcessdoes not append PATHEXT, so thisraises
FileNotFoundError(WinError 2) even thoughshutil.which("npm")finds it.Fix
Both reuse patterns that already exist elsewhere in the codebase:
Call the existing
agent_reach.utils.paths.get_ytdlp_config_path()instead of hardcoding~/.config/yt-dlp— thesame helper
doctor/youtube.pyalready use.Resolve command names via
shutil.which()beforesubprocess.run(), mirroringprobe.py:probe_command()and theexisting undici install path (
npm_cmd = shutil.which("npm")).Verification
Tested on Windows 11 with Anaconda Python 3.11.7 + venv install:
installwrites--js-runtimes nodeto%APPDATA%/yt-dlp/config(verified the file is created thereand
~/.config/yt-dlp/configis no longer touched).doctornow reportsYouTube 视频和字幕 — 可提取视频信息和字幕(install and doctor agree).subprocess.run([shutil.which("npm"), "--version"])succeeds (returns11.13.0); the bare["npm", ...]formraises
FileNotFoundError: WinError 2. Same formcporter.Change scope:
1 file changed, 28 insertions(+), 8 deletions(-)—agent_reach/cli.pyonly.Notes
_install_system_depsWindows branch forgh(separate issue — gh CLI auto-install has noWindows path, only linux/darwin). Happy to send a follow-up PR if welcome.