Skip to content

Commit e6a5bec

Browse files
cursoragentcursoragent
committed
Fix package map bugs: remove stale maps and rebuild after islands
- Bug fix #1: Package map saved before island linking - Move package map persistence to after all islands are linked - Rebuild package map with island packages when islands are present - Ensures .package-map.json reflects the final disk layout - Bug fix #2: Stale map after build failure - Remove old .package-map.json at the start of node-modules linking - Prevents leftover maps from earlier installs when build() fails - Matches pnpm linker behavior of cleaning up at link start
1 parent 6eadee7 commit e6a5bec

5 files changed

Lines changed: 96 additions & 4 deletions

File tree

‎packages/zpm/src/linker/mod.rs‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ pub mod venv;
2222
pub struct LinkResult {
2323
pub packages_by_location: BTreeMap<Path, Locator>,
2424
pub build_requests: BuildRequests,
25+
pub package_map: Option<package_map::PackageMap>,
2526
}
2627

2728
pub async fn link_project<'a>(project: &'a Project, install: &'a Install) -> Result<LinkResult, Error> {
@@ -38,6 +39,8 @@ pub async fn link_project<'a>(project: &'a Project, install: &'a Install) -> Res
3839
=> pnpm::link_project_pnpm(project, install).await?,
3940
};
4041

42+
let has_islands = !install.resolved_islands.is_empty();
43+
4144
// Per-island link steps
4245
for island in &install.resolved_islands {
4346
match island.linker {
@@ -55,9 +58,88 @@ pub async fn link_project<'a>(project: &'a Project, install: &'a Install) -> Res
5558
}
5659
}
5760

61+
// Persist package map after all islands have been linked
62+
// If islands were present, rebuild the package map to include island packages
63+
if let Some(package_map) = &result.package_map {
64+
if has_islands && matches!(project.config.settings.node_linker.value, NodeLinker::NodeModules | NodeLinker::Pnpm) {
65+
// Rebuild the package map to include island-added packages
66+
let rebuilt_package_map = rebuild_package_map_with_islands(project, install, &result.packages_by_location)?;
67+
package_map::persist_package_map(project, &rebuilt_package_map)?;
68+
} else {
69+
package_map::persist_package_map(project, package_map)?;
70+
}
71+
}
72+
5873
Ok(result)
5974
}
6075

