From 9d7174e434187ab938df52d8c1d778a26b8e4e66 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> Date: Tue, 17 Dec 2024 15:56:03 -0800 Subject: [PATCH] fix(outdated): ensure "Latest" version is greater than "Update" version (#27390) Fixes #27038. Previously, for NPM packages the latest version was the version with the "latest" tag. For JSR packages, the latest version was the greatest version that matched a `*` version requirement. Unfortunately, that doesn't work well with pre-release versions. This PR changes it so that the latest version is always > the currently requested version. For NPM: if "latest" tag > current then "latest" tag; otherwise the greatest version that is >= current For JSR: greatest version >= current This is the most reasonable behavior I could come up with. For example, ``` versions: 2.0.0-beta.2 2.0.0-beta.1 1.0.0 => "latest" tag with a version req `^2.0.0-beta.1` previously: "Update" column => 2.0.0-beta.2 "Latest" column => 1.0.0 now: "Update" column => 2.0.0-beta.2 "Latest" column => 2.0.0-beta.2 ``` --- cli/tools/registry/pm/deps.rs | 82 +++++++++++++------ .../has-only-pre-release/2.0.0-beta.1/mod.ts | 1 + .../2.0.0-beta.1_meta.json | 5 ++ .../has-only-pre-release/2.0.0-beta.2/mod.ts | 1 + .../2.0.0-beta.2_meta.json | 5 ++ .../@denotest/has-only-pre-release/meta.json | 6 ++ .../@denotest/has-pre-release/1.0.0/mod.ts | 1 + .../@denotest/has-pre-release/1.0.0_meta.json | 3 + .../has-pre-release/2.0.0-beta.1/mod.ts | 1 + .../has-pre-release/2.0.0-beta.1_meta.json | 5 ++ .../has-pre-release/2.0.0-beta.2/mod.ts | 1 + .../has-pre-release/2.0.0-beta.2_meta.json | 5 ++ .../jsr/@denotest/has-pre-release/meta.json | 7 ++ .../has-pre-release/1.0.0/package.json | 7 ++ .../has-pre-release/2.0.0-beta.1/package.json | 4 + .../has-pre-release/2.0.0-beta.2/package.json | 4 + tests/specs/update/pre_release/__test__.jsonc | 21 +++++ tests/specs/update/pre_release/deno.json | 7 ++ tests/specs/update/pre_release/deno.lock | 28 +++++++ tests/specs/update/pre_release/outdated.out | 11 +++ tests/specs/update/pre_release/update.out | 5 ++ tests/util/server/src/npm.rs | 14 +++- 22 files changed, 198 insertions(+), 26 deletions(-) create mode 100644 tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.1/mod.ts create mode 100644 tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.1_meta.json create mode 100644 tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.2/mod.ts create mode 100644 tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.2_meta.json create mode 100644 tests/registry/jsr/@denotest/has-only-pre-release/meta.json create mode 100644 tests/registry/jsr/@denotest/has-pre-release/1.0.0/mod.ts create mode 100644 tests/registry/jsr/@denotest/has-pre-release/1.0.0_meta.json create mode 100644 tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.1/mod.ts create mode 100644 tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.1_meta.json create mode 100644 tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.2/mod.ts create mode 100644 tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.2_meta.json create mode 100644 tests/registry/jsr/@denotest/has-pre-release/meta.json create mode 100644 tests/registry/npm/@denotest/has-pre-release/1.0.0/package.json create mode 100644 tests/registry/npm/@denotest/has-pre-release/2.0.0-beta.1/package.json create mode 100644 tests/registry/npm/@denotest/has-pre-release/2.0.0-beta.2/package.json create mode 100644 tests/specs/update/pre_release/__test__.jsonc create mode 100644 tests/specs/update/pre_release/deno.json create mode 100644 tests/specs/update/pre_release/deno.lock create mode 100644 tests/specs/update/pre_release/outdated.out create mode 100644 tests/specs/update/pre_release/update.out diff --git a/cli/tools/registry/pm/deps.rs b/cli/tools/registry/pm/deps.rs index e4c38276f7..bb03e97f2d 100644 --- a/cli/tools/registry/pm/deps.rs +++ b/cli/tools/registry/pm/deps.rs @@ -3,7 +3,6 @@ use std::borrow::Cow; use std::collections::HashMap; use std::path::PathBuf; -use std::sync::atomic::AtomicBool; use std::sync::Arc; use deno_ast::ModuleSpecifier; @@ -28,6 +27,7 @@ use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageNv; use deno_semver::package::PackageReq; use deno_semver::package::PackageReqReference; +use deno_semver::Version; use deno_semver::VersionReq; use import_map::ImportMap; use import_map::ImportMapWithDiagnostics; @@ -42,6 +42,7 @@ use crate::jsr::JsrFetchResolver; use crate::module_loader::ModuleLoadPreparer; use crate::npm::CliNpmResolver; use crate::npm::NpmFetchResolver; +use crate::util::sync::AtomicFlag; use super::ConfigUpdater; @@ -447,7 +448,7 @@ pub struct DepManager { pending_changes: Vec, - dependencies_resolved: AtomicBool, + dependencies_resolved: AtomicFlag, module_load_preparer: Arc, // TODO(nathanwhit): probably shouldn't be pub pub(crate) jsr_fetch_resolver: Arc, @@ -489,7 +490,7 @@ impl DepManager { resolved_versions: Vec::new(), latest_versions: Vec::new(), jsr_fetch_resolver, - dependencies_resolved: AtomicBool::new(false), + dependencies_resolved: AtomicFlag::lowered(), module_load_preparer, npm_fetch_resolver, npm_resolver, @@ -530,10 +531,7 @@ impl DepManager { } async fn run_dependency_resolution(&self) -> Result<(), AnyError> { - if self - .dependencies_resolved - .load(std::sync::atomic::Ordering::Relaxed) - { + if self.dependencies_resolved.is_raised() { return Ok(()); } @@ -556,9 +554,7 @@ impl DepManager { } DepKind::Jsr => graph.packages.mappings().contains_key(&dep.req), }) { - self - .dependencies_resolved - .store(true, std::sync::atomic::Ordering::Relaxed); + self.dependencies_resolved.raise(); graph_permit.commit(); return Ok(()); } @@ -613,6 +609,7 @@ impl DepManager { ) .await?; + self.dependencies_resolved.raise(); graph_permit.commit(); Ok(()) @@ -655,10 +652,6 @@ impl DepManager { if self.latest_versions.len() == self.deps.len() { return Ok(self.latest_versions.clone()); } - let latest_tag_req = deno_semver::VersionReq::from_raw_text_and_inner( - "latest".into(), - deno_semver::RangeSetOrTag::Tag("latest".into()), - ); let mut latest_versions = Vec::with_capacity(self.deps.len()); let npm_sema = Semaphore::new(32); @@ -670,14 +663,25 @@ impl DepManager { DepKind::Npm => futs.push_back( async { let semver_req = &dep.req; - let latest_req = PackageReq { - name: dep.req.name.clone(), - version_req: latest_tag_req.clone(), - }; let _permit = npm_sema.acquire().await; let semver_compatible = self.npm_fetch_resolver.req_to_nv(semver_req).await; - let latest = self.npm_fetch_resolver.req_to_nv(&latest_req).await; + let info = + self.npm_fetch_resolver.package_info(&semver_req.name).await; + let latest = info + .and_then(|info| { + let latest_tag = info.dist_tags.get("latest")?; + let lower_bound = &semver_compatible.as_ref()?.version; + if latest_tag > lower_bound { + Some(latest_tag.clone()) + } else { + latest_version(Some(latest_tag), info.versions.keys()) + } + }) + .map(|version| PackageNv { + name: semver_req.name.clone(), + version, + }); PackageLatestVersion { latest, semver_compatible, @@ -688,14 +692,29 @@ impl DepManager { DepKind::Jsr => futs.push_back( async { let semver_req = &dep.req; - let latest_req = PackageReq { - name: dep.req.name.clone(), - version_req: deno_semver::WILDCARD_VERSION_REQ.clone(), - }; let _permit = jsr_sema.acquire().await; let semver_compatible = self.jsr_fetch_resolver.req_to_nv(semver_req).await; - let latest = self.jsr_fetch_resolver.req_to_nv(&latest_req).await; + let info = + self.jsr_fetch_resolver.package_info(&semver_req.name).await; + let latest = info + .and_then(|info| { + let lower_bound = &semver_compatible.as_ref()?.version; + latest_version( + Some(lower_bound), + info.versions.iter().filter_map(|(version, version_info)| { + if !version_info.yanked { + Some(version) + } else { + None + } + }), + ) + }) + .map(|version| PackageNv { + name: semver_req.name.clone(), + version, + }); PackageLatestVersion { latest, semver_compatible, @@ -893,3 +912,18 @@ fn parse_req_reference( DepKind::Jsr => JsrPackageReqReference::from_str(input)?.into_inner(), }) } + +fn latest_version<'a>( + start: Option<&Version>, + versions: impl IntoIterator, +) -> Option { + let mut best = start; + for version in versions { + match best { + Some(best_version) if version > best_version => best = Some(version), + None => best = Some(version), + _ => {} + } + } + best.cloned() +} diff --git a/tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.1/mod.ts b/tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.1/mod.ts new file mode 100644 index 0000000000..6a8018af41 --- /dev/null +++ b/tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.1/mod.ts @@ -0,0 +1 @@ +export const foo = 1; \ No newline at end of file diff --git a/tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.1_meta.json b/tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.1_meta.json new file mode 100644 index 0000000000..6c213a9c05 --- /dev/null +++ b/tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.1_meta.json @@ -0,0 +1,5 @@ +{ + "exports": { + ".": "mod.ts" + } +} diff --git a/tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.2/mod.ts b/tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.2/mod.ts new file mode 100644 index 0000000000..6a8018af41 --- /dev/null +++ b/tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.2/mod.ts @@ -0,0 +1 @@ +export const foo = 1; \ No newline at end of file diff --git a/tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.2_meta.json b/tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.2_meta.json new file mode 100644 index 0000000000..6c213a9c05 --- /dev/null +++ b/tests/registry/jsr/@denotest/has-only-pre-release/2.0.0-beta.2_meta.json @@ -0,0 +1,5 @@ +{ + "exports": { + ".": "mod.ts" + } +} diff --git a/tests/registry/jsr/@denotest/has-only-pre-release/meta.json b/tests/registry/jsr/@denotest/has-only-pre-release/meta.json new file mode 100644 index 0000000000..cce8d0954f --- /dev/null +++ b/tests/registry/jsr/@denotest/has-only-pre-release/meta.json @@ -0,0 +1,6 @@ +{ + "versions": { + "2.0.0-beta.1": {}, + "2.0.0-beta.2": {} + } +} diff --git a/tests/registry/jsr/@denotest/has-pre-release/1.0.0/mod.ts b/tests/registry/jsr/@denotest/has-pre-release/1.0.0/mod.ts new file mode 100644 index 0000000000..6a8018af41 --- /dev/null +++ b/tests/registry/jsr/@denotest/has-pre-release/1.0.0/mod.ts @@ -0,0 +1 @@ +export const foo = 1; \ No newline at end of file diff --git a/tests/registry/jsr/@denotest/has-pre-release/1.0.0_meta.json b/tests/registry/jsr/@denotest/has-pre-release/1.0.0_meta.json new file mode 100644 index 0000000000..c5807f588c --- /dev/null +++ b/tests/registry/jsr/@denotest/has-pre-release/1.0.0_meta.json @@ -0,0 +1,3 @@ +{ + "exports": {} +} diff --git a/tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.1/mod.ts b/tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.1/mod.ts new file mode 100644 index 0000000000..6a8018af41 --- /dev/null +++ b/tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.1/mod.ts @@ -0,0 +1 @@ +export const foo = 1; \ No newline at end of file diff --git a/tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.1_meta.json b/tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.1_meta.json new file mode 100644 index 0000000000..6c213a9c05 --- /dev/null +++ b/tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.1_meta.json @@ -0,0 +1,5 @@ +{ + "exports": { + ".": "mod.ts" + } +} diff --git a/tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.2/mod.ts b/tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.2/mod.ts new file mode 100644 index 0000000000..6a8018af41 --- /dev/null +++ b/tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.2/mod.ts @@ -0,0 +1 @@ +export const foo = 1; \ No newline at end of file diff --git a/tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.2_meta.json b/tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.2_meta.json new file mode 100644 index 0000000000..6c213a9c05 --- /dev/null +++ b/tests/registry/jsr/@denotest/has-pre-release/2.0.0-beta.2_meta.json @@ -0,0 +1,5 @@ +{ + "exports": { + ".": "mod.ts" + } +} diff --git a/tests/registry/jsr/@denotest/has-pre-release/meta.json b/tests/registry/jsr/@denotest/has-pre-release/meta.json new file mode 100644 index 0000000000..ba7bc452d7 --- /dev/null +++ b/tests/registry/jsr/@denotest/has-pre-release/meta.json @@ -0,0 +1,7 @@ +{ + "versions": { + "1.0.0": {}, + "2.0.0-beta.1": {}, + "2.0.0-beta.2": {} + } +} diff --git a/tests/registry/npm/@denotest/has-pre-release/1.0.0/package.json b/tests/registry/npm/@denotest/has-pre-release/1.0.0/package.json new file mode 100644 index 0000000000..027b783d54 --- /dev/null +++ b/tests/registry/npm/@denotest/has-pre-release/1.0.0/package.json @@ -0,0 +1,7 @@ +{ + "name": "@denotest/has-pre-release", + "version": "1.0.0", + "publishConfig": { + "tag": "latest" + } +} \ No newline at end of file diff --git a/tests/registry/npm/@denotest/has-pre-release/2.0.0-beta.1/package.json b/tests/registry/npm/@denotest/has-pre-release/2.0.0-beta.1/package.json new file mode 100644 index 0000000000..b3d3b3e634 --- /dev/null +++ b/tests/registry/npm/@denotest/has-pre-release/2.0.0-beta.1/package.json @@ -0,0 +1,4 @@ +{ + "name": "@denotest/has-pre-release", + "version": "2.0.0-beta.1" +} \ No newline at end of file diff --git a/tests/registry/npm/@denotest/has-pre-release/2.0.0-beta.2/package.json b/tests/registry/npm/@denotest/has-pre-release/2.0.0-beta.2/package.json new file mode 100644 index 0000000000..b6f8ea4c76 --- /dev/null +++ b/tests/registry/npm/@denotest/has-pre-release/2.0.0-beta.2/package.json @@ -0,0 +1,4 @@ +{ + "name": "@denotest/has-pre-release", + "version": "2.0.0-beta.2" +} \ No newline at end of file diff --git a/tests/specs/update/pre_release/__test__.jsonc b/tests/specs/update/pre_release/__test__.jsonc new file mode 100644 index 0000000000..7e2ea39dab --- /dev/null +++ b/tests/specs/update/pre_release/__test__.jsonc @@ -0,0 +1,21 @@ +{ + "tempDir": true, + "steps": [ + { + "args": "i", + "output": "[WILDCARD]" + }, + { + "args": "outdated", + "output": "outdated.out" + }, + { + "args": "outdated --compatible", + "output": "outdated.out" + }, + { + "args": "outdated --update --latest", + "output": "update.out" + } + ] +} diff --git a/tests/specs/update/pre_release/deno.json b/tests/specs/update/pre_release/deno.json new file mode 100644 index 0000000000..07646b5059 --- /dev/null +++ b/tests/specs/update/pre_release/deno.json @@ -0,0 +1,7 @@ +{ + "imports": { + "@denotest/npm-has-pre-release": "npm:@denotest/has-pre-release@^2.0.0-beta.1", + "@denotest/jsr-has-pre-release": "jsr:@denotest/has-pre-release@^2.0.0-beta.1", + "@denotest/has-only-pre-release": "jsr:@denotest/has-only-pre-release@^2.0.0-beta.1" + } +} diff --git a/tests/specs/update/pre_release/deno.lock b/tests/specs/update/pre_release/deno.lock new file mode 100644 index 0000000000..33b136dd53 --- /dev/null +++ b/tests/specs/update/pre_release/deno.lock @@ -0,0 +1,28 @@ +{ + "version": "4", + "specifiers": { + "jsr:@denotest/has-only-pre-release@^2.0.0-beta.1": "2.0.0-beta.1", + "jsr:@denotest/has-pre-release@^2.0.0-beta.1": "2.0.0-beta.1", + "npm:@denotest/has-pre-release@^2.0.0-beta.1": "2.0.0-beta.1" + }, + "jsr": { + "@denotest/has-only-pre-release@2.0.0-beta.1": { + "integrity": "43fd680ea94bb5db5fe1a2d86101c47d0e2cc77323b881755cea9a0372e49537" + }, + "@denotest/has-pre-release@2.0.0-beta.1": { + "integrity": "43fd680ea94bb5db5fe1a2d86101c47d0e2cc77323b881755cea9a0372e49537" + } + }, + "npm": { + "@denotest/has-pre-release@2.0.0-beta.1": { + "integrity": "sha512-K1fHe1L2EUSLgijtzzALNpkkIO0SrX3z+IXvVjjOIE8HKd4T7lkpzDdoUp+WllwS3KXmuJh+9vIfY5lFp38pew==" + } + }, + "workspace": { + "dependencies": [ + "jsr:@denotest/has-only-pre-release@^2.0.0-beta.1", + "jsr:@denotest/has-pre-release@^2.0.0-beta.1", + "npm:@denotest/has-pre-release@^2.0.0-beta.1" + ] + } +} diff --git a/tests/specs/update/pre_release/outdated.out b/tests/specs/update/pre_release/outdated.out new file mode 100644 index 0000000000..b8369b25f7 --- /dev/null +++ b/tests/specs/update/pre_release/outdated.out @@ -0,0 +1,11 @@ +┌────────────────────────────────────┬──────────────┬──────────────┬──────────────┐ +│ Package │ Current │ Update │ Latest │ +├────────────────────────────────────┼──────────────┼──────────────┼──────────────┤ +│ jsr:@denotest/has-only-pre-release │ 2.0.0-beta.1 │ 2.0.0-beta.2 │ 2.0.0-beta.2 │ +├────────────────────────────────────┼──────────────┼──────────────┼──────────────┤ +│ jsr:@denotest/has-pre-release │ 2.0.0-beta.1 │ 2.0.0-beta.2 │ 2.0.0-beta.2 │ +├────────────────────────────────────┼──────────────┼──────────────┼──────────────┤ +│ npm:@denotest/has-pre-release │ 2.0.0-beta.1 │ 2.0.0-beta.2 │ 2.0.0-beta.2 │ +└────────────────────────────────────┴──────────────┴──────────────┴──────────────┘ + +[WILDCARD] diff --git a/tests/specs/update/pre_release/update.out b/tests/specs/update/pre_release/update.out new file mode 100644 index 0000000000..d019457f0a --- /dev/null +++ b/tests/specs/update/pre_release/update.out @@ -0,0 +1,5 @@ +[WILDCARD] +Updated 3 dependencies: + - jsr:@denotest/has-only-pre-release 2.0.0-beta.1 -> 2.0.0-beta.2 + - jsr:@denotest/has-pre-release 2.0.0-beta.1 -> 2.0.0-beta.2 + - npm:@denotest/has-pre-release 2.0.0-beta.1 -> 2.0.0-beta.2 diff --git a/tests/util/server/src/npm.rs b/tests/util/server/src/npm.rs index 081989ddb5..0261b2532c 100644 --- a/tests/util/server/src/npm.rs +++ b/tests/util/server/src/npm.rs @@ -267,6 +267,7 @@ fn get_npm_package( let mut tarballs = HashMap::new(); let mut versions = serde_json::Map::new(); let mut latest_version = semver::Version::parse("0.0.0").unwrap(); + let mut dist_tags = serde_json::Map::new(); for entry in fs::read_dir(&package_folder)? { let entry = entry?; let file_type = entry.file_type()?; @@ -345,6 +346,14 @@ fn get_npm_package( } } + if let Some(publish_config) = version_info.get("publishConfig") { + if let Some(tag) = publish_config.get("tag") { + if let Some(tag) = tag.as_str() { + dist_tags.insert(tag.to_string(), version.clone().into()); + } + } + } + versions.insert(version.clone(), version_info.into()); let version = semver::Version::parse(&version)?; if version.cmp(&latest_version).is_gt() { @@ -352,8 +361,9 @@ fn get_npm_package( } } - let mut dist_tags = serde_json::Map::new(); - dist_tags.insert("latest".to_string(), latest_version.to_string().into()); + if !dist_tags.contains_key("latest") { + dist_tags.insert("latest".to_string(), latest_version.to_string().into()); + } // create the registry file for this package let mut registry_file = serde_json::Map::new();