diff --git a/crates/pm/src/helper/lock.rs b/crates/pm/src/helper/lock.rs index 65a9f00839..f9969db1db 100644 --- a/crates/pm/src/helper/lock.rs +++ b/crates/pm/src/helper/lock.rs @@ -4,7 +4,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context as _, Result, anyhow}; use serde_json::Value; use utoo_ruborist::lock::{LockPackage, PackageLock}; -use utoo_ruborist::manifest::{DepsView, EnginesView, PackageJson}; +use utoo_ruborist::manifest::PackageJson; use utoo_ruborist::registry::resolve_package; use utoo_ruborist::spec::{PackageSpec, Protocol, resolve_catalog_spec}; use utoo_ruborist::util::PackageNameStr; @@ -17,12 +17,14 @@ use crate::util::cli_enum::{PackageAction, SaveType}; use crate::util::cloner::clone_package; use crate::util::downloader::{is_git_url, resolve_cache_path}; use crate::util::git_resolver::{resolve_git_spec, resolve_github_spec}; -use crate::util::json::{load_package_json, load_package_lock_json_from_path, read_json_file}; +use crate::util::json::{load_package_lock_json_from_path, read_json_file}; use crate::util::logger::{finish_progress_bar, start_progress_bar}; use utoo_ruborist::builder::PeerDeps; use crate::util::platform_const::GLOBAL_NODE_MODULES; -use crate::util::user_config::{get_catalogs, get_peer_deps, set_package_json}; +use crate::util::user_config::{ + get_catalogs, get_or_load_package_json, get_peer_deps, set_package_json, +}; // Platform-specific line endings #[cfg(target_os = "windows")] @@ -349,16 +351,22 @@ pub fn path_to_pkg_name(path_str: &str) -> Option<&str> { } pub async fn is_pkg_lock_outdated(root_path: &Path) -> Result { - // Always read from disk — this function checks whether the on-disk - // package.json has diverged from the lock file, so it must not use the - // in-process cache. - let pkg: DepsView = load_package_json(root_path).await?; + // Root package.json is served from the in-process cache. The cache + // stays consistent with disk across the full `ut install` lifetime + // because `update_package_json` calls `set_package_json` write-through + // after every edit, and a fresh `ut install` process starts with a + // cold cache (so external edits / git checkout always see a re-read). + // The lockfile itself is still read from disk each time — it's the + // thing whose freshness we're checking. + let root_pkg = get_or_load_package_json(root_path).await?; let lock_file: PackageLock = read_json_file(&root_path.join("package-lock.json")).await?; let catalogs = get_catalogs().await; - let deps_match = |pkg_deps: &HashMap, + let deps_match = |pkg_deps: Option<&HashMap>, lock_deps: Option<&HashMap>| -> bool { + let empty = HashMap::new(); + let pkg_deps = pkg_deps.unwrap_or(&empty); match lock_deps { None => pkg_deps.is_empty(), Some(ld) => { @@ -372,24 +380,20 @@ pub async fn is_pkg_lock_outdated(root_path: &Path) -> Result { }; let packages = &lock_file.packages; + let peer_deps = get_peer_deps().await; - // prepare packages to check: (relative_path, deps) - let mut pkgs_to_check: Vec<(String, DepsView)> = vec![("".to_string(), pkg)]; - - // populate all workspaces (load only the deps view for each) let workspaces = find_workspaces(root_path).await?; - for (_, path, _) in workspaces { + let mut pkgs_to_check: Vec<(String, &PackageJson)> = Vec::with_capacity(1 + workspaces.len()); + pkgs_to_check.push((String::new(), &root_pkg)); + for (_, path, pkg) in &workspaces { let target_path = path .strip_prefix(root_path) - .unwrap_or(&path) + .unwrap_or(path) .to_string_lossy() .to_string(); - let ws_deps: DepsView = load_package_json(&path).await?; - pkgs_to_check.push((target_path, ws_deps)); + pkgs_to_check.push((target_path, pkg)); } - let peer_deps = get_peer_deps().await; - for (path, pkg) in &pkgs_to_check { let lock = match packages.get(path.as_str()) { Some(lock) => lock, @@ -402,40 +406,43 @@ pub async fn is_pkg_lock_outdated(root_path: &Path) -> Result { let name = if path.is_empty() { "root" } else { path }; - // check dependencies whether changed - if !deps_match(&pkg.dependencies, lock.dependencies.as_ref()) { + if !deps_match(pkg.dependencies.as_ref(), lock.dependencies.as_ref()) { tracing::warn!("package-lock.json is outdated, {name} dependencies changed"); return Ok(true); } if !deps_match( - &pkg.optional_dependencies, + pkg.optional_dependencies.as_ref(), lock.optional_dependencies.as_ref(), ) { tracing::warn!("package-lock.json is outdated, {name} optionalDependencies changed"); return Ok(true); } - if !deps_match(&pkg.dev_dependencies, lock.dev_dependencies.as_ref()) { + if !deps_match( + pkg.dev_dependencies.as_ref(), + lock.dev_dependencies.as_ref(), + ) { tracing::warn!("package-lock.json is outdated, {name} devDependencies changed"); return Ok(true); } if peer_deps == PeerDeps::Include - && !deps_match(&pkg.peer_dependencies, lock.peer_dependencies.as_ref()) + && !deps_match( + pkg.peer_dependencies.as_ref(), + lock.peer_dependencies.as_ref(), + ) { tracing::warn!("package-lock.json is outdated, {name} peerDependencies changed"); return Ok(true); } } - // engines: root package only - let root_engines: EnginesView = load_package_json(root_path).await?; + // engines: root only — derived from the same loaded `root_pkg` let root_lock = packages .get("") .ok_or_else(|| anyhow!("Missing root in package-lock.json"))?; - - let pkg_engines = root_engines.engines.as_ref().filter(|m| !m.is_empty()); + let pkg_engines = root_pkg.engines.as_ref().filter(|m| !m.is_empty()); let lock_engines = root_lock.engines.as_ref().filter(|m| !m.is_empty()); let engines_match = match (pkg_engines, lock_engines) { @@ -560,25 +567,13 @@ mod tests { ); } - #[tokio::test] - async fn test_is_pkg_lock_outdated() { - // Create a temporary directory - let temp_dir = TempDir::new().unwrap(); - let temp_path = temp_dir.path(); - - // Test case 1: package.json and package-lock.json are in sync - let pkg_json = json!({ - "name": "test-package", - "version": "1.0.0", - "dependencies": { - "lodash": "^4.17.20" - }, - "devDependencies": { - "typescript": "^4.9.0" - } - }); - - let pkg_lock = json!({ + /// Baseline lockfile for the outdated-check scenarios. + /// + /// Each scenario uses a fresh `TempDir` (unique path) so the in-process + /// package.json cache never serves a stale entry — which mirrors real + /// `ut install` where every invocation starts with a cold cache. + fn baseline_pkg_lock() -> Value { + json!({ "name": "test-package", "version": "1.0.0", "lockfileVersion": 3, @@ -587,94 +582,100 @@ mod tests { "": { "name": "test-package", "version": "1.0.0", - "dependencies": { - "lodash": "^4.17.20" - }, - "devDependencies": { - "typescript": "^4.9.0" - } + "dependencies": { "lodash": "^4.17.20" }, + "devDependencies": { "typescript": "^4.9.0" } } } - }); + }) + } - // Write test files to temporary directory + async fn assert_outdated(pkg_json: Value, expected: bool) { + let temp_dir = TempDir::new().unwrap(); + let temp_path = temp_dir.path(); fs::write(temp_path.join("package.json"), pkg_json.to_string()).unwrap(); - fs::write(temp_path.join("package-lock.json"), pkg_lock.to_string()).unwrap(); - - // Test that files are in sync - assert!(!is_pkg_lock_outdated(temp_path).await.unwrap()); - - // Test case 2: package.json has new dependency - let pkg_json_updated = json!({ - "name": "test-package", - "version": "1.0.0", - "dependencies": { - "lodash": "^4.17.20", - "react": "^18.0.0" // New dependency - }, - "devDependencies": { - "typescript": "^4.9.0" - } - }); - - fs::write(temp_path.join("package.json"), pkg_json_updated.to_string()).unwrap(); - let outdated = is_pkg_lock_outdated(temp_path).await.unwrap(); - assert!(outdated); - - // Test case 3: package.json has updated version - let pkg_json_version_updated = json!({ - "name": "test-package", - "version": "1.0.0", - "dependencies": { - "lodash": "^4.17.21" // Updated version - }, - "devDependencies": { - "typescript": "^4.9.0" - } - }); - fs::write( - temp_path.join("package.json"), - pkg_json_version_updated.to_string(), + temp_path.join("package-lock.json"), + baseline_pkg_lock().to_string(), ) .unwrap(); - assert!(is_pkg_lock_outdated(temp_path).await.unwrap()); + assert_eq!( + is_pkg_lock_outdated(temp_path).await.unwrap(), + expected, + "package.json:\n{pkg_json}" + ); + } - // Test case 4: package.json has removed dependency - let pkg_json_removed = json!({ - "name": "test-package", - "version": "1.0.0", - "dependencies": { - "lodash": "^4.17.20" - } - // Removed devDependencies - }); + #[tokio::test] + async fn test_is_pkg_lock_outdated_in_sync() { + assert_outdated( + json!({ + "name": "test-package", + "version": "1.0.0", + "dependencies": { "lodash": "^4.17.20" }, + "devDependencies": { "typescript": "^4.9.0" } + }), + false, + ) + .await; + } - fs::write(temp_path.join("package.json"), pkg_json_removed.to_string()).unwrap(); - assert!(is_pkg_lock_outdated(temp_path).await.unwrap()); + #[tokio::test] + async fn test_is_pkg_lock_outdated_new_dep_added() { + assert_outdated( + json!({ + "name": "test-package", + "version": "1.0.0", + "dependencies": { + "lodash": "^4.17.20", + "react": "^18.0.0" + }, + "devDependencies": { "typescript": "^4.9.0" } + }), + true, + ) + .await; + } - // Test case 4: package.json has removed dependency - let pkg_json_engines_changed = json!({ - "name": "test-package", - "version": "1.0.0", - "dependencies": { - "lodash": "^4.17.20" - }, - "devDependencies": { - "typescript": "^4.9.0" - }, - "engines": { - "install-node": "16" - } - // Removed devDependencies - }); + #[tokio::test] + async fn test_is_pkg_lock_outdated_version_bumped() { + assert_outdated( + json!({ + "name": "test-package", + "version": "1.0.0", + "dependencies": { "lodash": "^4.17.21" }, + "devDependencies": { "typescript": "^4.9.0" } + }), + true, + ) + .await; + } - fs::write( - temp_path.join("package.json"), - pkg_json_engines_changed.to_string(), + #[tokio::test] + async fn test_is_pkg_lock_outdated_dev_deps_removed() { + assert_outdated( + json!({ + "name": "test-package", + "version": "1.0.0", + "dependencies": { "lodash": "^4.17.20" } + }), + true, ) - .unwrap(); - assert!(is_pkg_lock_outdated(temp_path).await.unwrap()); + .await; + } + + #[tokio::test] + async fn test_is_pkg_lock_outdated_engines_added() { + assert_outdated( + json!({ + "name": "test-package", + "version": "1.0.0", + "dependencies": { "lodash": "^4.17.20" }, + "devDependencies": { "typescript": "^4.9.0" }, + "engines": { "install-node": "16" } + }), + true, + ) + .await; } #[test] diff --git a/crates/pm/src/service/install.rs b/crates/pm/src/service/install.rs index 738c1026fd..e4591d3b38 100644 --- a/crates/pm/src/service/install.rs +++ b/crates/pm/src/service/install.rs @@ -8,7 +8,7 @@ use crate::cmd::deps::build_deps; use crate::fs; use crate::helper::global_bin::get_global_bin_dir; use crate::helper::lock::{ - Package, UpdatePackageJsonOptions, extract_package_name, group_by_depth, + Package, UpdatePackageJsonOptions, extract_package_name, group_by_depth, is_pkg_lock_outdated, prepare_global_package_json, update_package_json, }; use crate::helper::workspace::init_project_root; @@ -224,14 +224,16 @@ impl InstallService { omit: &HashSet, ) -> Result<()> { let lock_path = root_path.join("package-lock.json"); + // Treat a failing freshness check as stale: regenerate rather than + // install from a lockfile we couldn't validate. `is_pkg_lock_outdated` + // itself emits a `tracing::warn` with the specific mismatch reason. + let use_fresh_lock = fs::try_exists(&lock_path).await.unwrap_or(false) + && !is_pkg_lock_outdated(root_path).await.unwrap_or(true); - let (package_lock, pipeline_handles) = if fs::try_exists(&lock_path).await.unwrap_or(false) - { - // Lock exists: install directly, skip manifest resolution + let (package_lock, pipeline_handles) = if use_fresh_lock { let lock = load_package_lock_json_from_path(root_path).await?; (lock, None) } else { - // No lock: full pipeline flow (resolve + concurrent download/clone) start_progress_bar(); let result = super::pipeline::resolve_with_pipeline(root_path).await?; finish_progress_bar("package-lock.json resolved"); diff --git a/e2e/pm/stale-lockfile/package.json b/e2e/pm/stale-lockfile/package.json new file mode 100644 index 0000000000..5be1caa0bd --- /dev/null +++ b/e2e/pm/stale-lockfile/package.json @@ -0,0 +1,10 @@ +{ + "name": "e2e-stale-lockfile", + "version": "1.0.0", + "private": true, + "description": "e2e: package.json declares deps the lockfile hasn't been resolved for yet", + "dependencies": { + "abbrev": "^2.0.0", + "ini": "^5.0.0" + } +} diff --git a/e2e/utoo-pm.sh b/e2e/utoo-pm.sh index 3782c619a3..a3ff0a55b1 100755 --- a/e2e/utoo-pm.sh +++ b/e2e/utoo-pm.sh @@ -182,6 +182,49 @@ done echo -e "${GREEN}PASS: HTTP tarball warm install successful${NC}" cd ../../.. +# Case 8.4: stale lockfile is detected and regenerated on `ut install` +# +# package.json declares two deps; we seed an empty lockfile. A correct +# `ut install` must notice the mismatch, re-resolve, and install both. +# Regression guard for #2576. +echo -e "${YELLOW}Case 8.4: stale lockfile detection${NC}" +cd e2e/pm/stale-lockfile +rm -rf node_modules +cat > package-lock.json <<'JSON' +{ + "name": "e2e-stale-lockfile", + "version": "1.0.0", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "e2e-stale-lockfile", + "version": "1.0.0", + "dependencies": {} + } + } +} +JSON +utoo install --ignore-scripts || { echo -e "${RED}FAIL: utoo install failed on stale lockfile fixture${NC}"; exit 1; } +for pkg in abbrev ini; do + if [ ! -f "node_modules/$pkg/package.json" ]; then + echo -e "${RED}FAIL: $pkg not installed (stale lockfile not regenerated)${NC}"; exit 1 + fi +done +# Assert the ROOT entry's dependencies got rewritten — grep would match +# transitive node_modules/... entries even if the regen failed. +node -e ' +const lock = require("./package-lock.json"); +const root = (lock.packages || {})[""] || {}; +const deps = root.dependencies || {}; +if (!deps.abbrev || !deps.ini) { + console.error("root deps after install:", deps); + process.exit(1); +} +' || { echo -e "${RED}FAIL: package-lock.json root deps not regenerated${NC}"; exit 1; } +echo -e "${GREEN}PASS: stale lockfile detected + regenerated${NC}" +cd ../../.. + # Case 9: reinstall ant-design by npmjs.org echo -e "${YELLOW}Case 9: reinstall ant-design${NC} by npmjs.org" cd e2e/pm/ant-design/ant-design