diff --git a/src/lib.rs b/src/lib.rs index 033d9469..dd77dd71 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -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 + // + // 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 - // - // 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) diff --git a/tests/cflags.rs b/tests/cflags.rs index e93c2907..06bdb737 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 67516a3b..0ca29ebb 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