Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 58 additions & 14 deletions src/install/PackageManager/PackageManagerEnqueue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,32 @@
let _ = this.pending_pre_calc_hashes.fetch_add(1, Ordering::Relaxed);
}

/// Mirrors the npm arm's `should_update` gate in
/// `get_or_put_resolved_package_with_find_result`: a `bun update` target must
/// not bind to the in-memory lockfile package on its first enqueue, or the
/// update never re-fetches.
Comment thread
robobun marked this conversation as resolved.
fn is_update_target(this: &mut PackageManager, dependency: &Dependency, id: DependencyID) -> bool {
if !this.to_update {
return false;
}
let this_ptr: *mut PackageManager = this;
// Update direct deps of the current workspace; catalogs are root-scoped.
// SAFETY: `is_root_dependency` reads `manager.root_package_id` /
// `manager.workspace_name_hash` only — disjoint from `manager.lockfile`.
if dependency.version.tag != dependency::version::Tag::Catalog
&& !unsafe { &*(*this_ptr).lockfile }.is_root_dependency(unsafe { &mut *this_ptr }, id)

Check failure on line 635 in src/install/PackageManager/PackageManagerEnqueue.rs

View workflow job for this annotation

GitHub Actions / cargo clippy

unsafe block missing a safety comment

Check failure on line 635 in src/install/PackageManager/PackageManagerEnqueue.rs

View workflow job for this annotation

GitHub Actions / cargo clippy

unsafe block missing a safety comment
{
return false;
}
// By name_hash, not `updating_packages`: that map only records
// npm/dist-tag deps. Matches `Diff::generate`'s update-target test.
Comment thread
robobun marked this conversation as resolved.
this.update_requests.is_empty()
|| this
.update_requests
.iter()
.any(|r| r.name_hash == dependency.name_hash)
}

/// Q: "What do we do with a dependency in a package.json?"
/// A: "We enqueue it!"
pub fn enqueue_dependency_with_main_and_success_fn(
Expand All @@ -641,7 +667,14 @@
return Ok(());
}

