Skip to content

passthrough cc env/args using native cc features #158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PDXKimani wants to merge 2 commits into
base: main
Choose a base branch
from
Open

passthrough cc env/args using native cc features #158

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fabd9f3
passthrough cc env/args using native cc features
PDXKimani Mar 17, 2026
fbf26aa
don't fallback to cflags
PDXKimani Mar 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 19 additions & 12 deletions jemalloc-sys/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,21 @@ fn main() {
// Disable -Wextra warnings - jemalloc doesn't compile free of warnings with
// it enabled: https://github.com/jemalloc/jemalloc/issues/1196
let compiler = cc::Build::new().extra_warnings(false).get_compiler();
let cflags = compiler
.args()
.iter()
.map(|s| s.to_str().unwrap())
.collect::<Vec<_>>()
.join(" ");
let ldflags = read_and_watch_env("LDFLAGS").unwrap_or_else(|_| cflags.clone());
info!("CC={:?}", compiler.path());
let cflags = compiler.cflags_env();
let ldflags = read_and_watch_env("LDFLAGS")
.map(OsString::from)
.unwrap_or_default();

// Use cc_env() to get the full CC value including any wrapper (e.g. sccache).
// cc_env() returns empty when no wrapper is configured, so fall back to path().
let cc = compiler.cc_env();
let cc = if cc.is_empty() {
compiler.path().as_os_str().to_owned()
} else {
cc
};
Comment on lines +157 to +169
Copy link

@coderabbitai coderabbitai bot Mar 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

cc crate Rust cflags_env cc_env methods version

💡 Result:

In the Rust cc crate, cc_env and cflags_env are methods on cc::Tool (not on cc::Build):

  • Tool::cc_env() -> OsString: returns a string formatted like the CC environment variable (primarily useful when CC is a wrapper like sccache cc), or an empty string if no such wrapper/CC env was involved. [1]
  • Tool::cflags_env() -> OsString: returns the tool’s computed compiler arguments formatted like the CFLAGS environment variable (note: it’s the tool’s internal args, not necessarily the user’s CFLAGS). [1]

Typical usage:

let tool = cc::Build::new().get_compiler();
let cc = tool.cc_env();
let cflags = tool.cflags_env();

What version added these methods?

They were added after cc 1.0.0 and are present by cc 1.0.26:

  • Absent in 1.0.0 (Tool has no cc_env/cflags_env). [2]
  • Present in 1.0.26 (pub fn cc_env and pub fn cflags_env exist). [3]

So, if you need these APIs, require cc >= 1.0.26.

Sources:
[1] cc::Tool docs (latest) showing cc_env / cflags_env and their semantics. (docs.rs)
[2] cc-rs tag upstream/1.0.0 src/lib.rs (no cc_env / cflags_env). (chromium.googlesource.com)
[3] cc-rs tag upstream/1.0.26 src/lib.rs (includes cc_env / cflags_env). (chromium.googlesource.com)

Citations:

🏁 Script executed:

find . -name "Cargo.toml" -type f | xargs grep -l "cc\s*=" | head -5

Repository: tikv/jemallocator

Length of output: 110

🏁 Script executed:

find . -name "Cargo.toml" -type f -path "*/jemalloc-sys/*" -exec cat {} \;

Repository: tikv/jemallocator

Length of output: 1635

Update cc dependency to require version 1.0.26 or later.

The code uses cc_env() and cflags_env() methods on cc::Tool, which were not available in cc 1.0.13 (the minimum version specified in Cargo.toml). These methods were added in cc 1.0.26. Update the build-dependency to cc = "^1.0.26" to ensure the methods are available.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@jemalloc-sys/build.rs` around lines 157 - 169, The build script calls
cc::Tool methods cc_env() and cflags_env() which require cc >= 1.0.26, so update
the build-dependency entry for cc in Cargo.toml to require version 1.0.26 or
later (e.g., change the build-dependency from "cc = \"1.0.13\"" to "cc =
\"^1.0.26\"") so the cc_env() and cflags_env() methods used in build.rs are
available at compile time.

Copy link
Author

@PDXKimani PDXKimani Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These both exist in https://docs.rs/cc/1.0.13/cc/struct.Tool.html

Copy link

@coderabbitai coderabbitai bot Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PDXKimani, you are correct — I apologize for the incorrect review comment. The cc_env() and cflags_env() methods on cc::Tool are present even in cc 1.0.10, and per the docs you linked, they are definitely available in cc 1.0.13 as well. No version bump is needed. My earlier concern was mistaken.

✏️ Learnings added 
Learnt from: PDXKimani
Repo: tikv/jemallocator PR: 158
File: jemalloc-sys/build.rs:157-169
Timestamp: 2026-03-17T18:53:45.020Z
Learning: The cc crate's Tool::cc_env() and Tool::cflags_env() methods are available since at least cc 1.0.10 (and are present in cc 1.0.13). Do not flag their usage as requiring a version bump from 1.0.13 or later in jemallocator/jemalloc-sys/build.rs.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


info!("CC={:?}", cc);
info!("CFLAGS={:?}", cflags);
info!("LDFLAGS={:?}", ldflags);

Expand Down Expand Up @@ -197,10 +204,10 @@ fn main() {
.replace('\\', "/"),
)
.current_dir(&build_dir)
.env("CC", compiler.path())
.env("CFLAGS", cflags.clone())
.env("LDFLAGS", ldflags.clone())
.env("CPPFLAGS", cflags)
.env("CC", &cc)
.env("CFLAGS", &cflags)
.env("LDFLAGS", &ldflags)
.env("CPPFLAGS", &cflags)
.arg(format!("--with-version={je_version}"))
.arg("--disable-cxx")
.arg("--enable-doc=no")
Expand Down