Detect rustc(drivers) more precisely by sagudev · Pull Request #1897 · mozilla/sccache

sagudev · 2023-10-07T06:17:31Z

If we are not sure it's rustc like and we check -vV output to accept valid rustc drivers.

Fixes #861

codecov-commenter · 2023-10-07T06:55:28Z

Codecov Report

Attention: 125 lines in your changes are missing coverage. Please review.

Comparison is base (abbd88e) 31.00% compared to head (0bf8ee2) 30.55%.

❗ Current head 0bf8ee2 differs from pull request most recent head 1a58db1. Consider uploading reports for the commit 1a58db1 to get more accurate results

Files	Patch %	Lines
src/compiler/compiler.rs	28.07%	17 Missing and 106 partials ⚠️
src/test/tests.rs	33.33%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1897      +/-   ##
==========================================
- Coverage   31.00%   30.55%   -0.46%     
==========================================
  Files          52       51       -1     
  Lines       19753    19254     -499     
  Branches     9506     9238     -268     
==========================================
- Hits         6125     5883     -242     
+ Misses       7925     7731     -194     
+ Partials     5703     5640      -63

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sagudev · 2023-10-07T07:23:45Z

Aarch fails on unrelated error (installation of musl).

Xuanwo · 2023-10-07T11:23:50Z

Aarch fails on unrelated error (installation of musl).

Yep. It's unrelated. I'm working on the fix. Please ignore it for now. Thanks for the patience.

src/compiler/compiler.rs

drahnr · 2023-10-17T12:40:19Z

I have yet to do a full review.

sagudev · 2023-10-18T08:04:13Z

I squashed all commits as they were not meant to be reviewed separately. I also updated verify_rust_compiler to not depend on must_be_rust and moved this logic back to detect_compiler

src/compiler/compiler.rs

sagudev · 2023-11-27T16:28:00Z

rebased

drahnr · 2024-01-23T13:14:31Z

I'll do a full review as soon as all CI steps went green

sagudev · 2024-01-25T15:13:25Z

Checks are green in my fork: https://github.com/sagudev/sccache/actions/runs/7656273169 & https://github.com/sagudev/sccache/actions/runs/7656273174

src/compiler/compiler.rs

drahnr · 2024-01-25T17:31:26Z

src/compiler/compiler.rs

-                if stdout.starts_with("rustc ") {
-                    return Ok(stdout);
-                }
+    if !must_be_rust && is_like_c_compiler(&rustc_executable) {


I don't see the need for the must_be_rust. I do not understand why we go back to mutable state if we can use scoped Option<PathBuf> if there is a rustc-like or None, and use that to determine the full compiler path or fallback to c_compiler.

This is to be compatible with current behavior. If name of compiler is rustc and we cannot detect it we fast fail (we do not try it for c compiler).

That may be, but it can be expressed more concisely.

So, should I just change behavior and simply code or express it differently (currently have no idea how)?

drahnr · 2024-01-25T17:39:07Z

src/compiler/compiler.rs

+                "",
+                "gcc: error: unrecognized command-line option '-vV'; did you mean '-v'?",
+            )),
+        );


Iirc, the result will be cached, so the impact of another resolution call is very small, even for small projects. Correct me if I am wrong.

Is this comment to #1897 (comment)?

This is a principled question, if we incur much overhead for the compiler resolution with this changeset.

I am not sure, to be honest. But if the results are cached then it should be ok.

drahnr

I don't particularly like the use mutable bool variables in deciding the compiler. There is an outstanding question on the efficiency of test runs and ifwe can assume -vV is unique to rust.

sagudev · 2024-01-25T18:32:58Z

if we can assume -vV is unique to rust.

Even if it is not we try to parse the result and if it does not contain rustc(-drivers) specific output we know it is not rusty.

sylvestre · 2024-02-20T07:50:25Z

moved into a different PR #1897

* compiler: Add an additional fallback attempt if no vanilla rustc or known cc was found Fixes #861 Co-authored-by: sagudev <[email protected]>

sagudev marked this pull request as ready for review October 7, 2023 06:42

sagudev mentioned this pull request Oct 9, 2023

Replace script_plugins with a clippy like rustc driver (named crown) servo/servo#30508

Merged

3 tasks

sylvestre reviewed Oct 10, 2023

View reviewed changes

src/compiler/compiler.rs Outdated Show resolved Hide resolved

