From 61c49a186e62aac84667b9325c440209d3ee6f7f Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Thu, 14 Jul 2022 08:14:20 +0200 Subject: [PATCH 1/6] chore: create function to get local crate version This is done so that the submit function can easily acquire the version it has to wait for to appear in the crates.io registry after submitting it. Like that, we will only advance to the next crate, when the current one was fully populated upstream --- tools/release-operator/src/registry.rs | 48 ++++++++++++++------------ 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/tools/release-operator/src/registry.rs b/tools/release-operator/src/registry.rs index 85b92bbeb..3b4cf5be1 100644 --- a/tools/release-operator/src/registry.rs +++ b/tools/release-operator/src/registry.rs @@ -63,6 +63,30 @@ impl Crate { } } + fn get_local_version(&self) -> anyhow::Result { + let name = format!("{self}"); + let cargo_toml_location = std::fs::canonicalize(&self.path) + .context("absolute path to Cargo.toml")?; + let mut cmd = cargo_metadata::MetadataCommand::new(); + cmd.manifest_path(format!( + "{}/Cargo.toml", + cargo_toml_location.to_string_lossy() + )) + .no_deps(); + + let metadata = cmd.exec()?; + let package = metadata + .packages + .iter() + .find(|p| p.name == name) + .ok_or_else(|| anyhow!("could not find package"))?; + + let version = package.version.to_owned(); + log::debug!("{self} found as {version} on our side"); + + Ok(version) + } + fn determine_state(&self) -> anyhow::Result { let theirs = { #[derive(Deserialize)] @@ -112,29 +136,7 @@ impl Crate { versions.versions.get(0).unwrap().version.to_owned() }; - let ours = { - let name = format!("{self}"); - let cargo_toml_location = std::fs::canonicalize(&self.path) - .context("absolute path to Cargo.toml")?; - let mut cmd = cargo_metadata::MetadataCommand::new(); - cmd.manifest_path(format!( - "{}/Cargo.toml", - cargo_toml_location.to_string_lossy() - )) - .no_deps(); - - let metadata = cmd.exec()?; - let package = metadata - .packages - .iter() - .find(|p| p.name == name) - .ok_or_else(|| anyhow!("could not find package"))?; - - let version = package.version.to_owned(); - log::debug!("{self} found as {version} on our side"); - - version - }; + let ours = self.get_local_version()?; if ours == theirs { log::info!("{self} has already been published as {ours}"); From 0e4e288a08e0813c4b3a37daf7795619946b218f Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Thu, 14 Jul 2022 08:40:55 +0200 Subject: [PATCH 2/6] chore: create function to get upstream crate version --- tools/release-operator/src/registry.rs | 102 +++++++++++++++---------- 1 file changed, 60 insertions(+), 42 deletions(-) diff --git a/tools/release-operator/src/registry.rs b/tools/release-operator/src/registry.rs index 3b4cf5be1..baa42f18e 100644 --- a/tools/release-operator/src/registry.rs +++ b/tools/release-operator/src/registry.rs @@ -53,6 +53,17 @@ impl Registry { } } +#[derive(Debug)] +struct CrateNotFoundError; + +impl std::fmt::Display for CrateNotFoundError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "crate was not found on crates.io") + } +} + +impl std::error::Error for CrateNotFoundError {} + impl Crate { fn validate(&self) -> anyhow::Result<()> { match self.path.exists() { @@ -87,53 +98,60 @@ impl Crate { Ok(version) } - fn determine_state(&self) -> anyhow::Result { - let theirs = { - #[derive(Deserialize)] - struct CrateVersions { - versions: Vec, - } + fn get_upstream_version(&self) -> anyhow::Result { + #[derive(Deserialize)] + struct CrateVersions { + versions: Vec, + } - #[derive(Deserialize)] - struct CrateVersion { - #[serde(rename = "num")] - version: semver::Version, - } + #[derive(Deserialize)] + struct CrateVersion { + #[serde(rename = "num")] + version: semver::Version, + } - let client = reqwest::blocking::ClientBuilder::new() - .user_agent(concat!( - env!("CARGO_PKG_NAME"), - "/", - env!("CARGO_PKG_VERSION") - )) - .build() - .context("build http client")?; - - let resp = client - .get(format!("https://crates.io/api/v1/crates/{self}")) - .send() - .context("fetching crate versions from the registry")?; - - if resp.status() == reqwest::StatusCode::NOT_FOUND { - log::info!("{self} has not been published yet"); - return Ok(CrateState::Unknown); - } + let client = reqwest::blocking::ClientBuilder::new() + .user_agent(concat!( + env!("CARGO_PKG_NAME"), + "/", + env!("CARGO_PKG_VERSION") + )) + .build() + .context("build http client")?; + + let resp = client + .get(format!("https://crates.io/api/v1/crates/{self}")) + .send() + .context("fetching crate versions from the registry")?; + + if resp.status() == reqwest::StatusCode::NOT_FOUND { + return Err(anyhow::Error::new(CrateNotFoundError)); + } - if resp.status() != reqwest::StatusCode::OK { - return Err(anyhow!( - "{self} request to crates.io failed with {} '{}'", - resp.status(), - resp.text().unwrap_or_else(|_| { - "[response body could not be read]".to_string() - }) - )); - } + if resp.status() != reqwest::StatusCode::OK { + return Err(anyhow!( + "{self} request to crates.io failed with {} '{}'", + resp.status(), + resp.text().unwrap_or_else(|_| { + "[response body could not be read]".to_string() + }) + )); + } + + let versions = + serde_json::from_str::(resp.text()?.as_str()) + .context("deserializing crates.io response")?; - let versions = - serde_json::from_str::(resp.text()?.as_str()) - .context("deserializing crates.io response")?; + Ok(versions.versions.get(0).unwrap().version.to_owned()) + } - versions.versions.get(0).unwrap().version.to_owned() + fn determine_state(&self) -> anyhow::Result { + let theirs = match self.get_upstream_version() { + Ok(version) => version, + Err(error) => match error.downcast_ref::() { + Some(_) => return Ok(CrateState::Unknown), + None => return Err(error), + }, }; let ours = self.get_local_version()?; From b145c448f19d82994020fff6ef8231ef0aa38ca7 Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Thu, 14 Jul 2022 08:51:05 +0200 Subject: [PATCH 3/6] feat: wait for crate version to appeat upstream after submitting it The operator will wait for 10 minutes per crate. --- tools/release-operator/src/registry.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tools/release-operator/src/registry.rs b/tools/release-operator/src/registry.rs index baa42f18e..19fccc259 100644 --- a/tools/release-operator/src/registry.rs +++ b/tools/release-operator/src/registry.rs @@ -5,6 +5,7 @@ use std::fmt::{Display, Formatter}; use std::path::PathBuf; use std::process::Command; use std::str::FromStr; +use std::time::{Duration, Instant}; pub struct Registry { token: SecUtf8, @@ -195,6 +196,31 @@ impl Crate { } } + let ours = self.get_local_version()?; + let delay = Duration::from_secs(10); + let start_time = Instant::now(); + let timeout = Duration::from_secs(600); + + log::info!( + "{self} should appear as {ours} on the registry, waiting... [{delay:?}|{timeout:?}]" + ); + + loop { + if Instant::now() - start_time > timeout { + return Err(anyhow!("{self} did not appear as {ours} on the registry within {timeout:?}")); + } + + let theirs = self.get_upstream_version()?; + + if theirs == ours { + log::info!("{self} appeared as {ours} on the registry"); + break; + } + + log::info!("{self} waiting for {ours}..."); + std::thread::sleep(delay); + } + Ok(()) } } From 4890ec0cf77134b70c77d8bb9ec7ebb26b5ff3d3 Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Thu, 14 Jul 2022 08:58:16 +0200 Subject: [PATCH 4/6] chore: replace if statement with a proper comparison --- tools/release-operator/src/registry.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tools/release-operator/src/registry.rs b/tools/release-operator/src/registry.rs index 19fccc259..23ed29357 100644 --- a/tools/release-operator/src/registry.rs +++ b/tools/release-operator/src/registry.rs @@ -212,9 +212,15 @@ impl Crate { let theirs = self.get_upstream_version()?; - if theirs == ours { - log::info!("{self} appeared as {ours} on the registry"); - break; + match theirs.cmp(&ours) { + std::cmp::Ordering::Less => (), + std::cmp::Ordering::Equal => { + log::info!("{self} appeared as {ours} on the registry"); + break; + } + std::cmp::Ordering::Greater => { + return Err(anyhow!("{self} appeared as {theirs} on the registry which is newer than the current release ({ours})")) + }, } log::info!("{self} waiting for {ours}..."); From 0ca118e898580a1676d298e61d039413c193769c Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Thu, 14 Jul 2022 09:03:09 +0200 Subject: [PATCH 5/6] chore: replace previous comparison with match as well --- tools/release-operator/src/registry.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tools/release-operator/src/registry.rs b/tools/release-operator/src/registry.rs index 23ed29357..5d339c2ec 100644 --- a/tools/release-operator/src/registry.rs +++ b/tools/release-operator/src/registry.rs @@ -157,17 +157,17 @@ impl Crate { let ours = self.get_local_version()?; - if ours == theirs { - log::info!("{self} has already been published as {ours}"); - return Ok(CrateState::Published); - } - - if ours < theirs { - log::warn!("{self} has already been published as {ours}, which is a newer version"); - return Ok(CrateState::Behind); + match theirs.cmp(&ours) { + std::cmp::Ordering::Less => Ok(CrateState::Ahead), + std::cmp::Ordering::Equal => { + log::info!("{self} has already been published as {ours}"); + Ok(CrateState::Published) + } + std::cmp::Ordering::Greater => { + log::warn!("{self} has already been published as {ours}, which is a newer version"); + Ok(CrateState::Behind) + } } - - Ok(CrateState::Ahead) } fn submit(&self, token: &SecUtf8, dry_run: bool) -> anyhow::Result<()> { From ea759e09adef1e4dd17a4382404d516e5f3813c4 Mon Sep 17 00:00:00 2001 From: Hendrik Maus Date: Fri, 15 Jul 2022 21:18:25 +0200 Subject: [PATCH 6/6] chore: pull in thiserror --- Cargo.lock | 1 + tools/release-operator/Cargo.toml | 1 + tools/release-operator/src/registry.rs | 23 +++++++++-------------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d905fb5a0..2749b0476 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2445,6 +2445,7 @@ dependencies = [ "semver", "serde", "serde_json", + "thiserror", ] [[package]] diff --git a/tools/release-operator/Cargo.toml b/tools/release-operator/Cargo.toml index 099e3f88c..709990527 100644 --- a/tools/release-operator/Cargo.toml +++ b/tools/release-operator/Cargo.toml @@ -16,6 +16,7 @@ regex = "1.6.0" secstr = "0.5.0" semver = "1.0.12" serde_json = "1.0.82" +thiserror = "1.0.31" [dependencies.reqwest] version = "0.11.11" diff --git a/tools/release-operator/src/registry.rs b/tools/release-operator/src/registry.rs index 5d339c2ec..1568cfdca 100644 --- a/tools/release-operator/src/registry.rs +++ b/tools/release-operator/src/registry.rs @@ -7,6 +7,12 @@ use std::process::Command; use std::str::FromStr; use std::time::{Duration, Instant}; +#[derive(thiserror::Error, Debug)] +enum Error { + #[error("crate was not found on crates.io")] + CrateNotFound, +} + pub struct Registry { token: SecUtf8, crates: Vec, @@ -54,17 +60,6 @@ impl Registry { } } -#[derive(Debug)] -struct CrateNotFoundError; - -impl std::fmt::Display for CrateNotFoundError { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "crate was not found on crates.io") - } -} - -impl std::error::Error for CrateNotFoundError {} - impl Crate { fn validate(&self) -> anyhow::Result<()> { match self.path.exists() { @@ -126,7 +121,7 @@ impl Crate { .context("fetching crate versions from the registry")?; if resp.status() == reqwest::StatusCode::NOT_FOUND { - return Err(anyhow::Error::new(CrateNotFoundError)); + return Err(anyhow::Error::new(Error::CrateNotFound)); } if resp.status() != reqwest::StatusCode::OK { @@ -149,8 +144,8 @@ impl Crate { fn determine_state(&self) -> anyhow::Result { let theirs = match self.get_upstream_version() { Ok(version) => version, - Err(error) => match error.downcast_ref::() { - Some(_) => return Ok(CrateState::Unknown), + Err(error) => match error.downcast_ref::() { + Some(Error::CrateNotFound) => return Ok(CrateState::Unknown), None => return Err(error), }, };