Skip to content

Commit

Permalink
fix(install): auto node-gyp script bugfix (#9289)
Browse files Browse the repository at this point in the history
* preinstall and install

* comment
  • Loading branch information
dylan-conway committed Mar 7, 2024
1 parent 3f12d71 commit ea6bf12
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 23 deletions.
4 changes: 2 additions & 2 deletions src/install/install.zig
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -2586,7 +2586,7 @@ for (const forceWaiterThread of [false, true]) {
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 @@ for (const forceWaiterThread of [false, true]) {
"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 @@ for (const forceWaiterThread of [false, true]) {
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

0 comments on commit ea6bf12

Please sign in to comment.