Skip to content

Commit

Permalink
fix(install): invalidate manifest cache on registry change (#11606)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dylan-conway
dylan-conway authored Jun 6, 2024
1 parent fe7b040 commit 3e4c091
Show file tree
Hide file tree
Showing 10 changed files with 2,015 additions and 68 deletions.
54 changes: 36 additions & 18 deletions src/install/install.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -674,8 +674,10 @@ const Task = struct {
defer {
bun.default_allocator.free(body);
}

const package_manifest = Npm.Registry.getPackageMetadata(
allocator,
manager.scopeForPackageName(manifest.name.slice()),
manifest.network.http.response.?,
body,
&this.log,
Expand Down Expand Up @@ -2352,23 +2354,28 @@ const PackageManifestMap = struct {
};
const HashMap = std.HashMapUnmanaged(PackageNameHash, Value, IdentityContext(PackageNameHash), 80);

pub fn byName(this: *PackageManifestMap, name: []const u8) ?*Npm.PackageManifest {
return this.byNameHash(String.Builder.stringHash(name));
pub fn byName(this: *PackageManifestMap, scope: *const Npm.Registry.Scope, name: []const u8) ?*Npm.PackageManifest {
return this.byNameHash(scope, String.Builder.stringHash(name));
}

pub fn insert(this: *PackageManifestMap, name_hash: PackageNameHash, manifest: *const Npm.PackageManifest) !void {
try this.hash_map.put(bun.default_allocator, name_hash, .{ .manifest = manifest.* });
}

pub fn byNameHash(this: *PackageManifestMap, name_hash: PackageNameHash) ?*Npm.PackageManifest {
return byNameHashAllowExpired(this, name_hash, null);
pub fn byNameHash(this: *PackageManifestMap, scope: *const Npm.Registry.Scope, name_hash: PackageNameHash) ?*Npm.PackageManifest {
return byNameHashAllowExpired(this, scope, name_hash, null);
}

pub fn byNameAllowExpired(this: *PackageManifestMap, name: string, is_expired: ?*bool) ?*Npm.PackageManifest {
return byNameHashAllowExpired(this, String.Builder.stringHash(name), is_expired);
pub fn byNameAllowExpired(this: *PackageManifestMap, scope: *const Npm.Registry.Scope, name: string, is_expired: ?*bool) ?*Npm.PackageManifest {
return byNameHashAllowExpired(this, scope, String.Builder.stringHash(name), is_expired);
}

pub fn byNameHashAllowExpired(this: *PackageManifestMap, name_hash: PackageNameHash, is_expired: ?*bool) ?*Npm.PackageManifest {
pub fn byNameHashAllowExpired(
this: *PackageManifestMap,
scope: *const Npm.Registry.Scope,
name_hash: PackageNameHash,
is_expired: ?*bool,
) ?*Npm.PackageManifest {
const entry = this.hash_map.getOrPut(bun.default_allocator, name_hash) catch bun.outOfMemory();
if (entry.found_existing) {
if (entry.value_ptr.* == .manifest) {
Expand All @@ -2386,7 +2393,12 @@ const PackageManifestMap = struct {
}

if (PackageManager.instance.options.enable.manifest_cache) {
if (Npm.PackageManifest.Serializer.loadByFileID(PackageManager.instance.allocator, PackageManager.instance.getCacheDirectory(), name_hash) catch null) |manifest| {
if (Npm.PackageManifest.Serializer.loadByFileID(
PackageManager.instance.allocator,
scope,
PackageManager.instance.getCacheDirectory(),
name_hash,
) catch null) |manifest| {
if (PackageManager.instance.options.enable.manifest_cache_control and manifest.pkg.public_max_age > PackageManager.instance.timestamp_for_manifest_cache_control) {
entry.value_ptr.* = .{ .manifest = manifest };
return &entry.value_ptr.manifest;
Expand Down Expand Up @@ -3020,11 +3032,10 @@ pub const PackageManager = struct {

pub fn formatLaterVersionInCache(
this: *PackageManager,
name: []const u8,
package_name: string,
name_hash: PackageNameHash,
resolution: Resolution,
) ?Semver.Version.Formatter {
_ = name; // autofix
switch (resolution.tag) {
Resolution.Tag.npm => {
if (resolution.value.npm.version.tag.hasPre())
Expand All @@ -3036,7 +3047,7 @@ pub const PackageManager = struct {
if (this.isContinuousIntegration())
return null;

const manifest: *const Npm.PackageManifest = this.manifests.byNameHash(name_hash) orelse return null;
const manifest = this.manifests.byNameHash(this.scopeForPackageName(package_name), name_hash) orelse return null;

if (manifest.findByDistTag("latest")) |latest_version| {
if (latest_version.version.order(
Expand Down Expand Up @@ -4019,7 +4030,8 @@ pub const PackageManager = struct {
}

// Resolve the version from the loaded NPM manifest
const manifest = this.manifests.byNameHash(name_hash) orelse return null; // manifest might still be downloading. This feels unreliable.
const name_str = this.lockfile.str(&name);
const manifest = this.manifests.byNameHash(this.scopeForPackageName(name_str), name_hash) orelse return null; // manifest might still be downloading. This feels unreliable.
const find_result: Npm.PackageManifest.FindResult = switch (version.tag) {
.dist_tag => manifest.findByDistTag(this.lockfile.str(&version.value.dist_tag.tag)),
.npm => manifest.findBestVersion(version.value.npm.version, this.lockfile.buffers.string_bytes.items),
Expand Down Expand Up @@ -4602,7 +4614,7 @@ pub const PackageManager = struct {
if (!this.hasCreatedNetworkTask(task_id)) {
if (this.options.enable.manifest_cache) {
var expired = false;
if (this.manifests.byNameHashAllowExpired(name_hash, &expired)) |manifest| {
if (this.manifests.byNameHashAllowExpired(this.scopeForPackageName(name_str), name_hash, &expired)) |manifest| {
loaded_manifest = manifest.*;

// If it's an exact package version already living in the cache
Expand Down Expand Up @@ -5298,7 +5310,7 @@ pub const PackageManager = struct {
if (comptime log_level != .silent) {
const string_buf = manager.lockfile.buffers.string_bytes.items;
Output.prettyErrorln("<r><red>error:<r> expected package.json in <b>{any}<r> to be a JSON file: {s}\n", .{
resolution.fmtURL(&manager.options, string_buf),
resolution.fmtURL(string_buf),
@errorName(err),
});
}
Expand Down Expand Up @@ -5349,7 +5361,7 @@ pub const PackageManager = struct {
if (comptime log_level != .silent) {
const string_buf = manager.lockfile.buffers.string_bytes.items;
Output.prettyErrorln("<r><red>error:<r> expected package.json in <b>{any}<r> to be a JSON file: {s}\n", .{
resolution.fmtURL(&manager.options, string_buf),
resolution.fmtURL(string_buf),
@errorName(err),
});
}
Expand Down Expand Up @@ -5392,7 +5404,7 @@ pub const PackageManager = struct {
if (comptime log_level != .silent) {
const string_buf = manager.lockfile.buffers.string_bytes.items;
Output.prettyErrorln("<r><red>error:<r> expected package.json in <b>{any}<r> to be a JSON file: {s}\n", .{
resolution.fmtURL(&manager.options, string_buf),
resolution.fmtURL(string_buf),
@errorName(err),
});
}
Expand Down Expand Up @@ -5634,7 +5646,13 @@ pub const PackageManager = struct {
}

entry.value_ptr.manifest.pkg.public_max_age = timestamp_this_tick.?;
Npm.PackageManifest.Serializer.saveAsync(&entry.value_ptr.manifest, manager.getTemporaryDirectory(), manager.getCacheDirectory());

Npm.PackageManifest.Serializer.saveAsync(
&entry.value_ptr.manifest,
manager.scopeForPackageName(name.slice()),
manager.getTemporaryDirectory(),
manager.getCacheDirectory(),
);

const dependency_list_entry = manager.task_queue.getEntry(task.task_id).?;

Expand Down Expand Up @@ -6286,7 +6304,7 @@ pub const PackageManager = struct {
if (base.url.len == 0) base.url = Npm.Registry.default_url;
this.scope = try Npm.Registry.Scope.fromAPI("", base, allocator, env);
defer {
this.did_override_default_scope = !strings.eqlComptime(this.scope.url.href, Npm.Registry.default_url);
this.did_override_default_scope = this.scope.url_hash != Npm.Registry.default_url_hash;
}
if (bun_install_) |bun_install| {
if (bun_install.scoped) |scoped| {
Expand Down
9 changes: 7 additions & 2 deletions src/install/lockfile.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1448,7 +1448,8 @@ pub const Printer = struct {
const name = dependency.name.slice(string_buf);

if (this.manager) |manager| {
if (manager.formatLaterVersionInCache(name, dependency.name_hash, resolution)) |later_version_fmt| {
const package_name = packages_slice.items(.name)[package_id].slice(string_buf);
if (manager.formatLaterVersionInCache(package_name, dependency.name_hash, resolution)) |later_version_fmt| {
const fmt = comptime brk: {
if (enable_ansi_colors) {
break :brk Output.prettyFmt("<r><green>+<r> <b>{s}<r><d>@{}<r> <d>(<blue>v{} available<r><d>)<r>\n", enable_ansi_colors);
Expand Down Expand Up @@ -1846,7 +1847,7 @@ pub const Printer = struct {

try writer.writeAll(" resolved ");

const url_formatter = resolution.fmtURL(&this.options, string_buf);
const url_formatter = resolution.fmtURL(string_buf);

// Resolved URL is always quoted
try std.fmt.format(writer, "\"{any}\"\n", .{url_formatter});
Expand Down Expand Up @@ -6534,6 +6535,10 @@ pub fn jsonStringify(this: *const Lockfile, w: anytype) !void {
try w.objectField("value");
const formatted = try std.fmt.bufPrint(&buf, "{s}", .{res.fmt(sb, .posix)});
try w.write(formatted);

try w.objectField("resolved");
const formatted_url = try std.fmt.bufPrint(&buf, "{}", .{res.fmtURL(sb)});
try w.write(formatted_url);
}

try w.objectField("dependencies");
Expand Down
Loading

0 comments on commit 3e4c091

Please sign in to comment.