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

Don't print opt fuel messages to stdout because it breaks Rustbuild #55495

Merged
merged 2 commits into from Nov 7, 2018

Conversation

Projects
None yet
6 participants
@wesleywiser
Member

wesleywiser commented Oct 30, 2018

Rustbuild passes --message-format json to the compiler invocations
which causes JSON to be emitted on stdout. Printing optimization fuel
messages to stdout breaks the json and causes Rustbuild to fail.

Work around this by emitting optimization fuel related messages on
stderr instead.

Don't print opt fuel messages to stdout because it breaks Rustbuild
Rustbuild passes `--message-format json` to the compiler invocations
which causes JSON to be emitted on stdout. Printing optimization fuel
messages to stdout breaks the json and causes Rustbuild to fail.

Work around this by emitting optimization fuel related messages on
stderr instead.
@rust-highfive

This comment has been minimized.

Collaborator

rust-highfive commented Oct 30, 2018

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@wesleywiser

This comment has been minimized.

Member

wesleywiser commented Oct 30, 2018

This is another instance of #43855. I'm using the same workaround as found in that issue.

@rust-highfive

This comment has been minimized.

Collaborator

rust-highfive commented Oct 30, 2018

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:000ee380:start=1540868026625325352,finish=1540868027634650722,duration=1009325370
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
---
[00:48:39] .........................................i.ii....................................................... 3500/4977
[00:48:41] ..............................................................i..................................... 3600/4977
[00:48:42] .................................................................................................... 3700/4977
[00:48:44] ................i................................................................................... 3800/4977
[00:48:47] .......................................F............................................................ 3900/4977
[00:48:53] .................................................................................................... 4100/4977
[00:48:56] ...........................................................................i........................ 4200/4977
[00:49:01] .................................................................................................... 4300/4977
[00:49:04] .................................................................................................... 4400/4977
---
[00:49:22] 
[00:49:22] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:503:22
[00:49:22] 
[00:49:22] 
[00:49:22] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-5.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "5.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:49:22] 
[00:49:22] 
[00:49:22] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:49:22] Build completed unsuccessfully in 0:03:39
[00:49:22] Build completed unsuccessfully in 0:03:39
[00:49:22] Makefile:58: recipe for target 'check' failed
[00:49:22] make: *** [check] Error 1
2476224 ./obj
2476184 ./obj/build
1847472 ./obj/build/x86_64-unknown-linux-gnu
1072000 ./src

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Oct 30, 2018

@bors r+

@bors

This comment has been minimized.

Contributor

bors commented Oct 30, 2018

📌 Commit a7bea73 has been approved by nikomatsakis

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Oct 30, 2018

@bors r-

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Oct 30, 2018

Wait, some tests are failing...

@wesleywiser

This comment has been minimized.

Member

wesleywiser commented Oct 30, 2018

Yeah, there's a UI test that's still expecting the output on stdout. I'll fix that tonight.

@wesleywiser

This comment has been minimized.

Member

wesleywiser commented Oct 30, 2018

Hmm... I'm not sure what to do with this test.

If I leave the compile-pass flag, the test runner complains because there's output on stderr when the test is supposed to succeed. But if I remove the compile-pass flag, the runner complains because the compilation succeeds.

Thoughts?

@pnkfelix

This comment has been minimized.

Member

pnkfelix commented Oct 31, 2018

Make it a compile fail test and use #[rustc_error] on a new fn at the end?

@wesleywiser

This comment has been minimized.

Member

wesleywiser commented Oct 31, 2018

Hmm.. That doesn't seem to have done anything:

---- [ui] ui/print-fuel/print-fuel.rs stdout ----

error: ui test compiled successfully!
status: exit code: 0
command: "/home/wesley/code/rust/rust3/build/x86_64-unknown-linux-gnu/stage1/bin/rustc" "/home/wesley/code/rust/rust3/src/test/ui/print-fuel/print-fuel.rs" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/home/wesley/code/rust/rust3/build/x86_64-unknown-linux-gnu/test/ui/print-fuel/print-fuel/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/home/wesley/code/rust/rust3/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-Z" "print-fuel=foo" "-L" "/home/wesley/code/rust/rust3/build/x86_64-unknown-linux-gnu/test/ui/print-fuel/print-fuel/auxiliary" "-A" "unused"
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
Fuel used by foo: 3

