Injected copy: sync materializes a symlinked directory that the install keeps as a symlink #147

Activity

1. zkochan commented on Oct 17, 2026

   

   zkochan

   on Oct 17, 2026

   MemberAuthor

   More actions

   Another case for this: within one walk, the v12 directory walker visits a directory once per canonical path (the visited set in walk_all_inner). Two links to the same target inside a package (a -> ../shared, b -> ../shared) therefore give the injected copy the files under only one of them. Raised in review on pnpm/pnpm#17033.

   ---

   Written by an agent (Claude Code, claude-opus-5-5).
2. zkochan commented on Oct 17, 2026

   

   zkochan

   on Oct 17, 2026

   MemberAuthor

   More actions

   One more case, raised by Qodo on pnpm/pnpm#17033: v11's walk of every file (_fetchAllFilesFromDir) has no cycle guard. A link inside a package such as loop -> . makes it recurse through loop/loop/... until the path is too long. This affects the install with deployAllFiles and the sync of an injected copy. v12's walker has a visited set.

   ---

   Written by an agent (Claude Code, claude-opus-5-5).
3. zkochan commented on Oct 18, 2026

   

   zkochan

   on Oct 18, 2026

   MemberAuthor

   More actions

   Reproduced on main (ebbb1b3e1a) with both CLIs. app injects lib, the lockfile is shared, and syncInjectedDepsAfterScripts: [build] is set. Each cell shows what app's copy holds after the install, then after pnpm run build in lib. @ marks a symlink.

   lib holds	deployAllFiles	v12	v11
   link-to-a-dir -> files/foo	false	link kept, then kept	link kept, then kept
   link-to-a-dir -> files/foo	true	link kept, then kept	materialized by the install
   a -> shared, b -> shared	false	links kept	install OK, sync fails with EEXIST (it links lib/a/x.txt over the copy's a/x.txt, which is shared/x.txt through the kept link)
   a -> shared, b -> shared	true	links kept	materialized
   loop -> .	false	link kept	install OK, sync fails with ELOOP (the walk of the copy follows loop/loop/...)
   loop -> .	true	link kept	install fails with ELOOP in _fetchAllFilesFromDir
   build adds newlink -> files/foo	false	materialized by the sync	materialized by the sync
   build adds newlink -> files/foo	true	missing after the sync	materialized, as its install does

   So:

   1. v12's install keeps every link. Its sync turns a new directory link into a real directory, and with deployAllFiles it drops the new link entirely. The two-links walker case from the earlier comment did not show up here, because the install reads with preserve_symlinks.
   2. v11's sync crashes on a copy that holds a link to an ancestor or two links to one directory. Its walk of every file crashes on loop -> ., which breaks the install under deployAllFiles.
   3. Under deployAllFiles, v11's install materializes directory links and v12's keeps them.

   To make the sync write what the install writes, it would read both trees with the install's symlink policy and recreate links the way the importer does. The importer's rules live in deps-restorer (placement/symlinks.rs): it rejects targets outside the package, rewrites absolute targets as relative, recreates a directory link only when its target is imported, imports a file link as a file, and uses junction targets on Windows. The syncer should share them rather than copy them.

   ---

   Written by an agent (Claude Code, claude-opus-5-5).
4. zkochan commented on Oct 18, 2026

   

   zkochan

   on Oct 18, 2026

   MemberAuthor

   More actions

   The fix is in pnpm/pnpm#17088, for both CLIs. Each CLI's sync now writes a package's links to its own files the way its install does, by the importer's rules, and the v11 crashes are fixed. v11's install under deployAllFiles still materializes directory links, where v12 keeps them. That difference is left as it was.

   ---

   Written by an agent (Claude Code, claude-opus-5-5).
5. zkochan

   added a commit that references this issue on Oct 18, 2026

   fix: keep a package's links to its own files in synced injected copies (

   ...

   453870c
6. zkochan commented on Oct 18, 2026

   

   zkochan

   on Oct 18, 2026

   MemberAuthor

   More actions

   Fixed by pnpm/pnpm#17088, now merged (453870cc3f). In both CLIs, the sync after a listed script now writes a package's links to its own files the way the install does. The v11 EEXIST and ELOOP failures are fixed, and so is the v11 install failure on a looping link under deployAllFiles.

   One difference is left as decided: v11's install under deployAllFiles still materializes directory links, where v12 keeps them.

   ---

   Written by an agent (Claude Code, claude-opus-5-5).
7. zkochan

   closed this as completedon Oct 18, 2026