Use std::process::Command throughout compile-test #43907

euclio · 2017-08-16T16:53:45Z

Resubmission of #43798.

Fixes #43762.

r? @alexcrichton

alexcrichton · 2017-08-16T16:54:27Z

@bors: r+

bors · 2017-08-16T16:54:28Z

📌 Commit 88bac1f has been approved by alexcrichton

bors · 2017-08-18T13:32:58Z

⌛ Testing commit 88bac1f with merge 8ba7852589b76b9c92d29a09f90777d9b7869cf2...

bors · 2017-08-18T14:52:13Z

💔 Test failed - status-travis

kennytm · 2017-08-18T15:40:53Z

All GDB tests on android-arm failed with "Can't bind address: Address already in use". Possibly related to this PR since the testing code is modified.

Check compiletest suite=debuginfo mode=debuginfo-gdb (x86_64-unknown-linux-gnu -> arm-linux-androideabi)
[00:58:24] arm-linux-androideabi debug-info test uses tcp 5039 port.please reserve it
[00:58:24] 
[00:58:24] running 108 tests
[00:58:24] test [debuginfo-gdb] debuginfo-gdb/associated-types.rs ... [100%] /data/tmp/work/associated-types.stage2-arm-linux-androideabi
[00:58:24] /checkout/obj/build/x86_64-unknown-linux-gnu/test/debuginfo/associated-types.stage2-arm-linux-androideabi: 1 file pushed. 1.8 MB/s (18292 bytes in 0.010s)
[00:58:24] FAILED
[00:58:24] test [debuginfo-gdb] debuginfo-gdb/basic-types-globals-metadata.rs ... Process /data/tmp/work/associated-types.stage2-arm-linux-androideabi created; pid = 31093
[00:58:24] Listening on port 5039
[00:58:24] [100%] /data/tmp/work/basic-types-globals-metadata.stage2-arm-linux-androideabi
[00:58:24] /checkout/obj/build/x86_64-unknown-linux-gnu/test/debuginfo/basic-types-globals-metadata.stage2-arm-linux-androideabi: 1 file pushed. 1.5 MB/s (12668 bytes in 0.008s)
[00:58:24] FAILED
[00:58:24] test [debuginfo-gdb] debuginfo-gdb/basic-types-globals.rs ... Process /data/tmp/work/basic-types-globals-metadata.stage2-arm-linux-androideabi created; pid = 31098
[00:58:24] Can't bind address: Address already in use.
[00:58:24] Killing process(es): 31098
[00:58:24] [100%] /data/tmp/work/basic-types-globals.stage2-arm-linux-androideabi
[00:58:24] /checkout/obj/build/x86_64-unknown-linux-gnu/test/debuginfo/basic-types-globals.stage2-arm-linux-androideabi: 1 file pushed. 2.0 MB/s (12332 bytes in 0.006s)
[00:58:24] FAILED
<snip>
[00:58:53] test [debuginfo-gdb] debuginfo-gdb/vec.rs ... Process /data/tmp/work/vec-slices.stage2-arm-linux-androideabi created; pid = 31573
[00:58:53] Can't bind address: Address already in use.
[00:58:53] Killing process(es): 31573
[00:58:53] [100%] /data/tmp/work/vec.stage2-arm-linux-androideabi
[00:58:53] /checkout/obj/build/x86_64-unknown-linux-gnu/test/debuginfo/vec.stage2-arm-linux-androideabi: 1 file pushed. 1.6 MB/s (9412 bytes in 0.005s)
[00:58:53] FAILED
[00:58:53] 
[00:58:53] failures:
[00:58:53] 
[00:58:53] ---- [debuginfo-gdb] debuginfo-gdb/associated-types.rs stdout ----
[00:58:53] 	NOTE: compiletest thinks it is using GDB without native rust support
[00:58:53] thread '[debuginfo-gdb] debuginfo-gdb/associated-types.rs' panicked at 'called `Option::unwrap()` on a `None` value', /checkout/src/libcore/option.rs:335:20
[00:58:53] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:58:53] 
[00:58:53] ---- [debuginfo-gdb] debuginfo-gdb/basic-types-globals-metadata.rs stdout ----
[00:58:53] 	NOTE: compiletest thinks it is using GDB without native rust support
[00:58:53] thread '[debuginfo-gdb] debuginfo-gdb/basic-types-globals-metadata.rs' panicked at 'called `Option::unwrap()` on a `None` value', /checkout/src/libcore/option.rs:335:20
[00:58:53] 
<snip>
[00:58:53] ---- [debuginfo-gdb] debuginfo-gdb/vec.rs stdout ----
[00:58:53] 	NOTE: compiletest thinks it is using GDB without native rust support
[00:58:53] thread '[debuginfo-gdb] debuginfo-gdb/vec.rs' panicked at 'called `Option::unwrap()` on a `None` value', /checkout/src/libcore/option.rs:335:20
[00:58:53] 
[00:58:53] 
[00:58:53] failures:
[00:58:53]     [debuginfo-gdb] debuginfo-gdb/associated-types.rs
[00:58:53]     [debuginfo-gdb] debuginfo-gdb/basic-types-globals-metadata.rs
<snip>
[00:58:53]     [debuginfo-gdb] debuginfo-gdb/vec.rs
[00:58:53] 
[00:58:53] test result: FAILED. 0 passed; 98 failed; 10 ignored; 0 measured; 0 filtered out

euclio · 2017-08-19T17:25:42Z

Hm, looking at the GDB and ADB code, I don't see anything glaringly different.

