Use response files when spawning rustc commands when needed - #2789
Use response files when spawning rustc commands when needed#2789ranger-ross wants to merge 1 commit into
Conversation
| // On Windows there is a hard limit of ~32KB, so we cut off at 30KB to | ||
| // give some buffer just incase. | ||
| #[cfg(windows)] | ||
| let threshold: usize = 30 * 1024; | ||
| // On unix the limit is defined by ARG_MAX. If its not explicitly set we set it to 1MB | ||
| // which is fairly large but lower than the ~2MB that it defaults to on most systems. | ||
| #[cfg(unix)] | ||
| let threshold: usize = std::env::var("ARG_MAX") | ||
| .ok() | ||
| .and_then(|v| v.parse().ok()) | ||
| .unwrap_or(1024 * 1024); |
There was a problem hiding this comment.
Took these thresholds from rustc, see
There was a problem hiding this comment.
Besides ARG_MAX, this might fail if a single argument in the list exceeds MAX_ARG_STRLEN, which is 32 page sizes (128kb on most systems). This is a real issue (#2815) and I can confirm that lowering this threshold to 128kb solves it.
Even though setting a defensive limit solves the problem (32kb would cover both Windows and a single arg problem), it needs at least a comment explanation or a separate single-argument threshold to make it more transparent and less fragile to future changes.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2789 +/- ##
==========================================
- Coverage 74.32% 74.31% -0.02%
==========================================
Files 72 72
Lines 40772 40821 +49
==========================================
+ Hits 30305 30336 +31
- Misses 10467 10485 +18 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
CI failures should be unrelated to this change. |
c24a906 to
e02a9e9
Compare
|
@sylvestre friendly bump on this PR :) I'd be nice to get this PR merged before we stabilize the new Cargo build-dir layout. (which we are planning to do in the next beta branch) |
|
this PR also fixes #2815 |
3 participants
This is a follow up on #2782.
Unfortunately, I overlooked the fact that sccache modifies the args and does not pass the exact args to rustc that it receives.
This leads to the expanded args getting passed to rustc which blows up on windows when the command is large enough.
The solution is to have sccache create its own arg file (when needed) to pass the args to rustc.
fixes #2787
(I tested the repo in question and it fails without these changes and builds successfully with them)