diff --git a/Cargo.lock b/Cargo.lock index 0d45ad95..b6947096 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -458,7 +458,7 @@ version = "3.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fde0e0ec90c9dfb3b4b1a0891a7dcd0e2bffde2f7efed5fe7c9bb00e5bfb915e" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.48.0", ] [[package]] @@ -670,7 +670,7 @@ dependencies = [ "libc", "option-ext", "redox_users", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -758,7 +758,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -1573,7 +1573,7 @@ checksum = "3640c1c38b8e4e43584d8df18be5fc6b0aa314ce6ebf51b53313d4306cca8e46" dependencies = [ "hermit-abi", "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -2002,7 +2002,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -2294,9 +2294,9 @@ dependencies = [ [[package]] name = "pilota-build" -version = "0.13.5" +version = "0.13.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c25569294e2338a732b0eda3ec027aa3240f05df1aa1d26b27bfbb9c570d72d5" +checksum = "9009e333edaac3698d6da157cb3eafa38777ec77d7954ce7f93026be048323a1" dependencies = [ "ahash", "anyhow", @@ -2650,7 +2650,7 @@ dependencies = [ "once_cell", "socket2 0.5.10", "tracing", - "windows-sys 0.52.0", + "windows-sys 0.60.2", ] [[package]] @@ -3002,7 +3002,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.11.0", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -3167,7 +3167,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5b55fb86dfd3a2f5f76ea78310a88f96c4ea21a3031f8d212443d56123fd0521" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -3527,7 +3527,7 @@ dependencies = [ "getrandom 0.3.4", "once_cell", "rustix 1.1.3", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -4195,7 +4195,7 @@ dependencies = [ [[package]] name = "volo-build" -version = "0.12.2" +version = "0.12.3" dependencies = [ "ahash", "anyhow", diff --git a/examples/thrift/no_service.thrift b/examples/thrift/no_service.thrift new file mode 100644 index 00000000..8267a43f --- /dev/null +++ b/examples/thrift/no_service.thrift @@ -0,0 +1,6 @@ +namespace rs no_service_target + +struct NoServiceRecord { + 1: required string name, + 2: optional i64 version, +} diff --git a/examples/thrift/no_service_ignored.thrift b/examples/thrift/no_service_ignored.thrift new file mode 100644 index 00000000..0d726b2d --- /dev/null +++ b/examples/thrift/no_service_ignored.thrift @@ -0,0 +1,6 @@ +namespace rs no_service_ignored + +struct IgnoredRecord { + 1: required string name, + 2: optional i64 version, +} diff --git a/examples/volo-gen/src/lib.rs b/examples/volo-gen/src/lib.rs index 3fcd1fde..f62f58b4 100644 --- a/examples/volo-gen/src/lib.rs +++ b/examples/volo-gen/src/lib.rs @@ -1,5 +1,19 @@ +//! Positive doctest: the target file's types are generated and usable. +//! +//! ``` +//! let _ = volo_gen::thrift_no_service_gen::no_service_target::NoServiceRecord::default(); +//! ``` +//! +//! Negative doctest: the file that did NOT enable `no_service` must not +//! contribute any types to the generated module tree. +//! +//! ```compile_fail +//! let _ = volo_gen::thrift_no_service_gen::no_service_ignored::IgnoredRecord::default(); +//! ``` + mod r#gen { include!(concat!(env!("OUT_DIR"), "/thrift_gen.rs")); + include!(concat!(env!("OUT_DIR"), "/thrift_no_service_gen.rs")); include!(concat!(env!("OUT_DIR"), "/proto_gen.rs")); } diff --git a/examples/volo-gen/volo.yml b/examples/volo-gen/volo.yml index eb89de50..a04bad95 100644 --- a/examples/volo-gen/volo.yml +++ b/examples/volo-gen/volo.yml @@ -46,3 +46,21 @@ entries: path: ../thrift/echo_unknown.thrift codegen_option: keep_unknown_fields: true + # Regression scenario for service-level `no_service`. + # The first IDL enables `codegen_option.config.no_service`, so its types appear + # in the generated module. The second IDL is intentionally listed without + # `touch_all` to prove that service-level `no_service` only affects the + # files it is attached to, instead of whitelisting the whole entry. + thrift_no_service: + filename: thrift_no_service_gen.rs + protocol: thrift + services: + - idl: + source: local + path: ../thrift/no_service.thrift + codegen_option: + config: + no_service: true + - idl: + source: local + path: ../thrift/no_service_ignored.thrift diff --git a/volo-build/Cargo.toml b/volo-build/Cargo.toml index f651bec3..c60b4368 100644 --- a/volo-build/Cargo.toml +++ b/volo-build/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "volo-build" -version = "0.12.2" +version = "0.12.3" edition.workspace = true homepage.workspace = true repository.workspace = true diff --git a/volo-build/src/config_builder.rs b/volo-build/src/config_builder.rs index f17f0bae..9bb13018 100644 --- a/volo-build/src/config_builder.rs +++ b/volo-build/src/config_builder.rs @@ -7,14 +7,16 @@ use volo::FastStr; use crate::{ model::{self, Entry}, util::{ - DEFAULT_CONFIG_FILE, DEFAULT_DIR, ServiceBuilder, download_repos_to_target, - get_service_builders_from_services, open_config_file, read_config_from_file, + DEFAULT_CONFIG_FILE, DEFAULT_DIR, ServiceBuilder, collect_no_service_paths, + download_repos_to_target, get_service_builders_from_services, open_config_file, + read_config_from_file, }, }; pub struct ConfigBuilder { filename: PathBuf, plugins: Vec, + out_dir: Option, } #[allow(clippy::large_enum_variant)] @@ -71,6 +73,13 @@ impl InnerBuilder { } } + fn out_dir>(self, out_dir: P) -> Self { + match self { + InnerBuilder::Protobuf(inner) => InnerBuilder::Protobuf(inner.out_dir(&out_dir)), + InnerBuilder::Thrift(inner) => InnerBuilder::Thrift(inner.out_dir(&out_dir)), + } + } + pub fn add_service

(self, path: P) -> Self where P: AsRef, @@ -89,12 +98,12 @@ impl InnerBuilder { keep_unknown_fields, } in service_builders { - self = self - .add_service(path.clone()) - .includes(includes) - .touch([(path.clone(), touch)]); + self = self.add_service(path.clone()).includes(includes); + if !touch.is_empty() { + self = self.touch([(path.clone(), touch)]); + } if keep_unknown_fields { - self = self.keep_unknown_fields([path]) + self = self.keep_unknown_fields([path]); } } self @@ -116,6 +125,13 @@ impl InnerBuilder { } } + pub fn touch_files(self, paths: impl IntoIterator) -> Self { + match self { + InnerBuilder::Protobuf(inner) => InnerBuilder::Protobuf(inner.touch_files(paths)), + InnerBuilder::Thrift(inner) => InnerBuilder::Thrift(inner.touch_files(paths)), + } + } + pub fn keep_unknown_fields(self, keep: impl IntoIterator) -> Self { match self { InnerBuilder::Protobuf(inner) => { @@ -194,6 +210,7 @@ impl ConfigBuilder { ConfigBuilder { filename, plugins: Vec::new(), + out_dir: None, } } @@ -203,10 +220,32 @@ impl ConfigBuilder { self } + /// Overrides the output directory used by the underlying code generator. + /// This also relocates downloaded IDL repos from `${OUT_DIR}/idl` to `/idl`. + pub fn out_dir>(mut self, out_dir: P) -> Self { + self.out_dir = Some(out_dir.as_ref().to_path_buf()); + self + } + + fn get_out_dir(&self) -> anyhow::Result { + if let Some(out_dir) = &self.out_dir { + return Ok(out_dir.clone()); + } + + // Default to OUT_DIR derived from `DEFAULT_DIR` (which is `${OUT_DIR}/idl`). + // We intentionally don't check `OUT_DIR` here, because `DEFAULT_DIR` already + // provides a clear panic message when used outside build.rs. + Ok(PathBuf::from(DEFAULT_DIR.parent().expect( + "DEFAULT_DIR should always have a parent directory", + ))) + } + pub fn write(self) -> anyhow::Result<()> { println!("cargo:rerun-if-changed={}", self.filename.display()); let mut f = open_config_file(self.filename.clone())?; let config = read_config_from_file(&mut f)?; + let out_dir = self.get_out_dir()?; + let idl_dir = out_dir.join("idl"); config .entries .into_iter() @@ -215,23 +254,30 @@ impl ConfigBuilder { model::IdlProtocol::Thrift => InnerBuilder::thrift(), model::IdlProtocol::Protobuf => InnerBuilder::protobuf(), } - .filename(entry.filename.clone()); + .filename(entry.filename.clone()) + .out_dir(&out_dir); for p in self.plugins.iter() { builder = builder.plugin(p.clone()); } // download repos and get the repo paths - let target_dir = PathBuf::from(&*DEFAULT_DIR).join(entry_name); + let target_dir = idl_dir.join(&entry_name); let repo_dir_map = download_repos_to_target(&entry.repos, target_dir)?; + // collect per-IDL no_service flags from `codegen_option.config.no_service` + let no_service_paths = collect_no_service_paths(&entry.services, &repo_dir_map); + // get idl builders from services let service_builders = get_service_builders_from_services(&entry.services, &repo_dir_map); // add build options to the builder and build + let mut builder = builder.add_services(service_builders); + if !no_service_paths.is_empty() { + builder = builder.touch_files(no_service_paths); + } builder - .add_services(service_builders) .ignore_unused(!entry.common_option.touch_all) .special_namings(entry.common_option.special_namings) .split_generated_files(entry.common_option.split_generated_files) @@ -253,6 +299,75 @@ impl Default for ConfigBuilder { } } +#[cfg(test)] +mod tests { + use std::{ + fs, + path::{Path, PathBuf}, + }; + + use tempfile::tempdir; + + use super::ConfigBuilder; + + fn write_thrift(path: &Path, namespace: &str, type_name: &str) { + fs::write( + path, + format!( + "namespace rs {namespace}\n\nstruct {type_name} {{\n 1: required string \ + value,\n}}\n" + ), + ) + .unwrap(); + } + + #[test] + fn write_respects_service_level_no_service_per_path() { + let dir = tempdir().unwrap(); + let target_idl = dir.path().join("target.thrift"); + let ignored_idl = dir.path().join("ignored.thrift"); + let config_path = dir.path().join("volo.yml"); + let out_dir = dir.path().join("out"); + + write_thrift(&target_idl, "no_service_target", "WantedRecord"); + write_thrift(&ignored_idl, "no_service_ignored", "IgnoredRecord"); + + fs::write( + &config_path, + format!( + "entries:\n sample:\n filename: generated.rs\n protocol: thrift\n services:\n - idl:\n source: local\n path: {}\n codegen_option:\n config:\n no_service: true\n - idl:\n source: local\n path: {}\n", + target_idl.display(), + ignored_idl.display() + ), + ) + .unwrap(); + + ConfigBuilder::new(config_path) + .out_dir(&out_dir) + .write() + .unwrap(); + + let generated = fs::read_to_string(out_dir.join("generated.rs")).unwrap(); + assert!(generated.contains("WantedRecord")); + assert!(!generated.contains("IgnoredRecord")); + } + + #[test] + fn get_out_dir_prefers_explicit_out_dir() { + let explicit = tempfile::tempdir() + .unwrap() + .path() + .join("volo-build-explicit-out"); + + let out_dir = ConfigBuilder::new(PathBuf::from("volo.yml")) + .out_dir(&explicit) + .get_out_dir() + .unwrap(); + + assert_eq!(out_dir, explicit); + } +} + pub struct InitBuilder { entry: Entry, } @@ -273,11 +388,17 @@ impl InitBuilder { let temp_target_dir = tempfile::TempDir::new()?; let repo_dir_map = download_repos_to_target(&self.entry.repos, temp_target_dir.as_ref())?; + // collect per-IDL no_service flags from `codegen_option.config.no_service` + let no_service_paths = collect_no_service_paths(&self.entry.services, &repo_dir_map); + // get idl builders from services let idl_builders = get_service_builders_from_services(&self.entry.services, &repo_dir_map); // add services to the builder builder = builder.add_services(idl_builders); + if !no_service_paths.is_empty() { + builder = builder.touch_files(no_service_paths); + } builder.init_service() } diff --git a/volo-build/src/lib.rs b/volo-build/src/lib.rs index 3dc35aec..0d8910de 100644 --- a/volo-build/src/lib.rs +++ b/volo-build/src/lib.rs @@ -103,6 +103,11 @@ impl Builder { self } + pub fn touch_files(mut self, items: impl IntoIterator) -> Self { + self.pilota_builder = self.pilota_builder.touch_files(items); + self + } + pub fn keep_unknown_fields( mut self, keep_unknown_fields: impl IntoIterator, diff --git a/volo-build/src/model.rs b/volo-build/src/model.rs index 79d6fe72..1dfa6223 100644 --- a/volo-build/src/model.rs +++ b/volo-build/src/model.rs @@ -107,6 +107,13 @@ impl CodegenOption { && !self.keep_unknown_fields && self.config.is_null() } + + pub(crate) fn no_service(&self) -> bool { + self.config + .get("no_service") + .and_then(serde_yaml::Value::as_bool) + .unwrap_or(false) + } } #[derive(Serialize, Deserialize, Debug, Clone)] diff --git a/volo-build/src/thrift_backend.rs b/volo-build/src/thrift_backend.rs index 2100cebc..f9ba24a6 100644 --- a/volo-build/src/thrift_backend.rs +++ b/volo-build/src/thrift_backend.rs @@ -327,25 +327,25 @@ impl VoloThriftBackend { write_item( stream, base_dir, - format!("enum_{}.rs", &req_recv_name), + format!("enum_{}.rs", req_recv_name), req_recv_stream, ); write_item( stream, base_dir, - format!("enum_{}.rs", &res_recv_name), + format!("enum_{}.rs", res_recv_name), res_recv_stream, ); write_item( stream, base_dir, - format!("enum_{}.rs", &req_send_name), + format!("enum_{}.rs", req_send_name), req_send_stream, ); write_item( stream, base_dir, - format!("enum_{}.rs", &res_send_name), + format!("enum_{}.rs", res_send_name), res_send_stream, ); } else { diff --git a/volo-build/src/util.rs b/volo-build/src/util.rs index 8fa44008..44e75d9a 100644 --- a/volo-build/src/util.rs +++ b/volo-build/src/util.rs @@ -449,6 +449,21 @@ pub fn get_service_builders_from_services( .collect() } +/// Collect the resolved IDL build paths for services whose `codegen_option.config.no_service` +/// is set to `true`. These paths are later passed to the underlying builder via `touch_files`, +/// which switches the root-selection mode away from "pick init service" so that services +/// without any RPC definition can still be code-generated as types-only. +pub(crate) fn collect_no_service_paths( + services: &[Service], + repo_dir_map: &HashMap, +) -> Vec { + services + .iter() + .filter(|s| s.codegen_option.no_service()) + .map(|s| get_idl_build_path_and_includes(&s.idl, repo_dir_map).0) + .collect() +} + pub fn check_and_get_repo_name( entry_name: &String, repos: &HashMap, @@ -595,7 +610,7 @@ pub(crate) fn write_item(stream: &mut String, base_dir: &Path, name: String, imp let path_buf = base_dir.join(&name); let path = path_buf.as_path(); write_file(path, impl_str); - stream.push_str(format!("include!(\"{}\");", &name).as_str()); + stream.push_str(format!("include!(\"{}\");", name).as_str()); } pub(crate) fn write_file(path: &Path, stream: String) { @@ -652,6 +667,7 @@ mod tests { use tempfile::{NamedTempFile, tempdir}; use super::*; + use crate::model::CodegenOption; #[test] fn test_ensure_path() { @@ -774,6 +790,37 @@ mod tests { assert_eq!(builders[0].path, idl.path); } + #[test] + fn test_collect_no_service_paths() { + let selected_idl = Idl { + source: Source::Local, + path: PathBuf::from("../idl/selected.thrift"), + includes: vec![PathBuf::from("../idl")], + }; + let ignored_idl = Idl { + source: Source::Local, + path: PathBuf::from("../idl/ignored.thrift"), + includes: vec![PathBuf::from("../idl")], + }; + let services = vec![ + Service { + idl: selected_idl.clone(), + codegen_option: CodegenOption { + config: serde_yaml::from_str("no_service: true").unwrap(), + ..Default::default() + }, + }, + Service { + idl: ignored_idl, + codegen_option: Default::default(), + }, + ]; + + let paths = collect_no_service_paths(&services, &HashMap::new()); + + assert_eq!(paths, vec![selected_idl.path]); + } + #[test] fn test_check_and_get_repo_name() { let mut repos = HashMap::new(); diff --git a/volo-build/src/workspace.rs b/volo-build/src/workspace.rs index 43ef2ca7..2a48c166 100644 --- a/volo-build/src/workspace.rs +++ b/volo-build/src/workspace.rs @@ -76,29 +76,44 @@ where .into_iter() .map(|s| { let (path, includes) = get_idl_build_path_and_includes(&s.idl, &repo_dir_map); + let no_service = s.codegen_option.no_service(); ( IdlService { path: path.clone(), config: s.codegen_option.config, }, - ServiceBuilder { - path, - includes, - touch: s.codegen_option.touch, - keep_unknown_fields: s.codegen_option.keep_unknown_fields, - }, + ( + ServiceBuilder { + path, + includes, + touch: s.codegen_option.touch, + keep_unknown_fields: s.codegen_option.keep_unknown_fields, + }, + no_service, + ), ) }) .unzip(); - for ServiceBuilder { - path, - includes, - touch, - keep_unknown_fields, - } in service_builders + for ( + ServiceBuilder { + path, + includes, + touch, + keep_unknown_fields, + }, + no_service, + ) in service_builders { - self = self.include_dirs(includes).touch([(path.clone(), touch)]); - if keep_unknown_fields { + self = self.include_dirs(includes); + if !touch.is_empty() { + self = self.touch([(path.clone(), touch)]); + } + if no_service { + if keep_unknown_fields { + self = self.keep_unknown_fields([path.clone()]); + } + self = self.touch_files([path]); + } else if keep_unknown_fields { self = self.keep_unknown_fields([path]); } } @@ -152,6 +167,11 @@ where self } + pub fn touch_files(mut self, paths: impl IntoIterator) -> Self { + self.pilota_builder = self.pilota_builder.touch_files(paths); + self + } + pub fn split_generated_files(mut self, split_generated_files: bool) -> Self { self.pilota_builder = self .pilota_builder