use gcc::Build rather than deprecated gcc::Config #43975

RalfJung · 2017-08-18T10:09:43Z

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

alexcrichton · 2017-08-18T16:03:49Z

@bors: r+

bors · 2017-08-18T16:03:49Z

📌 Commit dac9a4e has been approved by alexcrichton

bors · 2017-08-19T05:02:55Z

⌛ Testing commit dac9a4eaff803747a2216263d97724701b0bcd40 with merge 51fd08cc86b50b4f5fbe3ff1afb32026e0a92102...

bors · 2017-08-19T05:29:46Z

💔 Test failed - status-appveyor

RalfJung · 2017-08-19T10:19:47Z

Retrying 3 of 3
Chocolatey v0.10.7
Installing the following packages:
InnoSetup
By installing you accept licenses for the packages.
[NuGet] An error occurred while loading packages from 'https://chocolatey.org/api/v2/': The operation has timed out.
InnoSetup not installed. The package was not found with the source(s) listed.
 If you specified a particular version and are receiving this message, it is possible that the package name exists but the version does not.
 Version: ""
 Source(s): "https://chocolatey.org/api/v2/"
Chocolatey installed 0/1 packages. 1 packages failed.

AppVeyor network issue.

kennytm · 2017-08-19T11:58:28Z

@bors retry #43985

bors · 2017-08-19T17:34:43Z

⌛ Testing commit dac9a4eaff803747a2216263d97724701b0bcd40 with merge 8448844271c663e5023915febf9797506d2a4676...

bors · 2017-08-19T17:58:26Z

💔 Test failed - status-appveyor

kennytm · 2017-08-19T18:16:45Z

I don't know, the log does not make any sense at all. Only the section from [00:06:19] to [00:07:07] is viewable. The error indicates sccache error but sccache.log contains no error.

@bors retry

[00:06:54] [161/1909] Building CXX object lib\Transforms\InstCombine\CMakeFiles\LLVMInstCombine.dir\InstCombineCompares.cpp.obj
[00:06:54] FAILED: lib/Transforms/InstCombine/CMakeFiles/LLVMInstCombine.dir/InstCombineCompares.cpp.obj 
[00:06:54] C:\projects\rust\build\bootstrap\debug\sccache-plus-cl.exe  /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib\Transforms\InstCombine -IC:\projects\rust\src\llvm\lib\Transforms\InstCombine -Iinclude -IC:\projects\rust\src\llvm\include /nologo /MD /Wall /W4 -wd4141 -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 -wd4355 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4800 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4324 -w14062 -we4238 /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /MT /O2 /Ob2   -UNDEBUG  /EHs-c- /GR- /showIncludes /Folib\Transforms\InstCombine\CMakeFiles\LLVMInstCombine.dir\InstCombineCompares.cpp.obj /Fdlib\Transforms\InstCombine\CMakeFiles\LLVMInstCombine.dir\LLVMInstCombine.pdb /FS -c C:\projects\rust\src\llvm\lib\Transforms\InstCombine\InstCombineCompares.cpp
[00:06:54] error: failed to execute compile
[00:06:54] caused by: error reading compile response from server
[00:06:54] caused by: Failed to read response header
[00:06:54] caused by: failed to fill whole buffer
[00:07:07] [162/1909] Building CXX object lib\Transforms\InstCombine\CMakeFiles\LLVMInstCombine.dir\InstCombineCalls.cpp.obj
[00:07:07] FAILED: lib/Transforms/InstCombine/CMakeFiles/LLVMInstCombine.dir/InstCombineCalls.cpp.obj 
[00:07:07] C:\projects\rust\build\bootstrap\debug\sccache-plus-cl.exe  /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib\Transforms\InstCombine -IC:\projects\rust\src\llvm\lib\Transforms\InstCombine -Iinclude -IC:\projects\rust\src\llvm\include /nologo /MD /Wall /W4 -wd4141 -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 -wd4355 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4800 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4324 -w14062 -we4238 /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /MT /O2 /Ob2   -UNDEBUG  /EHs-c- /GR- /showIncludes /Folib\Transforms\InstCombine\CMakeFiles\LLVMInstCombine.dir\InstCombineCalls.cpp.obj /Fdlib\Transforms\InstCombine\CMakeFiles\LLVMInstCombine.dir\LLVMInstCombine.pdb /FS -c C:\projects\rust\src\llvm\lib\Transforms\InstCombine\InstCombineCalls.cpp
[00:07:07] error: failed to execute compile
[00:07:07] caused by: error reading compile response from server
[00:07:07] caused by: Failed to read response header
[00:07:07] caused by: failed to fill whole buffer
[00:07:07] ninja: build stopped: subcommand failed.
[00:07:07] thread 'main' panicked at '
[00:07:07] command did not execute successfully, got: exit code: 1

bors · 2017-08-19T19:22:28Z

⌛ Testing commit dac9a4eaff803747a2216263d97724701b0bcd40 with merge 5c0a340d472cbac99e58e30acf21facec2429974...

