sonos / dinghy

Easier cross-compilation for phones and single boards computers
Other
367 stars 44 forks source link

Change IosSimDevice to use xcrun simctl launch rather than lldb #96

Closed simlay closed 4 years ago

simlay commented 4 years ago

Given that https://bugs.llvm.org/show_bug.cgi?id=36580 seems to be a bug in lldb, I figure it's worth having a work around using xcrun simctl launch

I've spent a bunch of time trying to get the exit status of the app to actually propagate but I'm not sure how to make simctl do it. I don't have a machine with Xcode 8 to see if it propagated before but, I think it's a step forward to just have the test outputs running and being displayed.

Sample output:

$ cargo dinghy --platform auto-ios-x86_64 test
 INFO  cargo_dinghy > Targeting platform 'auto-ios-x86_64' and device '078869FD-7B1E-4BB2-AE23-8FBDDF952715'
    Finished dev [unoptimized + debuginfo] target(s) in 0.03s
Dinghy: 19151

running 4 tests
test tests::pass::it_works ... ok
test tests::fails::it_fails ... FAILED
test tests::pass::it_finds_source_files ... ok
test tests::pass::it_finds_test_data_files ... ok

failures:

---- tests::fails::it_fails stdout ----
thread 'tests::fails::it_fails' panicked at 'Failing as expected', test-app/src/lib.rs:56:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

failures:
    tests::fails::it_fails

test result: FAILED. 3 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out

You'll notice Dinghy: 19151 is in the output. To remove that would need to use a tempfile and just redirect stdout to it. Really not a hard change but I just didn't feel like adding a new dependency.

Thoughts?

kali commented 4 years ago

Hey !

Thanks for making this work again. iOS have a bit slipped out of or scope and did not get the love it deserve. This is great progress.

If it's not too complicated, I still think we should try to propagate the exit status, because that's ultimately what you want to use dinghy in CI. It was working when we were able to launch through lldb. We have struggled a bit with android on that side, as adb was not propagating it either.

I don't mind too much about adding a dep... this is a developer side project, and we have strategies to deploy pre-built binaries on CI, so size or build time does not matter too much imho.

simlay commented 4 years ago

After much pain I'm excited to say that it works. Here's why I did it the way I did.

I tried all of the following:

I think there might have been another few ways I tried.

Thoughts?

simlay commented 4 years ago

Oh and here's what the output looks like:

$ ../target/debug/cargo-dinghy -d 078869FD-7B1E-4BB2-AE23-8FBDDF952715 test pass
 INFO  cargo_dinghy > Targeting platform 'auto-ios-x86_64' and device '078869FD-7B1E-4BB2-AE23-8FBDDF952715'
    Finished dev [unoptimized + debuginfo] target(s) in 0.10s

running 3 tests
test tests::pass::it_finds_source_files ... ok
test tests::pass::it_works ... ok
test tests::pass::it_finds_test_data_files ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out
$ ../target/debug/cargo-dinghy -d 078869FD-7B1E-4BB2-AE23-8FBDDF952715 test fail
 INFO  cargo_dinghy > Targeting platform 'auto-ios-x86_64' and device '078869FD-7B1E-4BB2-AE23-8FBDDF952715'
    Finished dev [unoptimized + debuginfo] target(s) in 0.10s

running 1 test
test tests::fails::it_fails ... FAILED

failures:

---- tests::fails::it_fails stdout ----
thread 'tests::fails::it_fails' panicked at 'Failing as expected', test-app/src/lib.rs:56:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

failures:
    tests::fails::it_fails

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3 filtered out

thread 'main' panicked at 'Non-zero exit code from lldb: 101', dinghy-lib/src/ios/device.rs:750:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
kali commented 4 years ago

Well, lldb and its ios extensions has always been incredibly painful. I can't say I'm incredibly happy about what you had to do to get there, but all this project is like that, slaloming between bugs and finding ugly workarounds. It is awesome that you found a way to fix this. I'll merge it tomorrow if travis is happy.

kali commented 4 years ago

@fredszaq @Deluvi any last words before i merge this ?

kali commented 4 years ago

Thanks for this contribution, release done.