Ensure <ref> is provided in github.com/<org>/<repo>/tarball/<ref>

GitLab package registry URLs like
`https://gitlab.example.com/api/v4/projects/1/packages/npm/@group/repo/-/@group/repo-x.y.y.tgz`
were incorrectly inferred as `git` type due to hostname pattern matching.

These URLs now correctly resolve as `tarball` type.

src/install/dependency.zig

@@ -212,28 +212,58 @@ pub inline fn isGitHubRepoPath(dependency: string) bool {
     return hash_index != dependency.len - 1 and first_slash_index > 0 and first_slash_index != dependency.len - 1;
 }
 
-/// Github allows for the following format of URL:
-/// https://github.com/<org>/<repo>/tarball/<ref>
-/// This is a legacy (but still supported) method of retrieving a tarball of an
-/// entire source tree at some git reference. (ref = branch, tag, etc. Note: branch
-/// can have arbitrary number of slashes)
+/// Test whether a string of the form github.com/<dependency-string> is a
+/// tarball.
 ///
-/// This also checks for a github url that ends with ".tar.gz"
+/// This, at best, runs heuristics to determine whether the path is a tarball
+/// or not.
+///
+/// TODO(markovejnovic): Do not solely rely on this function to determine
+/// whether something is a tarball or not. Ask the server.
 pub inline fn isGitHubTarballPath(dependency: string) bool {
-    if (isTarball(dependency)) return true;
+    var url_parts_it = strings.split(dependency, "/");
 
-    var parts = strings.split(dependency, "/");
+    // First let's grab the first URL path.
+    const username = url_parts_it.next() orelse return false;
+    _ = username;
 
-    var n_parts: usize = 0;
+    // Then let's grab the second part
+    const repo_name = url_parts_it.next() orelse {
+        // If we don't find a repo-name, then we're at a user's page or one of
+        // /about, /enterprise, etc. Either way, no tarball here.
 
-    while (parts.next()) |part| {
-        n_parts += 1;
-        if (n_parts == 3) {
-            return strings.eqlComptime(part, "tarball");
-        }
+        // Importantly, repo names may contain the words .tar.gz or .tgz and
+        // we should be really careful to not misinterpret those.
+        return false;
+    };
+    _ = repo_name;
+
+    const third_part = url_parts_it.next() orelse {
+        // If we don't find a third part, then we're definitely at a repo path.
+        // These are never tarballs.
+        return false;
+    };
+
+    // Github allows for the following format of URL:
+    // https://github.com/<org>/<repo>/tarball/<ref>
+    // This is a legacy (but still supported) method of retrieving a
+    // tarball of an entire source tree at some git reference. (ref =
+    // branch, tag, etc. Note: branch can have arbitrary number of slashes)
+    if (strings.eqlComptime(third_part, "tarball")) {
+        // The <ref> is mandatory!! There may actually be any more subpaths
+        // (for weird branch names, or not -- github ignores them)
+        return if (url_parts_it.next()) |ref| ref.len > 0 else false;
     }
 
-    return false;
+    // We're not done... Branches may end with .tar.gz...
+    if (strings.eqlComptime(third_part, "tree")) {
+        // We're looking at a branch and branches are definitely not tarballs.
+        return false;
+    }
+
+    // Excellent! Now we can test whether the part ends with .tar.gz or not and
+    // we're good to go.
+    return isTarball(dependency);
 }
 
 // This won't work for query string params, but I'll let someone file an issue
@@ -662,6 +692,10 @@ pub const Version = struct {
                                 if (isGitHubRepoPath(path)) return .github;
                             }
 
+                            if (isTarball(url)) {
+                                return .tarball;
+                            }
+
                             if (strings.indexOfChar(url, '.')) |dot| {
                                 if (Repository.Hosts.has(url[0..dot])) return .git;
                             }

test/cli/install/__snapshots__/bun-install-dep.test.ts.snap

@@ -395,3 +395,36 @@ exports[`dependencies: {"bar": "1.0.0 - 2.0.0"} 1`] = `
   "version": ">=1.0.0 <=2.0.0-bar",
 }
 `;
+
+exports[`npa https://gitlab.com/inkscape/inkscape/-/archive/INKSCAPE_1_4/inkscape-INKSCAPE_1_4.tar.gz 1`] = `
+{
+  "name": "",
+  "version": {
+    "name": "",
+    "type": "tarball",
+    "url": "https://gitlab.com/inkscape/inkscape/-/archive/INKSCAPE_1_4/inkscape-INKSCAPE_1_4.tar.gz",
+  },
+}
+`;
+
+exports[`npa file:./path/to/folder 1`] = `
+{
+  "name": "",
+  "version": {
+    "folder": "./path/to/folder",
+    "type": "folder",
+  },
+}
+`;
+
+exports[`npa https://github.com/Jarred-Sumner/test-tarball-url.tgz 1`] = `
+{
+  "name": "",
+  "version": {
+    "owner": "Jarred-Sumner",
+    "ref": "",
+    "repo": "test-tarball-url.tgz",
+    "type": "github",
+  },
+}
+`;

test/cli/install/bun-install-dep.test.ts

@@ -1,50 +1,61 @@
 import { npa } from "bun:internal-for-testing";
-import { expect, test } from "bun:test";
+import { describe, expect, test } from "bun:test";
 
-const bitbucket = [
-  "bitbucket:dylan-conway/public-install-test",
-  "bitbucket.org:dylan-conway/public-install-test",
-  "bitbucket.com:dylan-conway/public-install-test",
-  "git@bitbucket.org:dylan-conway/public-install-test",
-];
+const TEST_DEPENDENCIES_BY_TYPE = {
+  "dist_tag": ["package", "@scoped/package"],
+  "npm": [
+    "@scoped/package@1.0.0",
+    "@scoped/package@1.0.0-beta.1",
+    "@scoped/package@1.0.0-beta.1+build.123",
+    "package@1.0.0",
+    "package@1.0.0-beta.1",
+    "package@1.0.0-beta.1+build.123",
+  ],
+  "tarball": [
+    "./path/to/tarball.tgz",
+    "file:./path/to/tarball.tgz",
+    "http://localhost:5000/no-deps/-/no-deps-2.0.0.tgz",
+    "https://gitlab.com/inkscape/inkscape/-/archive/INKSCAPE_1_4/inkscape-INKSCAPE_1_4.tar.gz",
+    "https://registry.npmjs.org/no-deps/-/no-deps-2.0.0.tgz",
+  ],
+  "folder": ["file:./path/to/folder"],
+  "git": [
+    "bitbucket.com:dylan-conway/public-install-test",
+    "bitbucket.org:dylan-conway/public-install-test",
+    "bitbucket:dylan-conway/public-install-test",
+    "git@bitbucket.org:dylan-conway/public-install-test",
+    "git@github.com:dylan-conway/public-install-test",
+    "gitlab.com:dylan-conway/public-install-test",
+    "gitlab:dylan-conway/public-install-test",
+    "https://github.com/dylan-conway/public-install-test.git#semver:^1.0.0",
+  ],
+  "github": [
+    "foo/bar",
+    "github:dylan-conway/public-install-test",
+    "https://github.com/Jarred-Sumner/test-tarball-url.tgz",
+    "https://github.com/dylan-conway/public-install-test",
+    "https://github.com/dylan-conway/public-install-test.git",
+  ],
+};
 
-const tarball_remote = [
-  "http://localhost:5000/no-deps/-/no-deps-2.0.0.tgz",
-  "https://registry.npmjs.org/no-deps/-/no-deps-2.0.0.tgz",
-];
+const ALL_TEST_DEPENDENCIES = Object.values(TEST_DEPENDENCIES_BY_TYPE).flat();
 
-const local_tarball = ["file:./path/to/tarball.tgz", "./path/to/tarball.tgz"];
-const github = ["foo/bar"];
-const folder = ["file:./path/to/folder"];
-
-const gitlab = ["gitlab:dylan-conway/public-install-test", "gitlab.com:dylan-conway/public-install-test"];
-
-const all = [
-  "@scoped/package",
-  "@scoped/package@1.0.0",
-  "@scoped/package@1.0.0-beta.1",
-  "@scoped/package@1.0.0-beta.1+build.123",
-  "package",
-  "package@1.0.0",
-  "package@1.0.0-beta.1",
-  "package@1.0.0-beta.1+build.123",
-  ...bitbucket,
-  ...github,
-  ...gitlab,
-  ...tarball_remote,
-  ...local_tarball,
-  ...github,
-  "github:dylan-conway/public-install-test",
-  "git@github.com:dylan-conway/public-install-test",
-  "https://github.com/dylan-conway/public-install-test",
-  "https://github.com/dylan-conway/public-install-test.git",
-  "https://github.com/dylan-conway/public-install-test.git#semver:^1.0.0",
-];
-
-test.each(all)("npa %s", dep => {
+test.each(ALL_TEST_DEPENDENCIES)("npa %s", dep => {
   expect(npa(dep)).toMatchSnapshot();
 });
 
+describe("Dependency resolution", () => {
+  describe("Resolves to the correct type", () => {
+    const testSeries = Object.entries(TEST_DEPENDENCIES_BY_TYPE).flatMap(([key, depStrs]) =>
+      depStrs.map(dep => [dep, key]),
+    );
+
+    test.each(testSeries)("%s resolves as %s", (depStr, expectedType) => {
+      expect(npa(depStr).version.type).toBe(expectedType);
+    });
+  });
+});
+
 const pkgJsonLike = [
   ["foo", "1.2.3"],
   ["foo", "latest"],