also looking at this
fix(discovery): a package's own bin programs outrank its nested npm copies
#271
1 file
+65
-9
| @@ -737,21 +737,14 @@ fn discoverNpmBinaries( | ||
| 737 | 737 | } |
| 738 | 738 | |
| 739 | 739 | /// Discover binaries in a package directory |
| 740 | /// 1. Checks for npm package structure (lib/node_modules/*/bin/*) | |
| 741 | /// 2. Scans standard bin/ and sbin/ directories | |
| 740 | /// 1. Scans standard bin/ and sbin/ directories | |
| 741 | /// 2. Falls back to npm package structure (lib/node_modules/*/bin/*) | |
| 742 | 742 | /// 3. Falls back to scanning the package root for native executables |
| 743 | 743 | /// (handles packages like zig that ship the binary at root level) |
| 744 | 744 | pub fn discoverBinaries( |
| 745 | 745 | allocator: std.mem.Allocator, |
| 746 | 746 | package_dir: []const u8, |
| 747 | 747 | ) ![]BinaryInfo { |
| 748 | // First, try to find npm binaries (these take precedence) | |
| 749 | const npm_bins = try discoverNpmBinaries(allocator, package_dir); | |
| 750 | if (npm_bins.len > 0) { | |
| 751 | return npm_bins; | |
| 752 | } | |
| 753 | defer allocator.free(npm_bins); | |
| 754 | ||
| 755 | 748 | var binaries = try std.ArrayList(BinaryInfo).initCapacity(allocator, 8); |
| 756 | 749 | errdefer { |
| 757 | 750 | for (binaries.items) |*bin| { |
| @@ -797,6 +790,20 @@ pub fn discoverBinaries( | ||
| 797 | 790 | } |
| 798 | 791 | } |
| 799 | 792 | |
| 793 | // Fallback: a package that ships no bin/ or sbin/ programs of its own may still | |
| 794 | // carry programs in an npm layout. This is consulted second, not first: a recipe's | |
| 795 | // own wrappers in bin/ (npmjs.com replaces bin/npx with a shim) must keep owning | |
| 796 | // their names, and the nested directory also holds files that are executable | |
| 797 | // without being commands, such as npm-cli.js. | |
| 798 | if (binaries.items.len == 0) { | |
| 799 | const npm_bins = try discoverNpmBinaries(allocator, package_dir); | |
| 800 | if (npm_bins.len > 0) { | |
| 801 | binaries.deinit(allocator); | |
| 802 | return npm_bins; | |
| 803 | } | |
| 804 | allocator.free(npm_bins); | |
| 805 | } | |
| 806 | ||
| 800 | 807 | // Fallback: if no binaries found in bin/sbin, scan the package root for native executables. |
| 801 | 808 | // This handles packages (e.g. zig) that ship the binary at the top level instead of bin/. |
| 802 | 809 | // Only pick up files that are executable and look like native binaries (no extension), |
| @@ -1200,6 +1207,55 @@ test "discoverBinaries" { | ||
| 1200 | 1207 | try std.testing.expectEqualStrings("testbin", binaries[0].name); |
| 1201 | 1208 | } |
| 1202 | 1209 | |
| 1210 | test "discoverBinaries keeps a package's own bin programs over its nested npm copies" { | |
| 1211 | const allocator = std.testing.allocator; | |
| 1212 | ||
| 1213 | // Mirrors the npmjs.com layout: the recipe's own bin/ wrappers (one of which is a | |
| 1214 | // deliberate shim) alongside the upstream package's nested bin directory, which | |
| 1215 | // also carries non-command files such as npm-cli.js. | |
| 1216 | const test_dir = "test_pkg_npm_shadow"; | |
| 1217 | io_helper.makePath(test_dir) catch {}; | |
| 1218 | defer io_helper.deleteTree(test_dir) catch {}; | |
| 1219 | ||
| 1220 | const own_bin = try std.fmt.allocPrint(allocator, "{s}/bin", .{test_dir}); | |
| 1221 | defer allocator.free(own_bin); | |
| 1222 | try io_helper.makePath(own_bin); | |
| 1223 | ||
| 1224 | const nested_bin = try std.fmt.allocPrint(allocator, "{s}/lib/node_modules/npm/bin", .{test_dir}); | |
| 1225 | defer allocator.free(nested_bin); | |
| 1226 | try io_helper.makePath(nested_bin); | |
| 1227 | ||
| 1228 | const own_programs = [_][]const u8{ "npm", "npx" }; | |
| 1229 | for (own_programs) |name| { | |
| 1230 | const path = try std.fmt.allocPrint(allocator, "{s}/{s}", .{ own_bin, name }); | |
| 1231 | defer allocator.free(path); | |
| 1232 | const file = try io_helper.cwd().createFile(io_helper.io, path, .{ .permissions = .executable_file }); | |
| 1233 | file.close(io_helper.io); | |
| 1234 | } | |
| 1235 | ||
| 1236 | const nested_programs = [_][]const u8{ "npm", "npx", "npm-cli.js" }; | |
| 1237 | for (nested_programs) |name| { | |
| 1238 | const path = try std.fmt.allocPrint(allocator, "{s}/{s}", .{ nested_bin, name }); | |
| 1239 | defer allocator.free(path); | |
| 1240 | const file = try io_helper.cwd().createFile(io_helper.io, path, .{ .permissions = .executable_file }); | |
| 1241 | file.close(io_helper.io); | |
| 1242 | } | |
| 1243 | ||
| 1244 | const binaries = try discoverBinaries(allocator, test_dir); | |
| 1245 | defer { | |
| 1246 | for (binaries) |*bin| { | |
| 1247 | var b = bin.*; | |
| 1248 | b.deinit(allocator); | |
| 1249 | } | |
| 1250 | allocator.free(binaries); | |
| 1251 | } | |
| 1252 | ||
| 1253 | try std.testing.expectEqual(@as(usize, own_programs.len), binaries.len); | |
| 1254 | for (binaries) |bin| { | |
| 1255 | try std.testing.expect(std.mem.indexOf(u8, bin.path, "node_modules") == null); | |
| 1256 | } | |
| 1257 | } | |
| 1258 | ||
| 1203 | 1259 | test "createShimsFromBinConfig normalizes scoped bin object keys" { |
| 1204 | 1260 | const allocator = std.testing.allocator; |
| 1205 | 1261 | const test_dir = "test_scoped_npm_shim"; |