Repository navigation
install: store the manifests that bun update and bun audit fix fetch #46303
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -706,10 +706,8 @@ impl Options { | |
| ) -> crate::Result<()> { | ||
| let bunfigs = [runtime_bunfigs[0], bun_install_ref, runtime_bunfigs[1]]; | ||
| if let Some(manifest_cache) = env.get(b"BUN_MANIFEST_CACHE") { | ||
| self.enable.set( | ||
| Enable::MANIFEST_CACHE, | ||
| matches!(manifest_cache, b"1" | b"2"), | ||
| ); | ||
| self.enable | ||
| .set_manifest_cache(matches!(manifest_cache, b"1" | b"2")); | ||
| self.enable | ||
| .set(Enable::MANIFEST_CACHE_CONTROL, manifest_cache == b"2"); | ||
| } | ||
|
|
@@ -816,11 +814,11 @@ impl Options { | |
|
|
||
| if config.disable_cache.unwrap_or(false) { | ||
| self.enable.set(Enable::CACHE, false); | ||
| self.enable.set(Enable::MANIFEST_CACHE, false); | ||
| self.enable.set_manifest_cache(false); | ||
| } | ||
|
|
||
| if config.disable_manifest_cache.unwrap_or(false) { | ||
| self.enable.set(Enable::MANIFEST_CACHE, false); | ||
| self.enable.set_manifest_cache(false); | ||
| } | ||
|
|
||
| if config.force.unwrap_or(false) { | ||
|
|
@@ -1076,7 +1074,7 @@ impl Options { | |
| self.json_output = cli.json_output; | ||
|
|
||
| if cli.no_cache { | ||
| self.enable.set(Enable::MANIFEST_CACHE, false); | ||
| self.enable.set_manifest_cache(false); | ||
| self.enable.set(Enable::MANIFEST_CACHE_CONTROL, false); | ||
| } | ||
|
|
||
|
|
@@ -1277,7 +1275,7 @@ impl Options { | |
| // The manifest cache is the data source for --prefer-offline/--offline; keep it on | ||
| // even where it is otherwise bypassed (`bun update`, `--no-cache`, `--force`). | ||
| if self.offline != OfflineMode::Online { | ||
| self.enable.set(Enable::MANIFEST_CACHE, true); | ||
| self.enable.set_manifest_cache(true); | ||
| } | ||
| // Prefetching resolved tarballs is a latency optimisation for downloads; under | ||
| // --offline there is nothing to download and the install phase reports misses. | ||
|
|
@@ -1331,6 +1329,7 @@ impl Default for Do { | |
| bitflags::bitflags! { | ||
| #[derive(Copy, Clone, PartialEq, Eq)] | ||
| pub struct Enable: u16 { | ||
| /// Lookups read the manifests in the cache. | ||
| const MANIFEST_CACHE = 1 << 0; | ||
| const MANIFEST_CACHE_CONTROL = 1 << 1; | ||
| const CACHE = 1 << 2; | ||
|
|
@@ -1353,13 +1352,18 @@ bitflags::bitflags! { | |
| const GLOBAL_VIRTUAL_STORE = 1 << 9; | ||
| /// `bun add` was given a group, so an entry that's in another group moves into it. | ||
| const MOVE_TO_GROUP = 1 << 10; | ||
| // _: u5 padding | ||
| /// Fetched manifests are stored. `bun update` clears `MANIFEST_CACHE` and keeps this. | ||
|
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) Users who run an online Why this was flaggedTrigger: an online Verification: src/install/PackageManager/PackageManagerOptions.rs:1033-1037 clears only |
||
| const MANIFEST_CACHE_STORE = 1 << 11; | ||
| // _: u4 padding | ||
| } | ||
| } | ||
|
|
||
| impl Default for Enable { | ||
| fn default() -> Self { | ||
| Enable::MANIFEST_CACHE | Enable::MANIFEST_CACHE_CONTROL | Enable::CACHE | ||
| Enable::MANIFEST_CACHE | ||
| | Enable::MANIFEST_CACHE_CONTROL | ||
| | Enable::MANIFEST_CACHE_STORE | ||
| | Enable::CACHE | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1423,6 +1427,15 @@ impl Enable { | |
| self.contains(Enable::MANIFEST_CACHE_CONTROL) | ||
| } | ||
| #[inline] | ||
| pub(crate) fn manifest_cache_store(self) -> bool { | ||
| self.contains(Enable::MANIFEST_CACHE_STORE) | ||
| } | ||
| /// The settings that name the manifest cache turn reading and storing on or off together. | ||
| #[inline] | ||
| pub(crate) fn set_manifest_cache(&mut self, on: bool) { | ||
| self.set(Enable::MANIFEST_CACHE | Enable::MANIFEST_CACHE_STORE, on); | ||
| } | ||
| #[inline] | ||
| pub(crate) fn fail_early(self) -> bool { | ||
| self.contains(Enable::FAIL_EARLY) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -731,7 +731,7 @@ pub mod registry { | |
| log: &mut bun_ast::Log, | ||
| package_name: &[u8], | ||
| loaded_manifest: Option<PackageManifest>, | ||
| package_manager: &mut PackageManager, | ||
| store: Option<directories::ManifestStore>, | ||
| is_extended_manifest: bool, | ||
| ) -> Result<PackageVersionResponse, Error> { | ||
| match response.status_code { | ||
|
|
@@ -766,15 +766,12 @@ pub mod registry { | |
| manifest_expiry(&response.headers), | ||
| is_extended_manifest, | ||
| )? { | ||
| // If the cache cannot be written, the manifest is still used. It is just not saved. | ||
| if package_manager.options.enable.manifest_cache() | ||
| && let Ok(temp_dir) = directories::try_get_temporary_directory(package_manager) | ||
| { | ||
| if let Some(store) = store { | ||
|
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) Users with a Why this was flaggedTwo projects use the default shared cache dir: project A sets Verification:
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) Every online Why this was flaggedTrigger: any online Verification: On base the write was gated on the same read bit, so |
||
| package_manifest::Serializer::save_async( | ||
| &package, | ||
| scope, | ||
| temp_dir.handle.fd, | ||
| package_manager.get_cache_directory(), | ||
| store.tmpdir, | ||
| store.cache_dir, | ||
| ); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Qualify the
disableManifestresolution claim.If
bun.lockalready holds a version,disableManifestdoes not resolve a newer one. An exact version constraint also cannot select the latest version. Say that this setting bypasses cached registry metadata when Bun needs to resolve a version. Apply the same correction todocs/runtime/bunfig.mdxLines 1072-1073. The lockfile exception is documented indocs/pm/cli/install.mdxLine 537.🤖 Prompt for AI Agents