------------------------------------------

thread '[ui] ui/print-fuel/print-fuel.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3284:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    [ui] ui/print-fuel/print-fuel.rs

Looks like there's also some run-pass tests which are failing as well:

---- [run-pass] run-pass/optimization-fuel-1.rs stdout ----
diff of stderr:

-	optimization-fuel-exhausted: Reorder fields of "S2"
-	


error: failed to delete `/home/wesley/code/rust/rust3/build/x86_64-unknown-linux-gnu/test/run-pass/optimization-fuel-1/optimization-fuel-1.stderr`: No such file or directory (os error 2)
thread '[run-pass] run-pass/optimization-fuel-1.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:2022:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- [run-pass] run-pass/optimization-fuel-0.rs stdout ----
diff of stderr:

-	optimization-fuel-exhausted: Reorder fields of "S1"
-	


error: failed to delete `/home/wesley/code/rust/rust3/build/x86_64-unknown-linux-gnu/test/run-pass/optimization-fuel-0/optimization-fuel-0.stderr`: No such file or directory (os error 2)
thread '[run-pass] run-pass/optimization-fuel-0.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:2022:9


failures:
    [run-pass] run-pass/optimization-fuel-0.rs
    [run-pass] run-pass/optimization-fuel-1.rs

Does the test harness not actually capture stderr or something?

@pnkfelix

This comment has been minimized.

Member

pnkfelix commented Nov 6, 2018

Let me take a look at this

@wesleywiser

This comment has been minimized.

Member

wesleywiser commented Nov 6, 2018

Thanks. If you can't figure it out either, I'll just close this. I don't think it's worth sinking a lot of time into.

@pnkfelix

This comment has been minimized.

Member

pnkfelix commented Nov 6, 2018

Well if nothing else, its making me discover things that I consider bugs. E.g. the fact that the timestamp system used by compiletest does not look at the timestamp (or existence?) of a foo.stderr file; just foo.rs...

@pnkfelix

This comment has been minimized.

Member

pnkfelix commented Nov 6, 2018

Ah, I see the problem.

By default we expect errors to be fed via --error-format json.

The optimization fuel stuff does not seem to be respecting that flag; instead of printing in a JSON format, it just spits to stderr. And then the json parsing subsequently throws that away, yielding an empty string for what it thinks the stderr was.

I think you can get around this by adding an explicit --error-format option to the // compile-flags: ... for each test.

I think I'll probably just push such a commit to this branch in a jiffy.

Add explcit `--error-format` options to tests of print-fuel to sidest…
…ep compiletests presumption of JSON.

As a driveby, rename the stdout files to stderr, which should have been part of the earlier commit.
@wesleywiser

This comment has been minimized.

Member

wesleywiser commented Nov 6, 2018

The optimization fuel stuff does not seem to be respecting that flag; instead of printing in a JSON format, it just spits to stderr. And then the json parsing subsequently throws that away, yielding an empty string for what it thinks the stderr was.

That's probably my fault. Is there a better way to print something to stderr which won't break the parsing?

@pnkfelix

This comment has been minimized.

Member

pnkfelix commented Nov 7, 2018

@bors r=nikomatsakis

@bors

This comment has been minimized.

Contributor

bors commented Nov 7, 2018

📌 Commit 22b571d has been approved by nikomatsakis

kennytm added a commit to kennytm/rust that referenced this pull request Nov 7, 2018

Rollup merge of rust-lang#55495 - wesleywiser:opt_fuel_rustbuild, r=n…
…ikomatsakis

Don't print opt fuel messages to stdout because it breaks Rustbuild

Rustbuild passes `--message-format json` to the compiler invocations
which causes JSON to be emitted on stdout. Printing optimization fuel
messages to stdout breaks the json and causes Rustbuild to fail.

Work around this by emitting optimization fuel related messages on
stderr instead.

bors added a commit that referenced this pull request Nov 7, 2018

Auto merge of #55746 - kennytm:rollup, r=kennytm
Rollup of 15 pull requests

