diff --git a/wasm-rpc-stubgen/src/cargo.rs b/wasm-rpc-stubgen/src/cargo.rs index 53f04d2c..b104ae8b 100644 --- a/wasm-rpc-stubgen/src/cargo.rs +++ b/wasm-rpc-stubgen/src/cargo.rs @@ -23,7 +23,6 @@ use cargo_toml::{ }; use serde::{Deserialize, Serialize}; use toml::Value; -use wit_parser::UnresolvedPackage; use golem_wasm_rpc::WASM_RPC_VERSION; diff --git a/wasm-rpc-stubgen/src/lib.rs b/wasm-rpc-stubgen/src/lib.rs index 3d0260eb..f2ad5781 100644 --- a/wasm-rpc-stubgen/src/lib.rs +++ b/wasm-rpc-stubgen/src/lib.rs @@ -23,7 +23,9 @@ use crate::cargo::generate_cargo_toml; use crate::compilation::compile; use crate::rust::generate_stub_source; use crate::stub::StubDefinition; -use crate::wit::{copy_wit_files, generate_stub_wit, get_stub_wit, verify_action, WitAction}; +use crate::wit::{ + copy_wit_files, generate_stub_wit, get_stub_wit, verify_action, StubTypeGen, WitAction, +}; use anyhow::{anyhow, Context}; use clap::Parser; use fs_extra::dir::CopyOptions; @@ -32,10 +34,10 @@ use golem_wasm_ast::component::Component; use golem_wasm_ast::IgnoreAllButMetadata; use heck::ToSnakeCase; use std::fs; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use tempdir::TempDir; use wasm_compose::config::Dependency; -use wit_parser::{Resolve, UnresolvedPackage}; +use wit_parser::UnresolvedPackage; #[derive(Parser, Debug)] #[command(name = "wasm-rpc-stubgen", version)] @@ -239,7 +241,7 @@ pub async fn build(args: BuildArgs) -> anyhow::Result<()> { Ok(()) } -fn find_if_same_package(dep_dir: &PathBuf, target_wit: &UnresolvedPackage) -> anyhow::Result { +fn find_if_same_package(dep_dir: &Path, target_wit: &UnresolvedPackage) -> anyhow::Result { let dep_package_name = UnresolvedPackage::parse_dir(dep_dir)?.name; let dest_package = target_wit.name.clone(); @@ -248,8 +250,7 @@ fn find_if_same_package(dep_dir: &PathBuf, target_wit: &UnresolvedPackage) -> an } else { println!( "Skipping the copy of cyclic dependencies {} to the the same as {}", - dep_package_name, - dest_package + dep_package_name, dest_package ); Ok(false) } @@ -265,21 +266,21 @@ pub fn add_stub_dependency(args: AddStubDependencyArgs) -> anyhow::Result<()> { // We filter the dependencies of stub that's already existing in dest_wit_root let filtered_source_deps = source_deps .into_iter() - .filter(|dep| find_if_same_package(&dep, &destination_wit_root).unwrap()) + .filter(|dep| find_if_same_package(dep, &destination_wit_root).unwrap()) .collect::>(); let main_wit = args.stub_wit_root.join("_stub.wit"); let stub_dependency = StubDefinition::new( &args.dest_wit_root, - &args.stub_wit_root.parent().unwrap(), + args.stub_wit_root.parent().unwrap(), &None, // Unavailable at this stage because of the disconnect between stub creation and adding stub as a dependency - &"0.0.1".to_string(), // Unavailable at this stage because of the disconnect between stub creation and adding stub as a dependency + "0.0.1", // Unavailable at this stage because of the disconnect between stub creation and adding stub as a dependency &None, // Unavailable at this stage because of the disconnect between stub creation and adding stub as a dependency )?; - let new_stub = - get_stub_wit(&stub_dependency, false).context("Failed to regenerate inlined stub")?; + let new_stub = get_stub_wit(&stub_dependency, StubTypeGen::InlineRootTypes) + .context("Failed to regenerate inlined stub")?; let main_wit_package_name = wit::get_package_name(&main_wit)?; diff --git a/wasm-rpc-stubgen/src/stub.rs b/wasm-rpc-stubgen/src/stub.rs index 08cc5906..c51b071e 100644 --- a/wasm-rpc-stubgen/src/stub.rs +++ b/wasm-rpc-stubgen/src/stub.rs @@ -276,7 +276,7 @@ fn collect_stub_interfaces(resolve: &Resolve, world: &World) -> anyhow::Result Some((name.clone().unwrap_name(), t)), + WorldItem::Type(t) => Some((name.clone().unwrap_name(), t)), _ => None, }) .collect::>(); diff --git a/wasm-rpc-stubgen/src/wit.rs b/wasm-rpc-stubgen/src/wit.rs index 65295aea..8eff51b6 100644 --- a/wasm-rpc-stubgen/src/wit.rs +++ b/wasm-rpc-stubgen/src/wit.rs @@ -14,16 +14,15 @@ use crate::stub::{FunctionParamStub, FunctionResultStub, InterfaceStubTypeDef, StubDefinition}; use anyhow::{anyhow, bail, Context}; +use regex::Regex; use std::ffi::OsStr; use std::fmt::{Display, Formatter, Write}; use std::fs; use std::path::{Path, PathBuf}; use wit_parser::{Field, Handle, PackageName, Resolve, Type, TypeDefKind, UnresolvedPackage}; -use regex::Regex; -use crate::stub; pub fn generate_stub_wit(def: &StubDefinition) -> anyhow::Result<()> { - let out = get_stub_wit(def, true)?; + let out = get_stub_wit(def, StubTypeGen::ImportRootTypes)?; println!( "Generating stub WIT to {}", def.target_wit_path().to_string_lossy() @@ -33,7 +32,15 @@ pub fn generate_stub_wit(def: &StubDefinition) -> anyhow::Result<()> { Ok(()) } -pub fn get_stub_wit(def: &StubDefinition, bool: bool) -> anyhow::Result { +pub enum StubTypeGen { + InlineRootTypes, + ImportRootTypes, +} + +pub fn get_stub_wit( + def: &StubDefinition, + type_gen_strategy: StubTypeGen, +) -> anyhow::Result { let world = def.resolve.worlds.get(def.world_id).unwrap(); let mut out = String::new(); @@ -50,54 +57,57 @@ pub fn get_stub_wit(def: &StubDefinition, bool: bool) -> anyhow::Result writeln!(out, " use golem:rpc/types@0.1.0.{{uri}};")?; - if bool { - for import in all_imports { - writeln!(out, " use {}.{{{}}};", import.path, import.name)?; + match type_gen_strategy { + StubTypeGen::ImportRootTypes => { + for import in all_imports { + writeln!(out, " use {}.{{{}}};", import.path, import.name)?; + } + writeln!(out)?; } - writeln!(out)?; - } else { - let mut inline_types: Vec = vec![]; + StubTypeGen::InlineRootTypes => { + let mut inline_types: Vec = vec![]; - for import in all_imports { - match &import.package_name { - Some(package) if package == &def.root_package_name => { - inline_types.push(import.clone()); + for import in all_imports { + match &import.package_name { + Some(package) if package == &def.root_package_name => { + inline_types.push(import.clone()); + } + _ => writeln!(out, " use {}.{{{}}};", import.path, import.name)?, } - _ => writeln!(out, " use {}.{{{}}};", import.path, import.name)?, } - } - writeln!(out)?; + writeln!(out)?; + + for typ in inline_types { + let typ_kind = typ.clone().type_def.kind; + let kind_str = typ_kind.as_str(); + let name = typ.clone().name; + + write!(out, " {}", kind_str)?; + write!(out, " {}", name)?; - for typ in inline_types { - let typ_kind = typ.clone().type_def.kind; - let kind_str = typ_kind.as_str(); - let name = typ.clone().name; - - write!(out, " {}", kind_str)?; - write!(out, " {}", name)?; - - match typ_kind { - TypeDefKind::Record(record) => { - write!(out, " {{")?; - writeln!(out)?; - write_field_list(&mut out, record.fields, def)?; - writeln!(out)?; - writeln!(out, " }}")?; + match typ_kind { + TypeDefKind::Record(record) => { + write!(out, " {{")?; + writeln!(out)?; + write_field_list(&mut out, record.fields, def)?; + writeln!(out)?; + writeln!(out, " }}")?; + } + TypeDefKind::Resource => {} + TypeDefKind::Handle(_) => {} + TypeDefKind::Flags(_) => {} + TypeDefKind::Tuple(_) => {} + TypeDefKind::Variant(_) => {} + TypeDefKind::Enum(_) => {} + TypeDefKind::Option(_) => {} + TypeDefKind::Result(_) => {} + TypeDefKind::List(_) => {} + TypeDefKind::Future(_) => {} + TypeDefKind::Stream(_) => {} + TypeDefKind::Type(_) => {} + TypeDefKind::Unknown => {} } - TypeDefKind::Resource => {} - TypeDefKind::Handle(_) => {} - TypeDefKind::Flags(_) => {} - TypeDefKind::Tuple(_) => {} - TypeDefKind::Variant(_) => {} - TypeDefKind::Enum(_) => {} - TypeDefKind::Option(_) => {} - TypeDefKind::Result(_) => {} - TypeDefKind::List(_) => {} - TypeDefKind::Future(_) => {} - TypeDefKind::Stream(_) => {} - TypeDefKind::Type(_) => {} - TypeDefKind::Unknown => {} } } } @@ -227,7 +237,7 @@ fn write_param_list( pub fn copy_wit_files(def: &StubDefinition) -> anyhow::Result<()> { let mut all = def.unresolved_deps.clone(); all.push(def.unresolved_root.clone()); - let stub_package_name = format!("{}-stub", def.root_package_name); + let stub_package_name = format!("{}-stub", def.root_package_name); let dest_wit_root = def.target_wit_root(); fs::create_dir_all(&dest_wit_root)?; @@ -245,19 +255,26 @@ pub fn copy_wit_files(def: &StubDefinition) -> anyhow::Result<()> { fs::create_dir_all(&dep_dir)?; for source in unresolved.source_files() { - // While copying this root WIT (from which the stub is generated) to the deps directory - // of the stub, we ensure there is no import of the same stub's types. This happens - // when you regenerate a stub from a WIT, which the user may have manually modified to refer to the same stub. - // This occurs only when there is a cyclic dependency. That's when the package from which the stub is generated is using the stub itself. - // - // Replacing this import with `none` is closer to being the right thing to do at this stage, - // because there is no reason a stub WIT needs to cyclically depend on its own types to generate the stub. - // Now the question is whether we need to parse this WIT to Resolved and then have our own Display - // for it, or just use regex replace. The latter is chosen because we hardly refer to the same stub without - // `import`, and the actual stub package name. - // The former may look cleaner, but slower, and may not bring about anymore significant correctness or reliability. - let read_data = fs::read_to_string(&source)?; - let re = Regex::new(format!(r"import\s+{}(/[^;]*)?;", regex::escape(stub_package_name.as_str())).as_str()).unwrap(); + // While copying this root WIT (from which the stub is generated) to the deps directory + // of the stub, we ensure there is no import of the same stub's types. This happens + // when you regenerate a stub from a WIT, which the user may have manually modified to refer to the same stub. + // This occurs only when there is a cyclic dependency. That's when the package from which the stub is generated is using the stub itself. + // + // Replacing this import with `none` is closer to being the right thing to do at this stage, + // because there is no reason a stub WIT needs to cyclically depend on its own types to generate the stub. + // Now the question is whether we need to parse this WIT to Resolved and then have our own Display + // for it, or just use regex replace. The latter is chosen because we hardly refer to the same stub without + // `import`, and the actual stub package name. + // The former may look cleaner, but slower, and may not bring about anymore significant correctness or reliability. + let read_data = fs::read_to_string(source)?; + let re = Regex::new( + format!( + r"import\s+{}(/[^;]*)?;", + regex::escape(stub_package_name.as_str()) + ) + .as_str(), + ) + .unwrap(); let new_data = re.replace_all(&read_data, ""); let dest = dep_dir.join(source.file_name().unwrap()); @@ -274,7 +291,7 @@ pub fn copy_wit_files(def: &StubDefinition) -> anyhow::Result<()> { println!("Copying package {}", unresolved.name); for source in unresolved.source_files() { - let parsed = UnresolvedPackage::parse_path(&source)?; + let parsed = UnresolvedPackage::parse_path(source)?; let stub_package_name = format!("{}-stub", def.root_package_name); if parsed.name.to_string() == stub_package_name { @@ -409,11 +426,6 @@ pub enum WitAction { CopyDepDir { source_dir: PathBuf, }, - CopyDepWit { - source_wit: PathBuf, - dir_name: String, - }, - CopyWitStr { source_wit: String, dir_name: String, @@ -425,9 +437,9 @@ impl Display for WitAction { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { WitAction::CopyWitStr { - source_wit, dir_name, file_name, + .. } => { write!( f, @@ -442,17 +454,6 @@ impl Display for WitAction { source_dir.to_string_lossy() ) } - WitAction::CopyDepWit { - source_wit, - dir_name, - } => { - write!( - f, - "copy stub WIT from {} as dependency {}", - source_wit.to_string_lossy(), - dir_name - ) - } } } } @@ -491,17 +492,6 @@ impl WitAction { ) .context("Failed to copy the dependency directory")?; } - WitAction::CopyDepWit { - source_wit, - dir_name, - } => { - let target_dir = target_wit_root.join("deps").join(dir_name); - if !target_dir.exists() { - fs::create_dir_all(&target_dir).context("Create target directory")?; - } - fs::copy(source_wit, target_dir.join(source_wit.file_name().unwrap())) - .context("Copy the WIT file")?; - } } Ok(()) @@ -514,7 +504,6 @@ impl WitAction { .context("Get wit dependency directory name")? .to_string_lossy() .to_string()), - WitAction::CopyDepWit { dir_name, .. } => Ok(dir_name.clone()), WitAction::CopyWitStr { dir_name, .. } => Ok(dir_name.clone()), } } @@ -566,42 +555,5 @@ pub fn verify_action( Ok(true) } } - WitAction::CopyDepWit { - source_wit, - dir_name, - } => { - let target_dir = target_wit_root.join("deps").join(dir_name); - let source_file_name = source_wit.file_name().context("Get source wit file name")?; - let target_wit = target_dir.join(source_file_name); - if target_dir.exists() && target_dir.is_dir() { - let mut existing_entries = Vec::new(); - for entry in fs::read_dir(&target_dir)? { - let entry = entry?; - let name = entry - .path() - .file_name() - .context("Get existing wit directory's name")? - .to_string_lossy() - .to_string(); - existing_entries.push(name); - } - if existing_entries.contains(&source_file_name.to_string_lossy().to_string()) { - let source_contents = fs::read_to_string(source_wit)?; - let target_contents = fs::read_to_string(&target_wit)?; - if source_contents == target_contents { - Ok(true) - } else if overwrite { - println!("Overwriting {}", target_wit.to_string_lossy()); - Ok(true) - } else { - Ok(false) - } - } else { - Ok(true) - } - } else { - Ok(true) - } - } } }