Skip to content
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

ndk-build: Return PID after launching the app #331

Merged
merged 1 commit into from
Sep 7, 2022

Conversation

Jasper-Bekkers
Copy link
Contributor

Depends on #329 to apply cleanly.

Shows the process PID after launching. Requires -W as an am start arg to wait for process completion to prevent a race condition. Having the PID is useful in a lot of cases, such as filtering logs with adb logcat --pid=...

.arg("am")
.arg("start")
.arg("-W")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's neat, I was wondering how this was implemented by @dvc94ch in x, turns out to be a sleep-loop: https://github.com/cloudpeers/xbuild/blob/e0ebb50bfa78cfad61931bb30c70a624b85fdddd/xbuild/src/devices/adb.rs#L175-L186

Copy link
Member

@MarijnS95 MarijnS95 Sep 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably semi-smart to keep a loop so that duplicate PIDs for yet-to-exit app instances are not causing issues:

https://github.com/cloudpeers/xbuild/blob/e58cc09b4f2cb07c1fa0591f9d65cea435dd5646/xbuild/src/devices/adb.rs#L179

But IMO am start should be able to emit the pid instead of this hackery. Android Studio must also have a smart yet concise way of dealing with this, that isn't error-prone. Perhaps just outputting all the PIDs (--pid is probably a coma-separated list or can be stacked), and we can combine that with a last timestamp filter.

@MarijnS95
Copy link
Member

Rebased now that #329 is in, and dropped #330 from it to be merged separately.

Copy link
Member

@MarijnS95 MarijnS95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the pid thing is good enough for now. Was seeing duplicate PIDs because one of my gdb sessions got stuck and the processes couldn't get killed and detached even under root.

If we ever observe problems with it we'll make it more fallible or change it altogether.

@MarijnS95 MarijnS95 changed the title Output the PID after launching the process ndk-build: Return PID after launching the process Sep 7, 2022
@MarijnS95 MarijnS95 changed the title ndk-build: Return PID after launching the process ndk-build: Return PID after launching the app Sep 7, 2022
@MarijnS95 MarijnS95 merged commit f1e0089 into rust-mobile:master Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants