Repository navigation
fix(desktop): recursive copyFileSync fallback for native-binary dirs on Windows - #76211
Closed
ruguanvip-beep wants to merge 1 commit into
Closed
ruguanvip-beep wants to merge 1 commit into
ruguanvip-beep wants to merge 1 commit into
Conversation
…on Windows Node fs.cpSync fails with EIO / "Access is denied" on Windows when copying directories that contain in-use native binaries (notably node-pty's conpty/ with OpenConsole.exe + conpty.dll). This breaks the desktop rebuild on Windows (reproduced repeatedly 2025-07-09 -> 07-24). Add copyDirRecursive() which walks the tree with copyFileSync, bypassing the cpSync code path that triggers the defect, and use it for native dependency staging so the Windows desktop rebuild completes.
teknium1
reviewed
Aug 1, 2026
teknium1
left a comment
teknium1
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for targeting the active native-dependency staging path. Current main still recursively calls cpSync for build/Release directories (apps/desktop/scripts/stage-native-deps.mjs:97-100) and for the conpty prebuild directory (apps/desktop/scripts/stage-native-deps.mjs:262-265).
Problems
- The new helper is used unconditionally in the PR at
apps/desktop/scripts/stage-native-deps.mjs:124, so macOS/Linuxbuild/Releasecopies also change despite the stated Windows-only scope. - No regression test accompanies the change. The current staging fixture at
apps/desktop/scripts/stage-native-deps.test.mjs:34-47does not create aconptydirectory or nested payload.
Suggested changes
- Keep
cpSyncfor non-Windows hosts and scope the workaround toprocess.platform === 'win32'. - Add a fixture with nested
conptyfiles and assertstageNodePtyIntostages them.
Automated hermes-sweeper review.
| @@ -96,7 +124,7 @@ function copyBuildRelease(srcDir, destDir) { | |||
| mkdirSync(destDir, { recursive: true }) | |||
Collaborator
There was a problem hiding this comment.
This replacement runs for every host platform whenever copyBuildRelease is used, not only Windows. Please retain cpSync on macOS/Linux and scope the workaround to Windows so the stated platform-limited behavior is preserved.
alt-glitch
added
type/bug
teknium1
added
the
sweeper:blast-contained
teknium1
added a commit
that referenced
this pull request
Sep 17, 2026
…k .bak wipe The salvaged commits drop the cpSync/rmSync imports, but stageGetWindowsInto landed on main after the PR was opened and still called both, so the module threw ReferenceError on every platform (7 vitest failures on the cherry-picked head). Route its six sites through copyFileSync/removeDirSync so get-windows staging survives the same non-ASCII profile paths as node-pty. preserveRollbackBackup had the same rmSync no-op as cleanStaleAppOutDir: a surviving .bak makes renameSync fail, so the previous working build gets wiped instead of kept as rollback material. Same existsSync -> removeDirSync fallback. Earlier fixes for the same crash: #60480 (@liuhao1024), #61832 (@danilofalcao), #76211, #103458, #109273, #111590. Co-authored-by: liuhao1024 <liuhao1024@users.noreply.github.com> Co-authored-by: danilofalcao <danilofalcao@users.noreply.github.com>
Collaborator
|
Fixed on main by #113919 ( |
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
3 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.
问题
Windows 桌面构建的
stage-native-deps.mjs用fs.cpSync复制含原生二进制的目录。当源目录含被占用的 DLL/EXE（典型是 node-pty 的conpty/，含OpenConsole.exe+conpty.dll）时，cpSync报EIO / "Access is denied"，导致桌面重建中断，hermes update自更新无法完成（在bootstrap-installer.log中反复出现 2025-07-09 -> 07-24）。根因
Node 的
cpSync在 Windows 上复制含被占用原生二进制的目录时触发该缺陷。更新器执行git reset --hard origin/main后重建，又跑到同一段会失败的拷贝 -> 自更新死循环。修复
新增
copyDirRecursive(srcDir, destDir)：用copyFileSync逐文件递归拷贝（单文件拷贝不走cpSync的缺陷代码路径），并在原生依赖 staging 处用它替代cpSync。影响范围
apps/desktop/scripts/stage-native-deps.mjs（构建期 staging 脚本，不影响运行时）。测试计划
hermes update/ 完整桌面重建在 conpty 路径不再报 EIO，能完成。背景
这原本是本地补丁——
hermes update会git reset --hard origin/main，每次自更新后都得手动重打，且正是它让工作树dirty、与桌面「启动强制自更新」形成死结，导致桌面端启动死循环。上游合入后：本地补丁不再需要；hermes update重建不再 EIO -> 自更新收敛 -> 桌面端恢复正常。🤖 Generated with WorkBuddy