Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add -C flag to build command #734

Merged
merged 5 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
121 changes: 117 additions & 4 deletions crates/cli/src/commands.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use anyhow::{anyhow, bail};
use clap::{Parser, Subcommand};
use std::path::PathBuf;
use std::{path::PathBuf, str::FromStr};

#[derive(Debug, Parser)]
#[command(
Expand Down Expand Up @@ -27,10 +28,10 @@ pub enum Command {
///
/// Use the `build` command instead.
#[command(arg_required_else_help = true)]
Compile(CompileAndBuildCommandOpts),
Compile(CompileCommandOpts),
/// Generates WebAssembly from a JavaScript source.
#[command(arg_required_else_help = true)]
Build(CompileAndBuildCommandOpts),
Build(BuildCommandOpts),
/// Emits the provider binary that is required to run dynamically
/// linked WebAssembly modules.
EmitProvider(EmitProviderCommandOpts),
Expand All @@ -44,7 +45,7 @@ impl Command {
}

#[derive(Debug, Parser)]
pub struct CompileAndBuildCommandOpts {
pub struct CompileCommandOpts {
#[arg(value_name = "INPUT", required = true)]
/// Path of the JavaScript input file.
pub input: PathBuf,
Expand Down Expand Up @@ -73,9 +74,121 @@ pub struct CompileAndBuildCommandOpts {
pub no_source_compression: bool,
}

#[derive(Debug, Parser)]
pub struct BuildCommandOpts {
#[arg(value_name = "INPUT", required = true)]
/// Path of the JavaScript input file.
pub input: PathBuf,

#[arg(short, default_value = "index.wasm")]
/// Desired path of the WebAssembly output file.
pub output: PathBuf,

#[arg(
short = 'C',
long_help = "Available codegen options:
-C dynamic[=y|n] -- Creates a smaller module that requires a dynamically linked QuickJS provider Wasm module to execute (see `emit-provider` command).
-C wit=path -- Optional path to WIT file describing exported functions. Only supports function exports with no arguments and no return values.
-C wit-world=val -- Optional WIT world name for WIT file. Must be specified if WIT is file path is specified.
-C no-source-compression[=y|n] -- Disable source code compression, which reduces compile time at the expense of generating larger WebAssembly files.
"
)]
/// Codegen options.
pub codegen: Vec<CodegenOption>,
}

#[derive(Debug, Parser)]
pub struct EmitProviderCommandOpts {
#[structopt(short, long)]
/// Output path for the provider binary (default is stdout).
pub out: Option<PathBuf>,
}

#[derive(Clone, Debug, Parser)]
pub struct CodegenOptionGroup {
/// Creates a smaller module that requires a dynamically linked QuickJS provider Wasm
/// module to execute (see `emit-provider` command).
pub dynamic: bool,
/// Optional path to WIT file describing exported functions.
/// Only supports function exports with no arguments and no return values.
pub wit: Option<PathBuf>,
/// Optional path to WIT file describing exported functions.
/// Only supports function exports with no arguments and no return values.
pub wit_world: Option<String>,
/// Disable source code compression, which reduces compile time at the expense of generating larger WebAssembly files.
pub no_source_compression: bool,
}

#[derive(Clone, Debug)]
pub enum CodegenOption {
/// Creates a smaller module that requires a dynamically linked QuickJS provider Wasm
/// module to execute (see `emit-provider` command).
Dynamic(bool),
/// Optional path to WIT file describing exported functions.
/// Only supports function exports with no arguments and no return values.
Wit(PathBuf),
/// Optional path to WIT file describing exported functions.
/// Only supports function exports with no arguments and no return values.
WitWorld(String),
jeffcharles marked this conversation as resolved.
Show resolved Hide resolved
/// Disable source code compression, which reduces compile time at the expense of generating larger WebAssembly files.
NoSourceCompression(bool),
jeffcharles marked this conversation as resolved.
Show resolved Hide resolved
}

impl FromStr for CodegenOption {
Copy link
Member

Choose a reason for hiding this comment

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

We had briefly discussed using https://docs.rs/clap/latest/clap/builder/trait.ValueParserFactory.html -- although the current approach seems to work well enough for the current PR. We can always iterate and refactor later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I should've mentioned in the description, I looked briefly at using it. My understanding is it works well if you need to keep track additional state outside of parsing the string to whatever type you're parsing it too. But the tradeoff is it forces you to define an additional type for the parser for the value, which is one more type to define. Ultimately since we have to collect a vector of something, I figured it would be easier to just write the state we need to collect to that something. That said, if we want to ensure there are no duplicate keys present and error out in the option collection phase, then having a custom parser would make that possible. Right now if an option is specified more than once, it'll take the last value as the one to use.

Copy link
Member

Choose a reason for hiding this comment

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

That said, if we want to ensure there are no duplicate keys present and error out in the option collection phase, then having a custom parser would make that possible

In the long run, I think we'd want to ensure this.

A custom parser also gives you more control over how to define/manage your group options and group help (e.g., you can have the help be automatically generated from the options without having to manually update the help string manually every time, which might be error prone).

I think it's fine to leave it as it for this first iteration, we can always refactor later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A custom parser also gives you more control over how to define/manage your group options and group help (e.g., you can have the help be automatically generated from the options without having to manually update the help string manually every time, which might be error prone).

Do you mean defining an implementation for possible_values?

type Err = anyhow::Error;

fn from_str(s: &str) -> std::result::Result<Self, Self::Err> {
let mut parts = s.splitn(2, '=');
let key = parts.next().ok_or_else(|| anyhow!("Invalid codegen key"))?;
let value = parts.next();
let option = match key {
"dynamic" => Self::Dynamic(match value {
None => true,
Some("y") => true,
Some("n") => false,
_ => bail!("Invalid value for dynamic"),
}),
"wit" => Self::Wit(PathBuf::from(
value.ok_or_else(|| anyhow!("Must provide value for wit"))?,
)),
"wit-world" => Self::WitWorld(
value
.ok_or_else(|| anyhow!("Must provide value for wit-world"))?
.to_string(),
),
"no-source-compression" => Self::NoSourceCompression(match value {
None => true,
Some("y") => true,
Some("n") => false,
_ => bail!("Invalid value for no-source-compression"),
}),
_ => bail!("Invalid codegen key"),
};
Ok(option)
}
}

impl From<Vec<CodegenOption>> for CodegenOptionGroup {
fn from(value: Vec<CodegenOption>) -> Self {
let mut dynamic = false;
let mut wit = None;
let mut wit_world = None;
let mut no_source_compression = false;

for option in value {
match option {
CodegenOption::Dynamic(enabled) => dynamic = enabled,
CodegenOption::Wit(path) => wit = Some(path),
CodegenOption::WitWorld(world) => wit_world = Some(world),
CodegenOption::NoSourceCompression(enabled) => no_source_compression = enabled,
}
}

Self {
dynamic,
wit,
wit_world,
no_source_compression,
}
}
}
34 changes: 28 additions & 6 deletions crates/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::commands::{Cli, Command, EmitProviderCommandOpts};
use anyhow::Result;
use clap::Parser;
use codegen::CodeGenBuilder;
use commands::CodegenOptionGroup;
use js::JS;
use std::fs;
use std::fs::File;
Expand All @@ -19,10 +20,9 @@ fn main() -> Result<()> {

match &args.command {
Command::EmitProvider(opts) => emit_provider(opts),
c @ Command::Compile(opts) | c @ Command::Build(opts) => {
if c.is_compile() {
eprintln!(
r#"
Command::Compile(opts) => {
eprintln!(
r#"
The `compile` command will be deprecated in the next major
release of the CLI (v4.0.0)

Expand All @@ -31,8 +31,7 @@ fn main() -> Result<()> {

Use the `build` command instead.
"#
);
}
);

let js = JS::from_file(&opts.input)?;
let mut builder = CodeGenBuilder::new();
Expand All @@ -52,6 +51,29 @@ fn main() -> Result<()> {

let wasm = gen.generate(&js)?;

fs::write(&opts.output, wasm)?;
Ok(())
}
Command::Build(opts) => {
let js = JS::from_file(&opts.input)?;
let codegen: CodegenOptionGroup = opts.codegen.clone().into();
let mut builder = CodeGenBuilder::new();
builder
.wit_opts(WitOptions::from_tuple((
codegen.wit.clone(),
codegen.wit_world.clone(),
))?)
.source_compression(!codegen.no_source_compression)
.provider_version("2");
jeffcharles marked this conversation as resolved.
Show resolved Hide resolved

let mut gen = if codegen.dynamic {
builder.build::<DynamicGenerator>()?
} else {
builder.build::<StaticGenerator>()?
};

let wasm = gen.generate(&js)?;

fs::write(&opts.output, wasm)?;
Ok(())
}
Expand Down
11 changes: 11 additions & 0 deletions crates/cli/tests/common/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use anyhow::Result;
use javy_runner::Builder;

pub fn run_with_compile_and_build<F>(test: F) -> Result<()>
where
F: Fn(&mut Builder) -> Result<()>,
{
test(Builder::default().use_compile())?;
test(&mut Builder::default())?;
Ok(())
}
Loading
Loading