From 375c589f7d9264afeb60e7a05f9d3105f21250d4 Mon Sep 17 00:00:00 2001 From: Ayush Jha Date: Fri, 24 Nov 2023 20:50:02 +0545 Subject: [PATCH 01/10] Introduce `--wait` flage to package publish The wait flag gets propagated to the backend via graphql params. This makes it so publish package waits for webc generation too pass. --- lib/cli/src/commands/publish.rs | 9 +++++++++ .../graphql/mutations/publish_package_chunked.graphql | 2 ++ lib/registry/src/package/builder.rs | 7 +++++++ lib/registry/src/publish.rs | 7 +++++-- 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/cli/src/commands/publish.rs b/lib/cli/src/commands/publish.rs index 46de87fb12f..09d96b86746 100644 --- a/lib/cli/src/commands/publish.rs +++ b/lib/cli/src/commands/publish.rs @@ -27,6 +27,13 @@ pub struct Publish { /// Defaults to current working directory. #[clap(name = "PACKAGE_PATH")] pub package_path: Option, + /// Wait for package to be available on the registry before exiting + #[clap(long)] + pub wait: bool, + + /// Timeout (in seconds) for the publish query to the registry + #[clap(long, default_value = "30")] + pub timeout: u64, } impl Publish { @@ -46,6 +53,8 @@ impl Publish { token, no_validate: self.no_validate, package_path: self.package_path.clone(), + wait: self.wait, + timeout: std::time::Duration::from_secs(self.timeout), }; publish.execute().map_err(on_error)?; diff --git a/lib/registry/graphql/mutations/publish_package_chunked.graphql b/lib/registry/graphql/mutations/publish_package_chunked.graphql index 76a1c67a7c0..398b4fbbaf4 100644 --- a/lib/registry/graphql/mutations/publish_package_chunked.graphql +++ b/lib/registry/graphql/mutations/publish_package_chunked.graphql @@ -12,6 +12,7 @@ mutation PublishPackageMutationChunked( $signature: InputSignature $signedUrl: String $private: Boolean + $wait: Boolean ) { publishPackage( input: { @@ -29,6 +30,7 @@ mutation PublishPackageMutationChunked( signature: $signature clientMutationId: "" private: $private + wait: $wait } ) { success diff --git a/lib/registry/src/package/builder.rs b/lib/registry/src/package/builder.rs index 544b403e2e6..5c8ab7074b1 100644 --- a/lib/registry/src/package/builder.rs +++ b/lib/registry/src/package/builder.rs @@ -1,5 +1,6 @@ use std::io::{self, Write}; use std::path::{Path, PathBuf}; +use std::time::Duration; use std::{fs, io::IsTerminal}; use anyhow::{anyhow, bail, Context}; @@ -38,6 +39,10 @@ pub struct Publish { pub no_validate: bool, /// Directory containing the `wasmer.toml` (defaults to current root dir) pub package_path: Option, + /// Wait for package to be available on the registry before exiting + pub wait: bool, + /// Timeout (in seconds) for the publish query to the registry + pub timeout: Duration, } #[derive(Debug, Error)] @@ -186,6 +191,8 @@ impl Publish { &maybe_signature_data, archived_data_size, self.quiet, + self.wait, + self.timeout, ) } diff --git a/lib/registry/src/publish.rs b/lib/registry/src/publish.rs index 1f69ef0741f..dcd7e67487a 100644 --- a/lib/registry/src/publish.rs +++ b/lib/registry/src/publish.rs @@ -1,7 +1,7 @@ -use std::collections::BTreeMap; use std::fmt::Write; use std::io::BufRead; use std::path::PathBuf; +use std::{collections::BTreeMap, time::Duration}; use console::{style, Emoji}; use graphql_client::GraphQLQuery; @@ -39,6 +39,8 @@ pub fn try_chunked_uploading( maybe_signature_data: &SignArchiveResult, archived_data_size: u64, quiet: bool, + wait_for_webc_generation: bool, + timeout: Duration, ) -> Result<(), anyhow::Error> { let registry = match registry.as_ref() { Some(s) => format_graphql(s), @@ -232,10 +234,11 @@ pub fn try_chunked_uploading( signature: maybe_signature_data, signed_url: Some(signed_url), private: Some(package.private), + wait: Some(wait_for_webc_generation), }); let _response: publish_package_mutation_chunked::ResponseData = - crate::graphql::execute_query(®istry, &token, &q)?; + crate::graphql::execute_query_with_timeout(®istry, &token, timeout, &q)?; println!( "Successfully published package `{}@{}`", From ad152541ed24cd36cecb6ca6557eedb5e8d64ef5 Mon Sep 17 00:00:00 2001 From: Ayush Jha Date: Sat, 25 Nov 2023 04:18:36 +0545 Subject: [PATCH 02/10] minor renaming of variables --- lib/cli/src/commands/publish.rs | 5 ++--- lib/registry/src/publish.rs | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/cli/src/commands/publish.rs b/lib/cli/src/commands/publish.rs index 09d96b86746..cc2bfa16dde 100644 --- a/lib/cli/src/commands/publish.rs +++ b/lib/cli/src/commands/publish.rs @@ -27,11 +27,10 @@ pub struct Publish { /// Defaults to current working directory. #[clap(name = "PACKAGE_PATH")] pub package_path: Option, - /// Wait for package to be available on the registry before exiting + /// Wait for package to be available on the registry before exiting. #[clap(long)] pub wait: bool, - - /// Timeout (in seconds) for the publish query to the registry + /// Timeout (in seconds) for the publish query to the registry. #[clap(long, default_value = "30")] pub timeout: u64, } diff --git a/lib/registry/src/publish.rs b/lib/registry/src/publish.rs index dcd7e67487a..c0f235bdff6 100644 --- a/lib/registry/src/publish.rs +++ b/lib/registry/src/publish.rs @@ -8,7 +8,6 @@ use graphql_client::GraphQLQuery; use indicatif::{ProgressBar, ProgressState, ProgressStyle}; use crate::graphql::{ - execute_query_modifier_inner, mutations::{publish_package_mutation_chunked, PublishPackageMutationChunked}, queries::{get_signed_url, GetSignedUrl}, }; @@ -39,7 +38,7 @@ pub fn try_chunked_uploading( maybe_signature_data: &SignArchiveResult, archived_data_size: u64, quiet: bool, - wait_for_webc_generation: bool, + wait: bool, timeout: Duration, ) -> Result<(), anyhow::Error> { let registry = match registry.as_ref() { @@ -234,7 +233,7 @@ pub fn try_chunked_uploading( signature: maybe_signature_data, signed_url: Some(signed_url), private: Some(package.private), - wait: Some(wait_for_webc_generation), + wait: Some(wait), }); let _response: publish_package_mutation_chunked::ResponseData = From 62a6b734c6a297ffda4f0c417be93b28546d5f4b Mon Sep 17 00:00:00 2001 From: Ayush Jha Date: Sat, 25 Nov 2023 04:18:48 +0545 Subject: [PATCH 03/10] Use execute_query_with_timeout to fetch the signed url as well --- lib/registry/src/publish.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/registry/src/publish.rs b/lib/registry/src/publish.rs index c0f235bdff6..9b51f93407c 100644 --- a/lib/registry/src/publish.rs +++ b/lib/registry/src/publish.rs @@ -103,8 +103,12 @@ pub fn try_chunked_uploading( expires_after_seconds: Some(60 * 30), }); - let _response: get_signed_url::ResponseData = - execute_query_modifier_inner(®istry, &token, &get_google_signed_url, None, |f| f)?; + let _response: get_signed_url::ResponseData = crate::graphql::execute_query_with_timeout( + ®istry, + &token, + timeout, + &get_google_signed_url, + )?; let url = _response.url.ok_or_else(|| { anyhow::anyhow!( From 833db91bca73dea0ef3eb81df2558cfd285c7342 Mon Sep 17 00:00:00 2001 From: Ayush Jha Date: Sat, 25 Nov 2023 04:25:07 +0545 Subject: [PATCH 04/10] better docs for --timeout flag --- lib/cli/src/commands/publish.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/cli/src/commands/publish.rs b/lib/cli/src/commands/publish.rs index cc2bfa16dde..ce8368fecd0 100644 --- a/lib/cli/src/commands/publish.rs +++ b/lib/cli/src/commands/publish.rs @@ -31,6 +31,10 @@ pub struct Publish { #[clap(long)] pub wait: bool, /// Timeout (in seconds) for the publish query to the registry. + /// + /// Note that this is not the timeout for the entire publish process, but + /// + /// for each individual query to the registry during the publish flow. #[clap(long, default_value = "30")] pub timeout: u64, } From 66cf4f5f8b31227a270c4b1e7a373759517522b7 Mon Sep 17 00:00:00 2001 From: Ayush Jha Date: Sat, 25 Nov 2023 04:25:21 +0545 Subject: [PATCH 05/10] use timeout (defaulting to 30 seconds) for chunked upload of targz file --- lib/registry/src/publish.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/registry/src/publish.rs b/lib/registry/src/publish.rs index 9b51f93407c..2d56dda3714 100644 --- a/lib/registry/src/publish.rs +++ b/lib/registry/src/publish.rs @@ -122,6 +122,7 @@ pub fn try_chunked_uploading( let url = url::Url::parse(&signed_url).unwrap(); let client = reqwest::blocking::Client::builder() .default_headers(reqwest::header::HeaderMap::default()) + .timeout(timeout) .build() .unwrap(); From d4bee712d3bc5dfc6684bf61ef57a2dba77b71bd Mon Sep 17 00:00:00 2001 From: Ayush Jha Date: Sat, 25 Nov 2023 05:02:51 +0545 Subject: [PATCH 06/10] Split the `try_chunked_uploading` function into smaller ones This makes it much more readable --- lib/registry/src/publish.rs | 114 ++++++++++++++++++++++++------------ 1 file changed, 76 insertions(+), 38 deletions(-) diff --git a/lib/registry/src/publish.rs b/lib/registry/src/publish.rs index 2d56dda3714..504984cddce 100644 --- a/lib/registry/src/publish.rs +++ b/lib/registry/src/publish.rs @@ -7,6 +7,7 @@ use console::{style, Emoji}; use graphql_client::GraphQLQuery; use indicatif::{ProgressBar, ProgressState, ProgressStyle}; +use crate::graphql::queries::get_signed_url::GetSignedUrlUrl; use crate::graphql::{ mutations::{publish_package_mutation_chunked, PublishPackageMutationChunked}, queries::{get_signed_url, GetSignedUrl}, @@ -41,6 +42,58 @@ pub fn try_chunked_uploading( wait: bool, timeout: Duration, ) -> Result<(), anyhow::Error> { + let (registry, token) = initialize_registry_and_token(registry, token)?; + + let maybe_signature_data = sign_package(maybe_signature_data); + + // fetch this before showing the `Uploading...` message + // because there is a chance that the registry may not return a signed url. + // This usually happens if the package version already exists in the registry. + let signed_url = google_signed_url(®istry, &token, package, timeout)?; + + if !quiet { + println!("{} {} Uploading...", style("[1/2]").bold().dim(), UPLOAD); + } + + upload_package(&signed_url.url, archive_path, archived_data_size, timeout)?; + + if !quiet { + println!("{} {}Publishing...", style("[2/2]").bold().dim(), PACKAGE); + } + + let q = + PublishPackageMutationChunked::build_query(publish_package_mutation_chunked::Variables { + name: package.name.to_string(), + version: package.version.to_string(), + description: package.description.clone(), + manifest: manifest_string.to_string(), + license: package.license.clone(), + license_file: license_file.to_owned(), + readme: readme.to_owned(), + repository: package.repository.clone(), + homepage: package.homepage.clone(), + file_name: Some(archive_name.to_string()), + signature: maybe_signature_data, + signed_url: Some(signed_url.url), + private: Some(package.private), + wait: Some(wait), + }); + + let _response: publish_package_mutation_chunked::ResponseData = + crate::graphql::execute_query_with_timeout(®istry, &token, timeout, &q)?; + + println!( + "Successfully published package `{}@{}`", + package.name, package.version + ); + + Ok(()) +} + +fn initialize_registry_and_token( + registry: Option, + token: Option, +) -> Result<(String, String), anyhow::Error> { let registry = match registry.as_ref() { Some(s) => format_graphql(s), None => { @@ -72,7 +125,13 @@ pub fn try_chunked_uploading( } }; - let maybe_signature_data = match maybe_signature_data { + Ok((registry, token)) +} + +fn sign_package( + maybe_signature_data: &SignArchiveResult, +) -> Option { + match maybe_signature_data { SignArchiveResult::Ok { public_key_id, signature, @@ -91,12 +150,15 @@ pub fn try_chunked_uploading( //warn!("Publishing package without a verifying signature. Consider registering a key pair with wasmer"); None } - }; - - if !quiet { - println!("{} {} Uploading...", style("[1/2]").bold().dim(), UPLOAD); } +} +fn google_signed_url( + registry: &str, + token: &str, + package: &wasmer_toml::Package, + timeout: Duration, +) -> Result { let get_google_signed_url = GetSignedUrl::build_query(get_signed_url::Variables { name: package.name.to_string(), version: package.version.to_string(), @@ -117,9 +179,16 @@ pub fn try_chunked_uploading( package.version ) })?; + Ok(url) +} - let signed_url = url.url; - let url = url::Url::parse(&signed_url).unwrap(); +fn upload_package( + signed_url: &str, + archive_path: &PathBuf, + archived_data_size: u64, + timeout: Duration, +) -> Result<(), anyhow::Error> { + let url = url::Url::parse(signed_url).unwrap(); let client = reqwest::blocking::Client::builder() .default_headers(reqwest::header::HeaderMap::default()) .timeout(timeout) @@ -218,36 +287,5 @@ pub fn try_chunked_uploading( } pb.finish_and_clear(); - - if !quiet { - println!("{} {}Publishing...", style("[2/2]").bold().dim(), PACKAGE); - } - - let q = - PublishPackageMutationChunked::build_query(publish_package_mutation_chunked::Variables { - name: package.name.to_string(), - version: package.version.to_string(), - description: package.description.clone(), - manifest: manifest_string.to_string(), - license: package.license.clone(), - license_file: license_file.to_owned(), - readme: readme.to_owned(), - repository: package.repository.clone(), - homepage: package.homepage.clone(), - file_name: Some(archive_name.to_string()), - signature: maybe_signature_data, - signed_url: Some(signed_url), - private: Some(package.private), - wait: Some(wait), - }); - - let _response: publish_package_mutation_chunked::ResponseData = - crate::graphql::execute_query_with_timeout(®istry, &token, timeout, &q)?; - - println!( - "Successfully published package `{}@{}`", - package.name, package.version - ); - Ok(()) } From f2caffd5d00f48b8dfcab38f906fe3f5181684b0 Mon Sep 17 00:00:00 2001 From: Ayush Jha Date: Tue, 28 Nov 2023 19:32:10 +0545 Subject: [PATCH 07/10] Use humantime for timeout duration --- Cargo.lock | 7 +++++++ lib/cli/Cargo.toml | 1 + lib/cli/src/commands/publish.rs | 7 +++---- lib/registry/src/publish.rs | 10 +++++----- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 863abecadff..ab420c26af3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2018,6 +2018,12 @@ version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9" +[[package]] +name = "humantime" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a3a5bfb195931eeb336b2a7b4d761daec841b97f947d34394601737a7bba5e4" + [[package]] name = "hyper" version = "0.14.27" @@ -5688,6 +5694,7 @@ dependencies = [ "futures", "hex", "http", + "humantime", "hyper", "indexmap 1.9.3", "indicatif", diff --git a/lib/cli/Cargo.toml b/lib/cli/Cargo.toml index f000e756315..d4f73c82d30 100644 --- a/lib/cli/Cargo.toml +++ b/lib/cli/Cargo.toml @@ -115,6 +115,7 @@ opener = "0.6.1" hyper = { version = "0.14.27", features = ["server"] } http = "0.2.9" futures = "0.3.29" +humantime = "2.1.0" # NOTE: Must use different features for clap because the "color" feature does not # work on wasi due to the anstream dependency not compiling. diff --git a/lib/cli/src/commands/publish.rs b/lib/cli/src/commands/publish.rs index ce8368fecd0..d2d83eecd51 100644 --- a/lib/cli/src/commands/publish.rs +++ b/lib/cli/src/commands/publish.rs @@ -33,10 +33,9 @@ pub struct Publish { /// Timeout (in seconds) for the publish query to the registry. /// /// Note that this is not the timeout for the entire publish process, but - /// /// for each individual query to the registry during the publish flow. - #[clap(long, default_value = "30")] - pub timeout: u64, + #[clap(long, default_value = "30s")] + pub timeout: humantime::Duration, } impl Publish { @@ -57,7 +56,7 @@ impl Publish { no_validate: self.no_validate, package_path: self.package_path.clone(), wait: self.wait, - timeout: std::time::Duration::from_secs(self.timeout), + timeout: self.timeout.into(), }; publish.execute().map_err(on_error)?; diff --git a/lib/registry/src/publish.rs b/lib/registry/src/publish.rs index 504984cddce..f7d17c12305 100644 --- a/lib/registry/src/publish.rs +++ b/lib/registry/src/publish.rs @@ -1,12 +1,12 @@ +use anyhow::Context; +use console::{style, Emoji}; +use graphql_client::GraphQLQuery; +use indicatif::{ProgressBar, ProgressState, ProgressStyle}; use std::fmt::Write; use std::io::BufRead; use std::path::PathBuf; use std::{collections::BTreeMap, time::Duration}; -use console::{style, Emoji}; -use graphql_client::GraphQLQuery; -use indicatif::{ProgressBar, ProgressState, ProgressStyle}; - use crate::graphql::queries::get_signed_url::GetSignedUrlUrl; use crate::graphql::{ mutations::{publish_package_mutation_chunked, PublishPackageMutationChunked}, @@ -188,7 +188,7 @@ fn upload_package( archived_data_size: u64, timeout: Duration, ) -> Result<(), anyhow::Error> { - let url = url::Url::parse(signed_url).unwrap(); + let url = url::Url::parse(signed_url).context("cannot parse signed url")?; let client = reqwest::blocking::Client::builder() .default_headers(reqwest::header::HeaderMap::default()) .timeout(timeout) From c6e08a71c14eb86828feb18c95d5c6d90a8748ef Mon Sep 17 00:00:00 2001 From: Ayush Jha Date: Tue, 28 Nov 2023 19:32:33 +0545 Subject: [PATCH 08/10] Make linter happy --- lib/registry/src/publish.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/registry/src/publish.rs b/lib/registry/src/publish.rs index f7d17c12305..126604e201e 100644 --- a/lib/registry/src/publish.rs +++ b/lib/registry/src/publish.rs @@ -166,8 +166,8 @@ fn google_signed_url( }); let _response: get_signed_url::ResponseData = crate::graphql::execute_query_with_timeout( - ®istry, - &token, + registry, + token, timeout, &get_google_signed_url, )?; From 16330a3391ee030fc25fba3e7ce4d7574f103224 Mon Sep 17 00:00:00 2001 From: Ayush Jha Date: Tue, 28 Nov 2023 19:32:41 +0545 Subject: [PATCH 09/10] add test that publishes pacakge with `--wait` flag then immediately runs it --- tests/integration/cli/tests/publish.rs | 63 ++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/tests/integration/cli/tests/publish.rs b/tests/integration/cli/tests/publish.rs index 1b583467c51..4c3e3c37681 100644 --- a/tests/integration/cli/tests/publish.rs +++ b/tests/integration/cli/tests/publish.rs @@ -1,4 +1,5 @@ use assert_cmd::prelude::OutputAssertExt; +use predicates::str::contains; use wasmer_integration_tests_cli::{fixtures, get_wasmer_path}; #[test] @@ -118,3 +119,65 @@ fn wasmer_init_publish() { "Successfully published package `{username}/randomversion@{random1}.{random2}.{random3}`\n" )); } + +#[test] +fn wasmer_publish_and_run() { + // Only run this test in the CI + if std::env::var("GITHUB_TOKEN").is_err() { + return; + } + + let wapm_dev_token = std::env::var("WAPM_DEV_TOKEN").ok(); + let tempdir = tempfile::tempdir().unwrap(); + let path = tempdir.path(); + let username = "ciuser"; + + let random_major = format!("{}", rand::random::()); + let random_minor = format!("{}", rand::random::()); + let random_patch = format!("{}", rand::random::()); + + std::fs::copy(fixtures::qjs(), path.join("largewasmfile.wasm")).unwrap(); + std::fs::write( + path.join("wasmer.toml"), + include_str!("./fixtures/init6.toml") + .replace("WAPMUSERNAME", username) // <-- TODO! + .replace("RANDOMVERSION1", &random_major) + .replace("RANDOMVERSION2", &random_minor) + .replace("RANDOMVERSION3", &random_patch), + ) + .unwrap(); + + let package_name = + format!("{username}/largewasmfile@{random_major}.{random_minor}.{random_patch}",); + + let mut cmd = std::process::Command::new(get_wasmer_path()); + cmd.arg("publish") + .arg("--quiet") + .arg("--wait") + .arg("--timeout=60s") + .arg("--registry=wasmer.wtf") + .arg(path); + + if let Some(token) = wapm_dev_token { + // Special case: GitHub secrets aren't visible to outside collaborators + if token.is_empty() { + return; + } + cmd.arg("--token").arg(token); + } + + cmd.assert() + .success() + .stdout(format!("Successfully published package `{package_name}`\n")); + + let assert = std::process::Command::new(get_wasmer_path()) + .arg("run") + .arg(format!("https://wasmer.wtf/{package_name}")) + .arg("--entrypoint=quickjs") + .arg("--") + .arg("--eval") + .arg("console.log('Hello, World!')") + .assert(); + + assert.success().stdout(contains("Hello, World!")); +} From b644394c3559b7f1ebbadf1c237760d15e52a44a Mon Sep 17 00:00:00 2001 From: Ayush Jha Date: Tue, 28 Nov 2023 19:40:19 +0545 Subject: [PATCH 10/10] no need to specify entrypoint when running the command --- tests/integration/cli/tests/publish.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/cli/tests/publish.rs b/tests/integration/cli/tests/publish.rs index 4c3e3c37681..b34ceae1974 100644 --- a/tests/integration/cli/tests/publish.rs +++ b/tests/integration/cli/tests/publish.rs @@ -173,7 +173,6 @@ fn wasmer_publish_and_run() { let assert = std::process::Command::new(get_wasmer_path()) .arg("run") .arg(format!("https://wasmer.wtf/{package_name}")) - .arg("--entrypoint=quickjs") .arg("--") .arg("--eval") .arg("console.log('Hello, World!')")