-
Notifications
You must be signed in to change notification settings - Fork 13.4k
use gcc::Build rather than deprecated gcc::Config #43975
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
@bors: r+ |
📌 Commit dac9a4e has been approved by |
⌛ Testing commit dac9a4eaff803747a2216263d97724701b0bcd40 with merge 51fd08cc86b50b4f5fbe3ff1afb32026e0a92102... |
💔 Test failed - status-appveyor |
AppVeyor network issue. |
⌛ Testing commit dac9a4eaff803747a2216263d97724701b0bcd40 with merge 8448844271c663e5023915febf9797506d2a4676... |
💔 Test failed - status-appveyor |
I don't know, the log does not make any sense at all. Only the section from @bors retry
|
⌛ Testing commit dac9a4eaff803747a2216263d97724701b0bcd40 with merge 5c0a340d472cbac99e58e30acf21facec2429974... |
💔 Test failed - status-appveyor |
This looks legit -- not sure why it only shows on Windows though:
@alexcrichton the build failure is inside the gcc crate, it seems? |
Yes that's apparently a bug in rustc, looks like it will need to be fixed before landing this |
Conversation with @eddyb revealed the cause: The gcc crate gets compiled with There are various ways this could be fixed:
I think the last fix makes most sense. |
It was simply extra conservative AFAICT. I'd double-check with @alexcrichton but I believe what he meant earlier was that the error is unnecessary/wrong. OTOH, your last fix does sound pretty nice if it works in practice, and if nobody else disagrees. |
Given #30206 (comment), I think the preference is the second fix, and eventually phase out |
I see. Well just removing the error would certainly be simpler to implement. However, are we sure that it is okay for the following code to compile:
This shows that when |
The flag is unstable anyway though. |
The second and third fix can coexist AFAIK 🙂. |
It may look wrong, but it's by design. The |
All right. My interpretation of this was that crates should opt-in to have staging API, and not just get that behavior as a side-effect of a flag that has a somewhat different purpose. Doesn't the feature change the behavior of the compiler for items that don't have stability annotation, making that a warning or even an error? IOW, this is unlike other features that, after stabilization, are just behaving as if the feature flag was present -- if In particular, isn't it the case that right now, we could actually remove I will change the error/lint to not fire if the crate doesn't set the |
The |
Yes a change in |
@alexcrichton like so? |
Ah right. I will fix both of that. My question was mostly about "do we really want to blankly disable all warnings"? It seems the answer is "yes".^^ |
Yeah when compiling upstream projects we for sure want to disable all warnings. |
@bors: r+ |
📌 Commit 13cf229 has been approved by |
⌛ Testing commit 13cf229 with merge e7fa363fddf90d05ecae17d0ecf38760d8021873... |
💔 Test failed - status-travis |
@bors: retry
|
@Others With japaric/xargo#166 now merged into xargo, xargo master should work again with the latest rustc nightly. |
⌛ Testing commit 13cf229 with merge bfa86877326c16a0e370525e5655112247e96162... |
💔 Test failed - status-travis |
@bors: retry
|
use gcc::Build rather than deprecated gcc::Config I did `cargo update -p gcc` to upgrade only this package. Is there further process that should be follwoed when updating a build dependency from crates.io? r? @alexcrichton Fixes #43973
☀️ Test successful - status-appveyor, status-travis |
@alexcrichton Is there any plan to update to gcc 0.3.54 soon to fix the regression in windows-msvc builds? Edit: That is, rust-lang/cc-rs@8ce7108 causes cmake-rs to send |
Ah, thank you! I searched for "0.3.53", not "0.3.54" ... |
I did
cargo update -p gcc
to upgrade only this package. Is there further process that should be follwoed when updating a build dependency from crates.io?r? @alexcrichton
Fixes #43973