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 JS runtime option infra and redirect-stdout-to-stderr option #739

Merged
merged 3 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
22 changes: 7 additions & 15 deletions crates/cli/src/codegen/builder.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::codegen::{CodeGen, CodeGenType, DynamicGenerator, StaticGenerator};
use crate::codegen::{CodeGen, DynamicGenerator, StaticGenerator};
use anyhow::{bail, Result};
use javy_config::Config;
use std::path::PathBuf;

/// Options for using WIT in the code generation process.
Expand Down Expand Up @@ -78,27 +79,18 @@ impl CodeGenBuilder {
self
}

/// Build a [`CodeGenerator`].
pub fn build<T>(self) -> Result<Box<dyn CodeGen>>
where
T: CodeGen,
{
match T::classify() {
CodeGenType::Static => self.build_static(),
CodeGenType::Dynamic => self.build_dynamic(),
}
}

fn build_static(self) -> Result<Box<dyn CodeGen>> {
let mut static_gen = Box::new(StaticGenerator::new());
/// Build a static [`CodeGenerator`].
pub fn build_static(self, js_runtime_config: Config) -> Result<Box<dyn CodeGen>> {
let mut static_gen = Box::new(StaticGenerator::new(js_runtime_config));

static_gen.source_compression = self.source_compression;
static_gen.wit_opts = self.wit_opts;

Ok(static_gen)
}

fn build_dynamic(self) -> Result<Box<dyn CodeGen>> {
/// Build a dynamic [`CodeGenerator`].
pub fn build_dynamic(self) -> Result<Box<dyn CodeGen>> {
let mut dynamic_gen = Box::new(DynamicGenerator::new());

if let Some(v) = self.provider_version {
Expand Down
6 changes: 1 addition & 5 deletions crates/cli/src/codegen/dynamic.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::transform::{self, SourceCodeSection};
use crate::{
codegen::{exports, CodeGen, CodeGenType, Exports, WitOptions},
codegen::{exports, CodeGen, Exports, WitOptions},
js::JS,
};
use anyhow::Result;
Expand Down Expand Up @@ -285,10 +285,6 @@ impl CodeGen for DynamicGenerator {
print_wat(&wasm)?;
Ok(wasm)
}

fn classify() -> CodeGenType {
CodeGenType::Dynamic
}
}

#[cfg(feature = "dump_wat")]
Expand Down
11 changes: 0 additions & 11 deletions crates/cli/src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,9 @@ pub(crate) use exports::*;

use crate::JS;

pub(crate) enum CodeGenType {
/// Static code generation.
Static,
/// Dynamic code generation.
Dynamic,
}

/// Code generator trait to abstract the multiple JS to Wasm code generation
/// paths.
pub(crate) trait CodeGen {
/// Generate Wasm from a given JS source.
fn generate(&mut self, source: &JS) -> anyhow::Result<Vec<u8>>;
/// Classify the [`CodeGen`] type.
fn classify() -> CodeGenType
where
Self: Sized;
}
18 changes: 9 additions & 9 deletions crates/cli/src/codegen/static.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{
codegen::{
exports,
transform::{self, SourceCodeSection},
CodeGen, CodeGenType, Exports, WitOptions,
CodeGen, Exports, WitOptions,
},
js::JS,
};
Expand All @@ -26,17 +26,20 @@ pub(crate) struct StaticGenerator {
function_exports: Exports,
/// WIT options for code generation.
pub wit_opts: WitOptions,
/// JS runtime options for code generation.
pub js_runtime_config: Config,
}

impl StaticGenerator {
/// Creates a new [`StaticGenerator`].
pub fn new() -> Self {
pub fn new(js_runtime_config: Config) -> Self {
let engine = include_bytes!(concat!(env!("OUT_DIR"), "/engine.wasm"));
Self {
engine,
source_compression: Default::default(),
function_exports: Default::default(),
wit_opts: Default::default(),
js_runtime_config,
}
}
}
Expand Down Expand Up @@ -71,9 +74,10 @@ impl CodeGen for StaticGenerator {
.unwrap()
.set_stdin(Box::new(ReadPipe::from(js.as_bytes())));

WASI.get_mut()
.unwrap()
.push_env("JS_RUNTIME_CONFIG", &Config::all().bits().to_string())?;
WASI.get_mut().unwrap().push_env(
"JS_RUNTIME_CONFIG",
&self.js_runtime_config.bits().to_string(),
)?;
};

let wasm = Wizer::new()
Expand Down Expand Up @@ -143,10 +147,6 @@ impl CodeGen for StaticGenerator {
transform::add_producers_section(&mut module.producers);
Ok(module.emit_wasm())
}

fn classify() -> CodeGenType {
CodeGenType::Static
}
}

fn export_exported_js_functions(
Expand Down
104 changes: 95 additions & 9 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 javy_config::Config;
use std::{path::PathBuf, str::FromStr};

use crate::codegen::WitOptions;
Expand Down Expand Up @@ -91,6 +92,16 @@ pub struct BuildCommandOpts {
)]
/// Codegen options.
pub codegen: Vec<CodegenOption>,

#[arg(
short = 'J',
long = "js",
long_help = "Available JS runtime options:
-J redirect-stdout-to-stderr[=y|n] -- Redirects console.log to stderr.
"
)]
/// JS runtime options.
pub js_runtime: Vec<JsRuntimeOption>,
}

#[derive(Debug, Parser)]
Expand All @@ -110,6 +121,16 @@ pub struct CodegenOptionGroup {
pub source_compression: bool,
}

impl Default for CodegenOptionGroup {
fn default() -> Self {
Self {
dynamic: false,
wit: WitOptions::default(),
source_compression: true,
}
}
}

