From 83e8f498de10295aec11c76675d0614107410ad2 Mon Sep 17 00:00:00 2001 From: Ashley Lewis Date: Tue, 5 Nov 2019 16:34:24 -0600 Subject: [PATCH 1/5] Site.entry_point: Option -> Option --- src/commands/kv/bucket/mod.rs | 58 +++++++++++++---------------------- src/settings/target/site.rs | 10 +++--- src/settings/target/target.rs | 8 +++-- 3 files changed, 33 insertions(+), 43 deletions(-) diff --git a/src/commands/kv/bucket/mod.rs b/src/commands/kv/bucket/mod.rs index bcba618e2..1bf9d0141 100644 --- a/src/commands/kv/bucket/mod.rs +++ b/src/commands/kv/bucket/mod.rs @@ -278,12 +278,9 @@ mod tests { #[test] fn it_can_ignore_node_modules() { - let target = make_target(Site { - bucket: "fake".to_string(), - entry_point: None, - include: None, - exclude: None, - }); + let mut site = Site::default(); + site.bucket = "fake".to_string(); + let target = make_target(site); let test_dir = "test1"; // If test dir already exists, delete it. @@ -308,12 +305,9 @@ mod tests { #[test] fn it_can_ignore_hidden() { - let target = make_target(Site { - bucket: "fake".to_string(), - entry_point: None, - include: None, - exclude: None, - }); + let mut site = Site::default(); + site.bucket = "fake".to_string(); + let target = make_target(site); let test_dir = "test2"; // If test dir already exists, delete it. @@ -338,12 +332,9 @@ mod tests { #[test] fn it_can_allow_unfiltered_files() { - let target = make_target(Site { - bucket: "fake".to_string(), - entry_point: None, - include: None, - exclude: None, - }); + let mut site = Site::default(); + site.bucket = "fake".to_string(); + let target = make_target(site); let test_dir = "test3"; // If test dir already exists, delete it. @@ -368,12 +359,10 @@ mod tests { #[test] fn it_can_filter_by_include() { - let target = make_target(Site { - bucket: "fake".to_string(), - entry_point: None, - include: Some(vec!["this_isnt_here.txt".to_string()]), - exclude: None, - }); + let mut site = Site::default(); + site.bucket = "fake".to_string(); + site.include = Some(vec!["this_isnt_here.txt".to_string()]); + let target = make_target(site); let test_dir = "test4"; // If test dir already exists, delete it. @@ -398,12 +387,10 @@ mod tests { #[test] fn it_can_filter_by_exclude() { - let target = make_target(Site { - bucket: "fake".to_string(), - entry_point: None, - include: None, - exclude: Some(vec!["ignore_me.txt".to_string()]), - }); + let mut site = Site::default(); + site.bucket = "fake".to_string(); + site.exclude = Some(vec!["ignore_me.txt".to_string()]); + let target = make_target(site); let test_dir = "test5"; // If test dir already exists, delete it. @@ -428,12 +415,11 @@ mod tests { #[test] fn it_can_prioritize_include_over_exclude() { - let target = make_target(Site { - bucket: "fake".to_string(), - entry_point: None, - include: Some(vec!["notice_me.txt".to_string()]), - exclude: Some(vec!["notice_me.txt".to_string()]), - }); + let mut site = Site::default(); + site.bucket = "fake".to_string(); + site.include = Some(vec!["notice_me.txt".to_string()]); + site.exclude = Some(vec!["notice_me.txt".to_string()]); + let target = make_target(site); let test_dir = "test6"; // If test dir already exists, delete it. diff --git a/src/settings/target/site.rs b/src/settings/target/site.rs index 46f98a5ce..5e137f20f 100644 --- a/src/settings/target/site.rs +++ b/src/settings/target/site.rs @@ -1,3 +1,4 @@ +use std::env; use std::path::PathBuf; use serde::{Deserialize, Serialize}; @@ -8,7 +9,7 @@ const SITE_ENTRY_POINT: &str = "workers-site"; pub struct Site { pub bucket: String, #[serde(rename = "entry-point")] - pub entry_point: Option, + entry_point: Option, pub include: Option>, pub exclude: Option>, } @@ -24,11 +25,12 @@ impl Site { // if the user has configured `site.entry-point`, use that // as the build directory. Otherwise use the default const // SITE_ENTRY_POINT - pub fn build_dir(&self, current_dir: PathBuf) -> Result { + pub fn entry_point(&self) -> Result { + let current_dir = env::current_dir()?; Ok(current_dir.join( self.entry_point .to_owned() - .unwrap_or_else(|| format!("./{}", SITE_ENTRY_POINT)), + .unwrap_or_else(|| PathBuf::from(SITE_ENTRY_POINT)), )) } } @@ -37,7 +39,7 @@ impl Default for Site { fn default() -> Site { Site { bucket: String::new(), - entry_point: Some(String::from(SITE_ENTRY_POINT)), + entry_point: Some(PathBuf::from(SITE_ENTRY_POINT)), include: None, exclude: None, } diff --git a/src/settings/target/target.rs b/src/settings/target/target.rs index 42dc49e64..8c8f341c6 100644 --- a/src/settings/target/target.rs +++ b/src/settings/target/target.rs @@ -37,12 +37,14 @@ impl Target { } pub fn build_dir(&self) -> Result { - let current_dir = env::current_dir()?; // if `site` is configured, we want to isolate worker code // and build artifacts away from static site application code. match &self.site { - Some(site_config) => site_config.build_dir(current_dir), - None => Ok(current_dir), + Some(site_config) => site_config.entry_point(), + None => { + let current_dir = env::current_dir()?; + Ok(current_dir) + } } } } From 7a43fb8845d32821e1cf0501eea10a786dd00b5a Mon Sep 17 00:00:00 2001 From: Ashley Lewis Date: Wed, 6 Nov 2019 12:15:34 -0600 Subject: [PATCH 2/5] refactor: make scaffold_worker a method on site --- src/commands/build/wranglerjs/mod.rs | 53 ++++++++++------------------ src/commands/generate/mod.rs | 9 ++--- src/commands/init/mod.rs | 31 ++++++++-------- src/settings/target/site.rs | 19 ++++++++++ 4 files changed, 59 insertions(+), 53 deletions(-) diff --git a/src/commands/build/wranglerjs/mod.rs b/src/commands/build/wranglerjs/mod.rs index 12f58dd7c..f74c65578 100644 --- a/src/commands/build/wranglerjs/mod.rs +++ b/src/commands/build/wranglerjs/mod.rs @@ -1,34 +1,33 @@ mod bundle; pub mod output; -use crate::commands::build::watch::wait_for_changes; -use crate::commands::build::watch::COOLDOWN_PERIOD; -use crate::commands::generate::run_generate; - -use crate::commands::publish::package::Package; -use crate::install; -use crate::util; pub use bundle::Bundle; -use fs2::FileExt; -use log::info; -use output::WranglerjsOutput; -use rand::distributions::Alphanumeric; -use rand::{thread_rng, Rng}; + use std::env; use std::fs; use std::fs::File; use std::iter; use std::path::{Path, PathBuf}; use std::process::Command; - -use crate::settings::target::Target; -use crate::terminal::message; - -use notify::{self, RecursiveMode, Watcher}; use std::sync::mpsc::{channel, Sender}; use std::thread; use std::time::Duration; +use fs2::FileExt; +use log::info; +use notify::{self, RecursiveMode, Watcher}; +use output::WranglerjsOutput; +use rand::distributions::Alphanumeric; +use rand::{thread_rng, Rng}; + +use crate::commands::build::watch::wait_for_changes; +use crate::commands::build::watch::COOLDOWN_PERIOD; +use crate::commands::publish::package::Package; +use crate::install; +use crate::settings::target::Target; +use crate::terminal::message; +use crate::util; + // Run the underlying {wranglerjs} executable. // In Rust we create a virtual file, pass it to {wranglerjs}, run the @@ -146,8 +145,8 @@ fn setup_build(target: &Target) -> Result<(Command, PathBuf, Bundle), failure::E let build_dir = target.build_dir()?; - if target.site.is_some() { - scaffold_site_worker(&target)?; + if let Some(site) = &target.site { + site.scaffold_worker()?; } run_npm_install(&build_dir).expect("could not run `npm install`"); @@ -229,22 +228,6 @@ fn build_with_default_webpack( Ok(()) } -pub fn scaffold_site_worker(target: &Target) -> Result<(), failure::Error> { - let build_dir = target.build_dir()?; - let template = "https://github.com/cloudflare/worker-sites-init"; - - if !Path::new(&build_dir).exists() { - // TODO: use site.entry_point instead of build_dir explicitly. - run_generate(build_dir.file_name().unwrap().to_str().unwrap(), template)?; - - // This step is to prevent having a git repo within a git repo after - // generating the scaffold into an existing project. - fs::remove_dir_all(&build_dir.join(".git"))?; - } - - Ok(()) -} - // Run {npm install} in the specified directory. Skips the install if a // {node_modules} is found in the directory. fn run_npm_install(dir: &PathBuf) -> Result<(), failure::Error> { diff --git a/src/commands/generate/mod.rs b/src/commands/generate/mod.rs index 38bc6dbc0..3efe39f86 100644 --- a/src/commands/generate/mod.rs +++ b/src/commands/generate/mod.rs @@ -12,7 +12,10 @@ pub fn generate( site: bool, ) -> Result<(), failure::Error> { validate_worker_name(name)?; + + log::info!("Generating a new worker project with name '{}'", name); run_generate(name, template)?; + let config_path = PathBuf::from("./").join(&name); // TODO: this is tightly coupled to our site template. Need to remove once // we refine our generate logic. @@ -32,15 +35,13 @@ pub fn run_generate(name: &str, template: &str) -> Result<(), failure::Error> { let args = ["generate", "--git", template, "--name", name, "--force"]; - let command = command(name, binary_path, &args); + let command = command(binary_path, &args); let command_name = format!("{:?}", command); commands::run(command, &command_name) } -fn command(name: &str, binary_path: PathBuf, args: &[&str]) -> Command { - log::info!("Generating a new worker project with name '{}'", name); - +fn command(binary_path: PathBuf, args: &[&str]) -> Command { let mut c = if cfg!(target_os = "windows") { let mut c = Command::new("cmd"); c.arg("/C"); diff --git a/src/commands/init/mod.rs b/src/commands/init/mod.rs index 8ccc41fd8..d4598b933 100644 --- a/src/commands/init/mod.rs +++ b/src/commands/init/mod.rs @@ -1,6 +1,5 @@ use std::path::{Path, PathBuf}; -use crate::commands; use crate::commands::validate_worker_name; use crate::settings::target::{Manifest, Site, TargetType}; use crate::terminal::message; @@ -8,7 +7,7 @@ use crate::terminal::message; pub fn init( name: Option<&str>, target_type: Option, - site: bool, + site_flag: bool, ) -> Result<(), failure::Error> { if Path::new("./wrangler.toml").exists() { failure::bail!("A wrangler.toml file already exists! Please remove it before running this command again."); @@ -19,20 +18,24 @@ pub fn init( let target_type = target_type.unwrap_or_default(); let config_path = PathBuf::from("./"); - let initialized_site = if site { Some(Site::default()) } else { None }; - let manifest = Manifest::generate( - name.to_string(), - Some(target_type), - &config_path, - initialized_site, - )?; - message::success("Succesfully created a `wrangler.toml`"); - if site { - let env = None; - let target = manifest.get_target(env)?; - commands::build::wranglerjs::scaffold_site_worker(&target)?; + if site_flag { + let site = Site::default(); + Manifest::generate( + name.to_string(), + Some(target_type), + &config_path, + Some(site.clone()), + )?; + message::success("Succesfully created a `wrangler.toml`"); + + site.scaffold_worker()?; + message::success("Succesfully scaffolded workers site"); + } else { + Manifest::generate(name.to_string(), Some(target_type), &config_path, None)?; + message::success("Succesfully created a `wrangler.toml`"); } + Ok(()) } diff --git a/src/settings/target/site.rs b/src/settings/target/site.rs index 5e137f20f..7f653ed39 100644 --- a/src/settings/target/site.rs +++ b/src/settings/target/site.rs @@ -1,8 +1,11 @@ use std::env; +use std::fs; use std::path::PathBuf; use serde::{Deserialize, Serialize}; +use crate::commands::generate::run_generate; + const SITE_ENTRY_POINT: &str = "workers-site"; #[derive(Clone, Debug, Deserialize, Serialize)] @@ -33,6 +36,22 @@ impl Site { .unwrap_or_else(|| PathBuf::from(SITE_ENTRY_POINT)), )) } + + pub fn scaffold_worker(&self) -> Result<(), failure::Error> { + let entry_point = &self.entry_point()?; + let template = "https://github.com/cloudflare/worker-sites-init"; + + if !entry_point.exists() { + log::info!("Generating a new workers site project"); + run_generate(entry_point.file_name().unwrap().to_str().unwrap(), template)?; + + // This step is to prevent having a git repo within a git repo after + // generating the scaffold into an existing project. + fs::remove_dir_all(&entry_point.join(".git"))?; + } + + Ok(()) + } } impl Default for Site { From f70826ba2dd15a5ac69798520b5022e2d798ab1d Mon Sep 17 00:00:00 2001 From: Ashley Lewis Date: Wed, 6 Nov 2019 13:52:34 -0600 Subject: [PATCH 3/5] =?UTF-8?q?=F0=9F=92=85:=20log::info!?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/commands/build/wranglerjs/mod.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/commands/build/wranglerjs/mod.rs b/src/commands/build/wranglerjs/mod.rs index f74c65578..240f2b5f4 100644 --- a/src/commands/build/wranglerjs/mod.rs +++ b/src/commands/build/wranglerjs/mod.rs @@ -14,7 +14,6 @@ use std::thread; use std::time::Duration; use fs2::FileExt; -use log::info; use notify::{self, RecursiveMode, Watcher}; use output::WranglerjsOutput; use rand::distributions::Alphanumeric; @@ -37,7 +36,7 @@ use crate::util; pub fn run_build(target: &Target) -> Result<(), failure::Error> { let (mut command, temp_file, bundle) = setup_build(target)?; - info!("Running {:?}", command); + log::info!("Running {:?}", command); let status = command.status()?; @@ -59,7 +58,7 @@ pub fn run_build_and_watch(target: &Target, tx: Option>) -> Result<() let is_site = target.site.clone(); - info!("Running {:?} in watch mode", command); + log::info!("Running {:?} in watch mode", command); // Turbofish the result of the closure so we can use ? thread::spawn::<_, Result<(), failure::Error>>(move || { @@ -69,13 +68,13 @@ pub fn run_build_and_watch(target: &Target, tx: Option>) -> Result<() let mut watcher = notify::watcher(watcher_tx, Duration::from_secs(1))?; watcher.watch(&temp_file, RecursiveMode::Recursive)?; - info!("watching temp file {:?}", &temp_file); + log::info!("watching temp file {:?}", &temp_file); if let Some(site) = is_site { let bucket = site.bucket; if Path::new(&bucket).exists() { watcher.watch(&bucket, RecursiveMode::Recursive)?; - info!("watching static sites asset file {:?}", &bucket); + log::info!("watching static sites asset file {:?}", &bucket); } else { failure::bail!( "Attempting to watch static assets bucket \"{}\" which doesn't exist", @@ -240,7 +239,7 @@ fn run_npm_install(dir: &PathBuf) -> Result<(), failure::Error> { let mut command = build_npm_command(); command.current_dir(dir.clone()); command.arg("install"); - info!("Running {:?} in directory {:?}", command, dir); + log::info!("Running {:?} in directory {:?}", command, dir); let status = command.status()?; @@ -248,7 +247,7 @@ fn run_npm_install(dir: &PathBuf) -> Result<(), failure::Error> { failure::bail!("failed to execute `{:?}`: exited with {}", command, status) } } else { - info!("skipping npm install because node_modules exists"); + log::info!("skipping npm install because node_modules exists"); } // TODO: (sven) figure out why the file doesn't exist in some cases @@ -298,13 +297,13 @@ fn install() -> Result { let wranglerjs_path = if install::target::DEBUG { let source_path = get_source_dir(); let wranglerjs_path = source_path.join("wranglerjs"); - info!("wranglerjs at: {:?}", wranglerjs_path); + log::info!("wranglerjs at: {:?}", wranglerjs_path); wranglerjs_path } else { let tool_name = "wranglerjs"; let version = env!("CARGO_PKG_VERSION"); let wranglerjs_path = install::install_artifact(tool_name, "cloudflare", version)?; - info!("wranglerjs downloaded at: {:?}", wranglerjs_path.path()); + log::info!("wranglerjs downloaded at: {:?}", wranglerjs_path.path()); wranglerjs_path.path() }; From eb94d00a48840dbd7096e789ee75ac348c43cb02 Mon Sep 17 00:00:00 2001 From: Ashley Lewis Date: Wed, 6 Nov 2019 17:43:45 -0600 Subject: [PATCH 4/5] refactor: use PathBuf for bucket --- src/commands/build/wranglerjs/mod.rs | 2 +- src/commands/kv/bucket/mod.rs | 12 ++++++------ src/commands/publish/mod.rs | 10 ++++++---- src/settings/target/kv_namespace.rs | 6 ++++-- src/settings/target/site.rs | 6 +++--- 5 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/commands/build/wranglerjs/mod.rs b/src/commands/build/wranglerjs/mod.rs index 240f2b5f4..ac81ea5b7 100644 --- a/src/commands/build/wranglerjs/mod.rs +++ b/src/commands/build/wranglerjs/mod.rs @@ -78,7 +78,7 @@ pub fn run_build_and_watch(target: &Target, tx: Option>) -> Result<() } else { failure::bail!( "Attempting to watch static assets bucket \"{}\" which doesn't exist", - bucket + bucket.display() ); } } diff --git a/src/commands/kv/bucket/mod.rs b/src/commands/kv/bucket/mod.rs index 1bf9d0141..c1b05f218 100644 --- a/src/commands/kv/bucket/mod.rs +++ b/src/commands/kv/bucket/mod.rs @@ -279,7 +279,7 @@ mod tests { #[test] fn it_can_ignore_node_modules() { let mut site = Site::default(); - site.bucket = "fake".to_string(); + site.bucket = PathBuf::from("fake"); let target = make_target(site); let test_dir = "test1"; @@ -306,7 +306,7 @@ mod tests { #[test] fn it_can_ignore_hidden() { let mut site = Site::default(); - site.bucket = "fake".to_string(); + site.bucket = PathBuf::from("fake"); let target = make_target(site); let test_dir = "test2"; @@ -333,7 +333,7 @@ mod tests { #[test] fn it_can_allow_unfiltered_files() { let mut site = Site::default(); - site.bucket = "fake".to_string(); + site.bucket = PathBuf::from("fake"); let target = make_target(site); let test_dir = "test3"; @@ -360,7 +360,7 @@ mod tests { #[test] fn it_can_filter_by_include() { let mut site = Site::default(); - site.bucket = "fake".to_string(); + site.bucket = PathBuf::from("fake"); site.include = Some(vec!["this_isnt_here.txt".to_string()]); let target = make_target(site); @@ -388,7 +388,7 @@ mod tests { #[test] fn it_can_filter_by_exclude() { let mut site = Site::default(); - site.bucket = "fake".to_string(); + site.bucket = PathBuf::from("fake"); site.exclude = Some(vec!["ignore_me.txt".to_string()]); let target = make_target(site); @@ -416,7 +416,7 @@ mod tests { #[test] fn it_can_prioritize_include_over_exclude() { let mut site = Site::default(); - site.bucket = "fake".to_string(); + site.bucket = PathBuf::from("fake"); site.include = Some(vec!["notice_me.txt".to_string()]); site.exclude = Some(vec!["notice_me.txt".to_string()]); let target = make_target(site); diff --git a/src/commands/publish/mod.rs b/src/commands/publish/mod.rs index 11bc246c8..51d4919bb 100644 --- a/src/commands/publish/mod.rs +++ b/src/commands/publish/mod.rs @@ -5,9 +5,9 @@ pub mod upload_form; pub use package::Package; -use crate::settings::target::KvNamespace; use route::Route; +use std::env; use std::path::Path; use crate::commands; @@ -17,8 +17,7 @@ use crate::commands::subdomain::Subdomain; use crate::commands::validate_worker_name; use crate::http; use crate::settings::global_user::GlobalUser; - -use crate::settings::target::{Site, Target}; +use crate::settings::target::{KvNamespace, Site, Target}; use crate::terminal::{emoji, message}; pub fn publish( @@ -158,7 +157,10 @@ pub fn upload_buckets( let mut asset_manifest = None; for namespace in &target.kv_namespaces() { if let Some(bucket) = &namespace.bucket { - if bucket.is_empty() { + // We don't want folks setting their bucket to the top level directory, + // which is where wrangler commands are always called from. + let current_dir = env::current_dir()?; + if bucket.as_os_str() == current_dir { failure::bail!( "{} You need to specify a bucket directory in your wrangler.toml", emoji::WARN diff --git a/src/settings/target/kv_namespace.rs b/src/settings/target/kv_namespace.rs index 8f9e53d98..50a0b3a51 100644 --- a/src/settings/target/kv_namespace.rs +++ b/src/settings/target/kv_namespace.rs @@ -1,13 +1,15 @@ -use crate::settings::binding::Binding; use std::fmt; +use std::path::PathBuf; use serde::{Deserialize, Serialize}; +use crate::settings::binding::Binding; + #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub struct KvNamespace { pub id: String, pub binding: String, - pub bucket: Option, + pub bucket: Option, } impl fmt::Display for KvNamespace { diff --git a/src/settings/target/site.rs b/src/settings/target/site.rs index 7f653ed39..8d0c901be 100644 --- a/src/settings/target/site.rs +++ b/src/settings/target/site.rs @@ -10,7 +10,7 @@ const SITE_ENTRY_POINT: &str = "workers-site"; #[derive(Clone, Debug, Deserialize, Serialize)] pub struct Site { - pub bucket: String, + pub bucket: PathBuf, #[serde(rename = "entry-point")] entry_point: Option, pub include: Option>, @@ -20,7 +20,7 @@ pub struct Site { impl Site { pub fn new(bucket: &str) -> Site { let mut site = Site::default(); - site.bucket = String::from(bucket); + site.bucket = PathBuf::from(bucket); site } @@ -57,7 +57,7 @@ impl Site { impl Default for Site { fn default() -> Site { Site { - bucket: String::new(), + bucket: PathBuf::new(), entry_point: Some(PathBuf::from(SITE_ENTRY_POINT)), include: None, exclude: None, From 8721ba63864b436d0506ad7e4f2476eb6070d506 Mon Sep 17 00:00:00 2001 From: Ashley Lewis Date: Wed, 6 Nov 2019 17:48:34 -0600 Subject: [PATCH 5/5] DRY up message per Avery's comment --- src/commands/init/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/commands/init/mod.rs b/src/commands/init/mod.rs index d4598b933..565070389 100644 --- a/src/commands/init/mod.rs +++ b/src/commands/init/mod.rs @@ -27,15 +27,14 @@ pub fn init( &config_path, Some(site.clone()), )?; - message::success("Succesfully created a `wrangler.toml`"); site.scaffold_worker()?; message::success("Succesfully scaffolded workers site"); } else { Manifest::generate(name.to_string(), Some(target_type), &config_path, None)?; - message::success("Succesfully created a `wrangler.toml`"); } + message::success("Succesfully created a `wrangler.toml`"); Ok(()) }