From 16e856a20307a3ca20524d96ea13348db7f2cffd Mon Sep 17 00:00:00 2001 From: nsfisis Date: Sat, 2 May 2026 17:40:07 +0900 Subject: feat(installer): add trace recorder and topo install order Adds TraceRecorderExecutor (Composer's InstallationManagerMock analog), which records every install/update/uninstall as a string matching Composer's *Operation::__toString output (after strip_tags) - the load-bearing assertion target for in-process fixture tests. Two changes were needed to make the recorder useful: - InstallerExecutor::uninstall_package gains a version parameter, and install_from_lock now looks up both the uninstall and the Update-from-version from installed.json. Previously the Update path passed the new version as a placeholder; the recorder needs the real old version to emit `Upgrading pkg (old => new)`. - compute_operations now topologically sorts the lock contents (deps before dependents) before computing actions, mirroring Composer's Transaction::calculateOperations. Without this, packages would install in alphabetical order and the trace would diverge from Composer's expectation. Also adds crates/mozart/tests/installer_in_process.rs with the in-process harness scaffold: parses the same .test fixtures, builds a tempdir, calls commands::install::run / update::run with an empty RepositorySet (no Packagist) and a TraceRecorderExecutor, then asserts exit code + EXPECT trace. One fixture wired up: suggest_replaced - the original CI failure that motivated this whole DI refactor. It now passes on the in-process path because the empty RepositorySet makes b/b unreachable just like Composer's `'packagist' => false` test config, and the resolver finds c/c (which replaces b/b) via the inline package repo's eager preload. Step F will migrate every fixture currently in installer.rs to the new harness; remaining divergences (alias handling, output ordering, replace trace shape, etc.) will surface as individual follow-ups. All 136 existing spawn-based fixtures + 114 mozart-registry tests + 541 mozart lib tests still green; clippy clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/mozart/src/commands/install.rs | 104 ++++++++++++++++++++++++++++++---- 1 file changed, 94 insertions(+), 10 deletions(-) (limited to 'crates/mozart/src') diff --git a/crates/mozart/src/commands/install.rs b/crates/mozart/src/commands/install.rs index c8b0431..b89793b 100644 --- a/crates/mozart/src/commands/install.rs +++ b/crates/mozart/src/commands/install.rs @@ -167,15 +167,23 @@ pub fn resolve_working_dir(cli: &super::Cli) -> PathBuf { /// Compute install operations by comparing locked packages against installed packages. /// /// Returns a tuple of (ops, removals) where: -/// - ops: list of (package, action) for each locked package +/// - ops: list of (package, action) ordered topologically — every package's +/// lock-internal `require` deps appear before it, so installs run in +/// dependency-first order to match Composer's `Transaction::calculateOperations`. /// - removals: list of package names that are installed but not locked pub fn compute_operations<'a>( locked: &[&'a lockfile::LockedPackage], installed: &installed::InstalledPackages, ) -> (Vec<(&'a lockfile::LockedPackage, Action)>, Vec) { - let mut ops: Vec<(&'a lockfile::LockedPackage, Action)> = Vec::new(); + // Topo-sort `locked` so each package's deps (within the lock set) come + // before it. Composer's solver yields operations in this order via the + // Transaction; Mozart writes the lock alphabetically, so the install + // loop must re-order before emitting trace lines or invoking the + // executor. + let ordered = topological_sort(locked); - for pkg in locked { + let mut ops: Vec<(&'a lockfile::LockedPackage, Action)> = Vec::new(); + for pkg in ordered { if installed.is_installed(&pkg.name, &pkg.version) { ops.push((pkg, Action::Skip)); } else if installed @@ -202,6 +210,72 @@ pub fn compute_operations<'a>( (ops, removals) } +/// Order a slice of locked packages so every package's `require` deps that +/// are present in the same slice come before it. Cycles fall back to the +/// input order (Composer rejects cycles earlier in the resolver, so Mozart +/// shouldn't see them here in practice). Mirrors the topological sort +/// inside `Composer\DependencyResolver\Transaction::calculateOperations`. +fn topological_sort<'a>( + packages: &[&'a lockfile::LockedPackage], +) -> Vec<&'a lockfile::LockedPackage> { + use std::collections::BTreeMap; + + let names: HashSet = packages.iter().map(|p| p.name.to_lowercase()).collect(); + let mut by_name: BTreeMap = BTreeMap::new(); + for pkg in packages { + by_name.insert(pkg.name.to_lowercase(), *pkg); + } + + let mut visited: HashSet = HashSet::new(); + let mut on_stack: HashSet = HashSet::new(); + let mut ordered: Vec<&'a lockfile::LockedPackage> = Vec::with_capacity(packages.len()); + + fn visit<'b>( + name: &str, + names: &HashSet, + by_name: &BTreeMap, + visited: &mut HashSet, + on_stack: &mut HashSet, + ordered: &mut Vec<&'b lockfile::LockedPackage>, + ) { + if visited.contains(name) || on_stack.contains(name) { + return; + } + let Some(pkg) = by_name.get(name) else { + return; + }; + on_stack.insert(name.to_string()); + for dep in pkg.require.keys() { + let dep_lower = dep.to_lowercase(); + if names.contains(&dep_lower) { + visit(&dep_lower, names, by_name, visited, on_stack, ordered); + } + } + on_stack.remove(name); + visited.insert(name.to_string()); + ordered.push(*pkg); + } + + // Seed iteration in the input order so two packages with no relation + // come out in the order Mozart's lock writer produced them + // (alphabetical), matching Composer's deterministic output. + for pkg in packages { + let lower = pkg.name.to_lowercase(); + if !visited.contains(&lower) { + visit( + &lower, + &names, + &by_name, + &mut visited, + &mut on_stack, + &mut ordered, + ); + } + } + + ordered +} + /// Convert a LockedPackage to an InstalledPackageEntry. /// /// `LockedPackage::extra_fields` is forwarded verbatim so flags like @@ -524,13 +598,17 @@ pub async fn install_from_lock( pkg.name, pkg.version )); - // The previous-version string is unknown to install_from_lock - // (it only sees the post-update lock). Pass the new version - // as a placeholder; this path is unused by the recorder, and - // Composer's `Upgrading` trace string is generated upstream - // by the resolver, not by InstallationManager itself. + // Pull the previously-installed version from installed.json + // so the trace recorder can format + // `Upgrading pkg (oldVersion => newVersion)`. + let from_version = installed + .packages + .iter() + .find(|p| p.name.eq_ignore_ascii_case(&pkg.name)) + .map(|p| p.version.as_str()) + .unwrap_or(""); PackageOperation::Update { - from_version: &pkg.version, + from_version, package: pkg, } } @@ -541,7 +619,13 @@ pub async fn install_from_lock( // Handle removals for name in &removals { console.info(&console_format!(" - Removing {}", name)); - executor.uninstall_package(name, &exec_ctx)?; + let from_version = installed + .packages + .iter() + .find(|p| p.name.eq_ignore_ascii_case(name)) + .map(|p| p.version.as_str()) + .unwrap_or(""); + executor.uninstall_package(name, from_version, &exec_ctx)?; } // Step 7: Clean up empty vendor namespace directories -- cgit v1.3.1