Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix findBestMatch so it finds the best match and not the first match #6611

Merged
merged 5 commits into from
Oct 20, 2023
Merged

fix findBestMatch so it finds the best match and not the first match #6611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4580780
fix findBestMatch so it finds the best match and not the first match
dylang Oct 20, 2023
b8d9690
update complex-workspaces to include lines-and-columns ^1.1.6
dylang Oct 20, 2023
0e39239
Merge branch 'main' into fix-findBestVersion
dylang Oct 20, 2023
33a9b0c
PR feedback
dylang Oct 20, 2023
528a695
PR feedback
dylang Oct 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/install/npm.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -823,16 +823,26 @@ pub const PackageManifest = struct {

{
const releases = this.pkg.releases.keys.get(this.versions);

var best_match_version: ?Semver.Version = null;
var best_match_package_index: usize = 0;
var i = releases.len;
// For now, this is the dumb way
while (i > 0) : (i -= 1) {
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] };
// If we find one that matches, save it, but keep looping because we might find a newer match.
// The versions from the registry are not in any particular order.
if (group.satisfies(version) and (best_match_version == null or Semver.Version.order(version, best_match_version.?, "", "") == .gt)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to account for pre/post tags? Usually the way we do this is to get the string buffer from lockfile.buffers.string_bytes.items and pass that to the parent function

Suggested change
if (group.satisfies(version) and (best_match_version == null or Semver.Version.order(version, best_match_version.?, "", "") == .gt)) {
if (group.satisfies(version) and (best_match_version == null or Semver.Version.order(version, best_match_version.?, string_bytes, string_bytes) == .gt)) {

best_match_version = version;
best_match_package_index = i - 1;
}
}

if (best_match_version != null) {
const packages = this.pkg.releases.values.get(this.package_versions);
return .{ .version = best_match_version.?, .package = &packages[best_match_package_index] };
}
}

if (group.flags.isSet(Semver.Query.Group.Flags.pre)) {
Expand Down
3 changes: 3 additions & 0 deletions test/cli/install/migration/complex-workspace.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,6 @@ mustNotExist("packages/second/node_modules/body-parser/node_modules/body-parser/
mustNotExist("packages/second/node_modules/body-parser/node_modules/iconv-lite");
mustNotExist("packages/second/node_modules/iconv-lite");
mustNotExist("node_modules/iconv-lite");

// Should pick 1.2.4, not 1.2.0.
validate("node_modules/lines-and-columns", "1.2.4");
1 change: 1 addition & 0 deletions test/cli/install/migration/complex-workspace/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"hello": "file:hello-0.3.2.tgz",
"install-test": "bitbucket:dylan-conway/public-install-test",
"install-test1": "git+ssh://[email protected]/dylan-conway/install-test.git#596234dab30564f37adae1e5c4d7123bcffce537",
"lines-and-columns": "^1.1.6",
"public-install-test": "gitlab:dylan-conway/public-install-test",
"svelte": "4.1.2"
},
Expand Down