Repository: rust-lang/cc-rs
Pull request #1403 (2025-02-14 15:34:24)

Change flag ordering
To allow flags set on the builder with `Build::flag`/`Build::flag_if_supported` to override other flags, and to allow `CFLAGS` to override _all_ other flags.

Found while doing https://github.com/rust-lang/cc-rs/pull/1401.

Now the order of flags are as follows, with later flags overriding previous ones. I've also here shown who "controls" which flag, to make it clear why we want the ordering to be like this.
1. Default flags
    - Controlled by `cc-rs`.
2. `rustc`-inherited flags
    - Controlled by `rustc`.
3. Builder flags
    - Controlled by the developer using `cc-rs` in e.g. their `build.rs`.
4. Environment flags
    - Controlled by the end user.



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b87da59..af55eb3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [1.2.15](https://github.com/rust-lang/cc-rs/compare/cc-v1.2.14...cc-v1.2.15) - 2025-02-21
+
+### Other
+
+- Regenerate target info ([#1406](https://github.com/rust-lang/cc-rs/pull/1406))
+- Always read from all `CFLAGS`-style flags ([#1401](https://github.com/rust-lang/cc-rs/pull/1401))
+- Simplify the error output on failed `Command` invocation ([#1397](https://github.com/rust-lang/cc-rs/pull/1397))
+
 ## [1.2.14](https://github.com/rust-lang/cc-rs/compare/cc-v1.2.13...cc-v1.2.14) - 2025-02-14
 
 ### Other
diff --git a/Cargo.toml b/Cargo.toml
index 9a7ab6c..ecff730 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "cc"
-version = "1.2.14"
+version = "1.2.15"
 authors = ["Alex Crichton <alex@alexcrichton.com>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/rust-lang/cc-rs"
diff --git a/dev-tools/gen-windows-sys-binding/Cargo.toml b/dev-tools/gen-windows-sys-binding/Cargo.toml
index 9b0309b..16231ee 100644
--- a/dev-tools/gen-windows-sys-binding/Cargo.toml
+++ b/dev-tools/gen-windows-sys-binding/Cargo.toml
@@ -5,6 +5,6 @@ edition = "2018"
 publish = false
 
 [dependencies]
-windows-bindgen = "0.59"
+windows-bindgen = "0.60"
 tempfile = "3"
 regex = "1"
diff --git a/src/lib.rs b/src/lib.rs
index f059d4f..dd77dd7 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -232,7 +232,7 @@ use std::io::{self, Write};
 use std::path::{Component, Path, PathBuf};
 #[cfg(feature = "parallel")]
 use std::process::Child;
-use std::process::Command;
+use std::process::{Command, Stdio};
 use std::sync::{
     atomic::{AtomicU8, Ordering::Relaxed},
     Arc, RwLock,
@@ -1892,13 +1892,39 @@ impl Build {
 
         let mut cmd = self.get_base_compiler()?;
 
+        // The flags below are added in roughly the following order:
+        // 1. Default flags
+        //   - Controlled by `cc-rs`.
+        // 2. `rustc`-inherited flags
+        //   - Controlled by `rustc`.
+        // 3. Builder flags
+        //   - Controlled by the developer using `cc-rs` in e.g. their `build.rs`.
+        // 4. Environment flags
+        //   - Controlled by the end user.
+        //
+        // This is important to allow later flags to override previous ones.
+
+        // Copied from <https://github.com/rust-lang/rust/blob/5db81020006d2920fc9c62ffc0f4322f90bffa04/compiler/rustc_codegen_ssa/src/back/linker.rs#L27-L38>
+        //
+        // Disables non-English messages from localized linkers.
+        // Such messages may cause issues with text encoding on Windows
+        // and prevent inspection of msvc output in case of errors, which we occasionally do.
+        // This should be acceptable because other messages from rustc are in English anyway,
+        // and may also be desirable to improve searchability of the compiler diagnostics.
+        if matches!(cmd.family, ToolFamily::Msvc { clang_cl: false }) {
+            cmd.env.push(("VSLANG".into(), "1033".into()));
+        } else {
+            cmd.env.push(("LC_ALL".into(), "C".into()));
+        }
+
         // Disable default flag generation via `no_default_flags` or environment variable
         let no_defaults = self.no_default_flags || self.getenv_boolean("CRATE_CC_NO_DEFAULTS");
-
         if !no_defaults {
             self.add_default_flags(&mut cmd, &target, &opt_level)?;
         }
 
+        // Specify various flags that are not considered part of the default flags above.
+        // FIXME(madsmtm): Should these be considered part of the defaults? If no, why not?
         if let Some(ref std) = self.std {
             let separator = match cmd.family {
                 ToolFamily::Msvc { .. } => ':',
@@ -1906,44 +1932,40 @@ impl Build {
             };
             cmd.push_cc_arg(format!("-std{}{}", separator, std).into());
         }
-
         for directory in self.include_directories.iter() {
             cmd.args.push("-I".into());
             cmd.args.push(directory.as_os_str().into());
         }
-
-        let flags = self.envflags(if self.cpp { "CXXFLAGS" } else { "CFLAGS" })?;
-        if let Some(flags) = &flags {
-            for arg in flags {
-                cmd.push_cc_arg(arg.into());
-            }
+        if self.warnings_into_errors {
+            let warnings_to_errors_flag = cmd.family.warnings_to_errors_flag().into();
+            cmd.push_cc_arg(warnings_to_errors_flag);
         }
 
         // If warnings and/or extra_warnings haven't been explicitly set,
         // then we set them only if the environment doesn't already have
         // CFLAGS/CXXFLAGS, since those variables presumably already contain
         // the desired set of warnings flags.
-
-        if self.warnings.unwrap_or(flags.is_none()) {
+        let envflags = self.envflags(if self.cpp { "CXXFLAGS" } else { "CFLAGS" })?;
+        if self.warnings.unwrap_or(envflags.is_none()) {
             let wflags = cmd.family.warnings_flags().into();
             cmd.push_cc_arg(wflags);
         }
-
-        if self.extra_warnings.unwrap_or(flags.is_none()) {
+        if self.extra_warnings.unwrap_or(envflags.is_none()) {
             if let Some(wflags) = cmd.family.extra_warnings_flags() {
                 cmd.push_cc_arg(wflags.into());
             }
         }
 
-        for flag in self.flags.iter() {
-            cmd.args.push((**flag).into());
-        }
-
-        // Add cc flags inherited from matching rustc flags
+        // Add cc flags inherited from matching rustc flags.
         if self.inherit_rustflags {
             self.add_inherited_rustflags(&mut cmd, &target)?;
         }
 
+        // Set flags configured in the builder (do this second-to-last, to allow these to override
+        // everything above).
+        for flag in self.flags.iter() {
+            cmd.args.push((**flag).into());
+        }
         for flag in self.flags_supported.iter() {
             if self
                 .is_flag_supported_inner(flag, &cmd, &target)
@@ -1952,7 +1974,6 @@ impl Build {
                 cmd.push_cc_arg((**flag).into());
             }
         }
-
         for (key, value) in self.definitions.iter() {
             if let Some(ref value) = *value {
                 cmd.args.push(format!("-D{}={}", key, value).into());
@@ -1961,22 +1982,11 @@ impl Build {
             }
         }
 
-        if self.warnings_into_errors {
-            let warnings_to_errors_flag = cmd.family.warnings_to_errors_flag().into();
-            cmd.push_cc_arg(warnings_to_errors_flag);
-        }
-
-        // Copied from <https://github.com/rust-lang/rust/blob/5db81020006d2920fc9c62ffc0f4322f90bffa04/compiler/rustc_codegen_ssa/src/back/linker.rs#L27-L38>
-        //
-        // Disables non-English messages from localized linkers.
-        // Such messages may cause issues with text encoding on Windows
-        // and prevent inspection of msvc output in case of errors, which we occasionally do.
-        // This should be acceptable because other messages from rustc are in English anyway,
-        // and may also be desirable to improve searchability of the compiler diagnostics.
-        if matches!(cmd.family, ToolFamily::Msvc { clang_cl: false }) {
-            cmd.env.push(("VSLANG".into(), "1033".into()));
-        } else {
-            cmd.env.push(("LC_ALL".into(), "C".into()));
+        // Set flags from the environment (do this last, to allow these to override everything else).
+        if let Some(flags) = &envflags {
+            for arg in flags {
+                cmd.push_cc_arg(arg.into());
+            }
         }
 
         Ok(cmd)
@@ -3226,7 +3236,6 @@ impl Build {
                 }
             });
 
-        let default = tool.to_string();
         let tool = match tool_opt {
             Some(t) => t,
             None => {
@@ -3287,7 +3296,7 @@ impl Build {
                     self.cmd(&name)
                 } else if self.get_is_cross_compile()? {
                     match self.prefix_for_target(&self.get_raw_target()?) {
-                        Some(p) => {
+                        Some(prefix) => {
                             // GCC uses $target-gcc-ar, whereas binutils uses $target-ar -- try both.
                             // Prefer -ar if it exists, as builds of `-gcc-ar` have been observed to be
                             // outright broken (such as when targeting freebsd with `--disable-lto`
@@ -3295,24 +3304,31 @@ impl Build {
                             // fails to find one).
                             //
                             // The same applies to ranlib.
-                            let mut chosen = default;
-                            for &infix in &["", "-gcc"] {
-                                let target_p = format!("{}{}-{}", p, infix, tool);
-                                if Command::new(&target_p).output().is_ok() {
-                                    chosen = target_p;
-                                    break;
-                                }
-                            }
+                            let chosen = ["", "-gcc"]
+                                .iter()
+                                .filter_map(|infix| {
+                                    let target_p = format!("{prefix}{infix}-{tool}");
+                                    let status = Command::new(&target_p)
+                                        .arg("--version")
+                                        .stdin(Stdio::null())
+                                        .stdout(Stdio::null())
+                                        .stderr(Stdio::null())
+                                        .status()
+                                        .ok()?;
+                                    status.success().then_some(target_p)
+                                })
+                                .next()
+                                .unwrap_or_else(|| tool.to_string());
                             name = chosen.into();
                             self.cmd(&name)
                         }
                         None => {
-                            name = default.into();
+                            name = tool.into();
                             self.cmd(&name)
                         }
                     }
                 } else {
-                    name = default.into();
+                    name = tool.into();
                     self.cmd(&name)
                 }
             }
diff --git a/src/target/generated.rs b/src/target/generated.rs
index 6b21366..2acfe95 100644
--- a/src/target/generated.rs
+++ b/src/target/generated.rs
@@ -3028,6 +3028,18 @@ pub(crate) const LIST: &[(&str, TargetInfo<'static>)] = &[
             llvm_target: "x86_64-linux-android",
         },
     ),
+    (
+        "x86_64-pc-cygwin",
+        TargetInfo {
+            full_arch: "x86_64",
+            arch: "x86_64",
+            vendor: "pc",
+            os: "cygwin",
+            env: "",
+            abi: "",
+            llvm_target: "x86_64-pc-cygwin",
+        },
+    ),
     (
         "x86_64-pc-nto-qnx710",
         TargetInfo {


Test patch:
diff --git a/tests/cflags.rs b/tests/cflags.rs
index e93c290..06bdb73 100644
--- a/tests/cflags.rs
+++ b/tests/cflags.rs
@@ -19,21 +19,39 @@ fn gnu_no_warnings_if_cflags() {
     test.cmd(0).must_not_have("-Wall").must_not_have("-Wextra");
 }
 
-/// Test the ordering of `CFLAGS*` variables.
+/// Test the ordering of flags.
+///
+/// 1. Default flags
+/// 2. Rustflags.
+/// 3. Builder flags.
+/// 4. Environment flags.
 fn cflags_order() {
-    unsafe { env::set_var("CFLAGS", "-arbitrary1") };
-    unsafe { env::set_var("HOST_CFLAGS", "-arbitrary2") };
-    unsafe { env::set_var("TARGET_CFLAGS", "-arbitrary2") };
-    unsafe { env::set_var("CFLAGS_x86_64_unknown_none", "-arbitrary3") };
-    unsafe { env::set_var("CFLAGS_x86_64-unknown-none", "-arbitrary4") };
+    // FIXME(madsmtm): Re-enable once `is_flag_supported` works in CI regardless of `target`.
+    // unsafe { std::env::set_var("CARGO_ENCODED_RUSTFLAGS", "-Cdwarf-version=5") };
+
+    unsafe { env::set_var("CFLAGS", "-Larbitrary1") };
+    unsafe { env::set_var("HOST_CFLAGS", "-Larbitrary2") };
+    unsafe { env::set_var("TARGET_CFLAGS", "-Larbitrary2") };
+    unsafe { env::set_var("CFLAGS_x86_64_unknown_none", "-Larbitrary3") };
+    unsafe { env::set_var("CFLAGS_x86_64-unknown-none", "-Larbitrary4") };
+
     let test = Test::gnu();
     test.gcc()
         .target("x86_64-unknown-none")
+        .static_flag(true)
+        .flag("-Lbuilder-flag1")
+        .flag("-Lbuilder-flag2")
         .file("foo.c")
         .compile("foo");
 
     test.cmd(0)
-        .must_have_in_order("-arbitrary1", "-arbitrary2")
-        .must_have_in_order("-arbitrary2", "-arbitrary3")
-        .must_have_in_order("-arbitrary3", "-arbitrary4");
+        // .must_have_in_order("-static", "-gdwarf-5")
+        // .must_have_in_order("-gdwarf-5", "-Lbuilder-flag1")
+        .must_have_in_order("-static", "-Lbuilder-flag1")
+        .must_have_in_order("-Lbuilder-flag1", "-Lbuilder-flag2")
+        .must_have_in_order("-Lbuilder-flag2", "-Larbitrary1")
+        .must_have_in_order("-Larbitrary1", "-Larbitrary2")
+        .must_have_in_order("-Larbitrary1", "-Larbitrary2")
+        .must_have_in_order("-Larbitrary2", "-Larbitrary3")
+        .must_have_in_order("-Larbitrary3", "-Larbitrary4");
 }
diff --git a/tests/support/mod.rs b/tests/support/mod.rs
index 67516a3..0ca29eb 100644
--- a/tests/support/mod.rs
+++ b/tests/support/mod.rs
@@ -160,8 +160,8 @@ impl Execution {
         match (before_position, after_position) {
             (Some(b), Some(a)) if b < a => {}
             (b, a) => panic!(
-                "{:?} (last position: {:?}) did not appear before {:?} (last position: {:?})",
-                before, b, after, a
+                "{:?} (last position: {:?}) did not appear before {:?} (last position: {:?}): {:?}",
+                before, b, after, a, self.args
             ),
         };
         self