Skip to content

Commit 9aff2d5

Browse files
divybotlittledivy
andauthored
fix(publish): skip already-published versions before type checking (#35134)
## Summary `deno publish` previously type checked and prepared every package before attempting to publish, only discovering that a version was already published when the registry rejected the upload (the `duplicateVersionPublish` path). For packages whose version is already on the registry this wasted the entire — and expensive — type-check + tarball step. This adds the up-front check requested in #30280: 1. Probe the registry for each package's version concurrently. 2. Drop already-published versions before preparing anything. If **every** version is already published, return early **without type checking at all**. 3. Type check + prepare the remaining packages. 4. Publish — the existing post-prepare `duplicateVersionPublish` handling still covers the race where another CI run lands the same version in the meantime. ### Skip-and-continue, not bail Following the feedback on the earlier #30347, already-published versions are **skipped with a warning** (matching today's idempotent `duplicateVersionPublish` behavior) rather than turned into a hard error. This keeps workspace partial publishes and idempotent CI re-runs working. Only a `200 OK` is treated as "already published"; a `404` (or any other non-success status) is treated as "not published" so a transient registry error never blocks a legitimate publish. ### Other changes - Extracts shared helpers `registry::parse_package_name` and `registry::get_package_version_api_url` (also used by `publish_package`). - Fixes a missing closing quote in the invalid-package-name error message. ## Tests - New spec test `tests/specs/publish/already_published` — publishing an already-published version skips it and exits successfully without type checking. The fake JSR registry now answers the version-existence `GET` (404 by default; a reserved `already-published` scope simulates an existing version). - `registry::test_parse_package_name` unit test. - Full `cargo test specs::publish` suite (64 tests) and `specs::init` pass. Closes #30280 Closes denoland/divybot#563 --------- Co-authored-by: divybot <divybot@users.noreply.github.com> Co-authored-by: Divy Srivastava <me@littledivy.com>
1 parent d9703a4 commit 9aff2d5

8 files changed

Lines changed: 178 additions & 13 deletions

File tree

cli/registry.rs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Copyright 2018-2026 the Deno authors. MIT license.
22

3+
use deno_core::anyhow::bail;
34
use deno_core::error::AnyError;
45
use deno_core::serde_json;
56
use deno_core::url::Url;
@@ -127,6 +128,26 @@ pub fn get_package_api_url(
127128
format!("{}scopes/{}/packages/{}", registry_api_url, scope, package)
128129
}
129130

131+
pub fn get_package_version_api_url(
132+
registry_api_url: &Url,
133+
scope: &str,
134+
package: &str,
135+
version: &str,
136+
params: Option<&str>,
137+
) -> String {
138+
if let Some(params) = params {
139+
format!(
140+
"{}scopes/{}/packages/{}/versions/{}?{}",
141+
registry_api_url, scope, package, version, params
142+
)
143+
} else {
144+
format!(
145+
"{}scopes/{}/packages/{}/versions/{}",
146+
registry_api_url, scope, package, version
147+
)
148+
}
149+
}
150+
130151
pub async fn get_package(
131152
client: &HttpClient,
132153
registry_api_url: &Url,
@@ -138,6 +159,43 @@ pub async fn get_package(
138159
Ok(response)
139160
}
140161

162+
/// Splits a fully qualified JSR package name (e.g. `@scope/package`) into its
163+
/// `(scope, package)` parts.
164+
pub fn parse_package_name(name: &str) -> Result<(&str, &str), AnyError> {
165+
let Some((scope, package)) = name
166+
.strip_prefix('@')
167+
.and_then(|no_at| no_at.split_once('/'))
168+
else {
169+
bail!("Invalid package name, use '@<scope_name>/<package_name>' format");
170+
};
171+
Ok((scope, package))
172+
}
173+
174+
/// Returns `true` if the given package version is already published to the
175+
/// registry.
176+
///
177+
/// Only a `200 OK` response is treated as "already published". A `404` (and any
178+
/// other non-success status) is treated as "not published" so that this
179+
/// up-front optimization never blocks a legitimate publish because of a
180+
/// transient registry error.
181+
pub async fn check_version_exists(
182+
client: &HttpClient,
183+
registry_api_url: &Url,
184+
scope: &str,
185+
package: &str,
186+
version: &str,
187+
) -> Result<bool, AnyError> {
188+
let url = get_package_version_api_url(
189+
registry_api_url,
190+
scope,
191+
package,
192+
version,
193+
None,
194+
);
195+
let response = client.get(url.parse()?)?.send().await?;
196+
Ok(response.status() == 200)
197+
}
198+
141199
pub fn get_jsr_alternative(imported: &Url) -> Option<String> {
142200
if matches!(imported.host_str(), Some("esm.sh")) {
143201
let mut segments = imported.path_segments()?;
@@ -197,4 +255,11 @@ mod test {
197255
Some("\"jsr:@std/path@1/something-underscore\""),
198256
);
199257
}
258+
259+
#[test]
260+
fn test_parse_package_name() {
261+
assert_eq!(parse_package_name("@deno/doc").unwrap(), ("deno", "doc"));
262+
assert!(parse_package_name("deno/doc").is_err());
263+
assert!(parse_package_name("@deno").is_err());
264+
}
200265
}

cli/tools/publish/mod.rs

Lines changed: 73 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,28 @@ pub async fn publish(
123123
}
124124
}
125125

126+
// Bail out early if the version is already published, before doing the
127+
// expensive type checking and tarball preparation. Already-published
128+
// versions are skipped with a warning (rather than erroring) so that
129+
// re-running `deno publish` after a partial or concurrent publish still
130+
// succeeds, matching the idempotent behavior of the publish step itself.
131+
if !publish_flags.dry_run {
132+
publish_configs = filter_out_published_packages(
133+
&cli_factory.http_client_provider().get_or_create()?,
134+
jsr_api_url(),
135+
publish_configs,
136+
)
137+
.await?;
138+
139+
if publish_configs.is_empty() {
140+
log::info!(
141+
"{} All packages are already published",
142+
colors::green("Success"),
143+
);
144+
return Ok(());
145+
}
146+
}
147+
126148
let specifier_unfurler = SpecifierUnfurler::new(
127149
cli_factory.node_resolver().await?.clone(),
128150
cli_factory.npm_req_resolver().await?.clone(),
@@ -205,6 +227,51 @@ pub async fn publish(
205227
Ok(())
206228
}
207229

230+
/// Queries the registry for each package's version concurrently and returns the
231+
/// subset of configs whose versions are not yet published. Already-published
232+
/// versions are reported with a warning and dropped from the returned list.
233+
async fn filter_out_published_packages(
234+
client: &HttpClient,
235+
registry_api_url: &Url,
236+
publish_configs: Vec<JsrPackageConfig>,
237+
) -> Result<Vec<JsrPackageConfig>, AnyError> {
238+
let checks = publish_configs.iter().map(|config| async move {
239+
let Some(version) = config.config_file.json.version.as_deref() else {
240+
// a missing version is reported with a better error later on
241+
return Ok::<bool, AnyError>(false);
242+
};
243+
let Ok((scope, package)) = registry::parse_package_name(&config.name)
244+
else {
245+
// an invalid package name is reported with a better error later on
246+
return Ok(false);
247+
};
248+
registry::check_version_exists(
249+
client,
250+
registry_api_url,
251+
scope,
252+
package,
253+
version,
254+
)
255+
.await
256+
});
257+
let results = deno_core::futures::future::join_all(checks).await;
258+
259+
let mut remaining = Vec::with_capacity(publish_configs.len());
260+
for (config, already_published) in publish_configs.into_iter().zip(results) {
261+
if already_published? {
262+
log::info!(
263+
"{} {}@{}",
264+
colors::yellow("Warning: Skipping, already published"),
265+
config.name,
266+
config.config_file.json.version.as_deref().unwrap_or(""),
267+
);
268+
} else {
269+
remaining.push(config);
270+
}
271+
}
272+
Ok(remaining)
273+
}
274+
208275
struct PreparedPublishPackage {
209276
scope: String,
210277
package: String,
@@ -427,12 +494,7 @@ impl PublishPreparer {
427494
deno_json.specifier
428495
)
429496
})?;
430-
let Some(name_no_at) = package.name.strip_prefix('@') else {
431-
bail!("Invalid package name, use '@<scope_name>/<package_name> format");
432-
};
433-
let Some((scope, name_no_scope)) = name_no_at.split_once('/') else {
434-
bail!("Invalid package name, use '@<scope_name>/<package_name> format");
435-
};
497+
let (scope, name_no_scope) = registry::parse_package_name(&package.name)?;
436498
let file_patterns = package.member_dir.to_publish_config()?.files;
437499

438500
let tarball = deno_core::unsync::spawn_blocking({
@@ -925,13 +987,12 @@ async fn publish_package(
925987
package.version
926988
);
927989

928-
let url = format!(
929-
"{}scopes/{}/packages/{}/versions/{}?config=/{}",
990+
let url = registry::get_package_version_api_url(
930991
registry_api_url,
931-
package.scope,
932-
package.package,
933-
package.version,
934-
package.config
992+
&package.scope,
993+
&package.package,
994+
&package.version,
995+
Some(&format!("config=/{}", package.config)),
935996
);
936997

937998
let body = deno_fetch::ReqBody::full(package.tarball.bytes.clone());

tests/specs/init/lib/dry_publish.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@ Check [WILDCARD]mod.ts
44
error: Failed preparing 'project'.
55

66
Caused by:
7-
Invalid package name, use '@<scope_name>/<package_name> format
7+
Invalid package name, use '@<scope_name>/<package_name>' format
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"args": "publish --token 'sadfasdf'",
3+
"output": "publish.out"
4+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "@already-published/pkg",
3+
"version": "1.0.0",
4+
"exports": {
5+
".": "./mod.ts"
6+
}
7+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export function add(a: number, b: number): number {
2+
return a + b;
3+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Warning: Skipping, already published @already-published/pkg@1.0.0
2+
Success All packages are already published

tests/util/server/servers/jsr_registry.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use bytes::Bytes;
1313
use http_body_util::Empty;
1414
use http_body_util::Full;
1515
use http_body_util::combinators::UnsyncBoxBody;
16+
use hyper::Method;
1617
use hyper::Request;
1718
use hyper::Response;
1819
use hyper::StatusCode;
@@ -120,6 +121,28 @@ async fn registry_server_handler(
120121
let body = serde_json::to_string_pretty(&json!({})).unwrap();
121122
let res = Response::new(UnsyncBoxBody::new(Full::from(body)));
122123
return Ok(res);
124+
} else if req.method() == Method::GET
125+
&& let Some(rest) = path.strip_prefix("/api/scopes/")
126+
&& let [scope, "packages", package, "versions", version] =
127+
rest.split('/').collect::<Vec<_>>().as_slice()
128+
{
129+
// `deno publish` probes this endpoint up front to skip already-published
130+
// versions. Report "not published" (404) by default; the reserved
131+
// `already-published` scope lets tests simulate an existing version.
132+
if *scope == "already-published" {
133+
let body = serde_json::to_string_pretty(&json!({
134+
"scope": scope,
135+
"package": package,
136+
"version": version,
137+
}))
138+
.unwrap();
139+
let res = Response::new(UnsyncBoxBody::new(Full::from(body)));
140+
return Ok(res);
141+
}
142+
let res = Response::builder()
143+
.status(StatusCode::NOT_FOUND)
144+
.body(UnsyncBoxBody::new(Empty::new()))?;
145+
return Ok(res);
123146
} else if path.starts_with("/api/scopes/") {
124147
let body = serde_json::to_string_pretty(&json!({
125148
"id": "sdfwqer-sffg-qwerasdf",

0 commit comments

Comments
 (0)