Repository navigation
install: keep an optional git, GitHub or tarball dependency on its locked package while it is read again #46276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -316,10 +316,7 @@ fn enqueue_download( | |
| task_context: TaskCallbackContext, | ||
| authorization: Authorization, | ||
| ) -> Result<(), ForTarballError> { | ||
| if this.network_task_has_failed(task_id) { | ||
| if is_asked_by_linker(&task_context) { | ||
| report_failed_download_as_required(this, task_id, dependency_id, package_id); | ||
| } | ||
| if has_already_failed(this, task_id, dependency_id, package_id, &task_context) { | ||
| return Err(ForTarballError::AlreadyFailed); | ||
| } | ||
| if this.options.offline == OfflineMode::Offline { | ||
|
|
@@ -363,12 +360,15 @@ pub fn enqueue_tarball_for_reading( | |
| alias: &[u8], | ||
| resolution: &Resolution, | ||
| task_context: TaskCallbackContext, | ||
| ) { | ||
| ) -> Result<(), ForTarballError> { | ||
| // `enqueue_local_tarball` copies `path` into the filename store before `string_bytes` can grow. | ||
| let path = this.lockfile.str_detached(resolution.local_tarball()); | ||
| let task_id = Task::Id::for_tarball(path); | ||
| if has_already_failed(this, task_id, dependency_id, package_id, &task_context) { | ||
| return Err(ForTarballError::AlreadyFailed); | ||
| } | ||
| if !push_task_callback(this, task_id, task_context) { | ||
| return; | ||
| return Ok(()); | ||
| } | ||
|
|
||
| let integrity = this.lockfile.packages.items_meta()[package_id as usize].integrity; | ||
|
|
@@ -383,6 +383,7 @@ pub fn enqueue_tarball_for_reading( | |
| Some(&integrity), | ||
| ); | ||
| this.task_batch.push(ThreadPool::Batch::from(task)); | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// Returns `Err(Offline)` if the repository isn't cached. Nothing is queued then, and the caller | ||
|
|
@@ -456,6 +457,23 @@ fn is_asked_by_linker(context: &TaskCallbackContext) -> bool { | |
| ) | ||
| } | ||
|
|
||
| /// Returns true if `task_id` has failed, which means that nothing would answer a request for it. | ||
| fn has_already_failed( | ||
| this: &mut PackageManager, | ||
| task_id: Task::Id, | ||
| dependency_id: DependencyID, | ||
| package_id: PackageID, | ||
| context: &TaskCallbackContext, | ||
| ) -> bool { | ||
| if !this.network_task_has_failed(task_id) { | ||
| return false; | ||
| } | ||
| if is_asked_by_linker(context) { | ||
| report_failed_download_as_required(this, task_id, dependency_id, package_id); | ||
| } | ||
| true | ||
| } | ||
|
|
||
| /// The download failed with only a warning, because only optional dependencies needed it at the | ||
| /// time. A linker that requires the package reports an error. | ||
| #[cold] | ||
|
|
@@ -986,6 +1004,87 @@ fn locked_repository( | |
| this.locked_tarball(res) | ||
| } | ||
|
|
||
| /// Only a package.json read from the project may name a tarball in the project. | ||
| fn may_name_local_tarball( | ||
| lockfile: &Lockfile::Lockfile, | ||
| declarer: PackageID, | ||
| dependency: &Dependency, | ||
| ) -> bool { | ||
| lockfile.is_local_declarer(declarer) | ||
| || lockfile.has_equal_root_dependency(dependency) | ||
| || lockfile.packages.items_resolution()[declarer as usize].tag | ||
| == ResolutionTag::LocalTarball | ||
| } | ||
|
|
||
| /// Returns false for a dependency that never `refreshes_in_place`, whatever bun.lock has for it. | ||
| #[inline] | ||
| pub(crate) fn may_refresh_in_place(dependency: &Dependency) -> bool { | ||
| use dependency::version::Tag; | ||
| dependency.behavior.is_optional() | ||
| && matches!( | ||
| dependency.version.tag, | ||
| Tag::Git | Tag::Github | Tag::Tarball | ||
| ) | ||
| } | ||
|
|
||
| /// Returns true if `dependency` of `declarer` keeps `locked` while this run reads its source again. | ||
| pub(crate) fn refreshes_in_place( | ||
| this: &PackageManager, | ||
| declarer: PackageID, | ||
| dependency: &Dependency, | ||
| locked: PackageID, | ||
| ) -> bool { | ||
| use dependency::version::Tag; | ||
| if !may_refresh_in_place(dependency) { | ||
| return false; | ||
| } | ||
| let lockfile = &*this.lockfile; | ||
| let Some(locked_res) = lockfile.packages.items_resolution().get(locked as usize) else { | ||
| return false; | ||
| }; | ||
| let buf = lockfile.buffers.string_bytes.as_slice(); | ||
| // An older bun.lock binds a dependency that never resolved to any package of its name. | ||
| let is_from_source = match dependency.version.tag { | ||
| Tag::Github => { | ||
| let asks = dependency.version.github(); | ||
| locked_res.tag == ResolutionTag::Github | ||
| && locked_res.github().owner.eql(asks.owner, buf, buf) | ||
| && locked_res.github().repo.eql(asks.repo, buf, buf) | ||
| } | ||
| Tag::Git => { | ||
| let asks = dependency.version.git().repo.slice(buf); | ||
| // bun.lock has `host:path` as a URL. | ||
| let recorded = crate::repository_real::locked_scp_like(asks); | ||
| locked_res.tag == ResolutionTag::Git | ||
| && locked_res.git().repo.slice(buf) == recorded.as_deref().unwrap_or(asks) | ||
| } | ||
| _ => match &dependency.version.tarball().uri { | ||
| dependency::tarball::Uri::Remote(url) => { | ||
| locked_res.tag == ResolutionTag::RemoteTarball | ||
| && locked_res.remote_tarball().eql(*url, buf, buf) | ||
| } | ||
| dependency::tarball::Uri::Local(path) => { | ||
| locked_res.tag == ResolutionTag::LocalTarball | ||
| && locked_res.local_tarball().eql(*path, buf, buf) | ||
| && may_name_local_tarball(lockfile, declarer, dependency) | ||
| } | ||
| }, | ||
| }; | ||
| let has_rule = |name_hash: PackageNameHash| { | ||
| lockfile.overrides.has_rule_for_name(name_hash) | ||
| || this | ||
| .summary | ||
| .overridden_names | ||
| .binary_search(&name_hash) | ||
| .is_ok() | ||
| }; | ||
| is_from_source | ||
| // A rule replaces what the dependency declares, and a changed rule can replace `locked`. | ||
| && !has_rule(dependency.name_hash) | ||
| && !has_rule(lockfile.packages.items_name_hash()[locked as usize]) | ||
| && unsafe_alias(lockfile, dependency).is_none() | ||
| } | ||
|
|
||
| /// Returns the `npm:` alias that the project declares for `name`, if any. A dependency of the root | ||
| /// package.json only follows an alias that the root package.json itself declares. | ||
| fn followed_npm_alias( | ||
|
|
@@ -1258,7 +1357,7 @@ fn request_remote_package_ahead( | |
| asked.version.tarball().uri, | ||
| dependency::tarball::Uri::Local(_) | ||
| ); | ||
| if resolution != invalid_package_id | ||
| if !this.lockfile.awaits_resolve(id) | ||
| || this.options.runtime_auto_install | ||
| // A deferred peer doesn't request anything, because the tree may provide it. | ||
| || (dependency.behavior.is_peer() && !install_peer) | ||
|
|
@@ -1810,15 +1909,7 @@ fn unsafe_name<'a>( | |
| dependency: &'a Dependency, | ||
| asked: &'a Asked<'_>, | ||
| ) -> Option<&'a [u8]> { | ||
| let alias = lockfile.str(&dependency.name); | ||
| let alias_is_safe = if alias == lockfile.str(&dependency.version.literal) { | ||
| // Until `assign_resolution` runs, the alias of `bun add <specifier>` is the specifier | ||
| // itself. It never becomes a folder name, but it is printed. | ||
| !dependency::contains_control_character(alias) | ||
| } else { | ||
| alias.is_empty() || crate::package_installer::alias_is_safe_install_target(alias) | ||
| }; | ||
| if !alias_is_safe { | ||
| if let Some(alias) = unsafe_alias(lockfile, dependency) { | ||
| return Some(alias); | ||
| } | ||
| let registry_name: Option<&[u8]> = match asked.version.tag { | ||
|
|
@@ -1832,6 +1923,23 @@ fn unsafe_name<'a>( | |
| registry_name.filter(|name| !crate::package_installer::alias_is_safe_install_target(name)) | ||
| } | ||
|
|
||
| /// The alias of a dependency, which is a future folder, if it is unsafe. | ||
| #[inline(always)] | ||
| fn unsafe_alias<'a>( | ||
| lockfile: &'a Lockfile::Lockfile, | ||
| dependency: &'a Dependency, | ||
| ) -> Option<&'a [u8]> { | ||
| let alias = lockfile.str(&dependency.name); | ||
| let alias_is_safe = if alias == lockfile.str(&dependency.version.literal) { | ||
| // Until `assign_resolution` runs, the alias of `bun add <specifier>` is the specifier | ||
| // itself. It never becomes a folder name, but it is printed. | ||
| !dependency::contains_control_character(alias) | ||
| } else { | ||
| alias.is_empty() || crate::package_installer::alias_is_safe_install_target(alias) | ||
| }; | ||
| (!alias_is_safe).then_some(alias) | ||
| } | ||
|
|
||
| /// Resolves one dependency: binds it to a package that the lockfile has, or appends the package. | ||
| pub(crate) fn bind_row( | ||
| this: &mut PackageManager, | ||
|
|
@@ -1880,6 +1988,13 @@ pub(crate) fn bind_row( | |
| version, | ||
| version_was_replaced, | ||
| } = asked; | ||
| // A rule for the name that its source has now replaces what the dependency asks for. | ||
| let resolution = if version_was_replaced && this.lockfile.is_refreshing(id) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 (optional) A refreshing row whose replaced version carries an unsafe install name stays bound to its locked package with the refreshing bit set, so Why this was flaggedA row marked refreshing by Verification: When that rule's target is unsafe, |
||
| this.lockfile.unbind_refreshing(id); | ||
| invalid_package_id | ||
| } else { | ||
| resolution | ||
| }; | ||
| let mut loaded_manifest: Option<Npm::PackageManifest> = None; | ||
|
|
||
| match version.tag { | ||
|
|
@@ -2170,8 +2285,7 @@ fn bind_row_without_manifest( | |
| let clone_id = Task::Id::for_git_clone(url); | ||
|
|
||
| if let Some(repo_fd) = this.git_repositories.get(&clone_id).copied() { | ||
| let needs_ctx = | ||
| this.lockfile.buffers.resolutions[id as usize] == invalid_package_id; | ||
| let needs_ctx = this.lockfile.awaits_resolve(id); | ||
|
|
||
| // Waits on a `git log` task; `run_tasks` fills `git_commits` and re-enters here. | ||
| let committish = this.lockfile.str_detached(&dep.committish); | ||
|
|
@@ -2336,14 +2450,10 @@ fn bind_row_without_manifest( | |
| dependency::tarball::Uri::Remote(url) => ResolutionTagged::RemoteTarball(*url), | ||
| }, | ||
| }); | ||
| // Only a package.json read from the project may name a tarball in the project. | ||
| if res.tag == ResolutionTag::LocalTarball | ||
| && !version_was_replaced | ||
| && !this.lockfile.is_dependency_of_local_package(id) | ||
| && !this.lockfile.has_equal_root_dependency(dependency) | ||
| && let Some(declarer) = this.lockfile.get_parent_pkg_of_dependency(id) | ||
| && this.lockfile.packages.items_resolution()[declarer as usize].tag | ||
| != ResolutionTag::LocalTarball | ||
| && !may_name_local_tarball(&this.lockfile, declarer, dependency) | ||
| { | ||
| if dependency.behavior.is_required() { | ||
| reject_local_tarball_of_remote_package(this, declarer, dependency); | ||
|
|
@@ -2373,7 +2483,7 @@ fn bind_row_without_manifest( | |
| }; | ||
| let task_id = Task::Id::for_tarball(url); | ||
|
|
||
| if this.lockfile.buffers.resolutions[id as usize] == invalid_package_id { | ||
| if this.lockfile.awaits_resolve(id) { | ||
| if let Some(pkg_id) = resolve_from_appended_task(this, task_id, id) { | ||
| success_fn(this, id, pkg_id); | ||
| return Ok(()); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.