#[derive(Clone, Debug)]
pub enum CodegenOption {
/// Creates a smaller module that requires a dynamically linked QuickJS provider Wasm
Expand Down Expand Up @@ -163,24 +184,89 @@ impl TryFrom<Vec<CodegenOption>> for CodegenOptionGroup {
type Error = anyhow::Error;

fn try_from(value: Vec<CodegenOption>) -> Result<Self, Self::Error> {
let mut dynamic = false;
let mut options = Self::default();
let mut wit = None;
let mut wit_world = None;
let mut source_compression = true;

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

Ok(Self {
dynamic,
wit: WitOptions::from_tuple((wit, wit_world))?,
source_compression,
})
options.wit = WitOptions::from_tuple((wit, wit_world))?;
Ok(options)
}
}

#[derive(Debug, PartialEq)]
pub struct JsRuntimeOptionGroup {
/// Whether to redirect console.log to stderr.
pub redirect_stdout_to_stderr: bool,
}

impl Default for JsRuntimeOptionGroup {
fn default() -> Self {
Self {
redirect_stdout_to_stderr: true,
}
}
}

#[derive(Clone, Debug)]
pub enum JsRuntimeOption {
/// Whether to redirect console.log to stderr.
RedirectStdoutToStderr(bool),
}

impl FromStr for JsRuntimeOption {
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 JS runtime key"))?;
let value = parts.next();
let option = match key {
"redirect-stdout-to-stderr" => Self::RedirectStdoutToStderr(match value {
None => true,
Some("y") => true,
Some("n") => false,
_ => bail!("Invalid value for redirect-stdout-to-stderr"),
}),
_ => bail!("Invalid JS runtime key"),
};
Ok(option)
}
}

impl From<Vec<JsRuntimeOption>> for JsRuntimeOptionGroup {
fn from(value: Vec<JsRuntimeOption>) -> Self {
let mut group = Self::default();

for option in value {
match option {
JsRuntimeOption::RedirectStdoutToStderr(enabled) => {
group.redirect_stdout_to_stderr = enabled;
}
}
}

group
}
}

impl From<JsRuntimeOptionGroup> for Config {
fn from(value: JsRuntimeOptionGroup) -> Self {
let mut config = Self::all();
config.set(
Config::REDIRECT_STDOUT_TO_STDERR,
value.redirect_stdout_to_stderr,
);
config
}
}
20 changes: 13 additions & 7 deletions crates/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ mod commands;
mod js;
mod wit;

use crate::codegen::{DynamicGenerator, StaticGenerator, WitOptions};
use crate::codegen::WitOptions;
use crate::commands::{Cli, Command, EmitProviderCommandOpts};
use anyhow::Result;
use anyhow::{bail, Result};
use clap::Parser;
use codegen::CodeGenBuilder;
use commands::CodegenOptionGroup;
use commands::{CodegenOptionGroup, JsRuntimeOptionGroup};
use javy_config::Config;
use js::JS;
use std::fs;
use std::fs::File;
Expand Down Expand Up @@ -44,9 +45,10 @@ fn main() -> Result<()> {
.provider_version("2");

let mut gen = if opts.dynamic {
builder.build::<DynamicGenerator>()?
builder.build_dynamic()?
} else {
builder.build::<StaticGenerator>()?
let config = Config::all();
builder.build_static(config)?
};

let wasm = gen.generate(&js)?;
Expand All @@ -63,10 +65,14 @@ fn main() -> Result<()> {
.source_compression(codegen.source_compression)
.provider_version("2");

let js_runtime_options: JsRuntimeOptionGroup = opts.js_runtime.clone().into();
let mut gen = if codegen.dynamic {
builder.build::<DynamicGenerator>()?
if js_runtime_options != JsRuntimeOptionGroup::default() {
bail!("Cannot set JS runtime options when building a dynamic module");
}
builder.build_dynamic()?
Copy link
Member

Choose a reason for hiding this comment

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

On a second thought, I wonder if the builder is better suited to perform this validation?

That way:

  1. We can keep the current build method, shared across dynamic/static codegen.
  2. Perform the validation of the option groups through javy::Config
  3. Keep main as slim as possible i.e., avoid baking in builder specific logic.

Copy link
Member

Choose a reason for hiding this comment

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

In other words, we could modify the existing build method signature to be:

fn build<T>(config: javy_config::Config) -> Result<..>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At some point in the future, if the defaults for build and compile diverge, the logic for validating the config for dynamic is going to depend on whether the build or compile command was used. But we can deal with that case when it actually comes up I guess.

} else {
builder.build::<StaticGenerator>()?
builder.build_static(js_runtime_options.into())?
};

let wasm = gen.generate(&js)?;
Expand Down
18 changes: 17 additions & 1 deletion crates/cli/tests/dynamic_linking_test.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use anyhow::Result;
use javy_runner::Builder;
use javy_runner::{Builder, JavyCommand};
use std::path::{Path, PathBuf};
use std::str;

Expand Down Expand Up @@ -152,6 +152,22 @@ fn test_producers_section_present() -> Result<()> {
})
}

#[test]
fn test_using_runtime_flag_with_dynamic_triggers_error() -> Result<()> {
let build_result = Builder::default()
.root(root())
.bin(BIN)
.input("console.js")
.preload("javy_quickjs_provider_v2".into(), provider_module_path())
.command(JavyCommand::Build)
.redirect_stdout_to_stderr(false)
.build();
assert!(build_result.is_err_and(|e| e
.to_string()
.contains("Error: Cannot set JS runtime options when building a dynamic module")));
Ok(())
}

#[test]
// Temporarily ignore given that Javy.JSON is disabled by default.
#[ignore]
Expand Down
Loading
Loading