76+
fn rebuild_package_map_with_islands(
77+
project: &Project,
78+
install: &Install,
79+
packages_by_location: &BTreeMap<Path, Locator>,
80+
) -> Result<package_map::PackageMap, Error> {
81+
match project.config.settings.node_linker.value {
82+
NodeLinker::NodeModules => {
83+
let mut builder = package_map::NodeModulesPackageMapBuilder::new(project, install);
84+
85+
// Register all packages from the final packages_by_location map
86+
for (rel_path, locator) in packages_by_location {
87+
let location_abs = project.project_cwd.with_join(rel_path);
88+
89+
// Determine the package_path (where the actual package files are)
90+
let package_path = match install.package_data.get(&locator.physical_locator()) {
91+
Some(crate::fetchers::PackageData::Local { package_directory, .. }) => {
92+
package_directory.clone()
93+
},
94+
_ => location_abs.clone(),
95+
};
96+
97+
builder.register_package(location_abs, package_path, locator);
98+
}
99+
100+
builder.build()
101+
},
102+
NodeLinker::Pnpm => {
103+
let mut builder = package_map::PnpmPackageMapBuilder::new(project);
104+
let tree = &install.install_state.resolution_tree;
105+
106+
// Register all packages
107+
for locator in packages_by_location.values() {
108+
let package_location = packages_by_location
109+
.iter()
110+
.find(|(_, l)| *l == locator)
111+
.map(|(path, _)| project.project_cwd.with_join(path))
112+
.unwrap();
113+
114+
builder.register_package(locator, package_location);
115+
}
116+
117+
// Register dependencies
118+
for (locator, resolution) in &tree.locator_resolutions {
119+
for (dep_name, descriptor) in &resolution.dependencies {
120+
if let Some(dep_locator) = tree.descriptor_to_locator.get(descriptor) {
121+
builder.register_dependency(locator, dep_name, dep_locator)?;
122+
}
123+
}
124+
125+
// Add implicit self-dependency for non-workspace packages
126+
if !locator.reference.is_workspace_reference() {
127+
let has_explicit_self = resolution.dependencies.contains_key(&locator.ident);
128+
if !has_explicit_self {
129+
builder.register_dependency(locator, &locator.ident, locator)?;
130+
}
131+
}
132+
}
133+
134+
builder.build()
135+
},
136+
_ => {
137+
// Other linkers don't use package maps
138+
Err(Error::Unsupported)
139+
},
140+
}
141+
}
142+
61143
fn cleanup_inactive_linker_artifacts(project: &Project) -> Result<(), Error> {
62144
let active = project.config.settings.node_linker.value;
63145

‎packages/zpm/src/linker/nm/mod.rs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use zpm_sync::{SyncItem, SyncTemplate, SyncTree};
55
use zpm_utils::{FromFileString, IoResultExt, Path, ToHumanString};
66

77
use crate::{
8-
build::{self, BuildRequest, BuildRequests}, content_flags, error::Error, fetchers::PackageData, install::Install, linker::{self, LinkResult, helpers::PackageMeta, nm::hoist::{Hoister, WorkTree}, package_map::{NodeModulesPackageMapBuilder, persist_package_map}}, project::Project
8+
build::{self, BuildRequest, BuildRequests}, content_flags, error::Error, fetchers::PackageData, install::Install, linker::{self, LinkResult, helpers::PackageMeta, nm::hoist::{Hoister, WorkTree}, package_map::NodeModulesPackageMapBuilder}, project::Project
99
};
1010

1111
pub mod hoist;
@@ -573,6 +573,7 @@ pub async fn link_island_nm(
573573
Ok(LinkResult {
574574
packages_by_location,
575575
build_requests,
576+
package_map: None,
576577
})
577578
}
578579

@@ -789,6 +790,11 @@ fn check_external_portal_conflicts(
789790
pub async fn link_project_nm(project: &Project, install: &Install) -> Result<LinkResult, Error> {
790791
warn_about_portals_if_any(install);
791792

793+
// Remove stale package map to prevent it from persisting after a build failure
794+
project.package_map_path()
795+
.fs_rm()
796+
.ok_missing()?;
797+
792798
let mut work_tree
793799
= WorkTree::new(project, &install.install_state);
794800

@@ -833,7 +839,7 @@ pub async fn link_project_nm(project: &Project, install: &Install) -> Result<Lin
833839

834840
run_cas_extractions(project, install, &cas_extractions)?;
835841

836-
persist_package_map(project, &package_map_builder.build()?)?;
842+
let package_map = package_map_builder.build()?;
837843

838844
let dependencies_meta
839845
= linker::helpers::TopLevelConfiguration::from_project(project);
@@ -849,5 +855,6 @@ pub async fn link_project_nm(project: &Project, install: &Install) -> Result<Lin
849855
Ok(LinkResult {
850856
packages_by_location,
851857
build_requests,
858+
package_map: Some(package_map),
852859
})
853860
}

‎packages/zpm/src/linker/pnp.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,5 +593,6 @@ pub async fn link_project_pnp<'a>(project: &'a Project, install: &'a Install) ->
593593
Ok(LinkResult {
594594
packages_by_location,
595595
build_requests,
596+
package_map: None,
596597
})
597598
}

‎packages/zpm/src/linker/pnpm.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::{
88
error::Error,
99
fetchers::PackageData,
1010
install::Install,
11-
linker::{self, LinkResult, package_map::{PnpmPackageMapBuilder, persist_package_map}},
11+
linker::{self, LinkResult, package_map::PnpmPackageMapBuilder},
1212
project::Project,
1313
tree_resolver::ResolutionTree,
1414
};
@@ -329,7 +329,7 @@ pub async fn link_project_pnpm<'a>(project: &'a Project, install: &'a Install) -
329329
}
330330
}
331331

332-
persist_package_map(project, &package_map_builder.build()?)?;
332+
let package_map = package_map_builder.build()?;
333333

334334
let package_build_dependencies = linker::helpers::populate_build_entry_dependencies(
335335
&package_build_entries,
@@ -345,5 +345,6 @@ pub async fn link_project_pnpm<'a>(project: &'a Project, install: &'a Install) -
345345
Ok(LinkResult {
346346
packages_by_location,
347347
build_requests,
348+
package_map: Some(package_map),
348349
})
349350
}

‎packages/zpm/src/linker/venv.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,5 +180,6 @@ pub async fn link_island_venv(
180180
entries: vec![],
181181
dependencies: BTreeMap::new(),
182182
},
183+
package_map: None,
183184
})
184185
}

0 commit comments

Comments
 (0)