Skip to content

Commit

Permalink
fix(install): npm version to git resolution package-lock.json migra…
Browse files Browse the repository at this point in the history 
…tion (#15810)
  • Loading branch information
@dylan-conway
dylan-conway authored Dec 18, 2024
1 parent df5f95b commit 2272b85
Show file tree
Hide file tree
Showing 13 changed files with 315 additions and 193 deletions.
4 changes: 1 addition & 3 deletions src/install/bun.lock.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1053,7 +1053,7 @@ pub fn parseIntoBinaryLockfile(
return error.InvalidLockfileVersion;
};

var string_buf = String.Buf.init(allocator);
var string_buf = lockfile.stringBuf();

if (root.get("trustedDependencies")) |trusted_dependencies_expr| {
var trusted_dependencies: BinaryLockfile.TrustedDependenciesSet = .{};
Expand Down Expand Up @@ -1477,8 +1477,6 @@ pub fn parseIntoBinaryLockfile(
lockfile.buffers.resolutions.expandToCapacity();
@memset(lockfile.buffers.resolutions.items, invalid_package_id);

string_buf.apply(lockfile);

const pkgs = lockfile.packages.slice();
const pkg_deps = pkgs.items(.dependencies);
var pkg_metas = pkgs.items(.meta);
Expand Down
128 changes: 97 additions & 31 deletions src/install/install.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -946,6 +946,8 @@ pub const Task = struct {
name: strings.StringOrTinyString,
url: strings.StringOrTinyString,
env: DotEnv.Map,
dep_id: DependencyID,
res: Resolution,
},
git_checkout: struct {
repo_dir: bun.FileDescriptor,
Expand Down Expand Up @@ -4838,7 +4840,9 @@ pub const PackageManager = struct {
task_id: u64,
name: string,
repository: *const Repository,
dep_id: DependencyID,
dependency: *const Dependency,
res: *const Resolution,
/// if patched then we need to do apply step after network task is done
patch_name_and_version_hash: ?u64,
) *ThreadPool.Task {
Expand All @@ -4860,6 +4864,8 @@ pub const PackageManager = struct {
FileSystem.FilenameStore.instance,
) catch unreachable,
.env = Repository.shared_env.get(this.allocator, this.env),
.dep_id = dep_id,
.res = res.*,
},
},
.id = task_id,
Expand Down Expand Up @@ -5452,7 +5458,7 @@ pub const PackageManager = struct {

if (this.hasCreatedNetworkTask(clone_id, dependency.behavior.isRequired())) return;

this.task_batch.push(ThreadPool.Batch.from(this.enqueueGitClone(clone_id, alias, dep, dependency, null)));
this.task_batch.push(ThreadPool.Batch.from(this.enqueueGitClone(clone_id, alias, dep, id, dependency, &res, null)));
}
},
.github => {
Expand Down Expand Up @@ -6754,13 +6760,20 @@ pub const PackageManager = struct {
manager.extracted_count += 1;
bun.Analytics.Features.extracted_packages += 1;

// GitHub and tarball URL dependencies are not fully resolved until after the tarball is downloaded & extracted.
if (manager.processExtractedTarballPackage(&package_id, dependency_id, resolution, &task.data.extract, comptime log_level)) |pkg| brk: {
if (comptime @TypeOf(callbacks.onExtract) != void and ExtractCompletionContext == *PackageInstaller) {
extract_ctx.fixCachedLockfilePackageSlices();
callbacks.onExtract(
extract_ctx,
dependency_id,
&task.data.extract,
log_level,
);
} else if (manager.processExtractedTarballPackage(&package_id, dependency_id, resolution, &task.data.extract, log_level)) |pkg| handle_pkg: {
// In the middle of an install, you could end up needing to downlaod the github tarball for a dependency
// We need to make sure we resolve the dependencies first before calling the onExtract callback
// TODO: move this into a separate function
var any_root = false;
var dependency_list_entry = manager.task_queue.getEntry(task.id) orelse break :brk;
var dependency_list_entry = manager.task_queue.getEntry(task.id) orelse break :handle_pkg;
var dependency_list = dependency_list_entry.value_ptr.*;
dependency_list_entry.value_ptr.* = .{};

Expand Down Expand Up @@ -6812,13 +6825,8 @@ pub const PackageManager = struct {

manager.setPreinstallState(package_id, manager.lockfile, .done);

// if (task.tag == .extract and task.request.extract.network.apply_patch_task != null) {
// manager.enqueuePatchTask(task.request.extract.network.apply_patch_task.?);
// } else
if (comptime @TypeOf(callbacks.onExtract) != void) {
if (ExtractCompletionContext == *PackageInstaller) {
extract_ctx.fixCachedLockfilePackageSlices();
}
if (comptime @TypeOf(callbacks.onExtract) != void and ExtractCompletionContext != *PackageInstaller) {
// handled *PackageInstaller above
callbacks.onExtract(extract_ctx, dependency_id, &task.data.extract, comptime log_level);
}

Expand All @@ -6831,10 +6839,11 @@ pub const PackageManager = struct {
},
.git_clone => {
const clone = &task.request.git_clone;
const repo_fd = task.data.git_clone;
const name = clone.name.slice();
const url = clone.url.slice();

manager.git_repositories.put(manager.allocator, task.id, task.data.git_clone) catch unreachable;
manager.git_repositories.put(manager.allocator, task.id, repo_fd) catch unreachable;

if (task.status == .fail) {
const err = task.err orelse error.Failed;
Expand All @@ -6861,11 +6870,49 @@ pub const PackageManager = struct {
continue;
}

const dependency_list_entry = manager.task_queue.getEntry(task.id).?;
const dependency_list = dependency_list_entry.value_ptr.*;
dependency_list_entry.value_ptr.* = .{};
if (comptime @TypeOf(callbacks.onExtract) != void and ExtractCompletionContext == *PackageInstaller) {
// Installing!
// this dependency might be something other than a git dependency! only need the name and
// behavior, use the resolution from the task.
const dep_id = clone.dep_id;
const dep = manager.lockfile.buffers.dependencies.items[dep_id];
const dep_name = dep.name.slice(manager.lockfile.buffers.string_bytes.items);

try manager.processDependencyList(dependency_list, ExtractCompletionContext, extract_ctx, callbacks, install_peer);
const git = clone.res.value.git;
const committish = git.committish.slice(manager.lockfile.buffers.string_bytes.items);
const repo = git.repo.slice(manager.lockfile.buffers.string_bytes.items);

const resolved = try Repository.findCommit(
manager.allocator,
manager.env,
manager.log,
task.data.git_clone.asDir(),
dep_name,
committish,
task.id,
);

const checkout_id = Task.Id.forGitCheckout(repo, resolved);

if (manager.hasCreatedNetworkTask(checkout_id, dep.behavior.isRequired())) continue;

manager.task_batch.push(ThreadPool.Batch.from(manager.enqueueGitCheckout(
checkout_id,
repo_fd,
dep_id,
dep_name,
clone.res,
resolved,
null,
)));
} else {
// Resolving!
const dependency_list_entry = manager.task_queue.getEntry(task.id).?;
const dependency_list = dependency_list_entry.value_ptr.*;
dependency_list_entry.value_ptr.* = .{};

try manager.processDependencyList(dependency_list, ExtractCompletionContext, extract_ctx, callbacks, install_peer);
}

if (comptime log_level.showProgress()) {
if (!has_updated_this_run) {
Expand Down Expand Up @@ -6897,15 +6944,29 @@ pub const PackageManager = struct {
continue;
}

if (manager.processExtractedTarballPackage(
if (comptime @TypeOf(callbacks.onExtract) != void and ExtractCompletionContext == *PackageInstaller) {
// We've populated the cache, package already exists in memory. Call the package installer callback
// and don't enqueue dependencies

// TODO(dylan-conway) most likely don't need to call this now that the package isn't appended, but
// keeping just in case for now
extract_ctx.fixCachedLockfilePackageSlices();

callbacks.onExtract(
extract_ctx,
git_checkout.dependency_id,
&task.data.git_checkout,
log_level,
);
} else if (manager.processExtractedTarballPackage(
&package_id,
git_checkout.dependency_id,
resolution,
&task.data.git_checkout,
comptime log_level,
)) |pkg| brk: {
log_level,
)) |pkg| handle_pkg: {
var any_root = false;
var dependency_list_entry = manager.task_queue.getEntry(task.id) orelse break :brk;
var dependency_list_entry = manager.task_queue.getEntry(task.id) orelse break :handle_pkg;
var dependency_list = dependency_list_entry.value_ptr.*;
dependency_list_entry.value_ptr.* = .{};

Expand All @@ -6932,18 +6993,15 @@ pub const PackageManager = struct {
},
}
}
}

if (comptime @TypeOf(callbacks.onExtract) != void) {
if (ExtractCompletionContext == *PackageInstaller) {
extract_ctx.fixCachedLockfilePackageSlices();
if (comptime @TypeOf(callbacks.onExtract) != void) {
callbacks.onExtract(
extract_ctx,
git_checkout.dependency_id,
&task.data.git_checkout,
comptime log_level,
);
}
callbacks.onExtract(
extract_ctx,
git_checkout.dependency_id,
&task.data.git_checkout,
comptime log_level,
);
}

if (comptime log_level.showProgress()) {
Expand Down Expand Up @@ -13517,7 +13575,15 @@ pub const PackageManager = struct {

if (clone_queue.found_existing) return;

this.task_batch.push(ThreadPool.Batch.from(this.enqueueGitClone(clone_id, alias, repository, &this.lockfile.buffers.dependencies.items[dependency_id], null)));
this.task_batch.push(ThreadPool.Batch.from(this.enqueueGitClone(
clone_id,
alias,
repository,
dependency_id,
&this.lockfile.buffers.dependencies.items[dependency_id],
resolution,
null,
)));
}
}

Expand Down
5 changes: 3 additions & 2 deletions src/install/lockfile.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2538,8 +2538,9 @@ pub inline fn stringBuilder(this: *Lockfile) Lockfile.StringBuilder {

pub fn stringBuf(this: *Lockfile) String.Buf {
return .{
.bytes = this.buffers.string_bytes.toManaged(this.allocator),
.pool = this.string_pool,
.bytes = &this.buffers.string_bytes,
.allocator = this.allocator,
.pool = &this.string_pool,
};
}

Expand Down
Loading

0 comments on commit 2272b85

Please sign in to comment.