Skip to content

Commit ad73f8b

Browse files
madsmtmmadsmtm
committed
Change flag ordering
To allow flags set on the builder to override other flags, and to allow CFLAGS to override _all_ other flags.
1 parent 0dd683c commit ad73f8b

File tree

3 files changed

+73
-45
lines changed

3 files changed

+73
-45
lines changed

‎src/lib.rs

+44-34
Original file line numberDiff line numberDiff line change
@@ -1892,58 +1892,80 @@ impl Build {
18921892

18931893
let mut cmd = self.get_base_compiler()?;
18941894

1895+
// The flags below are added in roughly the following order:
1896+
// - 1. Default flags
1897+
// - Controlled by `cc-rs`.
1898+
// - 2. `rustc`-inherited flags
1899+
// - Controlled by `rustc`.
1900+
// - 3. Builder flags
1901+
// - Controlled by the developer using `cc-rs` in e.g. their `build.rs`.
1902+
// - 4. Environment flags
1903+
// - Controlled by the end user.
1904+
//
1905+
// This is important to allow later flags to override previous ones.
1906+
1907+
// Copied from <https://github.com/rust-lang/rust/blob/5db81020006d2920fc9c62ffc0f4322f90bffa04/compiler/rustc_codegen_ssa/src/back/linker.rs#L27-L38>
1908+
//
1909+
// Disables non-English messages from localized linkers.
1910+
// Such messages may cause issues with text encoding on Windows
1911+
// and prevent inspection of msvc output in case of errors, which we occasionally do.
1912+
// This should be acceptable because other messages from rustc are in English anyway,
1913+
// and may also be desirable to improve searchability of the compiler diagnostics.
1914+
if matches!(cmd.family, ToolFamily::Msvc { clang_cl: false }) {
1915+
cmd.env.push(("VSLANG".into(), "1033".into()));
1916+
} else {
1917+
cmd.env.push(("LC_ALL".into(), "C".into()));
1918+
}
1919+
18951920
// Disable default flag generation via `no_default_flags` or environment variable
18961921
let no_defaults = self.no_default_flags || self.getenv_boolean("CRATE_CC_NO_DEFAULTS");
1897-
18981922
if !no_defaults {
18991923
self.add_default_flags(&mut cmd, &target, &opt_level)?;
19001924
}
19011925

1926+
// Specify various flags that are not considered part of the default flags above.
1927+
// FIXME(madsmtm): Should these be considered part of the defaults? If no, why not?
19021928
if let Some(ref std) = self.std {
19031929
let separator = match cmd.family {
19041930
ToolFamily::Msvc { .. } => ':',
19051931
ToolFamily::Gnu | ToolFamily::Clang { .. } => '=',
19061932
};
19071933
cmd.push_cc_arg(format!("-std{}{}", separator, std).into());
19081934
}
1909-
19101935
for directory in self.include_directories.iter() {
19111936
cmd.args.push("-I".into());
19121937
cmd.args.push(directory.as_os_str().into());
19131938
}
1914-
1915-
let flags = self.envflags(if self.cpp { "CXXFLAGS" } else { "CFLAGS" })?;
1916-
if let Some(flags) = &flags {
1917-
for arg in flags {
1918-
cmd.push_cc_arg(arg.into());
1919-
}
1939+
if self.warnings_into_errors {
1940+
let warnings_to_errors_flag = cmd.family.warnings_to_errors_flag().into();
1941+
cmd.push_cc_arg(warnings_to_errors_flag);
19201942
}
19211943

19221944
// If warnings and/or extra_warnings haven't been explicitly set,
19231945
// then we set them only if the environment doesn't already have
19241946
// CFLAGS/CXXFLAGS, since those variables presumably already contain
19251947
// the desired set of warnings flags.
1926-
1927-
if self.warnings.unwrap_or(flags.is_none()) {
1948+
let envflags = self.envflags(if self.cpp { "CXXFLAGS" } else { "CFLAGS" })?;
1949+
if self.warnings.unwrap_or(envflags.is_none()) {
19281950
let wflags = cmd.family.warnings_flags().into();
19291951
cmd.push_cc_arg(wflags);
19301952
}
1931-
1932-
if self.extra_warnings.unwrap_or(flags.is_none()) {
1953+
if self.extra_warnings.unwrap_or(envflags.is_none()) {
19331954
if let Some(wflags) = cmd.family.extra_warnings_flags() {
19341955
cmd.push_cc_arg(wflags.into());
19351956
}
19361957
}
19371958

1938-
for flag in self.flags.iter() {
1939-
cmd.args.push((**flag).into());
1940-
}
1941-
1942-
// Add cc flags inherited from matching rustc flags
1959+
// Add cc flags inherited from matching rustc flags.
19431960
if self.inherit_rustflags {
19441961
self.add_inherited_rustflags(&mut cmd, &target)?;
19451962
}
19461963

1964+
// Set flags configured in the builder (do this second-to-last, to allow these to override
1965+
// everything above).
1966+
for flag in self.flags.iter() {
1967+
cmd.args.push((**flag).into());
1968+
}
19471969
for flag in self.flags_supported.iter() {
19481970
if self
19491971
.is_flag_supported_inner(flag, &cmd, &target)
@@ -1952,7 +1974,6 @@ impl Build {
19521974
cmd.push_cc_arg((**flag).into());
19531975
}
19541976
}
1955-
19561977
for (key, value) in self.definitions.iter() {
19571978
if let Some(ref value) = *value {
19581979
cmd.args.push(format!("-D{}={}", key, value).into());
@@ -1961,22 +1982,11 @@ impl Build {
19611982
}
19621983
}
19631984

1964-
if self.warnings_into_errors {
1965-
let warnings_to_errors_flag = cmd.family.warnings_to_errors_flag().into();
1966-
cmd.push_cc_arg(warnings_to_errors_flag);
1967-
}
1968-
1969-
// Copied from <https://github.com/rust-lang/rust/blob/5db81020006d2920fc9c62ffc0f4322f90bffa04/compiler/rustc_codegen_ssa/src/back/linker.rs#L27-L38>
1970-
//
1971-
// Disables non-English messages from localized linkers.
1972-
// Such messages may cause issues with text encoding on Windows
1973-
// and prevent inspection of msvc output in case of errors, which we occasionally do.
1974-
// This should be acceptable because other messages from rustc are in English anyway,
1975-
// and may also be desirable to improve searchability of the compiler diagnostics.
1976-
if matches!(cmd.family, ToolFamily::Msvc { clang_cl: false }) {
1977-
cmd.env.push(("VSLANG".into(), "1033".into()));
1978-
} else {
1979-
cmd.env.push(("LC_ALL".into(), "C".into()));
1985+
// Set flags from the environment (do this last, to allow these to override everything else).
1986+
if let Some(flags) = &envflags {
1987+
for arg in flags {
1988+
cmd.push_cc_arg(arg.into());
1989+
}
19801990
}
19811991

19821992
Ok(cmd)

‎tests/cflags.rs

+27-9
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,39 @@ fn gnu_no_warnings_if_cflags() {
1919
test.cmd(0).must_not_have("-Wall").must_not_have("-Wextra");
2020
}
2121

22-
/// Test the ordering of `CFLAGS*` variables.
22+
/// Test the ordering of flags.
23+
///
24+
/// 1. Default flags
25+
/// 2. Rustflags.
26+
/// 3. Builder flags.
27+
/// 4. Environment flags.
2328
fn cflags_order() {
24-
unsafe { env::set_var("CFLAGS", "-arbitrary1") };
25-
unsafe { env::set_var("HOST_CFLAGS", "-arbitrary2") };
26-
unsafe { env::set_var("TARGET_CFLAGS", "-arbitrary2") };
27-
unsafe { env::set_var("CFLAGS_x86_64_unknown_none", "-arbitrary3") };
28-
unsafe { env::set_var("CFLAGS_x86_64-unknown-none", "-arbitrary4") };
29+
// FIXME(madsmtm): Re-enable once `is_flag_supported` works in CI regardless of `target`.
30+
// unsafe { std::env::set_var("CARGO_ENCODED_RUSTFLAGS", "-Cdwarf-version=5") };
31+
32+
unsafe { env::set_var("CFLAGS", "-Larbitrary1") };
33+
unsafe { env::set_var("HOST_CFLAGS", "-Larbitrary2") };
34+
unsafe { env::set_var("TARGET_CFLAGS", "-Larbitrary2") };
35+
unsafe { env::set_var("CFLAGS_x86_64_unknown_none", "-Larbitrary3") };
36+
unsafe { env::set_var("CFLAGS_x86_64-unknown-none", "-Larbitrary4") };
37+
2938
let test = Test::gnu();
3039
test.gcc()
3140
.target("x86_64-unknown-none")
41+
.static_flag(true)
42+
.flag("-Lbuilder-flag1")
43+
.flag("-Lbuilder-flag2")
3244
.file("foo.c")
3345
.compile("foo");
3446

3547
test.cmd(0)
36-
.must_have_in_order("-arbitrary1", "-arbitrary2")
37-
.must_have_in_order("-arbitrary2", "-arbitrary3")
38-
.must_have_in_order("-arbitrary3", "-arbitrary4");
48+
// .must_have_in_order("-static", "-gdwarf-5")
49+
// .must_have_in_order("-gdwarf-5", "-Lbuilder-flag1")
50+
.must_have_in_order("-static", "-Lbuilder-flag1")
51+
.must_have_in_order("-Lbuilder-flag1", "-Lbuilder-flag2")
52+
.must_have_in_order("-Lbuilder-flag2", "-Larbitrary1")
53+
.must_have_in_order("-Larbitrary1", "-Larbitrary2")
54+
.must_have_in_order("-Larbitrary1", "-Larbitrary2")
55+
.must_have_in_order("-Larbitrary2", "-Larbitrary3")
56+
.must_have_in_order("-Larbitrary3", "-Larbitrary4");
3957
}

‎tests/support/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,8 @@ impl Execution {
160160
match (before_position, after_position) {
161161
(Some(b), Some(a)) if b < a => {}
162162
(b, a) => panic!(
163-
"{:?} (last position: {:?}) did not appear before {:?} (last position: {:?})",
164-
before, b, after, a
163+
"{:?} (last position: {:?}) did not appear before {:?} (last position: {:?}): {:?}",
164+
before, b, after, a, self.args
165165
),
166166
};
167167
self

0 commit comments

Comments
 (0)