Merge pull request #851 from cloudflare/alewis/use-entry-point

add scaffold_worker method to Site, use all the PathBufs

src/commands/build/wranglerjs/mod.rs

@@ -1,34 +1,32 @@
 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 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
@@ -38,7 +36,7 @@ use std::time::Duration;
 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()?;
 
@@ -60,7 +58,7 @@ pub fn run_build_and_watch(target: &Target, tx: Option<Sender<()>>) -> 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 || {
@@ -70,17 +68,17 @@ pub fn run_build_and_watch(target: &Target, tx: Option<Sender<()>>) -> 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",
-                    bucket
+                    bucket.display()
                 );
             }
         }
@@ -146,8 +144,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`");
@@ -228,22 +226,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> {
@@ -256,15 +238,15 @@ 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()?;
 
         if !status.success() {
             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
@@ -314,13 +296,13 @@ fn install() -> Result<PathBuf, failure::Error> {
     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()
     };
 

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");

src/commands/init/mod.rs

@@ -1,14 +1,13 @@
 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;
 
 pub fn init(
     name: Option<&str>,
     target_type: Option<TargetType>,
-    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,23 @@ 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()),
+        )?;
+
+        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(())
 }
 

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 = PathBuf::from("fake");
+        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 = PathBuf::from("fake");
+        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 = PathBuf::from("fake");
+        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 = PathBuf::from("fake");
+        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 = PathBuf::from("fake");
+        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 = 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);
 
         let test_dir = "test6";
         // If test dir already exists, delete it.

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

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<String>,
+    pub bucket: Option<PathBuf>,
 }
 
 impl fmt::Display for KvNamespace {