diff options
| -rw-r--r-- | crates/mozart-registry/src/lockfile.rs | 1 | ||||
| -rw-r--r-- | crates/mozart-registry/src/resolver.rs | 100 | ||||
| -rw-r--r-- | crates/mozart/src/commands/create_project.rs | 1 | ||||
| -rw-r--r-- | crates/mozart/src/commands/remove.rs | 8 | ||||
| -rw-r--r-- | crates/mozart/src/commands/require.rs | 7 | ||||
| -rw-r--r-- | crates/mozart/src/commands/update.rs | 186 | ||||
| -rw-r--r-- | crates/mozart/tests/installer.rs | 15 |
7 files changed, 266 insertions, 52 deletions
diff --git a/crates/mozart-registry/src/lockfile.rs b/crates/mozart-registry/src/lockfile.rs index fa6c72f..da2384d 100644 --- a/crates/mozart-registry/src/lockfile.rs +++ b/crates/mozart-registry/src/lockfile.rs @@ -1407,6 +1407,7 @@ mod tests { root_replace: IndexMap::new(), root_conflict: IndexMap::new(), locked_package_names: IndexSet::new(), + locked_packages: Vec::new(), }; let resolved = resolve(&resolve_request) diff --git a/crates/mozart-registry/src/resolver.rs b/crates/mozart-registry/src/resolver.rs index 40acfad..6bce0f1 100644 --- a/crates/mozart-registry/src/resolver.rs +++ b/crates/mozart-registry/src/resolver.rs @@ -696,6 +696,31 @@ pub struct ResolveRequest { /// `require: "X as Y"` root aliases. Empty for installs and full updates, /// where every package can take aliases as usual. pub locked_package_names: IndexSet<String>, + /// Full data of packages pinned to their lock-file version (a partial + /// update). Each entry is added to the pool as a fixed entry, mirroring + /// Composer's `Request::lockPackage` + `PoolBuilder::buildPool`'s + /// `getFixedOrLockedPackages` loop: a locked-only package's pretty/normalized + /// version, requires, replaces, provides and conflicts all enter the pool + /// at exactly one version, so the SAT solver cannot pick a different + /// version (whether directly or via another package's `replace`). Empty + /// for installs and full updates. + pub locked_packages: Vec<LockedPackageInfo>, +} + +/// Full data for a lock-pinned package, used in partial updates. Carried on +/// `ResolveRequest::locked_packages` and turned into a fixed pool entry +/// inside `resolve()`. Mirrors what Composer's `PoolBuilder` reads off a +/// `BasePackage` retrieved from the locked repository. +pub struct LockedPackageInfo { + pub name: String, + /// Pretty (display) version, e.g. "1.2.3". + pub pretty_version: String, + /// Normalized version, e.g. "1.2.3.0". + pub version_normalized: String, + pub requires: Vec<(String, String)>, + pub replaces: Vec<(String, String)>, + pub provides: Vec<(String, String)>, + pub conflicts: Vec<(String, String)>, } /// A single package in the resolution output. @@ -899,6 +924,65 @@ pub async fn resolve(request: &ResolveRequest) -> Result<Vec<ResolvedPackage>, R builder.add_package(root_input); } + // Add lock-pinned packages as pool entries (partial-update case). + // + // Mirrors Composer's `PoolBuilder::buildPool` flow: every locked package + // not in the `updateAllowList` is added through `Request::lockPackage`, + // then re-entered into the pool via the `getFixedOrLockedPackages` + // loop. Crucially, a *locked* package is NOT a *fixed* package + // (Request.php:89-98): the SAT solver does not force its installation, + // so a locked package whose root require has been removed will simply + // drop out of the result. The locked entry's purpose is to constrain + // the pool to *only* the locked version for that name — every other + // version is filtered out below — so other packages cannot pick a + // different version (whether directly, or via `replace`, which would + // otherwise let an upgraded replacer silently drop the dependency). + // + // Build a map first so the filter below knows which (name, version) + // pairs are the only allowed entries for locked names. + let locked_name_to_version: IndexMap<String, String> = request + .locked_packages + .iter() + .map(|p| (p.name.to_lowercase(), p.version_normalized.clone())) + .collect(); + let lock_filter_allows = |name: &str, version: &str| -> bool { + match locked_name_to_version.get(&name.to_lowercase()) { + Some(locked_version) => locked_version == version, + None => true, + } + }; + for locked in &request.locked_packages { + let locked_name_lower = locked.name.to_lowercase(); + let input = PoolPackageInput { + name: locked_name_lower.clone(), + version: locked.version_normalized.clone(), + pretty_version: locked.pretty_version.clone(), + requires: make_pool_links( + &locked_name_lower, + &locked.version_normalized, + &locked.requires, + ), + replaces: make_pool_links( + &locked_name_lower, + &locked.version_normalized, + &locked.replaces, + ), + provides: make_pool_links( + &locked_name_lower, + &locked.version_normalized, + &locked.provides, + ), + conflicts: make_pool_links( + &locked_name_lower, + &locked.version_normalized, + &locked.conflicts, + ), + is_fixed: false, + is_alias_of: None, + }; + builder.add_package(input); + } + // Scan VCS repositories and collect packages from them let vcs_packages = vcs_bridge::scan_vcs_repositories(&request.raw_repositories).await; let mut vcs_package_names: IndexSet<String> = IndexSet::new(); @@ -911,6 +995,9 @@ pub async fn resolve(request: &ResolveRequest) -> Result<Vec<ResolvedPackage>, R let inputs = vcs_bridge::vcs_to_pool_inputs(vpkg, request.minimum_stability, &stability_flags); for input in inputs { + if !lock_filter_allows(&input.name, &input.version) { + continue; + } builder.add_package(input); } } @@ -929,6 +1016,9 @@ pub async fn resolve(request: &ResolveRequest) -> Result<Vec<ResolvedPackage>, R &stability_flags, ); for input in inputs { + if !lock_filter_allows(&input.name, &input.version) { + continue; + } builder.add_package(input); } } @@ -950,6 +1040,9 @@ pub async fn resolve(request: &ResolveRequest) -> Result<Vec<ResolvedPackage>, R &stability_flags, ); for input in inputs { + if !lock_filter_allows(&input.name, &input.version) { + continue; + } builder.add_package(input); } } @@ -991,6 +1084,9 @@ pub async fn resolve(request: &ResolveRequest) -> Result<Vec<ResolvedPackage>, R &stability_flags, ); for input in inputs { + if !lock_filter_allows(&input.name, &input.version) { + continue; + } builder.add_package(input); } } @@ -1030,6 +1126,9 @@ pub async fn resolve(request: &ResolveRequest) -> Result<Vec<ResolvedPackage>, R &request.stability_flags, ); for input in inputs { + if !lock_filter_allows(&input.name, &input.version) { + continue; + } builder.add_package(input); } } @@ -1581,6 +1680,7 @@ mod tests { root_replace: IndexMap::new(), root_conflict: IndexMap::new(), locked_package_names: IndexSet::new(), + locked_packages: Vec::new(), }; let result = resolve(&request).await; diff --git a/crates/mozart/src/commands/create_project.rs b/crates/mozart/src/commands/create_project.rs index 215eb20..ae7a550 100644 --- a/crates/mozart/src/commands/create_project.rs +++ b/crates/mozart/src/commands/create_project.rs @@ -441,6 +441,7 @@ pub async fn execute( .map(|(k, v)| (k.clone(), v.clone())) .collect(), locked_package_names: indexmap::IndexSet::new(), + locked_packages: Vec::new(), }; console.info("Resolving dependencies..."); diff --git a/crates/mozart/src/commands/remove.rs b/crates/mozart/src/commands/remove.rs index 15b3586..9c5f7fa 100644 --- a/crates/mozart/src/commands/remove.rs +++ b/crates/mozart/src/commands/remove.rs @@ -1,5 +1,5 @@ use clap::Args; -use indexmap::IndexMap; +use indexmap::{IndexMap, IndexSet}; use mozart_core::console::Verbosity; use mozart_core::console_format; use mozart_core::package; @@ -275,6 +275,7 @@ pub async fn execute( .map(|(k, v)| (k.clone(), v.clone())) .collect(), locked_package_names: indexmap::IndexSet::new(), + locked_packages: Vec::new(), }; // Print header messages @@ -340,7 +341,7 @@ pub async fn execute( super::update::expand_with_all_dependencies(removed_names, lock) } else { // Default: freed packages + their direct dependencies - super::update::expand_with_direct_dependencies(removed_names, lock) + super::update::expand_with_direct_dependencies(removed_names, lock, &IndexSet::new()) }; // For --minimal-changes, additionally pin packages beyond the allow list @@ -552,6 +553,7 @@ async fn remove_unused( .map(|(k, v)| (k.clone(), v.clone())) .collect(), locked_package_names: indexmap::IndexSet::new(), + locked_packages: Vec::new(), }; console.info("Resolving dependencies to detect unused packages..."); @@ -905,6 +907,7 @@ mod tests { root_replace: IndexMap::new(), root_conflict: IndexMap::new(), locked_package_names: IndexSet::new(), + locked_packages: Vec::new(), }; let resolved = resolve(&request) .await @@ -961,6 +964,7 @@ mod tests { root_replace: IndexMap::new(), root_conflict: IndexMap::new(), locked_package_names: IndexSet::new(), + locked_packages: Vec::new(), }; let resolved2 = resolve(&request2) .await diff --git a/crates/mozart/src/commands/require.rs b/crates/mozart/src/commands/require.rs index 6a2917b..caf88c1 100644 --- a/crates/mozart/src/commands/require.rs +++ b/crates/mozart/src/commands/require.rs @@ -1,5 +1,5 @@ use clap::Args; -use indexmap::IndexMap; +use indexmap::{IndexMap, IndexSet}; use mozart_core::console::Verbosity; use mozart_core::console_format; use mozart_core::package::{self, Stability}; @@ -663,6 +663,7 @@ pub async fn execute( .map(|(k, v)| (k.clone(), v.clone())) .collect(), locked_package_names: indexmap::IndexSet::new(), + locked_packages: Vec::new(), }; // Print header messages @@ -731,7 +732,7 @@ pub async fn execute( let allow_list = if with_all_deps { super::update::expand_with_all_dependencies(newly_required, lock) } else if with_deps { - super::update::expand_with_direct_dependencies(newly_required, lock) + super::update::expand_with_direct_dependencies(newly_required, lock, &IndexSet::new()) } else { // Default for `require`: only the newly added packages are allowed to change. additions.iter().map(|(name, _, _)| name.clone()).collect() @@ -1064,6 +1065,7 @@ mod tests { root_replace: IndexMap::new(), root_conflict: IndexMap::new(), locked_package_names: IndexSet::new(), + locked_packages: Vec::new(), }; let resolved = resolver::resolve(&request) @@ -1138,6 +1140,7 @@ mod tests { root_replace: IndexMap::new(), root_conflict: IndexMap::new(), locked_package_names: IndexSet::new(), + locked_packages: Vec::new(), }; let resolved = resolver::resolve(&request) diff --git a/crates/mozart/src/commands/update.rs b/crates/mozart/src/commands/update.rs index 6d314dc..6003dd0 100644 --- a/crates/mozart/src/commands/update.rs +++ b/crates/mozart/src/commands/update.rs @@ -4,7 +4,9 @@ use mozart_core::console; use mozart_core::console_format; use mozart_core::package::{self, Stability}; use mozart_registry::lockfile; -use mozart_registry::resolver::{self, PlatformConfig, ResolveRequest, ResolvedPackage}; +use mozart_registry::resolver::{ + self, LockedPackageInfo, PlatformConfig, ResolveRequest, ResolvedPackage, +}; #[derive(Args)] pub struct UpdateArgs { @@ -461,17 +463,26 @@ fn build_lock_map(lock: &lockfile::LockFile) -> IndexMap<String, &lockfile::Lock map } -/// Given a set of package names, add their direct `require` dependencies from -/// the lock file to the set. Returns the augmented set. +/// Expand the allow-list with transitive `require` dependencies, stopping at +/// any dependency that is also a root requirement. +/// +/// Mirrors Composer's `--with-dependencies` (UPDATE_LISTED_WITH_TRANSITIVE_DEPS_NO_ROOT_REQUIRE) +/// behaviour: in `PoolBuilder::loadPackage`, when a propagated package's +/// require points at a still-skipped (locked) dependency, the dependency is +/// unlocked and re-loaded — but only when it is NOT itself a root require. +/// A root require hit acts as a barrier: the locked version stays, a +/// warning is issued, and the cascade through that node stops. pub fn expand_with_direct_dependencies( packages: Vec<String>, lock: &lockfile::LockFile, + root_requires: &IndexSet<String>, ) -> Vec<String> { let lock_map = build_lock_map(lock); let mut result_set: IndexSet<String> = packages.iter().cloned().collect(); + let mut queue: Vec<String> = packages.clone(); let mut result: Vec<String> = packages; - for name in result.clone() { + while let Some(name) = queue.pop() { if let Some(pkg) = lock_map.get(&name) { for dep_name in pkg.require.keys() { // Skip platform packages (php, ext-*, lib-*) @@ -486,8 +497,13 @@ pub fn expand_with_direct_dependencies( continue; } let lower = dep_name.to_lowercase(); + // Root-require barrier: don't unlock and don't recurse. + if root_requires.contains(&lower) { + continue; + } if result_set.insert(lower.clone()) { - result.push(lower); + result.push(lower.clone()); + queue.push(lower); } } } @@ -541,6 +557,7 @@ pub fn expand_packages( lock: Option<&lockfile::LockFile>, with_dependencies: bool, with_all_dependencies: bool, + root_requires: &IndexSet<String>, console: &mozart_core::console::Console, ) -> Vec<String> { let mut packages: Vec<String> = if let Some(lock) = lock { @@ -555,7 +572,7 @@ pub fn expand_packages( if with_all_dependencies { packages = expand_with_all_dependencies(packages, lock); } else if with_dependencies { - packages = expand_with_direct_dependencies(packages, lock); + packages = expand_with_direct_dependencies(packages, lock, root_requires); } } @@ -805,6 +822,30 @@ pub async fn run( let dev_mode = !args.no_dev; + // Build the set of root require names (lowercase, excluding platform + // packages). Used as the barrier for `--with-dependencies` transitive + // expansion: Composer's UPDATE_LISTED_WITH_TRANSITIVE_DEPS_NO_ROOT_REQUIRE + // mode (Request.php) leaves root requires locked even when they are + // depended on by an allow-listed package, emitting a warning instead. + let root_requires: IndexSet<String> = { + let mut s: IndexSet<String> = composer_json + .require + .keys() + .filter(|k| !is_platform_package(k)) + .map(|k| k.to_lowercase()) + .collect(); + if dev_mode { + s.extend( + composer_json + .require_dev + .keys() + .filter(|k| !is_platform_package(k)) + .map(|k| k.to_lowercase()), + ); + } + s + }; + // Fix 1C + Fix 2: Parse --with constraints and inline constraint shorthand. let mut temporary_constraints: IndexMap<String, String> = IndexMap::new(); @@ -844,39 +885,91 @@ pub async fn run( .collect(); // For partial updates (specific package names given), eagerly read the - // lock file to collect names that stay pinned across this resolve. - // The resolver uses this set to skip materializing root `as` aliases - // for those packages — Composer's `PoolBuilder::loadPackage` only - // applies a root alias when the package's update is being propagated, - // so a locked-only package keeps its locked version unaliased. + // lock file to gather both the names that stay pinned across this + // resolve *and* the full package data needed to seed fixed pool + // entries for those names. The resolver uses the names to skip + // materializing root `as` aliases (Composer's `propagateUpdate=false` + // branch in `PoolBuilder::loadPackage`) and the full data to add a + // fixed entry per locked package — without the fixed entry the SAT + // solver may pick a different version of a locked package, including + // one that `replace`s an allow-listed dependency, silently dropping + // it from the install plan. // - // Only the *names* are needed — the full lock is re-read below for - // change reporting and `apply_partial_update` post-processing. Reading - // it twice is fine: it's a small JSON file. Errors here fall back to - // an empty set (treat as full update); the later read surfaces the + // The full lock is re-read below for change reporting and + // `apply_partial_update` post-processing. Reading it twice is fine: + // it's a small JSON file. Errors here fall back to empty + // collections (treat as full update); the later read surfaces the // failure to the user. - let locked_package_names: IndexSet<String> = if !raw_packages.is_empty() && lock_path.exists() { - match lockfile::LockFile::read_from_file(&lock_path) { - Ok(l) => { - let updated: IndexSet<String> = - raw_packages.iter().map(|s| s.to_lowercase()).collect(); - l.packages - .iter() - .map(|p| p.name.to_lowercase()) - .chain( - l.packages_dev - .iter() - .flatten() - .map(|p| p.name.to_lowercase()), + let (locked_package_names, locked_packages): (IndexSet<String>, Vec<LockedPackageInfo>) = + if !raw_packages.is_empty() && lock_path.exists() { + match lockfile::LockFile::read_from_file(&lock_path) { + Ok(l) => { + // Apply `--with-dependencies` / `--with-all-dependencies` + // expansion so transitive deps of allow-listed packages + // are not held back at their locked version. Mirrors + // Composer's `PoolBuilder::loadPackage` unlock cascade + // (line 524: when a propagated package's `require` + // points at a `skippedLoad` entry, the dep is unlocked + // and re-loaded). + let updated: IndexSet<String> = expand_packages( + &raw_packages, + Some(&l), + args.with_dependencies, + args.with_all_dependencies, + &root_requires, + console, ) - .filter(|n| !updated.contains(n)) - .collect() + .into_iter() + .collect(); + let mut names: IndexSet<String> = IndexSet::new(); + let mut infos: Vec<LockedPackageInfo> = Vec::new(); + for p in l.packages.iter().chain(l.packages_dev.iter().flatten()) { + let name_lower = p.name.to_lowercase(); + if updated.contains(&name_lower) { + continue; + } + // Path-repo packages are always reloaded from disk + // by Composer (PoolBuilder treats path repos as + // canonical sources, not lock-bound). Skip them so + // the pool sees the live on-disk version, matching + // Composer's "path repos always update" behaviour. + if p.dist.as_ref().map(|d| d.dist_type.as_str()) == Some("path") { + continue; + } + names.insert(name_lower.clone()); + infos.push(LockedPackageInfo { + name: name_lower, + pretty_version: p.version.clone(), + version_normalized: locked_version_normalized(p), + requires: p + .require + .iter() + .map(|(k, v)| (k.to_lowercase(), v.clone())) + .collect(), + replaces: p + .replace + .iter() + .map(|(k, v)| (k.to_lowercase(), v.clone())) + .collect(), + provides: p + .provide + .iter() + .map(|(k, v)| (k.to_lowercase(), v.clone())) + .collect(), + conflicts: p + .conflict + .iter() + .map(|(k, v)| (k.to_lowercase(), v.clone())) + .collect(), + }); + } + (names, infos) + } + Err(_) => (IndexSet::new(), Vec::new()), } - Err(_) => IndexSet::new(), - } - } else { - IndexSet::new() - }; + } else { + (IndexSet::new(), Vec::new()) + }; // Step 5: Build the resolve request from composer.json // Filter out platform packages from require list for the resolver (they're handled separately) @@ -948,6 +1041,7 @@ pub async fn run( .map(|(k, v)| (k.clone(), v.clone())) .collect(), locked_package_names, + locked_packages, }; // Step 6: Print header and run resolver @@ -1025,6 +1119,7 @@ pub async fn run( Some(lock), args.with_dependencies, args.with_all_dependencies, + &root_requires, console, ); @@ -1891,7 +1986,11 @@ mod tests { let lock = minimal_lock(vec![pkg, make_locked_package("psr/log", "3.0.0")]); - let result = expand_with_direct_dependencies(vec!["monolog/monolog".to_string()], &lock); + let result = expand_with_direct_dependencies( + vec!["monolog/monolog".to_string()], + &lock, + &IndexSet::new(), + ); let mut result_sorted = result.clone(); result_sorted.sort(); assert!(result_sorted.contains(&"monolog/monolog".to_string())); @@ -1908,7 +2007,11 @@ mod tests { let lock = minimal_lock(vec![pkg, make_locked_package("psr/log", "3.0.0")]); - let result = expand_with_direct_dependencies(vec!["monolog/monolog".to_string()], &lock); + let result = expand_with_direct_dependencies( + vec!["monolog/monolog".to_string()], + &lock, + &IndexSet::new(), + ); // Should NOT include php or ext-json assert!(!result.contains(&"php".to_string())); assert!(!result.contains(&"ext-json".to_string())); @@ -1929,8 +2032,11 @@ mod tests { let lock = minimal_lock(vec![pkg_a, pkg_b, make_locked_package("psr/log", "3.0.0")]); - let result = - expand_with_direct_dependencies(vec!["foo/a".to_string(), "foo/b".to_string()], &lock); + let result = expand_with_direct_dependencies( + vec!["foo/a".to_string(), "foo/b".to_string()], + &lock, + &IndexSet::new(), + ); let psr_count = result.iter().filter(|s| s.as_str() == "psr/log").count(); assert_eq!(psr_count, 1, "psr/log should appear only once"); } @@ -1996,6 +2102,7 @@ mod tests { Some(&lock), true, // with_dependencies false, // with_all_dependencies + &IndexSet::new(), &test_console(), ); @@ -2060,6 +2167,7 @@ mod tests { root_replace: IndexMap::new(), root_conflict: IndexMap::new(), locked_package_names: IndexSet::new(), + locked_packages: Vec::new(), }; let resolved = resolve(&request).await.expect("Resolution should succeed"); diff --git a/crates/mozart/tests/installer.rs b/crates/mozart/tests/installer.rs index 75ad3bf..4c877cc 100644 --- a/crates/mozart/tests/installer.rs +++ b/crates/mozart/tests/installer.rs @@ -247,7 +247,7 @@ installer_fixture!(deduplicate_solver_problems); installer_fixture!(disjunctive_multi_constraints); installer_fixture!(full_update_minimal_changes, ignore); installer_fixture!(github_issues_4319); -installer_fixture!(github_issues_4795, ignore); +installer_fixture!(github_issues_4795); installer_fixture!(github_issues_4795_2); installer_fixture!(github_issues_7051, ignore); installer_fixture!(github_issues_8902); @@ -298,9 +298,9 @@ installer_fixture!( partial_update_security_advisory_matching_locked_dep_with_dependencies, ignore ); -installer_fixture!(partial_update_with_dependencies_provide, ignore); +installer_fixture!(partial_update_with_dependencies_provide); installer_fixture!(partial_update_with_dependencies_replace, ignore); -installer_fixture!(partial_update_with_deps_warns_root, ignore); +installer_fixture!(partial_update_with_deps_warns_root); installer_fixture!(partial_update_with_symlinked_path_repos); installer_fixture!(partial_update_without_lock); installer_fixture!(platform_ext_solver_problems); @@ -326,10 +326,7 @@ installer_fixture!( ); installer_fixture!(provider_satisfies_its_own_requirement); installer_fixture!(remove_deletes_unused_deps); -installer_fixture!( - remove_does_nothing_if_removal_requires_update_of_dep, - ignore -); +installer_fixture!(remove_does_nothing_if_removal_requires_update_of_dep); installer_fixture!(replace_alias); installer_fixture!(replace_priorities); installer_fixture!(replace_range_require_single_version); @@ -377,7 +374,7 @@ installer_fixture!(update_allow_list_patterns_with_dependencies); installer_fixture!(update_allow_list_patterns_with_root_dependencies); installer_fixture!(update_allow_list_patterns_without_dependencies); installer_fixture!(update_allow_list_reads_lock); -installer_fixture!(update_allow_list_removes_unused, ignore); +installer_fixture!(update_allow_list_removes_unused); installer_fixture!(update_allow_list_require_new_replace); installer_fixture!(update_allow_list_warns_non_existing_patterns); installer_fixture!(update_allow_list_with_dependencies); @@ -389,7 +386,7 @@ installer_fixture!( update_allow_list_with_dependencies_require_new_replace_mutual, ignore ); -installer_fixture!(update_allow_list_with_dependency_conflict, ignore); +installer_fixture!(update_allow_list_with_dependency_conflict); installer_fixture!(update_changes_url, ignore); installer_fixture!(update_dev_ignores_providers); installer_fixture!(update_dev_packages_updates_repo_url, ignore); |