Successful merges:

 - #55377 (Slight copy-editing for `std::cell::Cell` docs)
 - #55441 (Remove unused re import in gdb_rust_pretty_printing)
 - #55453 (Choose predicates without inference variables over those with them)
 - #55495 (Don't print opt fuel messages to stdout because it breaks Rustbuild)
 - #55501 (Make `process_obligations`' computation of `completed` optional.)
 - #55510 (Fix feature gate only being checked on first repr attr.)
 - #55609 (Run name-anon-globals after LTO passes as well)
 - #55688 (Standardised names and location of ui issue tests)
 - #55692 (-C remark: fix incorrect warning about requiring "--debuginfo" instead of "-C debuginfo=n")
 - #55702 (Add `aarch64-pc-windows-msvc` to deployed targets)
 - #55717 (Bubble up an overflow error so that rustdoc can ignore it)
 - #55719 (Sidestep link error from rustfix'ed code by using a *defined* static.)
 - #55728 (Update lldb)
 - #55730 (Use trait impl method span when type param mismatch is due to impl Trait)
 - #55734 (refactor: use shorthand fields)

Failed merges:

r? @ghost
@eddyb

This comment has been minimized.

Member

eddyb commented Nov 7, 2018

Hmm, are there more like these? Does print-type-sizes use stderr?

@wesleywiser

This comment has been minimized.

Member

wesleywiser commented Nov 7, 2018

@eddyb print-type-sizes uses stdout

pub fn print_type_sizes(&self) {

@eddyb

This comment has been minimized.

Member

eddyb commented Nov 7, 2018

@wesleywiser I think that should get changed too, then.
Probably every *.stdout in src/test should be investigated.
(Feel free to open an issue and leave this PR as-is)

bors added a commit that referenced this pull request Nov 7, 2018

Auto merge of #55746 - kennytm:rollup, r=kennytm
Rollup of 14 pull requests

Successful merges:

 - #55377 (Slight copy-editing for `std::cell::Cell` docs)
 - #55441 (Remove unused re import in gdb_rust_pretty_printing)
 - #55453 (Choose predicates without inference variables over those with them)
 - #55495 (Don't print opt fuel messages to stdout because it breaks Rustbuild)
 - #55501 (Make `process_obligations`' computation of `completed` optional.)
 - #55510 (Fix feature gate only being checked on first repr attr.)
 - #55609 (Run name-anon-globals after LTO passes as well)
 - #55645 (do not print wrapping ranges like normal ranges in validity diagnostics)
 - #55688 (Standardised names and location of ui issue tests)
 - #55692 (-C remark: fix incorrect warning about requiring "--debuginfo" instead of "-C debuginfo=n")
 - #55702 (Add `aarch64-pc-windows-msvc` to deployed targets)
 - #55728 (Update lldb)
 - #55730 (Use trait impl method span when type param mismatch is due to impl Trait)
 - #55734 (refactor: use shorthand fields)

@bors bors merged commit 22b571d into rust-lang:master Nov 7, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

alexreg added a commit to alexreg/rust that referenced this pull request Nov 7, 2018

Auto merge of rust-lang#55746 - kennytm:rollup, r=kennytm
Rollup of 14 pull requests

Successful merges:

 - rust-lang#55377 (Slight copy-editing for `std::cell::Cell` docs)
 - rust-lang#55441 (Remove unused re import in gdb_rust_pretty_printing)
 - rust-lang#55453 (Choose predicates without inference variables over those with them)
 - rust-lang#55495 (Don't print opt fuel messages to stdout because it breaks Rustbuild)
 - rust-lang#55501 (Make `process_obligations`' computation of `completed` optional.)
 - rust-lang#55510 (Fix feature gate only being checked on first repr attr.)
 - rust-lang#55609 (Run name-anon-globals after LTO passes as well)
 - rust-lang#55645 (do not print wrapping ranges like normal ranges in validity diagnostics)
 - rust-lang#55688 (Standardised names and location of ui issue tests)
 - rust-lang#55692 (-C remark: fix incorrect warning about requiring "--debuginfo" instead of "-C debuginfo=n")
 - rust-lang#55702 (Add `aarch64-pc-windows-msvc` to deployed targets)
 - rust-lang#55728 (Update lldb)
 - rust-lang#55730 (Use trait impl method span when type param mismatch is due to impl Trait)
 - rust-lang#55734 (refactor: use shorthand fields)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment