Skip to content

rust: workaround --print file-names emitting staticlibs / dylibs. #658

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

Merged
merged 1 commit into from
Feb 3, 2020

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Feb 3, 2020

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.
Copy link
Contributor

@froydnj froydnj left a comment

Choose a reason for hiding this comment

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

Thanks.

@froydnj
Copy link
Contributor

froydnj commented Feb 3, 2020

I think we do have Rust version information at some point (maybe propagating it to here is hard), but maybe we should consider making this change conditional on the compiler version so that it's easy to detect regressions in the future?

@froydnj froydnj merged commit b1b347b into mozilla:master Feb 3, 2020
@emilio
Copy link
Contributor Author

emilio commented Feb 3, 2020

I think we do have Rust version information at some point (maybe propagating it to here is hard), but maybe we should consider making this change conditional on the compiler version so that it's easy to detect regressions in the future?

This works regardless of the other fix being in rust or not... Not sure what a condition for this workaround would look like.

@emilio emilio deleted the cargo-check-staticlib branch February 3, 2020 16:47
@froydnj
Copy link
Contributor

froydnj commented Feb 3, 2020

I think we do have Rust version information at some point (maybe propagating it to here is hard), but maybe we should consider making this change conditional on the compiler version so that it's easy to detect regressions in the future?

This works regardless of the other fix being in rust or not... Not sure what a condition for this workaround would look like.

I meant that if rustc_version >= fixed_rustc_version, we would not apply the workaround. It probably doesn't matter that much, but it would help people detect if rustc suddenly broke again.

@emilio
Copy link
Contributor Author

emilio commented Feb 3, 2020

Ah, that's fair... We'd need to fix rustc first though :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants