diff options
| author | nsfisis <nsfisis@gmail.com> | 2026-07-20 17:25:38 +0900 |
|---|---|---|
| committer | nsfisis <nsfisis@gmail.com> | 2026-07-20 17:25:38 +0900 |
| commit | daa580b97ad2d585c100927842ba88e891eb9aca (patch) | |
| tree | dcd04a2e76a63fa453cc1ac550f43b5658da58f9 /crates/shirabe | |
| parent | 5ef607e2afe7eaf7e2ccdf62f3ffacfede3c5fca (diff) | |
| download | php-shirabe-daa580b97ad2d585c100927842ba88e891eb9aca.tar.gz php-shirabe-daa580b97ad2d585c100927842ba88e891eb9aca.tar.zst php-shirabe-daa580b97ad2d585c100927842ba88e891eb9aca.zip | |
fix(pool-builder): fix three build_pool bugs found by un-ignoring test_pool_builder
build_pool never populated skipped_load for locked packages skipped due
to the update allow list (nor their replace targets), so load_package's
skipped_load-driven transitive-unlock branch never fired.
load_packages_marked_for_loading never recorded loaded packages into
loaded_per_repo, so repositories had no way to dedupe already-loaded
versions when a constraint got re-expanded, producing duplicate pool
entries.
unlock_package looked up a locked package's removal index via the
position in a Vec snapshot of self.packages.values() instead of its
actual IndexMap key, so after earlier removals the wrong entry (or
none) got removed, leaving stale locked packages in the pool.
Fixing all three lets test_pool_builder run un-ignored.
Diffstat (limited to 'crates/shirabe')
| -rw-r--r-- | crates/shirabe/src/dependency_resolver/pool_builder.rs | 32 | ||||
| -rw-r--r-- | crates/shirabe/tests/dependency_resolver/pool_builder_test.rs | 1 |
2 files changed, 27 insertions, 6 deletions
diff --git a/crates/shirabe/src/dependency_resolver/pool_builder.rs b/crates/shirabe/src/dependency_resolver/pool_builder.rs index 5cfaa33c..00d6b938 100644 --- a/crates/shirabe/src/dependency_resolver/pool_builder.rs +++ b/crates/shirabe/src/dependency_resolver/pool_builder.rs @@ -162,6 +162,18 @@ impl PoolBuilder { .get_canonical_packages()? { if !self.is_update_allowed(locked_package.clone()) { + // remember which packages we skipped loading remote content for in this partial update + self.skipped_load + .entry(locked_package.get_name()) + .or_default() + .push(locked_package.clone()); + for (_k, link) in &locked_package.get_replaces() { + self.skipped_load + .entry(link.get_target().to_string()) + .or_default() + .push(locked_package.clone()); + } + // Path repo packages are never loaded from lock, to force them to always remain in sync // unless symlinking is disabled in which case we probably should rather treat them like // regular packages. We mark them specially so they can be reloaded fully including update propagation @@ -518,6 +530,13 @@ impl PoolBuilder { let pkg_version = package.get_version().to_string(); let pkg_type = package.get_type().to_string(); + self.loaded_per_repo + .entry(repo_index as i64) + .or_default() + .entry(pkg_name.clone()) + .or_default() + .insert(pkg_version.clone(), package.clone()); + let pkg_type_mixed: PhpMixed = pkg_type.clone().into(); let ignored_mixed: PhpMixed = self .ignored_types @@ -542,7 +561,6 @@ impl PoolBuilder { { continue; } - let _ = (pkg_name, pkg_version); let propagate = !self.path_repo_unlocked.contains_key(&package.get_name()); self.load_package(request, repositories, package.clone(), propagate)?; } @@ -907,16 +925,20 @@ impl PoolBuilder { request.get_locked_packages().values().cloned().collect(); for locked_package in &locked_packages { if locked_package.as_alias().is_none() && locked_package.get_name() == name { - let pkgs: Vec<BasePackageHandle> = self.packages.values().cloned().collect(); - // PHP uses array_search with strict identity; map to pointer comparison. - let index_opt = pkgs.iter().position(|p| p.ptr_eq(locked_package)); + // PHP: array_search($lockedPackage, $this->packages, true) — identity lookup + // that returns the array KEY, not a positional index. + let index_opt = self + .packages + .iter() + .find(|(_, p)| p.ptr_eq(locked_package)) + .map(|(k, _)| *k); if let Some(index) = index_opt { request.unlock_package(locked_package.clone()); self.remove_loaded_package( request, repositories, locked_package.clone(), - index as i64, + index, ); // make sure that any requirements for this package by other locked or fixed packages are now diff --git a/crates/shirabe/tests/dependency_resolver/pool_builder_test.rs b/crates/shirabe/tests/dependency_resolver/pool_builder_test.rs index ce5310b2..9b82908d 100644 --- a/crates/shirabe/tests/dependency_resolver/pool_builder_test.rs +++ b/crates/shirabe/tests/dependency_resolver/pool_builder_test.rs @@ -621,7 +621,6 @@ fn run_test_pool_builder( std::env::set_current_dir(&old_cwd).unwrap(); } -#[ignore = "PoolBuilder::build_pool never populates skipped_load (PHP populates it at buildPool's lockedPackage loop, incl. its replace targets), so load_package's skipped_load-driven transitive unlock branch never fires; fails partial-update-unfixing-with-replacers-providers.test"] #[test] fn test_pool_builder() { let fixtures_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")) |
