Skip to content

Commit

Permalink
fix #11681 and some other stuff
Browse files Browse the repository at this point in the history
  • Loading branch information
zackradisic committed Jun 9, 2024
1 parent 2cba070 commit a62a62f
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 24 deletions.
8 changes: 8 additions & 0 deletions src/cli.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,8 @@ pub const HelpCommand = struct {
\\ <b><blue>update<r> <d>{s:<16}<r> Update outdated dependencies
\\ <b><blue>link<r> <d>[\<package\>]<r> Register or link a local npm package
\\ <b><blue>unlink<r> Unregister a local npm package
\\ <b><blue>patch <d>\<pkg\><r> Prepare a package for patching
\\ <b><blue>patch-commit <d>\<folder\><r> Install a modified package
\\ <b><blue>pm <d>\<subcommand\><r> Additional package management utilities
\\
\\ <b><yellow>build<r> <d>./a.ts ./b.jsx<r> Bundle TypeScript & JavaScript into a single file
Expand Down Expand Up @@ -2293,6 +2295,12 @@ pub const Command = struct {
Output.pretty("<b>Usage<r>: <b><green>bun completions<r>", .{});
Output.flush();
},
Command.Tag.PatchCommand => {
Install.PackageManager.CommandLineArguments.printHelp(.patch);
},
Command.Tag.PatchCommitCommand => {
Install.PackageManager.CommandLineArguments.printHelp(.@"patch-commit");
},
Command.Tag.ExecCommand => {
Output.pretty(
\\<b>Usage: bun exec <r><cyan>\<script\><r>
Expand Down
82 changes: 58 additions & 24 deletions src/install/install.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,6 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type {
return strings.eqlLong(repo.resolved.slice(buf), bun_tag_file, true);
}

// TODO: patched dependencies
pub fn verify(
this: *@This(),
resolution: *const Resolution,
Expand Down Expand Up @@ -2717,6 +2716,8 @@ pub const PackageManager = struct {
// dependency name -> original version information
updating_packages: bun.StringArrayHashMapUnmanaged(PackageUpdateInfo) = .{},

patched_dependencies_to_remove: std.ArrayHashMapUnmanaged(PackageNameAndVersionHash, void, ArrayIdentityContext.U64, false) = .{},

pub const PackageUpdateInfo = struct {
original_version_literal: string,
is_alias: bool,
Expand Down Expand Up @@ -5569,7 +5570,6 @@ pub const PackageManager = struct {
manager.network_resolve_batch = .{};
manager.patch_apply_batch = .{};
manager.patch_calc_hash_batch = .{};
// TODO probably have to put patch tasks here
return count;
}

Expand Down Expand Up @@ -7195,10 +7195,10 @@ pub const PackageManager = struct {

if (subcommand == .patch) {
// TODO args
} else if (subcommand == .patch_commit) {
} else if (subcommand == .@"patch-commit") {
this.patch_features = .{
.commit = .{
.patches_dir = cli.patch_commit.patches_dir,
.patches_dir = cli.@"patch-commit".patches_dir,
},
};
}
Expand Down Expand Up @@ -8145,7 +8145,7 @@ pub const PackageManager = struct {
link,
unlink,
patch,
patch_commit,
@"patch-commit",
};

pub fn init(ctx: Command.Context, comptime subcommand: Subcommand) !*PackageManager {
Expand Down Expand Up @@ -8634,7 +8634,7 @@ pub const PackageManager = struct {
}

pub inline fn patchCommit(ctx: Command.Context) !void {
try updatePackageJSONAndInstallCatchError(ctx, .patch_commit);
try updatePackageJSONAndInstallCatchError(ctx, .@"patch-commit");
}

pub inline fn update(ctx: Command.Context) !void {
Expand Down Expand Up @@ -9068,7 +9068,7 @@ pub const PackageManager = struct {
concurrent_scripts: ?usize = null,

patch: PatchOpts = .{},
patch_commit: PatchCommitOpts = .{},
@"patch-commit": PatchCommitOpts = .{},

const PatchOpts = struct {
edit_dir: ?[]const u8 = null,
Expand Down Expand Up @@ -9163,7 +9163,7 @@ pub const PackageManager = struct {
// Output.pretty("\n\n" ++ outro_text ++ "\n", .{});
Output.flush();
},
Subcommand.patch_commit => {
Subcommand.@"patch-commit" => {
const intro_text =
\\<b>Usage: bun patch-commit <r><cyan>\<directory\><r>
\\
Expand Down Expand Up @@ -9300,7 +9300,7 @@ pub const PackageManager = struct {
.link => link_params,
.unlink => unlink_params,
.patch => patch_params,
.patch_commit => patch_commit_params,
.@"patch-commit" => patch_commit_params,
};

var diag = clap.Diagnostic{};
Expand Down Expand Up @@ -9338,7 +9338,6 @@ pub const PackageManager = struct {

// link and unlink default to not saving, all others default to
// saving.
// TODO: I think `bun patch` command goes here
if (comptime subcommand == .link or subcommand == .unlink) {
cli.no_save = !args.flag("--save");
} else {
Expand All @@ -9349,8 +9348,8 @@ pub const PackageManager = struct {
cli.patch = .{};
}

if (comptime subcommand == .patch_commit) {
cli.patch_commit = .{
if (comptime subcommand == .@"patch-commit") {
cli.@"patch-commit" = .{
.patches_dir = args.option("--patches-dir") orelse "patches",
};
}
Expand Down Expand Up @@ -9430,7 +9429,7 @@ pub const PackageManager = struct {
Global.crash();
}

if (subcommand == .patch_commit and cli.positionals.len < 2) {
if (subcommand == .@"patch-commit" and cli.positionals.len < 2) {
Output.errGeneric("Missing pkg folder to patch\n", .{});
Global.crash();
}
Expand Down Expand Up @@ -9802,7 +9801,7 @@ pub const PackageManager = struct {
}
}
},
.patch_commit => {
.@"patch-commit" => {
_ = manager.lockfile.loadFromDisk(
manager,
manager.allocator,
Expand Down Expand Up @@ -10037,10 +10036,10 @@ pub const PackageManager = struct {
fn preparePatch(manager: *PackageManager) !void {
const @"pkg + maybe version to patch" = manager.options.positionals[1];
const name: []const u8, const version: ?[]const u8 = brk: {
if (std.mem.indexOfScalar(u8, @"pkg + maybe version to patch", '@')) |version_delimiter| {
if (std.mem.indexOfScalar(u8, @"pkg + maybe version to patch"[1..], '@')) |version_delimiter| {
break :brk .{
@"pkg + maybe version to patch"[0..version_delimiter],
@"pkg + maybe version to patch"[version_delimiter + 1 ..],
@"pkg + maybe version to patch"[1..][version_delimiter + 1 ..],
};
}
break :brk .{
Expand Down Expand Up @@ -11056,14 +11055,29 @@ pub const PackageManager = struct {
break :brk "";
} else std.fmt.bufPrint(&resolution_buf, "{}", .{resolution.fmt(buf, .posix)}) catch unreachable;

const patch_patch, const patch_contents_hash, const patch_name_and_version_hash = brk: {
if (this.manager.lockfile.patched_dependencies.entries.len == 0) break :brk .{ null, null, null };
if (std.mem.eql(u8, "is-even", name)) {
std.debug.print("HELLO!\n", .{});
}

const patch_patch, const patch_contents_hash, const patch_name_and_version_hash, const remove_patch = brk: {
if (this.manager.lockfile.patched_dependencies.entries.len == 0 and this.manager.patched_dependencies_to_remove.entries.len) break :brk .{ null, null, null, false };
var sfb = std.heap.stackFallback(1024, this.lockfile.allocator);
const name_and_version = std.fmt.allocPrint(sfb.get(), "{s}@{s}", .{ name, package_version }) catch unreachable;
defer sfb.get().free(name_and_version);
const name_and_version_hash = String.Builder.stringHash(name_and_version);

const patchdep = this.lockfile.patched_dependencies.get(name_and_version_hash) orelse break :brk .{ null, null, null };
const patchdep = this.lockfile.patched_dependencies.get(name_and_version_hash) orelse {
const to_remove = this.manager.patched_dependencies_to_remove.contains(name_and_version_hash);
if (to_remove) {
break :brk .{
null,
null,
name_and_version_hash,
true,
};
}
break :brk .{ null, null, null, false };
};
bun.assert(!patchdep.patchfile_hash_is_null);
// if (!patchdep.patchfile_hash_is_null) {
// this.manager.enqueuePatchTask(PatchTask.newCalcPatchHash(this, package_id, name_and_version_hash, dependency_id, url: string))
Expand All @@ -11072,6 +11086,7 @@ pub const PackageManager = struct {
patchdep.path.slice(this.lockfile.buffers.string_bytes.items),
patchdep.patchfileHash().?,
name_and_version_hash,
false,
};
};

Expand Down Expand Up @@ -11200,15 +11215,15 @@ pub const PackageManager = struct {
},
}

const needs_install = this.force_install or this.skip_verify_installed_version_number or !needs_verify or !installer.verify(
const needs_install = this.force_install or this.skip_verify_installed_version_number or !needs_verify or remove_patch or !installer.verify(
resolution,
buf,
this.root_node_modules_folder,
);
this.summary.skipped += @intFromBool(!needs_install);

if (needs_install) {
if (resolution.tag.canEnqueueInstallTask() and installer.packageMissingFromCache(this.manager, package_id)) {
if (!remove_patch and resolution.tag.canEnqueueInstallTask() and installer.packageMissingFromCache(this.manager, package_id)) {
if (comptime Environment.allow_assert) {
bun.assert(resolution.canEnqueueInstallTask());
}
Expand Down Expand Up @@ -11292,7 +11307,7 @@ pub const PackageManager = struct {
// above checks if unpatched package is in cache, if not null apply patch in temp directory, copy
// into cache, then install into node_modules
if (!installer.patch.isNull()) {
if (installer.patchedPackageMissingFromCache(this.manager, package_id, installer.patch.patch_contents_hash)) {
if (installer.patchedPackageMissingFromCache(this.manager, package_id)) {
const task = PatchTask.newApplyPatchHash(
this.manager,
package_id,
Expand Down Expand Up @@ -11337,7 +11352,7 @@ pub const PackageManager = struct {

const install_result = switch (resolution.tag) {
.symlink, .workspace => installer.installFromLink(this.skip_delete, destination_dir),
else => installer.install(this.skip_delete, destination_dir),
else => installer.install(this.skip_delete and (!remove_patch), destination_dir),
};

switch (install_result) {
Expand Down Expand Up @@ -12568,7 +12583,6 @@ pub const PackageManager = struct {
// Update patched dependencies
{
var iter = lockfile.patched_dependencies.iterator();
// TODO: if one key is present in manager.lockfile and not present in lockfile we should get rid of it
while (iter.next()) |entry| {
const pkg_name_and_version_hash = entry.key_ptr.*;
bun.debugAssert(entry.value_ptr.patchfile_hash_is_null);
Expand All @@ -12587,6 +12601,26 @@ pub const PackageManager = struct {
gop.value_ptr.setPatchfileHash(null);
}
}

var count: usize = 0;
iter = manager.lockfile.patched_dependencies.iterator();
while (iter.next()) |entry| {
if (!lockfile.patched_dependencies.contains(entry.key_ptr.*)) {
count += 1;
}
}
if (count > 0) {
try manager.patched_dependencies_to_remove.ensureTotalCapacity(manager.allocator, count);
iter = manager.lockfile.patched_dependencies.iterator();
while (iter.next()) |entry| {
if (!lockfile.patched_dependencies.contains(entry.key_ptr.*)) {
try manager.patched_dependencies_to_remove.put(manager.allocator, entry.key_ptr.*, {});
}
}
for (manager.patched_dependencies_to_remove.keys()) |hash| {
_ = manager.lockfile.patched_dependencies.orderedRemove(hash);
}
}
}

builder.clamp();
Expand Down
4 changes: 4 additions & 0 deletions src/install/lockfile.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3775,6 +3775,10 @@ pub const Package = extern struct {
)) break :patched_dependencies_changed true;
} else break :patched_dependencies_changed true;
}
iter = from_lockfile.patched_dependencies.iterator();
while (iter.next()) |entry| {
if (!to_lockfile.patched_dependencies.contains(entry.key_ptr.*)) break :patched_dependencies_changed true;
}
break :patched_dependencies_changed false;
};

Expand Down
104 changes: 104 additions & 0 deletions test/cli/install/bun-install-patch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,58 @@ index c8950c17b265104bcf27f8c345df1a1b13a78950..7ce57ab96400ab0ff4fac7e06f6e02c2
}
});

describe("should work when patches are removed", async () => {
for (const [version, patchVersion_] of versions) {
const patchFilename = filepathEscape(`is-even@${version}.patch`);
const patchVersion = patchVersion_ ?? version;
test(version, async () => {
const filedir = tempDirWithFiles("patch1", {
"package.json": JSON.stringify({
"name": "bun-patch-test",
"module": "index.ts",
"type": "module",
"patchedDependencies": {
[`is-even@${patchVersion}`]: `patches/${patchFilename}`,
},
"dependencies": {
"is-even": version,
},
}),
patches: {
[patchFilename]: is_even_patch2,
},
"index.ts": /* ts */ `import isEven from 'is-even'; isEven(2); console.log('lol')`,
});

console.log("FILEDIR", filedir);

await $`${bunExe()} i`.env(bunEnv).cwd(filedir);
throw new Error("woopsie!");

await $`echo ${JSON.stringify({
"name": "bun-patch-test",
"module": "index.ts",
"type": "module",
"patchedDependencies": {
[`[email protected]`]: `patches/[email protected]`,
},
"dependencies": {
"is-even": version,
},
})} > package.json`
.env(bunEnv)
.cwd(filedir);

await $`echo ${is_odd_patch2} > patches/[email protected]; ${bunExe()} i`.env(bunEnv).cwd(filedir);

const { stdout, stderr } = await $`${bunExe()} run index.ts`.env(bunEnv).cwd(filedir);
expect(stderr.toString()).toBe("");
expect(stdout.toString()).toContain("Hi from isOdd!\n");
expect(stdout.toString()).not.toContain("lmao\n");
});
}
});

it("should update a transitive dependency when the patchfile changes", async () => {
$.throws(true);
const filedir = tempDirWithFiles("patch1", {
Expand Down Expand Up @@ -379,4 +431,56 @@ index c8950c17b265104bcf27f8c345df1a1b13a78950..7ce57ab96400ab0ff4fac7e06f6e02c2
expect(stderr.toString()).toBe("");
expect(stdout.toString()).toContain("lmao\n");
});

it("should update a scoped package", async () => {
const patchfile = /* patch */ `diff --git a/private/var/folders/wy/3969rv2x63g63jf8jwlcb2x40000gn/T/.b7f7d77b9ffdd3ee-00000000.tmp/index.js b/index.js
new file mode 100644
index 0000000000000000000000000000000000000000..6edc0598a84632c41d9c770cfbbad7d99e2ab624
--- /dev/null
+++ b/index.js
@@ -0,0 +1,4 @@
+
+module.exports = () => {
+ return 'PATCHED!'
+}
diff --git a/package.json b/package.json
index aa7c7012cda790676032d1b01d78c0b69ec06360..6048e7cb462b3f9f6ac4dc21aacf9a09397cd4be 100644
--- a/package.json
+++ b/package.json
@@ -2,7 +2,7 @@
"name": "@zackradisic/hls-dl",
"version": "0.0.1",
"description": "",
- "main": "dist/hls-dl.commonjs2.js",
+ "main": "./index.js",
"dependencies": {
"m3u8-parser": "^4.5.0",
"typescript": "^4.0.5"
`;

$.throws(true);
const filedir = tempDirWithFiles("patch1", {
"package.json": JSON.stringify({
"name": "bun-patch-test",
"module": "index.ts",
"type": "module",
"patchedDependencies": {
"@zackradisic/[email protected]": "patches/thepatch.patch",
},
"dependencies": {
"@zackradisic/hls-dl": "0.0.1",
},
}),
patches: {
["thepatch.patch"]: patchfile,
},
"index.ts": /* ts */ `import hlsDl from '@zackradisic/hls-dl'; console.log(hlsDl())`,
});

await $`${bunExe()} i`.env(bunEnv).cwd(filedir);

const { stdout, stderr } = await $`${bunExe()} run index.ts`.env(bunEnv).cwd(filedir);
expect(stderr.toString()).toBe("");
expect(stdout.toString()).toContain("PATCHED!\n");
});
});

0 comments on commit a62a62f

Please sign in to comment.