Skip to content

Commit

Permalink
Change flag ordering
Browse files Browse the repository at this point in the history 
To allow flags set on the builder to override other flags, and to allow
CFLAGS to override _all_ other flags.
  • Loading branch information
@madsmtm
madsmtm committed Feb 14, 2025
1 parent 0dd683c commit ad73f8b
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 45 deletions.
78 changes: 44 additions & 34 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1892,58 +1892,80 @@ 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 { .. } => ':',
ToolFamily::Gnu | ToolFamily::Clang { .. } => '=',
};
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)
Expand All @@ -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());
Expand All @@ -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)
Expand Down
36 changes: 27 additions & 9 deletions tests/cflags.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
4 changes: 2 additions & 2 deletions tests/support/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit ad73f8b

Please sign in to comment.