Skip to content

Conversation

Copy link

Copilot AI commented Nov 25, 2025

  • Investigate the issue: Use CARGO_MAKEFLAGS instead of passing -jN to make
  • Review the current codebase
  • Verify fix is already implemented in jemalloc-sys/build.rs (make_command function, lines 369-384)
  • Confirm build works with the existing implementation
  • Run code review - no changed files
  • Run CodeQL security check - no issues

Summary

Issue #92 is already resolved in the main branch. No additional code changes are needed.

The make_command function in jemalloc-sys/build.rs implements the fix exactly as requested in the issue:

fn make_command(make_cmd: &str, build_dir: &Path, num_jobs: &str) -> Command {
    let mut cmd = Command::new(make_cmd);
    cmd.current_dir(build_dir);

    if let Ok(makeflags) = std::env::var("CARGO_MAKEFLAGS") {
        let makeflags = if let Ok(orig_makeflags) = std::env::var("MAKEFLAGS") {
            format!("{orig_makeflags} {makeflags}")
        } else {
            makeflags
        };
        cmd.env("MAKEFLAGS", makeflags);
    } else {
        cmd.arg("-j").arg(num_jobs);
    }
    cmd
}

This implementation:

  1. ✅ Checks if CARGO_MAKEFLAGS env var exists
  2. ✅ If it does, sets MAKEFLAGS to CARGO_MAKEFLAGS (preserving any existing MAKEFLAGS)
  3. ✅ Only falls back to using -jN if CARGO_MAKEFLAGS is not set

This matches the pattern from cmake-rs referenced in the issue and addresses the concern about CPU core spikes during parallel builds.

The fix was confirmed working by @BusyJay in the issue comments: "LGTM, in worse case, job server is overridden by user provided flags, which seems OK."

Original prompt

This section details on the original issue you should resolve

<issue_title>Do not pass -jN to make, use cargo jobserver instead</issue_title>
<issue_description>When compiling a crate that has jemallocator-sys as a dependency with cargo build -j4, I see a short spike where the number of cpu cores used is 8 instead of 4. The cause is this crate using the NUM_JOBS env var to call make, which ignores that cargo already has other running jobs, thus bringing the total number of running jobs to be higher than NUM_JOBS. This doesn't happen in any of the other *-sys crates, I guess because they use the cc crate with the parallel feature enabled instead of manually calling make.

The fix is to remove the -jN arg to make if the CARGO_MAKEFLAGS env var exists, and set MAKEFLAGS to CARGO_MAKEFLAGS instead.

I expect the implementation to be similar to this:

https://github.com/rust-lang/cmake-rs/blob/c4a60dd154dd90e469dffc41a1faa717704f90b3/src/lib.rs#L830

I guess using a crate to call make instead of doing it manually would handle this and other issues automatically, but I don't know what's the standard so I can't recommend any crates.</issue_description>

Comments on the Issue (you are @copilot in this section)

@BusyJay > The fix is to remove the -jN arg to make if the CARGO_MAKEFLAGS env var exists, and set MAKEFLAGS to CARGO_MAKEFLAGS instead.

Make sense. Would you like to send a PR?</comment_new>
<comment_new>@BusyJay
Likely the same problem as #32.</comment_new>
<comment_new>@BusyJay

MAKEFLAGS=" -- OBJECTS=init.o SHLIB=polars.so -j --jobserver-fds=8,9 --jobserver-auth=8,9" "make"

The problem may come from --, which requires argument for flags, and -j has no aurgument, so it will be parsed as a target instead of flags.</comment_new>
<comment_new>@BusyJay
LGTM, in worse case, job sever is overridden by user provided flags, which seems OK.</comment_new>


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@ti-chi-bot
Copy link

ti-chi-bot bot commented Nov 25, 2025

Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

📝 Please follow instructions in the contributing guide to update your commits with the DCO

Full details of the Developer Certificate of Origin can be found at developercertificate.org.

The list of commits missing DCO signoff:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do not pass -jN to make, use cargo jobserver instead

2 participants