From 8296fdf50745aa370b5e36e1f90d58550bfc6770 Mon Sep 17 00:00:00 2001 From: GuTS805 Date: Wed, 5 Aug 2026 07:30:09 +0530 Subject: [PATCH 1/2] fix(cli): normalize workspace path to forward slashes for CMake templates workspace_dir() returned a raw Windows path (backslash-separated), which gets substituted verbatim into __DORA_PATH__ in the generated CMakeLists.txt for --lang c/cxx --internal-create-with-path-dependencies. CMake treats backslash as a string escape character, so cmake -B failed immediately with 'Invalid character escape' before any compiler logic ran. Normalize to forward slashes, which both CMake and Windows accept. Also box the large Cached variant of CachedResult in the coordinator (pre-existing clippy::large_enum_variant failure surfaced while running clippy locally, related to #2979). --- binaries/cli/src/template/c/mod.rs | 4 ++-- binaries/cli/src/template/cxx/mod.rs | 4 ++-- binaries/cli/src/template/mod.rs | 24 +++++++++++++++++++++--- binaries/coordinator/src/state.rs | 14 +++++++++----- 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/binaries/cli/src/template/c/mod.rs b/binaries/cli/src/template/c/mod.rs index 7dbf56379a..c89627ec93 100644 --- a/binaries/cli/src/template/c/mod.rs +++ b/binaries/cli/src/template/c/mod.rs @@ -80,7 +80,7 @@ fn create_cmakefile(root: PathBuf, use_path_deps: bool) -> Result<(), eyre::ErrR const CMAKEFILE: &str = include_str!("cmake-template.txt"); let cmake_file = if use_path_deps { - CMAKEFILE.replace("__DORA_PATH__", super::workspace_dir()?) + CMAKEFILE.replace("__DORA_PATH__", &super::workspace_dir()?) } else { CMAKEFILE.replace("__DORA_PATH__", "") }; @@ -136,7 +136,7 @@ fn create_node_cmakefile( let cmake_content = if use_path_deps { NODE_CMAKE .replace("___name___", name) - .replace("__DORA_PATH__", super::workspace_dir()?) + .replace("__DORA_PATH__", &super::workspace_dir()?) } else { NODE_CMAKE .replace("___name___", name) diff --git a/binaries/cli/src/template/cxx/mod.rs b/binaries/cli/src/template/cxx/mod.rs index b486d6c41f..ee16372b7e 100644 --- a/binaries/cli/src/template/cxx/mod.rs +++ b/binaries/cli/src/template/cxx/mod.rs @@ -79,7 +79,7 @@ fn create_cmakefile(root: PathBuf, use_path_deps: bool) -> Result<(), eyre::ErrR const CMAKEFILE: &str = include_str!("cmake-template.txt"); let cmake_file = if use_path_deps { - CMAKEFILE.replace("__DORA_PATH__", super::workspace_dir()?) + CMAKEFILE.replace("__DORA_PATH__", &super::workspace_dir()?) } else { CMAKEFILE.replace("__DORA_PATH__", "") }; @@ -132,7 +132,7 @@ fn create_node_cmakefile( let cmake_content = if use_path_deps { NODE_CMAKE .replace("___name___", name) - .replace("__DORA_PATH__", super::workspace_dir()?) + .replace("__DORA_PATH__", &super::workspace_dir()?) } else { NODE_CMAKE .replace("___name___", name) diff --git a/binaries/cli/src/template/mod.rs b/binaries/cli/src/template/mod.rs index cc6b03dbd9..bc590b09b7 100644 --- a/binaries/cli/src/template/mod.rs +++ b/binaries/cli/src/template/mod.rs @@ -9,14 +9,32 @@ mod rust; /// Path to the dora workspace root (two levels above the CLI crate /// manifest), used by the C/C++ templates to reference dora via path /// dependencies when `use_path_deps` is set. -fn workspace_dir() -> eyre::Result<&'static str> { - Path::new(env!("CARGO_MANIFEST_DIR")) +fn workspace_dir() -> eyre::Result { + let dir = Path::new(env!("CARGO_MANIFEST_DIR")) .parent() .context("Could not get manifest parent folder")? .parent() .context("Could not get manifest grandparent folder")? .to_str() - .context("dora workspace path is not valid UTF-8") + .context("dora workspace path is not valid UTF-8")?; + // CMake treats `\` as a string escape character, so a raw Windows path + // (e.g. `C:\Users\...`) substituted into CMakeLists.txt fails to parse. + // Both CMake and Windows accept `/`, so normalize before substitution. + Ok(dir.replace('\\', "/")) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn workspace_dir_has_no_backslashes() { + let dir = workspace_dir().unwrap(); + assert!( + !dir.contains('\\'), + "workspace_dir() must use forward slashes for CMake compatibility, got: {dir}" + ); + } } pub fn create(args: crate::CommandNew, use_path_deps: bool) -> eyre::Result<()> { diff --git a/binaries/coordinator/src/state.rs b/binaries/coordinator/src/state.rs index f3ab123783..aa97a38e2a 100644 --- a/binaries/coordinator/src/state.rs +++ b/binaries/coordinator/src/state.rs @@ -486,7 +486,9 @@ impl RunningDataflow { node_stopped_at: BTreeMap::new(), network_metrics: None, spawn_result: CachedResult::Cached { - result: Ok(ControlRequestReply::DataflowSpawned { uuid: record.uuid }), + result: Box::new(Ok(ControlRequestReply::DataflowSpawned { + uuid: record.uuid, + })), }, stop_reply_senders: Vec::new(), buffered_log_messages: Vec::new(), @@ -542,7 +544,7 @@ pub(crate) enum CachedResult { result_senders: Vec>>, }, Cached { - result: eyre::Result, + result: Box>, }, } @@ -573,7 +575,9 @@ impl CachedResult { for sender in result_senders.drain(..) { Self::send_result_to(&result, sender); } - *self = CachedResult::Cached { result }; + *self = CachedResult::Cached { + result: Box::new(result), + }; } CachedResult::Cached { .. } => {} } @@ -592,7 +596,7 @@ impl CachedResult { /// the spawn-timeout watchdog (or any other terminal-failure path) /// has already marked as failed. pub(crate) fn is_terminal_error(&self) -> bool { - matches!(self, CachedResult::Cached { result: Err(_) }) + matches!(self, CachedResult::Cached { result } if result.is_err()) } /// Returns `true` if a successful result has been cached, i.e. the dataflow @@ -601,7 +605,7 @@ impl CachedResult { /// that are past spawn — spawn-pending ones remain the spawn-timeout /// watchdog's domain. See #2028. pub(crate) fn is_cached_ok(&self) -> bool { - matches!(self, CachedResult::Cached { result: Ok(_) }) + matches!(self, CachedResult::Cached { result } if result.is_ok()) } fn send_result_to( From b45176f775baf6174d5b71bdd2b77427f4796dc8 Mon Sep 17 00:00:00 2001 From: GuTS805 Date: Wed, 5 Aug 2026 16:28:50 +0530 Subject: [PATCH 2/2] fix: address review feedback - drop unrelated coordinator change, use OS-independent test The CachedResult::Cached boxing fix duplicated the already-open PR #3001, so it's dropped here to avoid merge conflicts (per 'don't fix unrelated warnings in PRs' convention). The regression test now asserts against a hardcoded Windows-style sample path instead of the live workspace_dir() output, since on Linux/macOS CI the real path never contains a backslash and the old assertion passed trivially without exercising the normalization logic. --- binaries/cli/src/template/mod.rs | 26 ++++++++++++++++---------- binaries/coordinator/src/state.rs | 14 +++++--------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/binaries/cli/src/template/mod.rs b/binaries/cli/src/template/mod.rs index bc590b09b7..23b0a8cfd0 100644 --- a/binaries/cli/src/template/mod.rs +++ b/binaries/cli/src/template/mod.rs @@ -17,23 +17,29 @@ fn workspace_dir() -> eyre::Result { .context("Could not get manifest grandparent folder")? .to_str() .context("dora workspace path is not valid UTF-8")?; - // CMake treats `\` as a string escape character, so a raw Windows path - // (e.g. `C:\Users\...`) substituted into CMakeLists.txt fails to parse. - // Both CMake and Windows accept `/`, so normalize before substitution. - Ok(dir.replace('\\', "/")) + Ok(normalize_for_cmake(dir)) +} + +// CMake treats `\` as a string escape character, so a raw Windows path +// (e.g. `C:\Users\...`) substituted into CMakeLists.txt fails to parse. +// Both CMake and Windows accept `/`, so normalize before substitution. +fn normalize_for_cmake(path: &str) -> String { + path.replace('\\', "/") } #[cfg(test)] mod tests { use super::*; + // Uses a hardcoded Windows-style sample path rather than the live + // `workspace_dir()` output, since on Linux/macOS CI runners the real + // path never contains a backslash and the assertion would pass + // trivially without exercising the normalization at all. #[test] - fn workspace_dir_has_no_backslashes() { - let dir = workspace_dir().unwrap(); - assert!( - !dir.contains('\\'), - "workspace_dir() must use forward slashes for CMake compatibility, got: {dir}" - ); + fn normalize_for_cmake_replaces_backslashes() { + let normalized = normalize_for_cmake(r"C:\Users\example\dora"); + assert_eq!(normalized, "C:/Users/example/dora"); + assert!(!normalized.contains('\\')); } } diff --git a/binaries/coordinator/src/state.rs b/binaries/coordinator/src/state.rs index aa97a38e2a..f3ab123783 100644 --- a/binaries/coordinator/src/state.rs +++ b/binaries/coordinator/src/state.rs @@ -486,9 +486,7 @@ impl RunningDataflow { node_stopped_at: BTreeMap::new(), network_metrics: None, spawn_result: CachedResult::Cached { - result: Box::new(Ok(ControlRequestReply::DataflowSpawned { - uuid: record.uuid, - })), + result: Ok(ControlRequestReply::DataflowSpawned { uuid: record.uuid }), }, stop_reply_senders: Vec::new(), buffered_log_messages: Vec::new(), @@ -544,7 +542,7 @@ pub(crate) enum CachedResult { result_senders: Vec>>, }, Cached { - result: Box>, + result: eyre::Result, }, } @@ -575,9 +573,7 @@ impl CachedResult { for sender in result_senders.drain(..) { Self::send_result_to(&result, sender); } - *self = CachedResult::Cached { - result: Box::new(result), - }; + *self = CachedResult::Cached { result }; } CachedResult::Cached { .. } => {} } @@ -596,7 +592,7 @@ impl CachedResult { /// the spawn-timeout watchdog (or any other terminal-failure path) /// has already marked as failed. pub(crate) fn is_terminal_error(&self) -> bool { - matches!(self, CachedResult::Cached { result } if result.is_err()) + matches!(self, CachedResult::Cached { result: Err(_) }) } /// Returns `true` if a successful result has been cached, i.e. the dataflow @@ -605,7 +601,7 @@ impl CachedResult { /// that are past spawn — spawn-pending ones remain the spawn-timeout /// watchdog's domain. See #2028. pub(crate) fn is_cached_ok(&self) -> bool { - matches!(self, CachedResult::Cached { result } if result.is_ok()) + matches!(self, CachedResult::Cached { result: Ok(_) }) } fn send_result_to(