diff options
| author | nsfisis <nsfisis@gmail.com> | 2026-05-03 13:14:00 +0900 |
|---|---|---|
| committer | nsfisis <nsfisis@gmail.com> | 2026-05-03 13:14:00 +0900 |
| commit | 3c0527aa63574f17c9f372b6187d5690e0cbaff0 (patch) | |
| tree | 8159e09b685f7faf2870970d465b9698093b8973 | |
| parent | c53dfc52f6449c8d5ca0b160a2a25f99790711f2 (diff) | |
| download | php-mozart-3c0527aa63574f17c9f372b6187d5690e0cbaff0.tar.gz php-mozart-3c0527aa63574f17c9f372b6187d5690e0cbaff0.tar.zst php-mozart-3c0527aa63574f17c9f372b6187d5690e0cbaff0.zip | |
fix(resolver): apply root "X as Y" aliases via pool second pass
Mirrors Composer's `RootPackageLoader::extractAliases` +
`PoolBuilder::loadPackage` flow: strip the `as` clause from each root
require so the SAT side sees only the LEFT-hand constraint, and after
every package is loaded run a second pass that materializes an alias
entry for any input matching `(name, version_normalized)`. Locked-only
packages in a partial update are excluded via a new
`ResolveRequest::locked_package_names` so they don't pick up the alias
(`propagateUpdate=false` in Composer).
Two adjacent fixes uncovered while making `install_aliased_alias`
green:
- `Version::cmp` treated unnamed wildcard branches (`1.0.x-dev`,
`is_dev_branch=true && name=None`) as below every numeric version.
They are semantically the same as the four-segment `*-dev` form
Composer's `normalizeBranch` emits, so let only *named* branches
take the shortcut.
- `Constraint::Exact` / `NotEqual` used the derived `==`, which
compared `is_dev_branch` field-by-field and missed the
wildcard/numeric equivalence. Switch to `cmp` so both forms count
as equal.
- `Pool::matches_package` now falls back to parsing `pretty_version`
when the `version` parse doesn't match the constraint, so a
`dev-master` query lines up with a pool entry stored as the
internal `9999999.x.x.x-dev` expansion.
Net effect on installer fixtures: `install_aliased_alias` newly
green, plus `aliased_priority`, `aliased_priority_conflicting`, and
`install_dev_using_dist` come along for the ride.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| -rw-r--r-- | crates/mozart-registry/src/lockfile.rs | 1 | ||||
| -rw-r--r-- | crates/mozart-registry/src/resolver.rs | 157 | ||||
| -rw-r--r-- | crates/mozart-sat-resolver/src/pool.rs | 26 | ||||
| -rw-r--r-- | crates/mozart-sat-resolver/src/pool_builder.rs | 8 | ||||
| -rw-r--r-- | crates/mozart-semver/src/lib.rs | 37 | ||||
| -rw-r--r-- | crates/mozart/src/commands/create_project.rs | 1 | ||||
| -rw-r--r-- | crates/mozart/src/commands/remove.rs | 6 | ||||
| -rw-r--r-- | crates/mozart/src/commands/require.rs | 5 | ||||
| -rw-r--r-- | crates/mozart/src/commands/update.rs | 37 | ||||
| -rw-r--r-- | crates/mozart/tests/installer.rs | 8 |
10 files changed, 267 insertions, 19 deletions
diff --git a/crates/mozart-registry/src/lockfile.rs b/crates/mozart-registry/src/lockfile.rs index 77a6b4c..fa6c72f 100644 --- a/crates/mozart-registry/src/lockfile.rs +++ b/crates/mozart-registry/src/lockfile.rs @@ -1406,6 +1406,7 @@ mod tests { root_provide: IndexMap::new(), root_replace: IndexMap::new(), root_conflict: IndexMap::new(), + locked_package_names: IndexSet::new(), }; let resolved = resolve(&resolve_request) diff --git a/crates/mozart-registry/src/resolver.rs b/crates/mozart-registry/src/resolver.rs index 48db7c3..40acfad 100644 --- a/crates/mozart-registry/src/resolver.rs +++ b/crates/mozart-registry/src/resolver.rs @@ -237,6 +237,77 @@ pub(crate) fn normalize_branch_alias_target(alias_target: &str) -> Option<String Some(format!("{}-dev", expanded.join("."))) } +/// Mirror Composer's `VersionParser::normalize` for the values that appear on +/// either side of an `as` clause (`require: "1.0.x-dev as dev-master"`). +/// +/// Composer sends both sides through `normalize`, which: +/// - Maps `master` / `trunk` / `default` (with optional `dev-` prefix) to +/// `9999999-dev`. Mozart's pool uses the four-segment expansion +/// `9999999.9999999.9999999.9999999-dev`, which is what +/// `make_default_branch_alias` emits — keep the same form here so a root +/// `as dev-master` lines up with synthetic default-branch aliases. +/// - Strips a leading `v` and treats numeric `*.x-dev` branches via +/// `normalizeBranch` (= `normalize_branch_alias_target`). +/// - Leaves other `dev-NAME` strings as `dev-NAME`. +fn normalize_root_alias_atom(atom: &str) -> Option<String> { + let trimmed = atom.trim(); + if trimmed.is_empty() { + return None; + } + let lower = trimmed.to_lowercase(); + let stripped = lower.strip_prefix("dev-").unwrap_or(&lower); + if matches!(stripped, "master" | "trunk" | "default") { + return Some("9999999.9999999.9999999.9999999-dev".to_string()); + } + if let Some(numeric) = normalize_branch_alias_target(trimmed) { + return Some(numeric); + } + if let Some(rest) = lower.strip_prefix("dev-") { + return Some(format!("dev-{rest}")); + } + parse_normalized(trimmed).map(|_| trimmed.to_string()) +} + +/// A root-level alias declared via the `require: "X as Y"` shorthand on the +/// root composer.json. Mirrors Composer's +/// `RootPackageLoader::extractAliases` entries: when the resolver loads a +/// package matching `(package, version_normalized)`, it materializes an extra +/// alias entry exposing the same install under `alias_normalized`/`alias`. +#[derive(Debug, Clone)] +struct RootAlias { + package: String, + /// Normalized form of the LEFT-hand side (the actual constraint). + version_normalized: String, + /// Pretty form of the RIGHT-hand side (the alias to expose). + alias: String, + /// Normalized form of the RIGHT-hand side. + alias_normalized: String, +} + +/// Strip a single-atom `<X> as <Y>` clause from a constraint string. Returns +/// the cleaned constraint plus the `(left, right)` pieces when an alias is +/// present. Mirrors Composer's `VersionParser::parseConstraint` `as`-strip: +/// the constraint passed to the resolver is the LEFT side, and a separate +/// alias entry is recorded for the RIGHT side. +fn strip_root_alias_clause(constraint: &str) -> (String, Option<(String, String)>) { + let trimmed = constraint.trim(); + if let Some(idx) = trimmed.find(" as ") { + let before = trimmed[..idx].trim(); + let after = trimmed[idx + 4..].trim(); + if !before.is_empty() + && !after.is_empty() + && !before.contains([' ', '\t', ',', '|']) + && !after.contains([' ', '\t', ',', '|']) + { + return ( + before.to_string(), + Some((before.to_string(), after.to_string())), + ); + } + } + (trimmed.to_string(), None) +} + // ───────────────────────────────────────────────────────────────────────────── // PackageName // ───────────────────────────────────────────────────────────────────────────── @@ -618,6 +689,13 @@ pub struct ResolveRequest { /// targeted version and any alias / replace / provide that would resolve /// to it. pub root_conflict: IndexMap<String, String>, + /// Lowercase names of packages that are pinned to their lock-file version + /// for this resolve (a partial update where the package is not in the + /// update list). Mirrors the `propagateUpdate=false` branch of Composer's + /// `PoolBuilder::loadPackage`: locked-only packages do not pick up + /// `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>, } /// A single package in the resolution output. @@ -653,10 +731,33 @@ pub async fn resolve(request: &ResolveRequest) -> Result<Vec<ResolvedPackage>, R // `RootPackageLoader::extractStabilityFlags`. Merged on top of the // request's caller-supplied flags (which today are usually empty). let mut stability_flags: IndexMap<String, Stability> = request.stability_flags.clone(); + // Root-level aliases extracted from `require: "X as Y"`. Mirrors + // Composer's `RootPackageLoader::extractAliases`: each entry adds a new + // alias package to the pool exposing the matched real package under the + // RIGHT-hand version label. + let mut root_aliases: Vec<RootAlias> = Vec::new(); let minimum_stability = request.minimum_stability; let mut insert_root_require = |name: &str, constraint: &str| { - let (clean, stability) = extract_stability_suffix(constraint); + // Strip any `<X> as <Y>` clause first (mirrors Composer's + // `parseConstraint` strip + `extractAliases` capture). The cleaned + // constraint feeds the resolver; the alias is recorded for a second + // pool-population pass once real packages are in. + let (constraint_no_as, alias_pieces) = strip_root_alias_clause(constraint); + if let Some((target_atom, alias_atom)) = alias_pieces + && let (Some(target_normalized), Some(alias_normalized)) = ( + normalize_root_alias_atom(&target_atom), + normalize_root_alias_atom(&alias_atom), + ) + { + root_aliases.push(RootAlias { + package: name.to_lowercase(), + version_normalized: target_normalized, + alias: alias_atom, + alias_normalized, + }); + } + let (clean, stability) = extract_stability_suffix(&constraint_no_as); let lower = name.to_lowercase(); if let Some(s) = stability { let entry = stability_flags.entry(lower.clone()).or_insert(s); @@ -934,6 +1035,59 @@ pub async fn resolve(request: &ResolveRequest) -> Result<Vec<ResolvedPackage>, R } } + // Second pass: materialize root aliases (`require: "X as Y"`). + // + // Mirrors Composer's `PoolBuilder::loadPackage` post-load step: when a + // package whose `(name, version)` matches a `rootAliases` entry is added, + // an extra `AliasPackage` exposing that install under + // `(alias_normalized, alias)` is appended to the pool. When the matched + // input is already an alias (e.g. an `extra.branch-alias` entry from + // `packagist_to_pool_inputs`), Composer follows `getAliasOf()` to the + // base package — we replicate by carrying the input's `is_alias_of` + // value forward, so the new alias points straight at the real package + // rather than chaining through the intermediate alias. + if !root_aliases.is_empty() { + let mut new_aliases: Vec<PoolPackageInput> = Vec::new(); + for input in builder.inputs() { + // Skip alias creation for packages locked to their lock-file + // version (partial update where this package wasn't requested). + // Mirrors Composer's `propagateUpdate=false` skip in + // `PoolBuilder::loadPackage`. + if request + .locked_package_names + .contains(&input.name.to_lowercase()) + { + continue; + } + for alias in &root_aliases { + if input.name.to_lowercase() != alias.package { + continue; + } + if input.version != alias.version_normalized { + continue; + } + let target_normalized = input + .is_alias_of + .clone() + .unwrap_or_else(|| input.version.clone()); + new_aliases.push(PoolPackageInput { + name: input.name.clone(), + version: alias.alias_normalized.clone(), + pretty_version: alias.alias.clone(), + requires: input.requires.clone(), + replaces: input.replaces.clone(), + provides: input.provides.clone(), + conflicts: input.conflicts.clone(), + is_fixed: false, + is_alias_of: Some(target_normalized), + }); + } + } + for alias_input in new_aliases { + builder.add_package(alias_input); + } + } + // Build the pool let mut pool = builder.build(); // Collect fixed package IDs @@ -1426,6 +1580,7 @@ mod tests { root_provide: IndexMap::new(), root_replace: IndexMap::new(), root_conflict: IndexMap::new(), + locked_package_names: IndexSet::new(), }; let result = resolve(&request).await; diff --git a/crates/mozart-sat-resolver/src/pool.rs b/crates/mozart-sat-resolver/src/pool.rs index 9268675..2c52791 100644 --- a/crates/mozart-sat-resolver/src/pool.rs +++ b/crates/mozart-sat-resolver/src/pool.rs @@ -272,11 +272,29 @@ impl Pool { return match constraint { None => true, Some(vc) => { - if let Ok(v) = mozart_semver::Version::parse(&candidate.version) { - vc.matches(&v) - } else { - false + // Try the normalized version first; fall back to the + // pretty version. Composer normalizes both sides of a + // constraint match to a single string form (e.g. + // `dev-master` → `9999999-dev`), so a query for + // `dev-master` matches a package whose pretty version + // is `dev-master` even when the pool stores its + // version field in a different normalized shape (e.g. + // the four-segment `9999999.9999999.9999999.9999999-dev` + // expansion Mozart uses internally for default-branch + // and root-alias entries). The pretty fallback bridges + // that gap without forcing the pool to commit to a + // single normalization. + if let Ok(v) = mozart_semver::Version::parse(&candidate.version) + && vc.matches(&v) + { + return true; } + if let Ok(pv) = mozart_semver::Version::parse(&candidate.pretty_version) + && vc.matches(&pv) + { + return true; + } + false } }; } diff --git a/crates/mozart-sat-resolver/src/pool_builder.rs b/crates/mozart-sat-resolver/src/pool_builder.rs index 3883d85..6088e7d 100644 --- a/crates/mozart-sat-resolver/src/pool_builder.rs +++ b/crates/mozart-sat-resolver/src/pool_builder.rs @@ -108,6 +108,14 @@ impl PoolBuilder { self.inputs.len() } + /// Read-only access to package inputs collected so far. Used by the + /// registry layer to materialize root aliases (`require: "X as Y"`) once + /// every base + branch-alias entry is in place: a second pass scans for + /// matching `(name, version)` and pushes the alias entry on top. + pub fn inputs(&self) -> &[PoolPackageInput] { + &self.inputs + } + /// Whether the builder has no packages. pub fn is_empty(&self) -> bool { self.inputs.is_empty() diff --git a/crates/mozart-semver/src/lib.rs b/crates/mozart-semver/src/lib.rs index a15db13..5f6b5fe 100644 --- a/crates/mozart-semver/src/lib.rs +++ b/crates/mozart-semver/src/lib.rs @@ -49,10 +49,17 @@ impl PartialOrd for Version { impl Ord for Version { fn cmp(&self, other: &Self) -> Ordering { - // Dev branches are always lowest - match (self.is_dev_branch, other.is_dev_branch) { + // Named dev branches (`dev-foo`) sort below every numeric version. + // A wildcard `1.0.x-dev` parses with `is_dev_branch=true` but + // `dev_branch_name=None` and is semantically identical to its + // normalized form `1.0.9999999.9999999-dev` (which parses with + // `is_dev_branch=false`). Only the *named* case takes the + // branch-comparison shortcut; unnamed wildcards fall through to + // numeric comparison so the two forms compare equal. + let self_named = self.is_dev_branch && self.dev_branch_name.is_some(); + let other_named = other.is_dev_branch && other.dev_branch_name.is_some(); + match (self_named, other_named) { (true, true) => { - // Compare branch names return self.dev_branch_name.cmp(&other.dev_branch_name); } (true, false) => return Ordering::Less, @@ -351,12 +358,19 @@ pub enum Constraint { impl Constraint { pub fn matches(&self, v: &Version) -> bool { match self { - Constraint::Exact(target) => v == target, + // Compare via `Ord` (rather than the derived `PartialEq`) so + // wildcard-branch / numeric-dev pairs that represent the same + // normalized version — e.g. `1.0.x-dev` (`is_dev_branch=true, + // name=None`) and its expanded form `1.0.9999999.9999999-dev` + // (`is_dev_branch=false`) — count as equal. The derived `==` + // would compare `is_dev_branch` field-by-field and miss the + // match. + Constraint::Exact(target) => v.cmp(target).is_eq(), Constraint::GreaterThan(target) => v > target, Constraint::GreaterThanOrEqual(target) => v >= target, Constraint::LessThan(target) => v < target, Constraint::LessThanOrEqual(target) => v <= target, - Constraint::NotEqual(target) => v != target, + Constraint::NotEqual(target) => !v.cmp(target).is_eq(), Constraint::Any => true, } } @@ -2266,11 +2280,16 @@ mod tests { #[test] fn test_x_dev_ordering_within_range() { - // "2.x-dev" version has patch=9999999, build=9999999 and is a dev branch. - // Dev branches are always lowest. So "2.x-dev" < "2.0.0" < "3.0.0". + // `2.x-dev` is the in-progress 2.x branch and normalizes to + // `2.9999999.9999999.9999999-dev`. Numerically that sorts above any + // concrete `2.N.M` release — Composer relies on this so a wildcard + // branch alias compares as the *latest* candidate within its major. + // Only *named* dev branches (`dev-foo`) sort below numeric versions. let x_dev = Version::parse("2.x-dev").unwrap(); - let stable = Version::parse("2.0.0").unwrap(); - assert!(x_dev < stable); + let stable_low = Version::parse("2.0.0").unwrap(); + let stable_next_major = Version::parse("3.0.0").unwrap(); + assert!(x_dev > stable_low); + assert!(x_dev < stable_next_major); } #[test] diff --git a/crates/mozart/src/commands/create_project.rs b/crates/mozart/src/commands/create_project.rs index c0faa76..215eb20 100644 --- a/crates/mozart/src/commands/create_project.rs +++ b/crates/mozart/src/commands/create_project.rs @@ -440,6 +440,7 @@ pub async fn execute( .iter() .map(|(k, v)| (k.clone(), v.clone())) .collect(), + locked_package_names: indexmap::IndexSet::new(), }; console.info("Resolving dependencies..."); diff --git a/crates/mozart/src/commands/remove.rs b/crates/mozart/src/commands/remove.rs index 08f7cc6..15b3586 100644 --- a/crates/mozart/src/commands/remove.rs +++ b/crates/mozart/src/commands/remove.rs @@ -274,6 +274,7 @@ pub async fn execute( .iter() .map(|(k, v)| (k.clone(), v.clone())) .collect(), + locked_package_names: indexmap::IndexSet::new(), }; // Print header messages @@ -550,6 +551,7 @@ async fn remove_unused( .iter() .map(|(k, v)| (k.clone(), v.clone())) .collect(), + locked_package_names: indexmap::IndexSet::new(), }; console.info("Resolving dependencies to detect unused packages..."); @@ -859,7 +861,7 @@ mod tests { #[tokio::test] #[ignore] async fn test_remove_full_e2e() { - use indexmap::IndexMap; + use indexmap::{IndexMap, IndexSet}; use mozart_registry::lockfile::{LockFileGenerationRequest, generate_lock_file}; use mozart_registry::resolver::{ResolveRequest, resolve}; use tempfile::tempdir; @@ -902,6 +904,7 @@ mod tests { root_provide: IndexMap::new(), root_replace: IndexMap::new(), root_conflict: IndexMap::new(), + locked_package_names: IndexSet::new(), }; let resolved = resolve(&request) .await @@ -957,6 +960,7 @@ mod tests { root_provide: IndexMap::new(), root_replace: IndexMap::new(), root_conflict: IndexMap::new(), + locked_package_names: IndexSet::new(), }; let resolved2 = resolve(&request2) .await diff --git a/crates/mozart/src/commands/require.rs b/crates/mozart/src/commands/require.rs index 97d6b02..6a2917b 100644 --- a/crates/mozart/src/commands/require.rs +++ b/crates/mozart/src/commands/require.rs @@ -662,6 +662,7 @@ pub async fn execute( .iter() .map(|(k, v)| (k.clone(), v.clone())) .collect(), + locked_package_names: indexmap::IndexSet::new(), }; // Print header messages @@ -1029,6 +1030,7 @@ mod tests { #[tokio::test] #[ignore] async fn test_require_full_e2e() { + use indexmap::IndexSet; use mozart_core::package::RawPackageData; use mozart_registry::lockfile::{LockFileGenerationRequest, generate_lock_file}; @@ -1061,6 +1063,7 @@ mod tests { root_provide: IndexMap::new(), root_replace: IndexMap::new(), root_conflict: IndexMap::new(), + locked_package_names: IndexSet::new(), }; let resolved = resolver::resolve(&request) @@ -1094,6 +1097,7 @@ mod tests { #[tokio::test] #[ignore] async fn test_require_no_install_writes_lock_only() { + use indexmap::IndexSet; use mozart_core::package::RawPackageData; use tempfile::tempdir; @@ -1133,6 +1137,7 @@ mod tests { root_provide: IndexMap::new(), root_replace: IndexMap::new(), root_conflict: IndexMap::new(), + locked_package_names: IndexSet::new(), }; let resolved = resolver::resolve(&request) diff --git a/crates/mozart/src/commands/update.rs b/crates/mozart/src/commands/update.rs index 130d7e3..6d314dc 100644 --- a/crates/mozart/src/commands/update.rs +++ b/crates/mozart/src/commands/update.rs @@ -843,6 +843,41 @@ pub async fn run( .filter(|p| !matches!(p.to_lowercase().as_str(), "lock" | "nothing" | "mirrors")) .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. + // + // 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 + // 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()), + ) + .filter(|n| !updated.contains(n)) + .collect() + } + Err(_) => IndexSet::new(), + } + } else { + IndexSet::new() + }; + // Step 5: Build the resolve request from composer.json // Filter out platform packages from require list for the resolver (they're handled separately) let require: Vec<(String, String)> = composer_json @@ -912,6 +947,7 @@ pub async fn run( .iter() .map(|(k, v)| (k.clone(), v.clone())) .collect(), + locked_package_names, }; // Step 6: Print header and run resolver @@ -2023,6 +2059,7 @@ mod tests { root_provide: IndexMap::new(), root_replace: IndexMap::new(), root_conflict: IndexMap::new(), + locked_package_names: IndexSet::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 6ab083d..b4eb512 100644 --- a/crates/mozart/tests/installer.rs +++ b/crates/mozart/tests/installer.rs @@ -223,8 +223,8 @@ installer_fixture!(alias_on_unloadable_package); installer_fixture!(alias_solver_problems); installer_fixture!(alias_solver_problems2); installer_fixture!(alias_with_reference, ignore); -installer_fixture!(aliased_priority, ignore); -installer_fixture!(aliased_priority_conflicting, ignore); +installer_fixture!(aliased_priority); +installer_fixture!(aliased_priority_conflicting); installer_fixture!(aliases_with_require_dev, ignore); installer_fixture!(broken_deps_do_not_replace, ignore); installer_fixture!(circular_dependency, ignore); @@ -255,10 +255,10 @@ installer_fixture!(github_issues_8903, ignore); installer_fixture!(github_issues_9012, ignore); installer_fixture!(github_issues_9290, ignore); installer_fixture!(hint_main_rename); -installer_fixture!(install_aliased_alias, ignore); +installer_fixture!(install_aliased_alias); installer_fixture!(install_branch_alias_composer_repo); installer_fixture!(install_dev); -installer_fixture!(install_dev_using_dist, ignore); +installer_fixture!(install_dev_using_dist); installer_fixture!(install_forces_reinstall_if_abandon_changes, ignore); installer_fixture!(install_from_incomplete_lock); installer_fixture!(install_from_incomplete_lock_with_ignore, ignore); |