Mark-Simulacrum · 2017-08-20T16:35:18Z

src/tools/compiletest/src/runtest.rs

+                test_client
+                    .args(&["run", &prog])
+                    .args(args)
+                    .envs(env.clone());


hm, I think passing &env here (and in other calls to envs) should work, but I'm not certain.

This won't work because envs requires the Item to be a tuple, not a tuple reference.

Mark-Simulacrum · 2017-08-20T16:37:25Z

Yeah, not clear to me why this would have failed CI. Could you try running locally and seeing if you can reproduce? ./src/ci/docker/run.sh arm-android.

alexcrichton · 2017-08-20T18:43:36Z

I think that the stdout for the spawned adb shell process isn't captured like it was before, which I think can cause problems like this in this situation.

euclio · 2017-08-21T05:32:27Z

Thanks for the hints. I am on vacation through Tuesday but I'll take a look when I can.

…

On Sun, Aug 20, 2017, 1:43 PM Alex Crichton ***@***.***> wrote: I think that the stdout for the spawned adb shell process isn't captured like it was before, which I think can cause problems like this in this situation. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#43907 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABTxFp2MrzGH1tehPUJAttylqpHdYRqSks5saH5sgaJpZM4O5Mar> .

euclio · 2017-08-22T15:17:01Z

I don't have a computer powerful enough to run the android tests locally for now, but I pushed a commit that reattaches the stdout and stderr for adb shell.

Mark-Simulacrum · 2017-08-22T17:23:01Z

You can temporarily add ALLOW_PR=1 to the android builder so that it runs on Travis and we can get feedback without going through bors.

alexcrichton · 2017-08-22T19:10:30Z

Er the stdout here should be Stdio::piped where the stderr is Stdio::inherit, we try to capture the stdout just below the call to spawn.

euclio · 2017-08-23T21:33:54Z

@alexcrichton Fixed.

alexcrichton · 2017-08-23T22:12:10Z

@bors: r+

Let's see what happens!

bors · 2017-08-23T22:12:11Z

📌 Commit 91bfe3f has been approved by alexcrichton

bors · 2017-08-24T10:02:25Z

⌛ Testing commit 91bfe3f with merge a125ec9...

@alexcrichton

Use std::process::Command throughout compile-test Resubmission of #43798. Fixes #43762. r? @alexcrichton

bors · 2017-08-24T12:43:08Z

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

euclio added 7 commits August 16, 2017 11:45

replace procsrv functions with Command

6601ffc

let compose_and_run take a Command

e385bc5

remove working_dir argument

0a666f8

make compose_and_run_compiler take Command

90a87be

return Command from make_compile_args

04dee08

remove extra_args argument

6f21239

remove procsrv module

88bac1f

rust-highfive assigned alexcrichton Aug 16, 2017

aidanhs added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 16, 2017

Mark-Simulacrum reviewed Aug 20, 2017

View reviewed changes

remove needless clone

02e95e5

arielb1 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 22, 2017

capture adb shell stdout

91bfe3f

euclio force-pushed the command branch from 95c4f26 to 91bfe3f Compare August 23, 2017 21:32

bors added a commit that referenced this pull request Aug 24, 2017

Auto merge of #43907 - euclio:command, r=alexcrichton

a125ec9

Use std::process::Command throughout compile-test Resubmission of #43798. Fixes #43762. r? @alexcrichton

bors merged commit 91bfe3f into rust-lang:master Aug 24, 2017

euclio deleted the command branch February 7, 2019 18:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use std::process::Command throughout compile-test #43907

Use std::process::Command throughout compile-test #43907

euclio commented Aug 16, 2017

alexcrichton commented Aug 16, 2017

bors commented Aug 16, 2017

bors commented Aug 18, 2017

bors commented Aug 18, 2017

kennytm commented Aug 18, 2017 •

edited

Loading

euclio commented Aug 19, 2017

Mark-Simulacrum Aug 20, 2017

euclio Aug 22, 2017

Mark-Simulacrum commented Aug 20, 2017

alexcrichton commented Aug 20, 2017

euclio commented Aug 21, 2017 via email

euclio commented Aug 22, 2017

Mark-Simulacrum commented Aug 22, 2017

alexcrichton commented Aug 22, 2017

euclio commented Aug 23, 2017

alexcrichton commented Aug 23, 2017

bors commented Aug 23, 2017

bors commented Aug 24, 2017

bors commented Aug 24, 2017

Use std::process::Command throughout compile-test #43907

Use std::process::Command throughout compile-test #43907

Conversation

euclio commented Aug 16, 2017

alexcrichton commented Aug 16, 2017

bors commented Aug 16, 2017

bors commented Aug 18, 2017

bors commented Aug 18, 2017

kennytm commented Aug 18, 2017 • edited Loading

euclio commented Aug 19, 2017

Mark-Simulacrum Aug 20, 2017

Choose a reason for hiding this comment

euclio Aug 22, 2017

Choose a reason for hiding this comment

Mark-Simulacrum commented Aug 20, 2017

alexcrichton commented Aug 20, 2017

euclio commented Aug 21, 2017 via email

euclio commented Aug 22, 2017

Mark-Simulacrum commented Aug 22, 2017

alexcrichton commented Aug 22, 2017

euclio commented Aug 23, 2017

alexcrichton commented Aug 23, 2017

bors commented Aug 23, 2017

bors commented Aug 24, 2017

bors commented Aug 24, 2017

kennytm commented Aug 18, 2017 •

edited

Loading