Skip to content

Commit 1b1c702

Browse files
AgentEnderFrozenPandaz
AgentEnder
authored and
FrozenPandaz
committed
fix(core): handle dangling symlinks during cache restore (#34396)
When cache outputs include both glob patterns and directory patterns containing symlinks, the cache restore fails with EEXIST (os error 17). This happens because `fs_extra::remove_items` silently skips dangling symlinks (since `is_dir()`/`is_file()` follow links and return false), leaving stale symlinks that cause `symlink()` to fail. The fix makes symlink creation idempotent by checking for and removing any existing symlink at the destination before creating a new one, using `symlink_metadata()` which correctly detects dangling symlinks. Fixes #34013 (cherry picked from commit 6570674)
1 parent 893404a commit 1b1c702

3 files changed

Lines changed: 253 additions & 0 deletions

File tree

‎e2e/nx/src/cache.test.ts‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,93 @@ describe('cache', () => {
250250
expect(outputsWithoutOutputs).not.toContain('z.md');
251251
});
252252

253+
it('should handle outputs with globs and directories containing symlinks', async () => {
254+
// Reproduces https://github.com/nrwl/nx/issues/34013
255+
// When outputs include both glob patterns (*.json) and directory patterns
256+
// (standalone, server), and those directories contain symlinks,
257+
// the cache put operation fails with "File exists (os error 17)" (EEXIST).
258+
const projectName = uniq('myapp');
259+
260+
// Create a build script that produces output with symlinks
261+
// (simulating Next.js standalone build with pnpm/bun node_modules)
262+
updateFile(
263+
`apps/${projectName}/build.js`,
264+
`
265+
const fs = require('fs');
266+
const path = require('path');
267+
268+
const projectRoot = path.join(process.cwd(), 'apps/${projectName}');
269+
const nextDir = path.join(projectRoot, '.next');
270+
271+
// Clean previous output
272+
fs.rmSync(nextDir, { recursive: true, force: true });
273+
274+
// Create .next directory with JSON files
275+
fs.mkdirSync(nextDir, { recursive: true });
276+
fs.writeFileSync(path.join(nextDir, 'build-manifest.json'), JSON.stringify({pages:{}}));
277+
fs.writeFileSync(path.join(nextDir, 'routes-manifest.json'), JSON.stringify({routes:[]}));
278+
279+
// Create .next/standalone with a symlink (simulating pnpm linker)
280+
const standaloneDir = path.join(nextDir, 'standalone');
281+
fs.mkdirSync(path.join(standaloneDir, 'real-pkg'), { recursive: true });
282+
fs.writeFileSync(path.join(standaloneDir, 'real-pkg', 'index.js'), 'module.exports = {}');
283+
fs.writeFileSync(path.join(standaloneDir, 'server.js'), 'require("./real-pkg")');
284+
285+
// Create a symlink inside standalone (outside of node_modules)
286+
// This simulates the kind of symlink structure pnpm/bun creates
287+
fs.symlinkSync(
288+
path.join(standaloneDir, 'real-pkg', 'index.js'),
289+
path.join(standaloneDir, 'linked-entry.js')
290+
);
291+
292+
// Create .next/server
293+
const serverDir = path.join(nextDir, 'server');
294+
fs.mkdirSync(serverDir, { recursive: true });
295+
fs.writeFileSync(path.join(serverDir, 'app.js'), 'module.exports = {}');
296+
297+
console.log('Build complete');
298+
`
299+
);
300+
301+
updateFile(
302+
`apps/${projectName}/project.json`,
303+
JSON.stringify({
304+
name: projectName,
305+
targets: {
306+
build: {
307+
cache: true,
308+
command: `node apps/${projectName}/build.js`,
309+
inputs: ['{projectRoot}/build.js'],
310+
outputs: [
311+
`{projectRoot}/.next/*.json`,
312+
`{projectRoot}/.next/standalone`,
313+
`{projectRoot}/.next/server`,
314+
],
315+
},
316+
},
317+
})
318+
);
319+
320+
// First run - should cache without error
321+
const firstRun = runCLI(`build ${projectName}`);
322+
expect(firstRun).not.toContain('read the output from the cache');
323+
expect(firstRun).toContain('Build complete');
324+
325+
// Verify the output files exist including the symlink
326+
expect(
327+
fileExists(tmpProjPath(`apps/${projectName}/.next/build-manifest.json`))
328+
).toBe(true);
329+
expect(
330+
fileExists(
331+
tmpProjPath(`apps/${projectName}/.next/standalone/linked-entry.js`)
332+
)
333+
).toBe(true);
334+
335+
// Second run - should hit cache and restore without EEXIST error
336+
const secondRun = runCLI(`build ${projectName}`);
337+
expect(secondRun).toContain('local cache');
338+
});
339+
253340
it('should use consider filesets when hashing', async () => {
254341
const parent = uniq('parent');
255342
const child1 = uniq('child1');

‎packages/nx/src/native/cache/expand_outputs.rs‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,156 @@ mod test {
312312
);
313313
}
314314

315+
#[test]
316+
#[cfg(unix)]
317+
fn should_expand_outputs_with_symlinks_and_globs() {
318+
// Reproduces https://github.com/nrwl/nx/issues/34013
319+
// When outputs include both glob patterns (*.json) and directory patterns
320+
// that contain symlinks, the expanded outputs may cause EEXIST errors
321+
// during cache operations.
322+
use std::os::unix::fs::symlink;
323+
324+
let temp = TempDir::new().unwrap();
325+
326+
// Create .next directory with JSON files
327+
temp.child(".next/build-manifest.json")
328+
.write_str("{}")
329+
.unwrap();
330+
temp.child(".next/routes-manifest.json")
331+
.write_str("{}")
332+
.unwrap();
333+
334+
// Create .next/standalone with files
335+
temp.child(".next/standalone/server.js")
336+
.write_str("server")
337+
.unwrap();
338+
temp.child(".next/standalone/real-pkg/index.js")
339+
.write_str("pkg")
340+
.unwrap();
341+
342+
// Create a symlink inside standalone (simulating pnpm/bun linker)
343+
symlink(
344+
temp.join(".next/standalone/real-pkg/index.js"),
345+
temp.join(".next/standalone/linked-entry.js"),
346+
)
347+
.unwrap();
348+
349+
// Create .next/server
350+
temp.child(".next/server/app.js").write_str("app").unwrap();
351+
352+
let entries = vec![
353+
".next/*.json".to_string(),
354+
".next/standalone".to_string(),
355+
".next/server".to_string(),
356+
];
357+
let mut result = expand_outputs(temp.display().to_string(), entries).unwrap();
358+
result.sort();
359+
360+
// Verify that the symlink is included in expanded outputs
361+
assert!(
362+
result.contains(&".next/standalone/linked-entry.js".to_string()),
363+
"Symlink should be in expanded outputs"
364+
);
365+
}
366+
367+
#[test]
368+
#[cfg(unix)]
369+
fn should_handle_cache_put_and_restore_with_symlinks() {
370+
// Full reproduction of issue #34013 including the cache put and restore cycle
371+
use crate::native::cache::file_ops::_copy;
372+
use std::os::unix::fs::symlink;
373+
374+
enable_logger();
375+
376+
let workspace = TempDir::new().unwrap();
377+
let cache = TempDir::new().unwrap();
378+
379+
// Create workspace structure simulating Next.js standalone build
380+
workspace
381+
.child("app/.next/build-manifest.json")
382+
.write_str("{}")
383+
.unwrap();
384+
workspace
385+
.child("app/.next/routes-manifest.json")
386+
.write_str("{}")
387+
.unwrap();
388+
workspace
389+
.child("app/.next/standalone/server.js")
390+
.write_str("server")
391+
.unwrap();
392+
workspace
393+
.child("app/.next/standalone/real-pkg/index.js")
394+
.write_str("pkg")
395+
.unwrap();
396+
397+
// Create symlink (simulating pnpm/bun linker)
398+
symlink(
399+
workspace.join("app/.next/standalone/real-pkg/index.js"),
400+
workspace.join("app/.next/standalone/linked-entry.js"),
401+
)
402+
.unwrap();
403+
404+
workspace
405+
.child("app/.next/server/app.js")
406+
.write_str("app")
407+
.unwrap();
408+
409+
let outputs = vec![
410+
"app/.next/*.json".to_string(),
411+
"app/.next/standalone".to_string(),
412+
"app/.next/server".to_string(),
413+
];
414+
415+
// === SIMULATE CACHE PUT ===
416+
let cache_hash_dir = cache.child("hash123");
417+
cache_hash_dir.create_dir_all().unwrap();
418+
419+
let expanded = _expand_outputs(workspace.path(), outputs.clone()).unwrap();
420+
421+
// Copy each expanded output to cache (mimicking cache.rs put())
422+
for output in expanded.iter() {
423+
let src = workspace.join(output);
424+
if src.exists() {
425+
let dest = cache_hash_dir.join(output);
426+
let result = _copy(&src, &dest);
427+
assert!(
428+
result.is_ok(),
429+
"PUT: Failed to copy {}: {:?}",
430+
output,
431+
result.err()
432+
);
433+
}
434+
}
435+
436+
// === SIMULATE CACHE RESTORE ===
437+
// (This is what copy_files_from_cache does)
438+
439+
// Step 1: Expand outputs from cache directory
440+
let restore_expanded = _expand_outputs(cache_hash_dir.path(), outputs.clone()).unwrap();
441+
442+
// Step 2: Remove expanded outputs from workspace
443+
let items_to_remove: Vec<_> = restore_expanded.iter().map(|p| workspace.join(p)).collect();
444+
fs_extra::remove_items(&items_to_remove).unwrap();
445+
446+
// Step 3: Copy entire cache hash dir to workspace (this is where EEXIST occurs)
447+
let restore_result = _copy(cache_hash_dir.path(), workspace.path());
448+
assert!(
449+
restore_result.is_ok(),
450+
"RESTORE: Failed to copy from cache to workspace: {:?}",
451+
restore_result.err()
452+
);
453+
454+
// Verify files were restored correctly
455+
assert!(workspace.child("app/.next/build-manifest.json").exists());
456+
assert!(workspace.child("app/.next/standalone/server.js").exists());
457+
assert!(
458+
workspace
459+
.child("app/.next/standalone/linked-entry.js")
460+
.exists()
461+
);
462+
assert!(workspace.child("app/.next/server/app.js").exists());
463+
}
464+
315465
#[test]
316466
fn should_get_files_for_outputs_when_gitignore_hides_files() {
317467
let temp = TempDir::new().unwrap();

‎packages/nx/src/native/cache/file_ops.rs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ where
4343
copied_size
4444
} else if src.is_symlink() {
4545
trace!("Copying symlink: {:?}", &src);
46+
remove_existing_symlink(&dest)?;
4647
symlink(fs::read_link(&src)?, &dest)?;
4748
trace!("Successfully copied symlink: {:?}", &src);
4849
0
@@ -87,6 +88,20 @@ fn symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) -> io::Result<(
8788
std::os::wasi::fs::symlink_path(original, link)
8889
}
8990

91+
/// Removes an existing symlink at `path` if one exists.
92+
/// Uses `symlink_metadata` which does not follow symlinks, so it correctly
93+
/// detects dangling symlinks that `is_file()`/`is_dir()` would miss.
94+
fn remove_existing_symlink(path: &Path) -> io::Result<()> {
95+
match fs::symlink_metadata(path) {
96+
Ok(meta) if meta.file_type().is_symlink() => {
97+
trace!("Removing existing symlink at {:?}", path);
98+
fs::remove_file(path)?;
99+
}
100+
_ => {}
101+
}
102+
Ok(())
103+
}
104+
90105
fn copy_dir_all(src: impl AsRef<Path>, dst: impl AsRef<Path>) -> io::Result<u64> {
91106
trace!("Creating directory: {:?}", dst.as_ref());
92107
fs::create_dir_all(&dst)?;
@@ -115,6 +130,7 @@ fn copy_dir_all(src: impl AsRef<Path>, dst: impl AsRef<Path>) -> io::Result<u64>
115130
subdir_size
116131
} else if ty.is_symlink() {
117132
trace!("Copying symlink: {:?}", entry.path());
133+
remove_existing_symlink(&dest_path)?;
118134
symlink(fs::read_link(entry.path())?, dest_path)?;
119135
symlinks_copied += 1;
120136
trace!("Successfully copied symlink: {:?}", entry_name);

0 commit comments

Comments
 (0)