Skip to content

Commit 7aceb22

Browse files
authored
fix(install): handle pre-existing node_modules symlink on Windows (#34659)
Fixes #30723. On Windows, reusing a `node_modules` directory across runs (for example one restored from a CI cache) can leave a stale directory symlink or junction behind. When the installer tries to recreate that link it first removes the previous entry with `remove_dir_all`, but on Windows that call can fail to remove a directory symlink or junction (notably a dangling one). The stale entry is then left in place, so both the symlink attempt and the junction fallback fail with `ERROR_ALREADY_EXISTS` (os error 183) and the install aborts with: ``` Warning Unexpected error symlinking node_modules: Cannot create a file when that file already exists. (os error 183) error: Creating junction in node_modules folder ``` This worked on Deno 2.4.x and regressed in 2.5.0. The intermittent nature of the failure matches a removal that only sometimes leaves the link behind. This removes the previous entry the Windows-correct way: a directory symlink or junction is removed with `RemoveDirectory`, falling back to `DeleteFile` for a file symlink and then to a recursive removal for a real directory. The same removal logic already lives in `cleanup_unused_packages`, so this brings the create path in line with it. As a safety net, symlink and junction creation now retry once after clearing a conflicting entry if creation still reports `AlreadyExists`.
1 parent 840a734 commit 7aceb22

1 file changed

Lines changed: 176 additions & 23 deletions

File tree

libs/npm_installer/local.rs

Lines changed: 176 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1332,6 +1332,8 @@ impl<TSys: NpmCacheSys> LocalSetupCache<TSys> {
13321332
pub(crate) fn symlink_package_dir(
13331333
sys: &(
13341334
impl sys_traits::FsSymlinkDir
1335+
+ sys_traits::FsRemoveDir
1336+
+ sys_traits::FsRemoveFile
13351337
+ sys_traits::FsRemoveDirAll
13361338
+ sys_traits::FsCreateDirAll
13371339
+ sys_traits::FsCreateJunction
@@ -1347,7 +1349,7 @@ pub(crate) fn symlink_package_dir(
13471349
}
13481350

13491351
// need to delete the previous symlink before creating a new one
1350-
let _ignore = sys.fs_remove_dir_all(new_path);
1352+
let _ignore = remove_existing_entry(sys.as_ref(), new_path);
13511353

13521354
let old_path_relative = relative_path(new_parent, old_path)
13531355
.unwrap_or_else(|| old_path.to_path_buf());
@@ -1368,8 +1370,85 @@ fn relative_path(from: &Path, to: &Path) -> Option<PathBuf> {
13681370
pathdiff::diff_paths(to, from)
13691371
}
13701372

1373+
/// Removes whatever currently exists at `path` (file, directory, symlink, or
1374+
/// junction) so a fresh symlink/junction can be created in its place. A stale
1375+
/// entry can be left behind when a `node_modules` directory is reused across
1376+
/// runs, for example when it's restored from a CI cache.
1377+
fn remove_existing_entry(
1378+
sys: &(
1379+
impl sys_traits::FsRemoveDir
1380+
+ sys_traits::FsRemoveFile
1381+
+ sys_traits::FsRemoveDirAll
1382+
),
1383+
path: &Path,
1384+
) -> Result<(), std::io::Error> {
1385+
let is_not_found =
1386+
|err: &std::io::Error| err.kind() == std::io::ErrorKind::NotFound;
1387+
// First try the syscall appropriate for the entry's actual type. Its error
1388+
// is discarded: if it fails we fall through to the recursive removal below
1389+
// and surface that error instead, since it's the last and most complete
1390+
// attempt.
1391+
if sys_traits::impls::is_windows() {
1392+
// On Windows a directory symlink or junction must be removed with
1393+
// RemoveDirectory rather than DeleteFile, and `remove_dir_all` may fail on
1394+
// a dangling directory symlink, so remove the link itself first.
1395+
match sys.fs_remove_dir(path) {
1396+
Ok(()) => return Ok(()),
1397+
Err(err) if is_not_found(&err) => return Ok(()),
1398+
Err(_) => {}
1399+
}
1400+
// It may instead be a file symlink, which needs DeleteFile.
1401+
match sys.fs_remove_file(path) {
1402+
Ok(()) => return Ok(()),
1403+
Err(err) if is_not_found(&err) => return Ok(()),
1404+
Err(_) => {}
1405+
}
1406+
} else {
1407+
// On Unix unlinking a symlink does not follow it.
1408+
match sys.fs_remove_file(path) {
1409+
Ok(()) => return Ok(()),
1410+
Err(err) if is_not_found(&err) => return Ok(()),
1411+
Err(_) => {}
1412+
}
1413+
}
1414+
// Fall back to removing a real (possibly non-empty) directory, surfacing
1415+
// this error if even the recursive removal fails.
1416+
match sys.fs_remove_dir_all(path) {
1417+
Ok(()) => Ok(()),
1418+
Err(err) if is_not_found(&err) => Ok(()),
1419+
Err(err) => Err(err),
1420+
}
1421+
}
1422+
1423+
/// Runs `create`, and if it fails because something already exists at `path`,
1424+
/// removes that entry and tries once more. This makes creating a symlink or
1425+
/// junction resilient to a stale entry left over from a previous run.
1426+
fn create_retry_if_exists(
1427+
sys: &(
1428+
impl sys_traits::FsRemoveDir
1429+
+ sys_traits::FsRemoveFile
1430+
+ sys_traits::FsRemoveDirAll
1431+
),
1432+
path: &Path,
1433+
mut create: impl FnMut() -> Result<(), std::io::Error>,
1434+
) -> Result<(), std::io::Error> {
1435+
match create() {
1436+
Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => {
1437+
remove_existing_entry(sys, path)?;
1438+
create()
1439+
}
1440+
result => result,
1441+
}
1442+
}
1443+
13711444
fn junction_or_symlink_dir(
1372-
sys: &(impl sys_traits::FsSymlinkDir + sys_traits::FsCreateJunction),
1445+
sys: &(
1446+
impl sys_traits::FsSymlinkDir
1447+
+ sys_traits::FsCreateJunction
1448+
+ sys_traits::FsRemoveDir
1449+
+ sys_traits::FsRemoveFile
1450+
+ sys_traits::FsRemoveDirAll
1451+
),
13731452
old_path_relative: &Path,
13741453
old_path: &Path,
13751454
new_path: &Path,
@@ -1379,29 +1458,37 @@ fn junction_or_symlink_dir(
13791458

13801459
let sys = sys.with_paths_in_errors();
13811460

1461+
// Use junctions because they're supported on ntfs file systems without
1462+
// needing to elevate privileges on Windows.
1463+
// Note: junctions don't support relative paths, so we need to use the
1464+
// absolute path here.
1465+
let create_junction = || {
1466+
create_retry_if_exists(sys.as_ref(), new_path, || {
1467+
sys.fs_create_junction(old_path, new_path)
1468+
})
1469+
};
1470+
13821471
if USE_JUNCTIONS.load(std::sync::atomic::Ordering::Relaxed) {
1383-
// Use junctions because they're supported on ntfs file systems without
1384-
// needing to elevate privileges on Windows.
1385-
// Note: junctions don't support relative paths, so we need to use the
1386-
// absolute path here.
1387-
return sys.fs_create_junction(old_path, new_path);
1472+
return create_junction();
13881473
}
13891474

1390-
match symlink_dir(sys.as_ref(), old_path_relative, new_path) {
1475+
match create_retry_if_exists(sys.as_ref(), new_path, || {
1476+
symlink_dir(sys.as_ref(), old_path_relative, new_path)
1477+
}) {
13911478
Ok(()) => Ok(()),
13921479
Err(symlink_err)
13931480
if symlink_err.kind() == std::io::ErrorKind::PermissionDenied =>
13941481
{
13951482
USE_JUNCTIONS.store(true, std::sync::atomic::Ordering::Relaxed);
1396-
sys.fs_create_junction(old_path, new_path)
1483+
create_junction()
13971484
}
13981485
Err(symlink_err) => {
13991486
log::warn!(
14001487
"{} Unexpected error symlinking node_modules: {symlink_err}",
14011488
colors::yellow("Warning")
14021489
);
14031490
USE_JUNCTIONS.store(true, std::sync::atomic::Ordering::Relaxed);
1404-
sys.fs_create_junction(old_path, new_path)
1491+
create_junction()
14051492
}
14061493
}
14071494
}
@@ -1507,17 +1594,6 @@ fn cleanup_unused_packages<TSys: LocalNpmInstallSys>(
15071594
})
15081595
.collect::<HashSet<_>>();
15091596

1510-
// Helper closure for removing symlinks cross-platform
1511-
let remove_symlink = |path: &Path| -> std::io::Result<()> {
1512-
if sys_traits::impls::is_windows() {
1513-
sys
1514-
.fs_remove_dir(path)
1515-
.or_else(|_| sys.fs_remove_file(path))
1516-
} else {
1517-
sys.fs_remove_file(path)
1518-
}
1519-
};
1520-
15211597
// Clean up .deno/node_modules/* symlinks for packages no longer needed
15221598
let deno_node_modules_dir = deno_local_registry_dir.join("node_modules");
15231599
let _ignore = remove_unused_node_modules_symlinks(
@@ -1526,7 +1602,7 @@ fn cleanup_unused_packages<TSys: LocalNpmInstallSys>(
15261602
&keep_names,
15271603
&mut |name, path| {
15281604
setup_cache.remove_deno_symlink(name);
1529-
remove_symlink(path)
1605+
remove_existing_entry(sys, path)
15301606
},
15311607
);
15321608

@@ -1537,7 +1613,7 @@ fn cleanup_unused_packages<TSys: LocalNpmInstallSys>(
15371613
&keep_names,
15381614
&mut |name, path| {
15391615
setup_cache.remove_root_symlink(name);
1540-
remove_symlink(path)
1616+
remove_existing_entry(sys, path)
15411617
},
15421618
);
15431619

@@ -1625,6 +1701,10 @@ pub fn remove_unused_node_modules_symlinks<TSys: LocalNpmInstallSys>(
16251701

16261702
#[cfg(test)]
16271703
mod test {
1704+
use sys_traits::FsCreateDirAll;
1705+
use sys_traits::FsMetadata;
1706+
use sys_traits::FsRead;
1707+
use sys_traits::FsWrite;
16281708
use test_util::TempDir;
16291709

16301710
use super::*;
@@ -1665,6 +1745,79 @@ mod test {
16651745
);
16661746
}
16671747

1748+
#[test]
1749+
fn test_symlink_package_dir_replaces_existing_link() {
1750+
let temp_dir = TempDir::new();
1751+
let sys = sys_traits::impls::RealSys;
1752+
let root = temp_dir.path().to_path_buf();
1753+
1754+
let target_a = root.join("target_a");
1755+
let target_b = root.join("target_b");
1756+
sys.fs_create_dir_all(&target_a).unwrap();
1757+
sys.fs_create_dir_all(&target_b).unwrap();
1758+
sys.fs_write(target_a.join("marker.txt"), "a").unwrap();
1759+
sys.fs_write(target_b.join("marker.txt"), "b").unwrap();
1760+
1761+
let node_modules = root.join("node_modules");
1762+
sys.fs_create_dir_all(&node_modules).unwrap();
1763+
let link = node_modules.join("pkg");
1764+
1765+
// First the link points at target_a.
1766+
symlink_package_dir(&sys, &target_a, &link).unwrap();
1767+
assert_eq!(sys.fs_read_to_string(link.join("marker.txt")).unwrap(), "a");
1768+
1769+
// Re-creating over the pre-existing link must succeed (this is what
1770+
// regressed on Windows: a stale directory symlink/junction has to be
1771+
// removed before the new one can be created) and now resolve to target_b.
1772+
symlink_package_dir(&sys, &target_b, &link).unwrap();
1773+
assert_eq!(sys.fs_read_to_string(link.join("marker.txt")).unwrap(), "b");
1774+
}
1775+
1776+
#[test]
1777+
fn test_create_retry_if_exists_clears_stale_entry() {
1778+
let temp_dir = TempDir::new();
1779+
let sys = sys_traits::impls::RealSys;
1780+
let path = temp_dir.path().join("entry").to_path_buf();
1781+
1782+
// A stale entry is sitting where we want to create something new.
1783+
sys.fs_create_dir_all(&path).unwrap();
1784+
1785+
let mut attempts = 0;
1786+
let result = create_retry_if_exists(&sys, &path, || {
1787+
attempts += 1;
1788+
if attempts == 1 {
1789+
// Simulate creation failing because the stale entry is in the way.
1790+
Err(std::io::Error::new(
1791+
std::io::ErrorKind::AlreadyExists,
1792+
"already exists",
1793+
))
1794+
} else {
1795+
Ok(())
1796+
}
1797+
});
1798+
1799+
assert!(result.is_ok());
1800+
assert_eq!(attempts, 2);
1801+
// The stale entry must have been removed before the retry.
1802+
assert!(!sys.fs_exists(&path).unwrap());
1803+
}
1804+
1805+
#[test]
1806+
fn test_create_retry_if_exists_passes_through_success() {
1807+
let temp_dir = TempDir::new();
1808+
let sys = sys_traits::impls::RealSys;
1809+
let path = temp_dir.path().join("entry").to_path_buf();
1810+
1811+
let mut attempts = 0;
1812+
let result = create_retry_if_exists(&sys, &path, || {
1813+
attempts += 1;
1814+
Ok(())
1815+
});
1816+
1817+
assert!(result.is_ok());
1818+
assert_eq!(attempts, 1);
1819+
}
1820+
16681821
#[test]
16691822
fn test_node_modules_package_actual_dir_to_name() {
16701823
assert_eq!(

0 commit comments

Comments
 (0)