Fix bunx command for github package #5974 (#6042)

* fix bunx command for github package

* refactor fmt package to use it when the path is seted

* use labeled block to assign const instead of use 'undefined'

3 files changed, 72 insertions, 18 deletions

diff --git a/src/cli/bunx_command.zig b/src/cli/bunx_command.zig
index 2643d33a6..dba2d5664 100644
--- a/src/cli/bunx_command.zig
+++ b/src/cli/bunx_command.zig
@@ -262,22 +262,37 @@ pub const BunxCommand = struct {
         else
             update_request.version.literal.slice(update_request.version_buf);
 
+        const package_fmt: []const u8 = brk: {
+            if (!strings.eql(update_request.version_buf, update_request.name)) {
+                break :brk try std.fmt.allocPrint(
+                    ctx.allocator,
+                    "{s}@{s}",
+                    .{
+                        update_request.name,
+                        display_version,
+                    },
+                );
+            }
+
+            break :brk update_request.name;
+        };
+
         const PATH_FOR_BIN_DIRS = PATH;
         if (PATH.len > 0) {
             PATH = try std.fmt.allocPrint(
                 ctx.allocator,
-                bun.fs.FileSystem.RealFS.PLATFORM_TMP_DIR ++ "/{s}@{s}--bunx/node_modules/.bin:{s}",
-                .{ update_request.name, display_version, PATH },
+                bun.fs.FileSystem.RealFS.PLATFORM_TMP_DIR ++ "/{s}--bunx/node_modules/.bin:{s}",
+                .{ package_fmt, PATH },
             );
         } else {
             PATH = try std.fmt.allocPrint(
                 ctx.allocator,
-                bun.fs.FileSystem.RealFS.PLATFORM_TMP_DIR ++ "/{s}@{s}--bunx/node_modules/.bin",
-                .{ update_request.name, display_version },
+                bun.fs.FileSystem.RealFS.PLATFORM_TMP_DIR ++ "/{s}--bunx/node_modules/.bin",
+                .{package_fmt},
             );
         }
         try this_bundler.env.map.put("PATH", PATH);
-        const bunx_cache_dir = PATH[0 .. bun.fs.FileSystem.RealFS.PLATFORM_TMP_DIR.len + "/--bunx@".len + update_request.name.len + display_version.len];
+        const bunx_cache_dir = PATH[0 .. bun.fs.FileSystem.RealFS.PLATFORM_TMP_DIR.len + "/--bunx".len + package_fmt.len];
 
         var absolute_in_cache_dir_buf: [bun.MAX_PATH_BYTES]u8 = undefined;
         var absolute_in_cache_dir = std.fmt.bufPrint(&absolute_in_cache_dir_buf, "/{s}/node_modules/.bin/{s}", .{ bunx_cache_dir, initial_bin_name }) catch unreachable;
@@ -362,8 +377,8 @@ pub const BunxCommand = struct {
 
         var bunx_install_dir_path = try std.fmt.allocPrint(
             ctx.allocator,
-            bun.fs.FileSystem.RealFS.PLATFORM_TMP_DIR ++ "/{s}@{s}--bunx",
-            .{ update_request.name, display_version },
+            bun.fs.FileSystem.RealFS.PLATFORM_TMP_DIR ++ "/{s}--bunx",
+            .{package_fmt},
         );
 
         // TODO: fix this after zig upgrade
@@ -378,19 +393,13 @@ pub const BunxCommand = struct {
         }
 
         var args_buf = [_]string{
-            try std.fs.selfExePathAlloc(ctx.allocator), "add", "--no-summary",
-            try std.fmt.allocPrint(
-                ctx.allocator,
-                "{s}@{s}",
-                .{
-                    update_request.name,
-                    display_version,
-                },
-            ),
+            try std.fs.selfExePathAlloc(ctx.allocator), "add",        "--no-summary",
+            package_fmt,
             // disable the manifest cache when a tag is specified
             // so that @latest is fetched from the registry
-            "--no-cache",
+                                           "--no-cache",
         };
+
         const argv_to_use: []const string = args_buf[0 .. args_buf.len - @as(usize, @intFromBool(update_request.version.tag != .dist_tag))];
 
         var child_process = std.ChildProcess.init(argv_to_use, default_allocator);
diff --git a/src/install/install.zig b/src/install/install.zig
index 65cc00d12..cee080116 100644
--- a/src/install/install.zig
+++ b/src/install/install.zig
@@ -6266,6 +6266,9 @@ pub const PackageManager = struct {
                     request.is_aliased = true;
                     request.name = allocator.dupe(u8, name) catch unreachable;
                     request.name_hash = String.Builder.stringHash(name);
+                } else if (version.tag == .github and version.value.github.committish.isEmpty()) {
+                    request.name = input;
+                    request.name_hash = String.Builder.stringHash(version.literal.slice(input));
                 } else {
                     request.name_hash = String.Builder.stringHash(version.literal.slice(input));
                 }
@@ -8179,6 +8182,7 @@ test "UpdateRequests.parse" {
         "baz",
         "boo@1.0.0",
         "bing@latest",
+        "github:bar/foo",
     };
     var reqs = PackageManager.UpdateRequest.parse(default_allocator, &log, updates, &array, .add);
 
@@ -8187,11 +8191,12 @@ test "UpdateRequests.parse" {
     try std.testing.expectEqualStrings(reqs[2].name, "bar");
     try std.testing.expectEqualStrings(reqs[3].name, "baz");
     try std.testing.expectEqualStrings(reqs[4].name, "boo");
+    try std.testing.expectEqualStrings(reqs[7].name, "github:bar/foo");
     try std.testing.expectEqual(reqs[4].version.tag, Dependency.Version.Tag.npm);
     try std.testing.expectEqualStrings(reqs[4].version.literal.slice("boo@1.0.0"), "1.0.0");
     try std.testing.expectEqual(reqs[5].version.tag, Dependency.Version.Tag.dist_tag);
     try std.testing.expectEqualStrings(reqs[5].version.literal.slice("bing@1.0.0"), "latest");
-    try std.testing.expectEqual(updates.len, 6);
+    try std.testing.expectEqual(updates.len, 7);
 }
 
 test "PackageManager.Options - default registry, default values" {
diff --git a/test/cli/install/bunx.test.ts b/test/cli/install/bunx.test.ts
index c353aa123..d60e80995 100644
--- a/test/cli/install/bunx.test.ts
+++ b/test/cli/install/bunx.test.ts
@@ -156,3 +156,43 @@ console.log(
   expect(await exited).toBe(0);
   expect(await readdirSorted(x_dir)).toEqual(["test.js"]);
 });
+
+it("should work for github repository", async () => {
+  await rm(join(await realpath(tmpdir()), "@withfig"), { force: true, recursive: true });
+  // without cache
+  const withoutCache = spawn({
+    cmd: [bunExe(), "x", "github:piuccio/cowsay", "--help"],
+    cwd: x_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+
+  expect(withoutCache.stderr).toBeDefined();
+  let err = await new Response(withoutCache.stderr).text();
+  expect(err).not.toContain("error");
+  expect(withoutCache.stdout).toBeDefined();
+  let out = await new Response(withoutCache.stdout).text();
+  expect(out.trim()).toContain("Usage: cowsay");
+  expect(await withoutCache.exited).toBe(0);
+
+  // cached
+  const cached = spawn({
+    cmd: [bunExe(), "x", "github:piuccio/cowsay", "--help"],
+    cwd: x_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+
+  expect(cached.stderr).toBeDefined();
+  err = await new Response(cached.stderr).text();
+  expect(err).not.toContain("error");
+  expect(cached.stdout).toBeDefined();
+  out = await new Response(cached.stdout).text();
+  expect(out.trim()).toContain("Usage: cowsay");
+  expect(await cached.exited).toBe(0);
+});
+