From 507b43b94cd2afc2adcc35bd0289865f070b9ee2 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Sat, 26 Aug 2023 22:30:19 +0200 Subject: [PATCH] adb: Use `uid` to limit `logcat` to the current application (#131) Having never really understood how Android Studio does it, I just stumbled upon this very new [stackoverflow answer] that has a rather beatiful solution to the current problems with `pidof`, without drawbacks. Pidof has always been flaky as it relies on the app to be running, which may either take some time or never happen if the app crashed before `pidof` is first run. This results in silly workarounds such as loops that induce extra delay and need to have an upper bound. And this `pid` changes every time the app is restarted, making it a tedious process that also doesn't react to manual app restarts on the device. Retrieving the `uid` via `pm list packages -U` on the other hand, and passing that to `logcat --uid` has the following advantages: - Always available immediately after the app has been installed, no need to check it in a loop (no extra delay); - Doesn't change after the app is (re!)installed, unless the user fully deletes and installs the app again; - Is resilient against app crashes because of that, and allows the user to see any error/crash related messages straight away; - Still includes logs printed by other system components that run or are invoked within an app, as before. [stackoverflow answer]: https://stackoverflow.com/a/76551835 --- xbuild/src/devices/adb.rs | 47 +++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/xbuild/src/devices/adb.rs b/xbuild/src/devices/adb.rs index 6179aab3..409a1e5e 100644 --- a/xbuild/src/devices/adb.rs +++ b/xbuild/src/devices/adb.rs @@ -1,7 +1,7 @@ use crate::config::AndroidDebugConfig; use crate::devices::{Backend, Device}; use crate::{Arch, Platform}; -use anyhow::Result; +use anyhow::{Context, Result}; use apk::Apk; use std::io::{BufRead, BufReader}; use std::path::{Path, PathBuf}; @@ -206,32 +206,35 @@ impl Adb { Ok(line[..18].to_string()) } - fn pidof(&self, device: &str, id: &str) -> Result { - loop { - let output = self.shell(device, None).arg("pidof").arg(id).output()?; - anyhow::ensure!( - output.status.success(), - "failed to get pid: {}", - std::str::from_utf8(&output.stderr)?.trim() - ); - let pid = std::str::from_utf8(&output.stdout)?.trim(); - // may return multiple space separated pids if the old process hasn't exited yet. - if pid.is_empty() || pid.contains(' ') { - std::thread::sleep(std::time::Duration::from_millis(100)); - continue; - } - println!("pid of {} is {}", id, pid); - return Ok(pid.parse()?); - } + fn uidof(&self, device: &str, id: &str) -> Result { + let output = self + .shell(device, None) + .arg("pm") + .arg("list") + .arg("package") + .arg("-U") + .arg(id) + .output()?; + anyhow::ensure!( + output.status.success(), + "failed to get uid: {}", + std::str::from_utf8(&output.stderr)?.trim() + ); + let output = std::str::from_utf8(&output.stdout)?; + let uid = output + .split_whitespace() + .find_map(|kv| kv.strip_prefix("uid:")) + .with_context(|| format!("Could not find `uid:`` in output `{output}`"))?; + Ok(uid.parse()?) } - fn logcat(&self, device: &str, pid: u32, last_timestamp: &str) -> Result { + fn logcat(&self, device: &str, uid: u32, last_timestamp: &str) -> Result { let child = self .shell(device, None) .arg("logcat") .arg("-T") .arg(format!("'{}'", last_timestamp)) - .arg(format!("--pid={}", pid)) + .arg(format!("--uid={}", uid)) .stdin(Stdio::null()) .stdout(Stdio::piped()) .spawn()?; @@ -343,8 +346,8 @@ impl Adb { self.forward_reverse(device, debug_config)?; let last_timestamp = self.logcat_last_timestamp(device)?; self.start(device, package, activity)?; - let pid = self.pidof(device, package)?; - let logcat = self.logcat(device, pid, &last_timestamp)?; + let uid = self.uidof(device, package)?; + let logcat = self.logcat(device, uid, &last_timestamp)?; for line in logcat { println!("{}", line); }