From ec907b1a02b481519c0d7ac886ce0fa4e9b9ede0 Mon Sep 17 00:00:00 2001
From: danakj <danakj@chromium.org>
Date: Wed, 12 Apr 2023 16:32:15 -0400
Subject: [PATCH] Escape spaces in the depfile output

Header file paths included from C/C++ files may include spaces in
them, and if they do, the spaces need to be escaped in order to
generate a proper depfile. Otherwise, the dep file ends up pointing
to non-existant paths.

For example:
../../third_party/depot_tools/win_toolchain/vs_files/27370823e7\Windows Kits\10\Include\10.0.22621.0\shared\winapifamily.h

This path ends up being parsed as two files, neither of which exist:
../../third_party/depot_tools/win_toolchain/vs_files/27370823e7\Windows
Kits\10\Include\10.0.22621.0\shared\winapifamily.h

This leads to build failures with ninja when it's verifying that the
build is complete:
```
$ ninja -C out rust_mojo_unittests -w dupbuild=err -n -d explain
ninja explain: output ../../third_party/depot_tools/win_toolchain/vs_files/27370823e7\Windows doesn't exist
ninja explain: output Kits/10/Include/10.0.22621.0/shared/winapifamily.h is dirty
```

ninja accepts spaces being escaped with backslash, so the path
Windows Kits\10\Include\10.0.22621.0\shared\winapifamily.h can be valid
if escaped as:

Windows\ Kits\10\Include\10.0.22621.0\shared\winapifamily.h

However this is less clear for humans as there is inconsistent use of
backslashes, so we opt to escape backslashes as well, which ninja also
accepts, and is standard for Windows paths.

Windows\ Kits\\10\\Include\\10.0.22621.0\\shared\\winapifamily.h

The same is also true for the target name, or `output_module` as its
named in bindgen. Issue https://github.com/ninja-build/ninja/issues/168
demonstrates this case as well. So we also escape the target name if
there is a space present.
---
 CHANGELOG.md    |  4 ++++
 bindgen/deps.rs | 47 ++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 098882a161..e271253915 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -177,6 +177,10 @@
   statement and only call the static function instead.
 * The `--wrap-static-fns` option no longer emits wrappers for static variadic
   functions.
+* Depfiles generated with `--depfile` or `Builder::depfile` will now be
+  properly generate module names and paths that include spaces by escaping
+  them. To make the escaping clear and consistent, backslashes are also
+  escaped.
    
 ## Removed
 
diff --git a/bindgen/deps.rs b/bindgen/deps.rs
index 987225b28e..2edeaa8886 100644
--- a/bindgen/deps.rs
+++ b/bindgen/deps.rs
@@ -9,12 +9,53 @@ pub(crate) struct DepfileSpec {
 
 impl DepfileSpec {
     pub fn write(&self, deps: &BTreeSet<String>) -> std::io::Result<()> {
-        let mut buf = format!("{}:", self.output_module);
+        std::fs::write(&self.depfile_path, &self.to_string(deps))
+    }
+
+    fn to_string(&self, deps: &BTreeSet<String>) -> String {
+        // Transforms a string by escaping spaces and backslashes.
+        let escape = |s: &str| s.replace('\\', "\\\\").replace(' ', "\\ ");
 
+        let mut buf = format!("{}:", escape(&self.output_module));
         for file in deps {
-            buf = format!("{} {}", buf, file);
+            buf = format!("{} {}", buf, escape(file));
         }
+        buf
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn escaping_depfile() {
+        let spec = DepfileSpec {
+            output_module: "Mod Name".to_owned(),
+            depfile_path: PathBuf::new(),
+        };
 
-        std::fs::write(&self.depfile_path, &buf)
+        let deps: BTreeSet<String> = vec![
+            r"/absolute/path".to_owned(),
+            r"C:\win\absolute\path".to_owned(),
+            r"../relative/path".to_owned(),
+            r"..\win\relative\path".to_owned(),
+            r"../path/with spaces/in/it".to_owned(),
+            r"..\win\path\with spaces\in\it".to_owned(),
+            r"path\with/mixed\separators".to_owned(),
+        ]
+        .into_iter()
+        .collect();
+        assert_eq!(
+            spec.to_string(&deps),
+            "Mod\\ Name: \
+            ../path/with\\ spaces/in/it \
+            ../relative/path \
+            ..\\\\win\\\\path\\\\with\\ spaces\\\\in\\\\it \
+            ..\\\\win\\\\relative\\\\path \
+            /absolute/path \
+            C:\\\\win\\\\absolute\\\\path \
+            path\\\\with/mixed\\\\separators"
+        );
     }
 }