sylvestre reviewed Oct 10, 2023

View reviewed changes

src/compiler/compiler.rs Show resolved Hide resolved

sagudev force-pushed the rustc-drivers branch from 13c9a17 to a93bb87 Compare October 11, 2023 07:48

sylvestre reviewed Oct 11, 2023

View reviewed changes

src/compiler/compiler.rs Outdated Show resolved Hide resolved

sylvestre reviewed Oct 11, 2023

View reviewed changes

src/compiler/compiler.rs Outdated Show resolved Hide resolved

sagudev requested a review from sylvestre October 17, 2023 04:28

sylvestre requested a review from drahnr October 17, 2023 08:04

drahnr reviewed Oct 17, 2023

View reviewed changes

src/compiler/compiler.rs Show resolved Hide resolved

drahnr reviewed Oct 17, 2023

View reviewed changes

src/compiler/compiler.rs Outdated Show resolved Hide resolved

drahnr reviewed Oct 17, 2023

View reviewed changes

src/compiler/compiler.rs Show resolved Hide resolved

sagudev force-pushed the rustc-drivers branch from 6910670 to d1ea6e3 Compare October 18, 2023 08:00

drahnr reviewed Oct 18, 2023

View reviewed changes

src/compiler/compiler.rs Outdated Show resolved Hide resolved

sagudev force-pushed the rustc-drivers branch from d1ea6e3 to eeffcf4 Compare November 27, 2023 16:27

sagudev force-pushed the rustc-drivers branch from eeffcf4 to 0bf8ee2 Compare November 27, 2023 16:29

sagudev requested a review from drahnr November 27, 2023 16:31

sagudev mentioned this pull request Dec 12, 2023

Failed to execute compile servo/servo#30850

Closed

sylvestre force-pushed the rustc-drivers branch from 0bf8ee2 to 72ac986 Compare January 23, 2024 10:21

Allow any rustc driver

1a58db1

sagudev force-pushed the rustc-drivers branch from 72ac986 to 1a58db1 Compare January 25, 2024 14:52

drahnr reviewed Jan 25, 2024

View reviewed changes

src/compiler/compiler.rs Outdated Show resolved Hide resolved

drahnr reviewed Jan 25, 2024

View reviewed changes

nit

a5082c4

drahnr requested changes Jan 25, 2024

View reviewed changes

drahnr mentioned this pull request Feb 12, 2024

rustc driver detection - includes #1897 #2085

Merged

sylvestre closed this Feb 20, 2024

sylvestre pushed a commit that referenced this pull request Feb 23, 2024

rustc driver detection - includes #1897 (#2085)

0cc0c62

* compiler: Add an additional fallback attempt if no vanilla rustc or known cc was found Fixes #861 Co-authored-by: sagudev <[email protected]>

Comments

Conversation

sagudev commented Oct 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Oct 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sagudev commented Oct 7, 2023

Uh oh!

Xuanwo commented Oct 7, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

drahnr commented Oct 17, 2023

Uh oh!

sagudev commented Oct 18, 2023

Uh oh!

Uh oh!

sagudev commented Nov 27, 2023

Uh oh!

drahnr commented Jan 23, 2024

Uh oh!

sagudev commented Jan 25, 2024

Uh oh!

Uh oh!

drahnr Jan 25, 2024

Choose a reason for hiding this comment

Uh oh!

sagudev Jan 25, 2024

Choose a reason for hiding this comment

Uh oh!

drahnr Jan 25, 2024

Choose a reason for hiding this comment

Uh oh!

sagudev Jan 25, 2024

Choose a reason for hiding this comment

Uh oh!

drahnr Jan 25, 2024

Choose a reason for hiding this comment

Uh oh!

sagudev Jan 25, 2024

Choose a reason for hiding this comment

Uh oh!

drahnr Jan 25, 2024

Choose a reason for hiding this comment

Uh oh!

sagudev Jan 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drahnr left a comment

Choose a reason for hiding this comment

Uh oh!

sagudev commented Jan 25, 2024

Uh oh!

sylvestre commented Feb 20, 2024 • edited by drahnr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sagudev commented Oct 7, 2023 •

edited

Loading

codecov-commenter commented Oct 7, 2023 •

edited

Loading

sagudev Jan 25, 2024 •

edited

Loading

sylvestre commented Feb 20, 2024 •

edited by drahnr

Loading