// Git/github/tarball `realname()` is only learned from the first extract,
// so on a fresh parse fall back to the alias; `package_index` is keyed by
// real names, and hashing "" would re-download lockfile-resolved packages.
Comment thread
robobun marked this conversation as resolved.
let mut name = dependency.realname();
let is_provisional_name = name.is_empty();
if is_provisional_name {
name = dependency.name;
}
let mut name_hash = match dependency.version.tag {
dependency::version::Tag::DistTag
| dependency::version::Tag::Git
Expand Down Expand Up @@ -1176,10 +1209,15 @@
let dep: Repository = *version.git();
let res = Resolution::init(ResolutionTagged::Git(dep));

// First: see if we already loaded the git package in-memory
if let Some(pkg_id) = this.lockfile.get_package_id(name_hash, None, &res) {
success_fn(this, id, pkg_id);
return Ok(());
// First: see if we already loaded the git package in-memory.
// On the fresh (alias-named) enqueue, `bun update` targets skip
// the bind so the update re-fetches; the post-checkout re-enqueue
// binds as before.
Comment thread
robobun marked this conversation as resolved.
if !(is_provisional_name && is_update_target(this, dependency, id)) {
if let Some(pkg_id) = this.lockfile.get_package_id(name_hash, None, &res) {
success_fn(this, id, pkg_id);
return Ok(());
}
}

// reshaped for borrowck — `alias`/`url` borrow
Expand Down Expand Up @@ -1286,10 +1324,13 @@
let dep: &Repository = version.github();
let res = Resolution::init(ResolutionTagged::Github(*dep));

// First: see if we already loaded the github package in-memory
if let Some(pkg_id) = this.lockfile.get_package_id(name_hash, None, &res) {
success_fn(this, id, pkg_id);
return Ok(());
// First: see if we already loaded the github package in-memory.
// See the git arm for the update-target gate.
Comment thread
robobun marked this conversation as resolved.
if !(is_provisional_name && is_update_target(this, dependency, id)) {
if let Some(pkg_id) = this.lockfile.get_package_id(name_hash, None, &res) {
success_fn(this, id, pkg_id);
return Ok(());
}
}

let url = this.alloc_github_url(dep);
Expand Down Expand Up @@ -1475,10 +1516,13 @@
}
};

// First: see if we already loaded the tarball package in-memory
if let Some(pkg_id) = this.lockfile.get_package_id(name_hash, None, &res) {
success_fn(this, id, pkg_id);
return Ok(());
// First: see if we already loaded the tarball package in-memory.
// See the git arm for the update-target gate.
Comment thread
robobun marked this conversation as resolved.
if !(is_provisional_name && is_update_target(this, dependency, id)) {
if let Some(pkg_id) = this.lockfile.get_package_id(name_hash, None, &res) {
success_fn(this, id, pkg_id);
return Ok(());
}
}

// reshaped for borrowck — `url` borrows `string_bytes`;
Expand Down Expand Up @@ -1968,8 +2012,8 @@
// borrows `this.lockfile` and `this` at once. Split via raw root.
let should_update = {
let this_ptr: *mut PackageManager = this;
// SAFETY: `is_root_dependency` reads `manager.root_dependency_list` /
// `manager.workspace_package_json_cache` only — disjoint from
// SAFETY: `is_root_dependency` reads `manager.root_package_id` /
// `manager.workspace_name_hash` only — disjoint from
// `manager.lockfile`.
this.to_update
// Update direct deps of the current workspace; catalogs are root-scoped.
Expand Down
58 changes: 58 additions & 0 deletions src/install/PackageManager/runTasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1387,6 +1387,64 @@ pub fn run_tasks<C: RunTasksCallbacks>(
None,
);
manager.task_batch.push(ThreadPoolBatch::from(queued));
} else if C::IS_STORE_INSTALLER {
// Installing! The isolated store parked its entry contexts
// under `checkout_id` (`enqueue_git_for_checkout`) with a
// `Dependency` waiter under the clone id. Enqueue each
// waiter's checkout directly from its lockfile resolution:
// re-running the dependency resolver here would bind the
// already-resolved dependency and return without ever
// scheduling the checkout, starving the parked entry.
Comment thread
robobun marked this conversation as resolved.
if let Some(waiters) = manager.task_queue.remove(&task.id) {
for waiter in waiters.iter() {
let dep_id = match waiter {
bun_install::TaskCallbackContext::Dependency(id) => *id,
_ => continue,
};
let pkg_id = manager.lockfile.buffers.resolutions[dep_id as usize];
if pkg_id == INVALID_PACKAGE_ID {
continue;
}
let res = manager.lockfile.packages.items_resolution()[pkg_id as usize];
if res.tag != bun_install::ResolutionTag::Git {
continue;
}
let (dep_name_handle, is_required) = {
let dep = &manager.lockfile.buffers.dependencies[dep_id as usize];
(dep.name, dep.behavior.is_required())
};
// SAFETY: `string_bytes` lives as long as
// `manager.lockfile` and is not reallocated while
// install-phase tasks are draining.
let string_buf = unsafe {
bun_ptr::detach_lifetime(
manager.lockfile.buffers.string_bytes.as_slice(),
)
};
// SAFETY: `res.tag == Git` checked above — `value.git`
// is the active union arm.
let res_git = res.git();
let resolved = res_git.resolved.slice(string_buf);
let checkout_id = Task::Id::for_git_checkout(
res_git.repo.slice(string_buf),
resolved,
);
if manager.has_created_network_task(checkout_id, is_required) {
continue;
}
let queued = enqueue::enqueue_git_checkout(
manager,
checkout_id,
repo_fd,
dep_id,
dep_name_handle.slice(string_buf),
&res,
resolved,
None,
);
manager.task_batch.push(ThreadPoolBatch::from(queued));
}
}
} else {
// Resolving!
let dependency_list_entry = manager
Expand Down
9 changes: 7 additions & 2 deletions src/install/lockfile/bun.lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3061,15 +3061,20 @@ fn resolve_peer_dep_version_based(
// `package_index` is keyed by *real* package names while `dep.name_hash`
// may hold an alias, so an `npm:`-aliased peer must be looked up under
// the real package name (`dep.realname()`). Mirrors the realname hashing
// in `enqueue_dependency_with_main_and_success_fn`.
// in `enqueue_dependency_with_main_and_success_fn`, including its
// empty-realname alias fallback.
Comment thread
robobun marked this conversation as resolved.
let name_hash = match dep.version.tag {
DependencyVersionTag::DistTag
| DependencyVersionTag::Git
| DependencyVersionTag::Github
| DependencyVersionTag::Npm
| DependencyVersionTag::Tarball
| DependencyVersionTag::Workspace => {
StringBuilder::string_hash(dep.realname().slice(string_buf))
let mut name = dep.realname();
if name.is_empty() {
name = dep.name;
}
StringBuilder::string_hash(name.slice(string_buf))
}
_ => dep.name_hash,
};
Expand Down
Loading
Loading