bors · 2017-08-19T19:42:44Z

💔 Test failed - status-appveyor

RalfJung · 2017-08-20T00:21:46Z

This looks legit -- not sure why it only shows on Windows though:

error: `#[deprecated]` cannot be used in staged api, use `#[rustc_deprecated]` instead
  --> C:\Users\appveyor\.cargo\registry\src\github.com-1ecc6299db9ec823\gcc-0.3.52\src\lib.rs:57:1
   |
57 | pub type Config = Build;
   | ^^^^^^^^^^^^^^^^^^^^^^^^
error: `#[deprecated]` cannot be used in staged api, use `#[rustc_deprecated]` instead
   --> C:\Users\appveyor\.cargo\registry\src\github.com-1ecc6299db9ec823\gcc-0.3.52\src\lib.rs:237:1
    |
237 | / pub fn compile_library(output: &str, files: &[&str]) {
238 | |     let mut c = Build::new();
239 | |     for f in files.iter() {
240 | |         c.file(*f);
241 | |     }
242 | |     c.compile(output);
243 | | }
    | |_^
error: aborting due to 2 previous errors
error: Could not compile `gcc`.

@alexcrichton the build failure is inside the gcc crate, it seems?

alexcrichton · 2017-08-20T12:40:41Z

Yes that's apparently a bug in rustc, looks like it will need to be fixed before landing this

RalfJung · 2017-08-20T13:20:33Z

Conversation with @eddyb revealed the cause: The gcc crate gets compiled with -Zforce-unstable-if-unmarked, like all crates in the compiler do. The compiler then considers all these crates to be using staged API, and rules out the #[deprecated] attribute. It just seems like gcc is the first crates.io-dependency of the compiler that deprecates something.^^

There are various ways this could be fixed:

We could try not passing this flag for build-deps, as it serves no purpose there. However, the problem will come back whenever an actual dependency tries to deprecate something, so this is not a good solution.
@eddyb suggests to just allow #[deprecated] in staged API crates. However, I assume that this error was added for a reason?
We currently actually allow stable/unstable to be used by crates.io dependencies of the compiler even though they do not enable staged API, just because that flag is that. That seems wrong to me. So I think this should be fixed by disallowing stable, unstable, rustc_deprecated in crates that don't use the staged_api feature, even if that flag is set -- and to allow them to use deprecated.

I think the last fix makes most sense.

eddyb · 2017-08-20T13:24:58Z

However, I assume that this error was added for a reason?

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.

kennytm · 2017-08-20T13:30:25Z

Given #30206 (comment), I think the preference is the second fix, and eventually phase out #[rustc_deprecated].

RalfJung · 2017-08-20T13:35:57Z

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:

// compile-flags:-Zforce-unstable-if-unmarked

#[unstable] //~ ERROR: stability attributes may not be used
#[stable] //~ ERROR: stability attributes may not be used
#[rustc_deprecated] //~ ERROR: stability attributes may not be used
fn main() { }

This shows that when -Zforce-unstable-if-unmarked is set, the crate can use stability annotations without opting in to them. That looks wrong, doesn't it?

eddyb · 2017-08-20T13:40:57Z

The flag is unstable anyway though.

kennytm · 2017-08-20T13:41:21Z

The second and third fix can coexist AFAIK 🙂.

petrochenkov · 2017-08-20T14:07:40Z

This shows that when -Zforce-unstable-if-unmarked is set, the crate can use stability annotations without opting in to them. That looks wrong, doesn't it?

It may look wrong, but it's by design.
I actually implemented the third fix in #43270, then but reverted it because it didn't make much sense and only caused more noise, -Z force-unstable-if-unmarked already requires unstable, so there's no need to guard stability attributes twice.

The #[deprecated] cannot be used in staged api error is purely a lint for the standard library writers, it can be removed if it causes trouble.

RalfJung · 2017-08-20T14:17:41Z

-Z force-unstable-if-unmarked already requires unstable, so there's no need to guard stability attributes twice.

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 staged_api is ever intended to be stabilized, we still want some indication whether the current crate actually uses this.

In particular, isn't it the case that right now, we could actually remove #[feature(staged_api)] from all crates in the compiler because they have -Z force-unstable-if-unmarked set? I still think that shouldn't be the case. But oh well, it's your call :)

I will change the error/lint to not fire if the crate doesn't set the staged_api feature. That will still leave the lint intact for libstd.

alexcrichton · 2017-08-20T18:34:24Z

The #[rustc_deprecated] and #[deprecated] attributes are subtly different and can't be "blindly unified" right now. It's fine to just turn this error into an allow-by-default lint and set it to deny-by-default in all of the standard library crates, we want this turned on for those regardless.

alexcrichton · 2017-09-05T14:30:02Z

Yes a change in gcc when upgrading is that warnings are on-by-default, so where were create a gcc::Build and inspect the flags we'll need to call warnings(false) to disable that

RalfJung · 2017-09-05T15:51:22Z

@alexcrichton like so?

