Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Module upload format + custom build commands + durable object support #1677

Merged
merged 5 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ tokio-native-tls = "0.1.0"
tokio-rustls = "0.14.1"
tokio-tungstenite = { version = "0.11.0", features = ["tls"] }
toml = "0.5.8"
toml_edit = "0.2.0"
twox-hash = "1.6.0"
url = "2.2.0"
uuid = { version = "0.8", features = ["v4"] }
Expand Down
31 changes: 25 additions & 6 deletions src/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,30 @@ use std::process::Command;
pub fn build_target(target: &Target) -> Result<String, failure::Error> {
let target_type = &target.target_type;
match target_type {
TargetType::JavaScript => {
let msg = "JavaScript project found. Skipping unnecessary build!".to_string();
Ok(msg)
}
TargetType::JavaScript => match &target.build {
None => {
let msg = "Basic JavaScript project found. Skipping unnecessary build!".to_string();
xortive marked this conversation as resolved.
Show resolved Hide resolved
Ok(msg)
}
Some(config) => {
if let Some((cmd_str, mut cmd)) = config.build_command() {
StdErr::working(format!("Running {}", cmd_str).as_ref());
Electroid marked this conversation as resolved.
Show resolved Hide resolved
let build_result = cmd.spawn()?.wait()?;
if build_result.success() {
Ok(String::from("Build completed successfully!"))
xortive marked this conversation as resolved.
Show resolved Hide resolved
} else if let Some(code) = build_result.code() {
Err(failure::err_msg(format!(
"Build failed! Status Code: {}",
code
)))
} else {
Err(failure::err_msg("Build failed."))
}
} else {
Ok(String::from("No build command specified, skipping build."))
xortive marked this conversation as resolved.
Show resolved Hide resolved
}
}
},
TargetType::Rust => {
let _ = which::which("rustc").map_err(|e| {
failure::format_err!(
Expand All @@ -31,6 +51,7 @@ pub fn build_target(target: &Target) -> Result<String, failure::Error> {
let command = command(&args, &binary_path);
let command_name = format!("{:?}", command);

StdErr::working("Compiling your project to WebAssembly...");
xortive marked this conversation as resolved.
Show resolved Hide resolved
commands::run(command, &command_name)?;
let msg = "Build succeeded".to_string();
Ok(msg)
Expand All @@ -49,8 +70,6 @@ pub fn build_target(target: &Target) -> Result<String, failure::Error> {
}

pub fn command(args: &[&str], binary_path: &PathBuf) -> Command {
StdErr::working("Compiling your project to WebAssembly...");

let mut c = if cfg!(target_os = "windows") {
let mut c = Command::new("cmd");
c.arg("/C");
Expand Down
1 change: 1 addition & 0 deletions src/commands/kv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ mod tests {
site: None,
vars: None,
text_blobs: None,
build: None,
};
assert!(kv::get_namespace_id(&target_with_dup_kv_bindings, "").is_err());
}
Expand Down
6 changes: 6 additions & 0 deletions src/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ pub fn publish(
}
Err(e) => Err(e),
}?;

// We verify early here, so we don't perform pre-upload tasks if the upload will fail
if let Some(build_config) = &target.build {
build_config.verify_upload_dir()?;
}

if let Some(site_config) = &target.site {
let path = &site_config.bucket.clone();
validate_bucket_location(path)?;
Expand Down
7 changes: 6 additions & 1 deletion src/preview/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,25 @@ use log::info;
use url::Url;
use ws::{Sender, WebSocket};

use crate::build::build_target;
use crate::http;
use crate::settings::global_user::GlobalUser;
use crate::settings::toml::Target;
use crate::terminal::message::{Message, StdOut};
use crate::terminal::open_browser;
use crate::watch::watch_and_build;
use crate::{build::build_target, settings::toml::ScriptFormat};

pub fn preview(
mut target: Target,
user: Option<GlobalUser>,
options: PreviewOpt,
verbose: bool,
) -> Result<(), failure::Error> {
if let Some(build) = &target.build {
if matches!(build.upload_format, ScriptFormat::Modules) {
failure::bail!("wrangler preview does not support previewing modules scripts. Please use wrangler dev instead.");
}
}
build_target(&target)?;

let sites_preview: bool = target.site.is_some();
Expand Down
9 changes: 0 additions & 9 deletions src/settings/metadata.rs

This file was deleted.

1 change: 0 additions & 1 deletion src/settings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ pub mod binding;
mod environment;
mod global_config;
pub mod global_user;
pub mod metadata;
pub mod toml;

pub use environment::{Environment, QueryEnvironment};
Expand Down
115 changes: 115 additions & 0 deletions src/settings/toml/builder.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
use std::env;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be really helpful to see an example TOML with the new additions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new additions would look something like this for a durable object

[builder_config]
build_dir = "./build"
src_dir = "./"
upload_format = "modules"

[durable_objects]
implements = [
  { class_name = "Counter" }
]

or for a regular service worker just

[builder_config]
build_command = "./dummy-build.sh"
build_dir = "./my-build-dir"
src_dir = "./does-not-exist"
upload_format = "service-worker"

Copy link
Contributor

@ObsidianMinor ObsidianMinor Jan 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of underscores and duplicate info... Perhaps it could be streamlined?

[build]
cwd = "."
command = "sh ./dummy-build.sh"
output = "./build"
format = "modules"

[durable_objects] # can't really avoid that one
implements = [{ class = "Counter" }]

I say this because toml is pretty flexible with how you can write out tables and while you may write a table like that, I may write it like

build.cwd = "."
build.command = "sh ./dummy-build.sh"
build.output = "./build"
build.format = "modules"

durable_objects.implements = [{ class = "Counter" }]

With the long names you get

builder_config.build_dir = "."
builder_config.build_command = "sh ./dummy-build.sh"
builder_config.src_dir = "./build"
builder_config.upload_format = "modules"

durable_objects.implements = [{ class_name = "Counter" }]

Toml accepts both but shorter names look better with the second format.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to prefer verbosity when it eliminates confusion, but I agree on build_command -> command.
I'll defer to @greg-mckeon on the rest

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like command, and I think that src_dir should be working_dir. And I mentioned earlier that build_dir should be upload_dir.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd personally want it to be configurable with reasonable defaults. So you can choose a different directory to watch, but it uses the build command's cwd by default, or perhaps just the root folder (where wrangler.toml is).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we talking about configuring cwd?
Wrangler won't work anywhere but the directory with wrangler.toml
If someone needs a different cwd for their build, they can change to it and back in their build script.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for explaining @xortive. I think src_dir is good.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah :/ I made the change to watch_dir already. It's easy to put it back, and I'm completely indifferent to which one we pick

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that's fine idc either way.

use std::path::PathBuf;
use std::process::Command;

use serde::{Deserialize, Serialize};

use super::ScriptFormat;

const UPLOAD_DIR: &str = "dist";
const WATCH_DIR: &str = "src";

#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
#[serde(deny_unknown_fields)]
pub struct Builder {
command: Option<String>,
#[serde(default = "project_root")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

project_dir? Would be in-line with the other names.

pub cwd: PathBuf,
#[serde(default = "upload_dir")]
pub upload_dir: PathBuf,
pub upload_format: ScriptFormat,
pub upload_include: Option<Vec<String>>,
pub upload_exclude: Option<Vec<String>>,
#[serde(default = "watch_dir")]
pub watch_dir: PathBuf,
}

fn project_root() -> PathBuf {
env::current_dir().unwrap()
}

fn upload_dir() -> PathBuf {
project_root().join(UPLOAD_DIR)
}

fn watch_dir() -> PathBuf {
project_root().join(WATCH_DIR)
}

impl Builder {
pub fn verify_watch_dir(&self) -> Result<(), failure::Error> {
let watch_canonical = match self.watch_dir.canonicalize() {
Ok(path) => path,
Err(e) if matches!(e.kind(), std::io::ErrorKind::NotFound) => failure::bail!(
"Your provided watch_dir {} does not exist.",
self.watch_dir.display()
),
Err(e) => failure::bail!(
"Error encountered when verifying watch_dir: {}, provided path: {}",
e,
self.watch_dir.display()
),
};
let root_canonical = project_root().canonicalize()?;
if watch_canonical == root_canonical {
failure::bail!("Wrangler doesn't support using the project root as the watch_dir.");
}
if !self.watch_dir.is_dir() {
failure::bail!(format!(
"A path was provided for watch_dir that is not a directory: {}",
self.watch_dir.display()
));
}
Ok(())
}

pub fn verify_upload_dir(&self) -> Result<(), failure::Error> {
let upload_canonical = match self.upload_dir.canonicalize() {
Ok(path) => path,
Err(e) if matches!(e.kind(), std::io::ErrorKind::NotFound) => failure::bail!(
"Your provided upload_dir {} does not exist.",
self.upload_dir.display()
),
Err(e) => failure::bail!(
"Error encountered when verifying upload_dir: {}, provided path: {}",
e,
self.upload_dir.display()
),
};
let root_canonical = project_root().canonicalize()?;
if upload_canonical == root_canonical {
failure::bail!("Wrangler doesn't support using the project root as the upload_dir.");
}
if !self.upload_dir.is_dir() {
failure::bail!(format!(
"A path was provided for upload_dir that is not a directory: {}",
self.upload_dir.display()
));
}
Ok(())
}

pub fn build_command(&self) -> Option<(&str, Command)> {
match &self.command {
Some(cmd) => {
let mut c = if cfg!(target_os = "windows") {
let args: Vec<&str> = cmd.split_whitespace().collect();
let mut c = Command::new("cmd");
c.arg("/C");
c.args(args.as_slice());
c
} else {
let mut c = Command::new("sh");
c.arg("-c");
c.arg(cmd);
c
};

c.current_dir(&self.cwd);

Some((cmd, c))
}
None => None,
}
}
}
2 changes: 2 additions & 0 deletions src/settings/toml/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::path::PathBuf;
use serde::{Deserialize, Serialize};
use serde_with::rust::string_empty_as_none;

use crate::settings::toml::builder::Builder;
use crate::settings::toml::kv_namespace::ConfigKvNamespace;
use crate::settings::toml::route::RouteConfig;
use crate::settings::toml::site::Site;
Expand All @@ -21,6 +22,7 @@ pub struct Environment {
#[serde(default, with = "string_empty_as_none")]
pub zone_id: Option<String>,
pub webpack_config: Option<String>,
pub build: Option<Builder>,
pub private: Option<bool>,
pub site: Option<Site>,
#[serde(alias = "kv-namespaces")]
Expand Down
Loading