Repository: rust-lang/cc-rs (rust)

Issue:
Encoded `CFLAGS` for supporting spaces
According to https://github.com/rust-lang/cc-rs/#external-configuration-via-environment-variables spaces nor escapes for spaces are not supported in e.g. `C(XX)FLAGS`. In [`xbuild`](https://github.com/rust-mobile/xbuild) we [use `CFLAGS` to set a `--sysroot` path](https://github.com/rust-mobile/xbuild/blob/875f933f5908b49436e7176f2c0cfd7f5233ee24/xbuild/src/cargo/mod.rs#L444-L445), and this can occasionally contain spaces on Windows machines (because the toolchain is unpacked to a user folder, and users like to have `Firstname Surname` as username and profile directory).

A likable solution is new `CARGO_ENCODED_RUSTFLAGS`-like variables, where spaces within individual args are supported by requiring the user to replace space delimiters in between separate arguments with the `\x1f` ASCII Unit Separator: https://doc.rust-lang.org/cargo/reference/environment-variables.html, to allow more mechanical piecing-together of these variables without messing around with nested quotes or backslash-escapes.

Referencing https://github.com/rust-mobile/xbuild/issues/124, where this is one of the deciding factors to build our Android compiler support _differently_.

Pull request description:
Add support for parsing `*FLAGS` with `shlex`
Hello,
This PR adds support for parsing `*FLAGS` with the `shlex` crate. This effectively fixes #847, and means `*FLAGS` with spaces designed for `meson`, `make` and `cmake` will work with no extra configuration.
In order to preserve compatibility with existing setups, the behavior must be opted into with `CC_PARSE_FLAGS_WITH_SHLEX`, or the `parse_flags_with_shlex` method on the `Builder`.

Patch:
diff --git a/Cargo.toml b/Cargo.toml
index a674643..c7be77e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -21,6 +21,7 @@ rust-version = "1.63"
 
 [dependencies]
 jobserver = { version = "0.1.30", default-features = false, optional = true }
+shlex = "1.3.0"
 
 [target.'cfg(unix)'.dependencies]
 # Don't turn on the feature "std" for this, see https://github.com/rust-lang/cargo/issues/4866
diff --git a/src/lib.rs b/src/lib.rs
index 9106660..9fd3860 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -84,6 +84,9 @@
 //! * `CC_ENABLE_DEBUG_OUTPUT` - if set, compiler command invocations and exit codes will
 //!   be logged to stdout. This is useful for debugging build script issues, but can be
 //!   overly verbose for normal use.
+//! * `CC_SHELL_ESCAPED_FLAGS` - if set, *FLAGS will be parsed as if they were shell
+//!   arguments, similar to `make` and `cmake`. For example, `CFLAGS='a "b c" d\ e'` will
+//!   be parsed as `["a", "b", "c", "d", "e"]` instead of `["a", "\"b", "c\", "d\\", "e"]`
 //! * `CXX...` - see [C++ Support](#c-support).
 //!
 //! Furthermore, projects using this crate may specify custom environment variables
@@ -224,6 +227,8 @@ use std::process::Child;
 use std::process::Command;
 use std::sync::{Arc, RwLock};
 
+use shlex::Shlex;
+
 #[cfg(feature = "parallel")]
 mod parallel;
 mod windows;
@@ -301,6 +306,7 @@ pub struct Build {
     apple_versions_cache: Arc<RwLock<HashMap<Box<str>, Arc<str>>>>,
     emit_rerun_if_env_changed: bool,
     cached_compiler_family: Arc<RwLock<HashMap<Box<Path>, ToolFamily>>>,
+    shell_escaped_flags: Option<bool>,
 }
 
 /// Represents the types of errors that may occur while using cc-rs.
@@ -425,6 +431,7 @@ impl Build {
             apple_versions_cache: Arc::new(RwLock::new(HashMap::new())),
             emit_rerun_if_env_changed: true,
             cached_compiler_family: Arc::default(),
+            shell_escaped_flags: None,
         }
     }
 
@@ -1277,6 +1284,15 @@ impl Build {
         self
     }
 
+    /// Configure whether *FLAGS variables are parsed using `shlex`, similarly to `make` and
+    /// `cmake`.
+    ///
+    /// This option defaults to `false`.
+    pub fn shell_escaped_flags(&mut self, shell_escaped_flags: bool) -> &mut Build {
+        self.shell_escaped_flags = Some(shell_escaped_flags);
+        self
+    }
+
     #[doc(hidden)]
     pub fn __set_env<A, B>(&mut self, a: A, b: B) -> &mut Build
     where
@@ -3634,6 +3650,11 @@ impl Build {
         })
     }
 
+    fn get_shell_escaped_flags(&self) -> bool {
+        self.shell_escaped_flags
+            .unwrap_or_else(|| self.getenv("CC_SHELL_ESCAPED_FLAGS").is_some())
+    }
+
     fn get_dwarf_version(&self) -> Option<u32> {
         // Tentatively matches the DWARF version defaults as of rustc 1.62.
         let target = self.get_target().ok()?;
@@ -3748,12 +3769,17 @@ impl Build {
     }
 
     fn envflags(&self, name: &str) -> Result<Vec<String>, Error> {
-        Ok(self
-            .getenv_with_target_prefixes(name)?
-            .to_string_lossy()
-            .split_ascii_whitespace()
-            .map(ToString::to_string)
-            .collect())
+        let env_os = self.getenv_with_target_prefixes(name)?;
+        let env = env_os.to_string_lossy();
+
+        if self.get_shell_escaped_flags() {
+            Ok(Shlex::new(&env).collect())
+        } else {
+            Ok(env
+                .split_ascii_whitespace()
+                .map(ToString::to_string)
+                .collect())
+        }
     }
 
     fn fix_env_for_apple_os(&self, cmd: &mut Command) -> Result<(), Error> {


Test patch:
diff --git a/tests/cflags_shell_escaped.rs b/tests/cflags_shell_escaped.rs
new file mode 100644
index 0000000..7d843e5
--- /dev/null
+++ b/tests/cflags_shell_escaped.rs
@@ -0,0 +1,24 @@
+mod support;
+
+use crate::support::Test;
+use std::env;
+
+/// This test is in its own module because it modifies the environment and would affect other tests
+/// when run in parallel with them.
+#[test]
+fn gnu_test_parse_shell_escaped_flags() {
+    env::set_var("CFLAGS", "foo \"bar baz\"");
+    env::set_var("CC_SHELL_ESCAPED_FLAGS", "1");
+    let test = Test::gnu();
+    test.gcc().file("foo.c").compile("foo");
+
+    test.cmd(0).must_have("foo").must_have("bar baz");
+
+    env::remove_var("CC_SHELL_ESCAPED_FLAGS");
+    let test = Test::gnu();
+    test.gcc().file("foo.c").compile("foo");
+
+    test.cmd(0)
+        .must_have("foo")
+        .must_have_in_order("\"bar", "baz\"");
+}