Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(install): auto node-gyp script bugfix #9289

Merged
merged 2 commits into from
Mar 7, 2024
Merged

fix(install): auto node-gyp script bugfix #9289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c60711
preinstall and install
dylan-conway Mar 7, 2024
ffc0f0a
comment
dylan-conway Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/install/install.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8367,7 +8367,7 @@ pub const PackageManager = struct {
if (scripts.hasAny()) {
const add_node_gyp_rebuild_script = if (this.lockfile.hasTrustedDependency(folder_name) and
scripts.install.isEmpty() and
scripts.postinstall.isEmpty())
scripts.preinstall.isEmpty())
brk: {
const binding_dot_gyp_path = Path.joinAbsStringZ(
this.node_modules_folder_path.items,
Expand Down Expand Up @@ -9450,7 +9450,7 @@ pub const PackageManager = struct {
.auto,
);
if (root.scripts.hasAny()) {
const add_node_gyp_rebuild_script = root.scripts.install.isEmpty() and root.scripts.postinstall.isEmpty() and Syscall.exists(binding_dot_gyp_path);
const add_node_gyp_rebuild_script = root.scripts.install.isEmpty() and root.scripts.preinstall.isEmpty() and Syscall.exists(binding_dot_gyp_path);

manager.root_lifecycle_scripts = root.scripts.enqueue(
manager.lockfile,
Expand Down
47 changes: 29 additions & 18 deletions src/install/lockfile.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2341,6 +2341,14 @@ pub const Package = extern struct {

meta: Meta = .{},
bin: Bin = .{},

/// If any of these scripts run, they will run in order:
/// 1. preinstall
/// 2. install
/// 3. postinstall
/// 4. preprepare
/// 5. prepare
/// 6. postprepare
scripts: Package.Scripts = .{},

pub const Scripts = extern struct {
Expand Down Expand Up @@ -2405,8 +2413,25 @@ pub const Package = extern struct {
var counter: u8 = 0;

if (add_node_gyp_rebuild_script) {
// missing install and postinstall, only need to check preinstall
if (!this.preinstall.isEmpty()) {
{
script_index += 1;
const entry: Lockfile.Scripts.Entry = .{
.cwd = cwd orelse brk: {
cwd = lockfile.allocator.dupe(u8, _cwd) catch unreachable;
break :brk cwd.?;
},
.script = lockfile.allocator.dupe(u8, "node-gyp rebuild") catch unreachable,
.package_name = package_name,
};
if (first_script_index == -1) first_script_index = @intCast(script_index);
scripts[script_index] = entry;
script_index += 1;
lockfile.scripts.install.append(lockfile.allocator, entry) catch unreachable;
counter += 1;
}

// missing install and preinstall, only need to check postinstall
if (!this.postinstall.isEmpty()) {
const entry: Lockfile.Scripts.Entry = .{
.cwd = cwd orelse brk: {
cwd = lockfile.allocator.dupe(u8, _cwd) catch unreachable;
Expand All @@ -2417,24 +2442,10 @@ pub const Package = extern struct {
};
if (first_script_index == -1) first_script_index = @intCast(script_index);
scripts[script_index] = entry;
lockfile.scripts.preinstall.append(lockfile.allocator, entry) catch unreachable;
lockfile.scripts.postinstall.append(lockfile.allocator, entry) catch unreachable;
counter += 1;
}
script_index += 1;

const entry: Lockfile.Scripts.Entry = .{
.cwd = cwd orelse brk: {
cwd = lockfile.allocator.dupe(u8, _cwd) catch unreachable;
break :brk cwd.?;
},
.script = lockfile.allocator.dupe(u8, "node-gyp rebuild") catch unreachable,
.package_name = package_name,
};
if (first_script_index == -1) first_script_index = @intCast(script_index);
scripts[script_index] = entry;
script_index += 2;
lockfile.scripts.install.append(lockfile.allocator, entry) catch unreachable;
counter += 1;
} else {
const install_scripts = .{
"preinstall",
Expand Down Expand Up @@ -2608,7 +2619,7 @@ pub const Package = extern struct {

const add_node_gyp_rebuild_script = if (lockfile.hasTrustedDependency(folder_name) and
this.install.isEmpty() and
this.postinstall.isEmpty())
this.preinstall.isEmpty())
brk: {
const binding_dot_gyp_path = Path.joinAbsStringZ(
cwd,
Expand Down
6 changes: 3 additions & 3 deletions test/cli/install/registry/bun-install-registry.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@

[err, out] = await Promise.all([new Response(stderr).text(), new Response(stdout).text()]);

expect(err).toContain("Saved lockfile");

Check failure on line 643 in test/cli/install/registry/bun-install-registry.test.ts

View workflow job for this annotation

GitHub Actions / Tests bun-darwin-aarch64

error: expect(received).toContain(expected) 

Expected to contain: "Saved lockfile"
Received: "bun install v1.0.30 (4aa3c39c)\n Resolving dependencies\n Resolved, downloaded and extracted [40]\nerror: failed to open lockfile: ENOENT\n"

      at /Users/runner/work/bun/bun/test/cli/install/registry/bun-install-registry.test.ts:643:5
expect(err).not.toContain("not found");
expect(err).not.toContain("error:");
expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
Expand Down Expand Up @@ -1548,7 +1548,7 @@
" 1 package installed",
]);
expect(await exited).toBe(0);
expect(Bun.which("what-bin", { PATH: join(packageDir, "node_modules", ".bin") })).toBe(

Check failure on line 1551 in test/cli/install/registry/bun-install-registry.test.ts

View workflow job for this annotation

GitHub Actions / Tests bun-darwin-aarch64

error: expect(received).toBe(expected) 

Expected: "/Users/runner/work/_temp/bun-test-tmp-386613752_91z8/bun-install-registry-167-CvqQg5/node_modules/.bin/what-bin"
Received: null

      at /Users/runner/work/bun/bun/test/cli/install/registry/bun-install-registry.test.ts:1551:3
join(packageDir, "node_modules", ".bin", "what-bin"),
);
expect(await file(join(packageDir, "node_modules", ".bin", "what-bin")).text()).toContain("[email protected]");
Expand Down Expand Up @@ -2586,7 +2586,7 @@
expect(await exists(join(packageDir, "build.node"))).toBeTrue();
});

test("auto node-gyp scripts work when scripts exists other than `install` and `postinstall`", async () => {
test("auto node-gyp scripts work when scripts exists other than `install` and `preinstall`", async () => {
await writeFile(
join(packageDir, "package.json"),
JSON.stringify({
Expand All @@ -2596,7 +2596,7 @@
"node-gyp": "1.5.0",
},
scripts: {
preinstall: "exit 0",
postinstall: "exit 0",
prepare: "exit 0",
postprepare: "exit 0",
},
Expand Down Expand Up @@ -2629,7 +2629,7 @@
expect(await exists(join(packageDir, "build.node"))).toBeTrue();
});

for (const script of ["install", "postinstall"]) {
for (const script of ["install", "preinstall"]) {
test(`does not add auto node-gyp script when ${script} script exists`, async () => {
const packageJSON: any = {
name: "foo",
Expand Down
Loading