alexcrichton · 2017-09-05T16:16:40Z

@RalfJung I suspect that 59f4e5d won't compile due to lifetime issues, and I think the main locations to target are those in src/bootstrap rather than src/librustc_llvm (although we probably want that too)

RalfJung · 2017-09-05T16:44:02Z

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".^^

alexcrichton · 2017-09-05T16:47:21Z

Yeah when compiling upstream projects we for sure want to disable all warnings.

alexcrichton · 2017-09-05T16:47:37Z

@bors: r+

bors · 2017-09-05T16:47:38Z

📌 Commit 13cf229 has been approved by alexcrichton

bors · 2017-09-05T20:30:22Z

⌛ Testing commit 13cf229 with merge e7fa363fddf90d05ecae17d0ecf38760d8021873...

bors · 2017-09-05T21:04:09Z

💔 Test failed - status-travis

alexcrichton · 2017-09-05T21:06:33Z

@bors: retry

good ol' network troubles

RalfJung · 2017-09-06T07:53:15Z

@Others With japaric/xargo#166 now merged into xargo, xargo master should work again with the latest rustc nightly.

bors · 2017-09-06T16:30:28Z

⌛ Testing commit 13cf229 with merge bfa86877326c16a0e370525e5655112247e96162...

bors · 2017-09-06T18:12:09Z

💔 Test failed - status-travis

alexcrichton · 2017-09-06T18:32:13Z

@bors: retry

fun times with travis

bors · 2017-09-06T19:28:43Z

⌛ Testing commit 13cf229 with merge 3681220...

@alexcrichton

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

bors · 2017-09-06T22:09:44Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 3681220 to master...

Arnavion · 2017-09-14T23:52:55Z

@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 /Wall in CMAKE_CXX_FLAGS -> llvm is built with EnableAllWarnings -> takes hours to complete. rust-lang/cc-rs@5cbb921 in 0.3.54 is needed to fix it.

alexcrichton · 2017-09-15T00:54:16Z

@Arnavion I think that's #44531

Arnavion · 2017-09-15T01:07:00Z

Ah, thank you! I searched for "0.3.53", not "0.3.54" ...

rust-highfive assigned alexcrichton Aug 18, 2017

RalfJung mentioned this pull request Aug 19, 2017

Run MIR-libstd tests for other architectures rust-lang/miri#202

Closed

RalfJung mentioned this pull request Aug 19, 2017

error: "can't handle type: TyClosure" rust-lang/miri#304

Closed

RalfJung mentioned this pull request Aug 20, 2017

Provide a way to tweak libstd compilation dependency resolution japaric/xargo#160

Closed

RalfJung force-pushed the gcc branch from dac9a4e to c738566 Compare August 20, 2017 12:47

disable gcc warnings

13cf229

RalfJung force-pushed the gcc branch from 59f4e5d to 13cf229 Compare September 5, 2017 16:46

bors merged commit 13cf229 into rust-lang:master Sep 6, 2017

RalfJung deleted the gcc branch July 10, 2018 09:05

use gcc::Build rather than deprecated gcc::Config #43975

use gcc::Build rather than deprecated gcc::Config #43975

Conversation

RalfJung commented Aug 18, 2017

alexcrichton commented Aug 18, 2017

bors commented Aug 18, 2017

bors commented Aug 19, 2017

bors commented Aug 19, 2017

RalfJung commented Aug 19, 2017

kennytm commented Aug 19, 2017

bors commented Aug 19, 2017

bors commented Aug 19, 2017

kennytm commented Aug 19, 2017

bors commented Aug 19, 2017

bors commented Aug 19, 2017

RalfJung commented Aug 20, 2017

alexcrichton commented Aug 20, 2017

RalfJung commented Aug 20, 2017

eddyb commented Aug 20, 2017

kennytm commented Aug 20, 2017

RalfJung commented Aug 20, 2017

eddyb commented Aug 20, 2017

kennytm commented Aug 20, 2017

petrochenkov commented Aug 20, 2017

RalfJung commented Aug 20, 2017

alexcrichton commented Aug 20, 2017

alexcrichton commented Sep 5, 2017

RalfJung commented Sep 5, 2017 • edited Loading

alexcrichton commented Sep 5, 2017

RalfJung commented Sep 5, 2017

alexcrichton commented Sep 5, 2017

alexcrichton commented Sep 5, 2017

bors commented Sep 5, 2017

bors commented Sep 5, 2017

bors commented Sep 5, 2017

alexcrichton commented Sep 5, 2017

RalfJung commented Sep 6, 2017

bors commented Sep 6, 2017

bors commented Sep 6, 2017

alexcrichton commented Sep 6, 2017

bors commented Sep 6, 2017

bors commented Sep 6, 2017

Arnavion commented Sep 14, 2017 • edited Loading

alexcrichton commented Sep 15, 2017

Arnavion commented Sep 15, 2017

RalfJung commented Sep 5, 2017 •

edited

Loading

Arnavion commented Sep 14, 2017 •

edited

Loading