ensure .bin folder is created before linking binaries (oven-sh#7166)

* bin might need to be created after iterating node_modules * Update bun-install-registry.test.ts * test this functionality in a few more tests * fix a test * another test * revert * more test
ryoppippi · Nov 17, 2023 · 3ca2d8a · 3ca2d8a
1 parent c23aed9
commit 3ca2d8a
Show file tree

Hide file tree

Showing 4 changed files with 551 additions and 42 deletions.
diff --git a/src/install/bin.zig b/src/install/bin.zig
@@ -2,6 +2,8 @@ const ExternalStringList = @import("./install.zig").ExternalStringList;
 const Semver = @import("./semver.zig");
 const ExternalString = Semver.ExternalString;
 const String = Semver.String;
+const Output = bun.Output;
+const Global = bun.Global;
 const std = @import("std");
 const strings = @import("root").bun.strings;
 const Environment = @import("../env.zig");
@@ -162,6 +164,7 @@ pub const Bin = extern struct {
     pub const Tag = enum(u8) {
         /// no bin field
         none = 0,
+
         /// "bin" is a string
         /// ```
         /// "bin": "./bin/foo",
@@ -175,13 +178,15 @@ pub const Bin = extern struct {
         /// }
         ///```
         named_file = 2,
+
         /// "bin" is a directory
         ///```
         /// "dirs": {
         ///     "bin": "./bin",
         /// }
         ///```
         dir = 3,
+
         // "bin" is a map of more than one
         ///```
         /// "bin": {
@@ -352,8 +357,31 @@ pub const Bin = extern struct {
 
             if (!link_global) {
                 const root_dir = std.fs.Dir{ .fd = bun.fdcast(this.package_installed_node_modules) };
-                const from = root_dir.realpath(dot_bin, &target_buf) catch |err| {
-                    this.err = err;
+                const from = root_dir.realpath(dot_bin, &target_buf) catch |realpath_err| brk: {
+                    if (realpath_err == error.FileNotFound) {
+                        if (comptime Environment.isWindows) {
+                            std.os.mkdiratW(root_dir.fd, bun.strings.w(".bin"), 0) catch |err| {
+                                this.err = err;
+                                return;
+                            };
+                        } else {
+                            root_dir.makeDirZ(".bin") catch |err| {
+                                this.err = err;
+                                return;
+                            };
+                        }
+
+                        if (comptime Environment.isPosix) {
+                            Bin.Linker.umask = C.umask(0);
+                        }
+
+                        break :brk root_dir.realpath(dot_bin, &target_buf) catch |err| {
+                            this.err = err;
+                            return;
+                        };
+                    }
+
+                    this.err = realpath_err;
                     return;
                 };
                 const to = bun.getFdPath(this.package_installed_node_modules, &dest_buf) catch |err| {

diff --git a/src/install/install.zig b/src/install/install.zig
@@ -7276,7 +7276,6 @@ pub const PackageManager = struct {
         bins: []const Bin,
         resolutions: []Resolution,
         node: *Progress.Node,
-        has_created_bin: bool = false,
         global_bin_dir: std.fs.IterableDir,
         destination_dir_subpath_buf: [bun.MAX_PATH_BYTES]u8 = undefined,
         folder_path_buf: [bun.MAX_PATH_BYTES]u8 = undefined,
@@ -7487,19 +7486,6 @@ pub const PackageManager = struct {
 
                         const bin = this.bins[package_id];
                         if (bin.tag != .none) {
-                            if (!this.has_created_bin) {
-                                if (!this.options.global) {
-                                    if (comptime Environment.isWindows) {
-                                        std.os.mkdiratW(this.node_modules_folder, strings.w(".bin"), 0) catch {};
-                                    } else {
-                                        this.node_modules_folder.dir.makeDirZ(".bin") catch {};
-                                    }
-                                }
-                                if (comptime Environment.isPosix)
-                                    Bin.Linker.umask = C.umask(0);
-                                this.has_created_bin = true;
-                            }
-
                             const bin_task_id = Task.Id.forBinLink(package_id);
                             var task_queue = this.manager.task_queue.getOrPut(this.manager.allocator, bin_task_id) catch unreachable;
                             if (!task_queue.found_existing) {
@@ -7961,9 +7947,6 @@ pub const PackageManager = struct {
                     break :brk try cwd.openIterableDir(node_modules.relative_path, .{});
                 };
 
-                // a .bin directory could be created in each node_modules directory
-                installer.has_created_bin = false;
-
                 var remaining = node_modules.dependencies;
 
                 // cache line is 64 bytes on ARM64 and x64
@@ -8068,19 +8051,6 @@ pub const PackageManager = struct {
 
                         const name = lockfile.str(&dependencies[dependency_id].name);
 
-                        if (!installer.has_created_bin) {
-                            if (!this.options.global) {
-                                if (comptime Environment.isWindows) {
-                                    std.os.mkdiratW(node_modules_folder.dir.fd, bun.strings.w(".bin"), 0) catch {};
-                                } else {
-                                    node_modules_folder.dir.makeDirZ(".bin") catch {};
-                                }
-                            }
-                            if (comptime Environment.isPosix)
-                                Bin.Linker.umask = C.umask(0);
-                            installer.has_created_bin = true;
-                        }
-
                         var bin_linker = Bin.Linker{
                             .bin = original_bin,
                             .package_installed_node_modules = bun.toFD(folder.dir.fd),

diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts
@@ -2571,7 +2571,7 @@ it("should handle aliased & direct dependency references", async () => {
     },
   });
   expect(await readdirSorted(join(package_dir, "bar"))).toEqual(["node_modules", "package.json"]);
-  expect(await readdirSorted(join(package_dir, "bar", "node_modules"))).toEqual([".bin", "moo"]);
+  expect(await readdirSorted(join(package_dir, "bar", "node_modules"))).toEqual(["moo"]);
   expect(await readdirSorted(join(package_dir, "bar", "node_modules", "moo"))).toEqual(["index.js", "package.json"]);
   expect(await file(join(package_dir, "bar", "node_modules", "moo", "package.json")).json()).toEqual({
     name: "baz",