Skip to content

Commit

Permalink
fix semver and repository host bugs (#6312)
Browse files Browse the repository at this point in the history
* releases before prereleases

* handle different repo hosts

* remove log

* Update bun-install.test.ts

* test for `bun add`

* gitlab test

* use comptime hash map, another test case

* don't need length

* bump timeout, use tld

* infer git dependencies for https and ssh
  • Loading branch information
dylan-conway authored Oct 5, 2023
1 parent 5a315f4 commit 4a2e157
Show file tree
Hide file tree
Showing 4 changed files with 271 additions and 18 deletions.
22 changes: 22 additions & 0 deletions src/install/dependency.zig
Original file line number Diff line number Diff line change
Expand Up @@ -513,15 +513,37 @@ pub const Version = struct {
},
else => {},
}

if (strings.hasPrefixComptime(url, "github.com/")) {
const path = url["github.com/".len..];
if (isGitHubTarballPath(path)) return .tarball;
if (isGitHubRepoPath(path)) return .github;
}

if (strings.indexOfChar(url, '.')) |dot| {
if (Repository.Hosts.has(url[0..dot])) return .git;
}

return .tarball;
}
}
},
's' => {
if (strings.hasPrefixComptime(dependency, "ssh")) {
var url = dependency["ssh".len..];
if (url.len > 2) {
if (url[0] == ':') {
if (strings.hasPrefixComptime(url, "://")) {
url = url["://".len..];
}
}

if (strings.indexOfChar(url, '.')) |dot| {
if (Repository.Hosts.has(url[0..dot])) return .git;
}
}
}
},
// lisp.tgz
// lisp/repo
// link:path/to/foo
Expand Down
29 changes: 16 additions & 13 deletions src/install/npm.zig
Original file line number Diff line number Diff line change
Expand Up @@ -798,29 +798,32 @@ pub const PackageManifest = struct {
return this.findByVersion(left.version);
}

const releases = this.pkg.releases.keys.get(this.versions);
if (this.findByDistTag("latest")) |result| {
if (group.satisfies(result.version)) {
return result;
}
}

