-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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.
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
librustc_driver: Make --print file-names do a minor accounting of --emit #68799
Conversation
…mit. Otherwise we print bogus output which prevents using `cargo check` with sccache, see https://bugzilla.mozilla.org/show_bug.cgi?id=1612855#c2 for a reduced test-case. Ideally --print file-names will emit all the files emitted by the session, but that seems a bit hard to do in a general way, so this is a best effort thing which fixes the cargo check use-case.
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
This is somewhat related to #54852 (sccache was working around that already). |
This fixes cargo check in mozilla-central. The issue is that rustc --print file-names emits a somewhat poor approximation of what's actually going to be emitted. So for a staticlib crate, it will also print the staticlib file, which is not great. See https://bugzilla.mozilla.org/show_bug.cgi?id=1612855#c2 for a more straight-forward explanation of the failure case. Sccache would try to find the library and fail, erroring as a consequence. Pile up on the existing workaround for rmeta files not showing up (rust-lang/rust#54852) by removing files that are not metadata when we only request metadata. rust-lang/rust#68799 contains a rust-side fix that would also fix this.
This fixes cargo check in mozilla-central. The issue is that rustc --print file-names emits a somewhat poor approximation of what's actually going to be emitted. So for a staticlib crate, it will also print the staticlib file, which is not great. See https://bugzilla.mozilla.org/show_bug.cgi?id=1612855#c2 for a more straight-forward explanation of the failure case. Sccache would try to find the library and fail, erroring as a consequence. Pile up on the existing workaround for rmeta files not showing up (rust-lang/rust#54852) by removing files that are not metadata when we only request metadata. rust-lang/rust#68799 contains a rust-side fix that would also fix this.
Thanks for the PR! It seems a bit odd though to add a helper function for just this one piece of functionality when presumably similar checks are happening elsewhere in the codebase. Is there somewhere else this new function could be used, or if not, is the new function necessary? |
☔ The latest upstream changes (presumably #69088) made this pull request unmergeable. Please resolve the merge conflicts. |
👋 @emilio |
I'm going to close this because I'm not convince of its correctness. The behavior of --print file-names is quite bizarre. |
Otherwise we print bogus output which prevents using
cargo check
with sccache,see https://bugzilla.mozilla.org/show_bug.cgi?id=1612855#c2 for a reduced
test-case.
Ideally --print file-names will emit only the files emitted by the session, but
that seems a bit hard to do in a general way, so this is a best effort thing
which fixes the cargo check use-case.