if (group.flags.isSet(Semver.Query.Group.Flags.pre)) {
const prereleases = this.pkg.prereleases.keys.get(this.versions);
var i = prereleases.len;
{
const releases = this.pkg.releases.keys.get(this.versions);
var i = releases.len;
// For now, this is the dumb way
while (i > 0) : (i -= 1) {
const version = prereleases[i - 1];
const packages = this.pkg.prereleases.values.get(this.package_versions);
const version = releases[i - 1];
const packages = this.pkg.releases.values.get(this.package_versions);

if (group.satisfies(version)) {
return .{ .version = version, .package = &packages[i - 1] };
}
}
} else if (this.findByDistTag("latest")) |result| {
if (group.satisfies(result.version)) return result;
}

{
var i = releases.len;
// // For now, this is the dumb way
if (group.flags.isSet(Semver.Query.Group.Flags.pre)) {
const prereleases = this.pkg.prereleases.keys.get(this.versions);
var i = prereleases.len;
while (i > 0) : (i -= 1) {
const version = releases[i - 1];
const packages = this.pkg.releases.values.get(this.package_versions);
const version = prereleases[i - 1];
const packages = this.pkg.prereleases.values.get(this.package_versions);

if (group.satisfies(version)) {
return .{ .version = version, .package = &packages[i - 1] };
Expand Down
28 changes: 25 additions & 3 deletions src/install/repository.zig
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ pub const Repository = extern struct {
resolved: GitSHA = .{},
package_name: String = .{},

pub const Hosts = bun.ComptimeStringMap(string, .{
.{ "bitbucket", ".org" },
.{ "github", ".com" },
.{ "gitlab", ".com" },
});

pub fn verify(this: *const Repository) void {
this.owner.assertDefined();
this.repo.assertDefined();
Expand Down Expand Up @@ -125,15 +131,31 @@ pub const Repository = extern struct {
if (strings.hasPrefixComptime(url, "ssh://")) {
final_path_buf[0.."https".len].* = "https".*;
bun.copy(u8, final_path_buf["https".len..], url["ssh".len..]);
return final_path_buf[0..(url.len - "ssh".len + "https".len)];
return final_path_buf[0 .. url.len - "ssh".len + "https".len];
}

if (Dependency.isSCPLikePath(url)) {
final_path_buf[0.."https://".len].* = "https://".*;
var rest = final_path_buf["https://".len..];

const colon_index = strings.indexOfChar(url, ':');

if (colon_index) |colon| {
// make sure known hosts have `.com` or `.org`
if (Hosts.get(url[0..colon])) |tld| {
bun.copy(u8, rest, url[0..colon]);
bun.copy(u8, rest[colon..], tld);
rest[colon + tld.len] = '/';
bun.copy(u8, rest[colon + tld.len + 1 ..], url[colon + 1 ..]);
return final_path_buf[0 .. url.len + "https://".len + tld.len];
}
}

bun.copy(u8, rest, url);
if (strings.indexOfChar(rest, ':')) |colon| rest[colon] = '/';
return final_path_buf[0..(url.len + "https://".len)];
if (colon_index) |colon| rest[colon] = '/';
return final_path_buf[0 .. url.len + "https://".len];
}

return null;
}

Expand Down
210 changes: 208 additions & 2 deletions test/cli/install/bun-install.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1496,6 +1496,50 @@ it("should handle ^0.0.2-alpha.3+b4d in dependencies", async () => {
await access(join(package_dir, "bun.lockb"));
});

it("should choose the right version with prereleases", async () => {
const urls: string[] = [];
setHandler(dummyRegistry(urls, { "0.0.2-alpha.3": { as: "0.0.2" } }));
await writeFile(
join(package_dir, "package.json"),
JSON.stringify({
name: "foo",
version: "0.0.1",
dependencies: {
bar: "^0.0.2-alpha.3+b4d",
},
}),
);
const { stdout, stderr, exited } = spawn({
cmd: [bunExe(), "install"],
cwd: package_dir,
stdout: null,
stdin: "pipe",
stderr: "pipe",
env,
});
expect(stderr).toBeDefined();
const err = await new Response(stderr).text();
expect(err).toContain("Saved lockfile");
expect(err).not.toContain("error:");
expect(stdout).toBeDefined();
const out = await new Response(stdout).text();
expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
" + bar@0.0.2-alpha.3",
"",
" 1 packages installed",
]);
expect(await exited).toBe(0);
expect(urls.sort()).toEqual([`${root_url}/bar`, `${root_url}/bar-0.0.2.tgz`]);
expect(requested).toBe(2);
expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".cache", "bar"]);
expect(await readdirSorted(join(package_dir, "node_modules", "bar"))).toEqual(["package.json"]);
expect(await file(join(package_dir, "node_modules", "bar", "package.json")).json()).toEqual({
name: "bar",
version: "0.0.2",
});
await access(join(package_dir, "bun.lockb"));
});

it("should handle ^0.0.2rc1 in dependencies", async () => {
const urls: string[] = [];
setHandler(dummyRegistry(urls, { "0.0.2rc1": { as: "0.0.2" } }));
Expand Down Expand Up @@ -2606,6 +2650,169 @@ it("should handle GitHub URL in dependencies (user/repo#tag)", async () => {
await access(join(package_dir, "bun.lockb"));
});

it("should handle bitbucket git dependencies", async () => {
const deps = [
"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",
];
for (const dep of deps) {
const urls: string[] = [];
setHandler(dummyRegistry(urls));
await writeFile(
join(package_dir, "package.json"),
JSON.stringify({
name: "foo",
version: "0.0.1",
dependencies: {
"public-install-test": dep,
},
}),
);
const { stdout, stderr, exited } = spawn({
cmd: [bunExe(), "install"],
cwd: package_dir,
stdout: null,
stdin: "pipe",
stderr: "pipe",
env,
});

expect(stderr).toBeDefined();
const err = await new Response(stderr).text();
expect(err).toContain("Saved lockfile");
expect(stdout).toBeDefined();
const out = await new Response(stdout).text();
expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
` + public-install-test@git+ssh://${dep}#56ee8a7e167c6ab10b672203f2ab6fbcb752788d`,
"",
" 1 packages installed",
]);
expect(await exited).toBe(0);
await access(join(package_dir, "bun.lockb"));
dummyAfterEach();
dummyBeforeEach();
}

for (const dep of deps) {
const urls: string[] = [];
setHandler(dummyRegistry(urls));
await writeFile(
join(package_dir, "package.json"),
JSON.stringify({
name: "foo",
version: "0.0.1",
}),
);

const { stdout, stderr, exited } = spawn({
cmd: [bunExe(), "add", dep],
cwd: package_dir,
stdout: null,
stdin: "pipe",
stderr: "pipe",
env,
});

expect(stderr).toBeDefined();
const err = await new Response(stderr).text();
expect(err).toContain("Saved lockfile");
expect(stdout).toBeDefined();
const out = await new Response(stdout).text();
expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
"",
` installed publicinstalltest@git+ssh://${dep}#56ee8a7e167c6ab10b672203f2ab6fbcb752788d`,
"",
"",
" 1 packages installed",
]);
expect(await exited).toBe(0);
await access(join(package_dir, "bun.lockb"));
dummyAfterEach();
dummyBeforeEach();
}
}, 20000);

it("should handle gitlab git dependencies", async () => {
const deps = ["gitlab:dylan-conway/public-install-test", "gitlab.com:dylan-conway/public-install-test"];
for (const dep of deps) {
const urls: string[] = [];
setHandler(dummyRegistry(urls));
await writeFile(
join(package_dir, "package.json"),
JSON.stringify({
name: "foo",
version: "0.0.1",
dependencies: {
"public-install-test": dep,
},
}),
);
const { stdout, stderr, exited } = spawn({
cmd: [bunExe(), "install"],
cwd: package_dir,
stdout: null,
stdin: "pipe",
stderr: "pipe",
env,
});

expect(stderr).toBeDefined();
const err = await new Response(stderr).text();
expect(err).toContain("Saved lockfile");
expect(stdout).toBeDefined();
const out = await new Response(stdout).text();
expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
` + public-install-test@git+ssh://${dep}#93f3aa4ec9ca8a0bacc010776db48bfcd915c44c`,
"",
" 1 packages installed",
]);
expect(await exited).toBe(0);
await access(join(package_dir, "bun.lockb"));
dummyAfterEach();
dummyBeforeEach();
}

for (const dep of deps) {
const urls: string[] = [];
setHandler(dummyRegistry(urls));
await writeFile(
join(package_dir, "package.json"),
JSON.stringify({
name: "foo",
version: "0.0.1",
}),
);

const { stdout, stderr, exited } = spawn({
cmd: [bunExe(), "add", dep],
cwd: package_dir,
stdout: null,
stdin: "pipe",
stderr: "pipe",
env,
});

expect(stderr).toBeDefined();
const err = await new Response(stderr).text();
expect(err).toContain("Saved lockfile");
expect(stdout).toBeDefined();
const out = await new Response(stdout).text();
expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual([
"",
` installed public-install-test@git+ssh://${dep}#93f3aa4ec9ca8a0bacc010776db48bfcd915c44c`,
"",
"",
" 1 packages installed",
]);
expect(await exited).toBe(0);
await access(join(package_dir, "bun.lockb"));
dummyAfterEach();
dummyBeforeEach();
}
}, 10000);

it("should handle GitHub URL in dependencies (github:user/repo#tag)", async () => {
const urls: string[] = [];
setHandler(dummyRegistry(urls));
Expand Down Expand Up @@ -5053,7 +5260,7 @@ cache = false
" + tsd@0.22.0",
" + typescript@5.0.4",
"",
" 119 packages installed",
" 118 packages installed",
]);
expect(await exited1).toBe(0);
expect(await readdirSorted(package_dir)).toEqual(["bun.lockb", "bunfig.toml", "node_modules", "package.json"]);
Expand Down Expand Up @@ -5089,7 +5296,6 @@ cache = false
"fastq",
"fill-range",
"find-up",
"function-bind",
"glob-parent",
"globby",
"hard-rejection",
Expand Down

0 comments on commit 4a2e157

Please sign in to comment.