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

devices/adb: Find adb executable in $ANDROID_HOME/$ANDROID_SDK_ROOT #116

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 39 additions & 11 deletions xbuild/src/command/doctor.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use anyhow::Result;
use anyhow::{bail, Result};
use std::path::PathBuf;
use std::process::Command;

#[derive(Clone, Debug)]
use crate::devices::adb::Adb;

#[derive(Debug)]
pub struct Doctor {
groups: Vec<Group>,
}
Expand Down Expand Up @@ -35,7 +37,11 @@ impl Default for Doctor {
Group {
name: "android",
checks: vec![
Check::new("adb", Some(VersionCheck::new("--version", 0, 4))),
Check::with_path(
"adb",
Adb::which(),
Some(VersionCheck::new("--version", 0, 4)),
),
Check::new("javac", Some(VersionCheck::new("--version", 0, 1))),
Check::new("java", Some(VersionCheck::new("--version", 0, 1))),
Check::new("kotlin", Some(VersionCheck::new("-version", 0, 2))),
Expand Down Expand Up @@ -77,7 +83,7 @@ impl std::fmt::Display for Doctor {
}
}

#[derive(Clone, Debug)]
#[derive(Debug)]
struct Group {
name: &'static str,
checks: Vec<Check>,
Expand Down Expand Up @@ -105,15 +111,32 @@ impl std::fmt::Display for Group {
}
}

#[derive(Clone, Copy, Debug)]
#[derive(Debug)]
struct Check {
name: &'static str,
location: Option<Result<PathBuf>>,
version: Option<VersionCheck>,
}

impl Check {
pub const fn new(name: &'static str, version: Option<VersionCheck>) -> Self {
Self { name, version }
Self {
name,
location: None,
version,
}
}

pub const fn with_path(
name: &'static str,
path: Result<PathBuf>,
version: Option<VersionCheck>,
) -> Self {
Self {
name,
location: Some(path),
version,
}
}
}

Expand All @@ -131,17 +154,22 @@ impl VersionCheck {
}

impl Check {
fn name(self) -> &'static str {
fn name(&self) -> &'static str {
self.name
}

fn path(self) -> Result<PathBuf> {
Ok(which::which(self.name)?)
fn path(&self) -> Result<PathBuf> {
Ok(match &self.location {
Some(Ok(path)) => path.clone(),
// Cannot clone the error:
Some(Err(e)) => bail!("{:?}", e),
None => which::which(self.name)?,
})
}

fn version(self) -> Result<Option<String>> {
fn version(&self) -> Result<Option<String>> {
if let Some(version) = self.version {
let output = Command::new(self.name)
let output = Command::new(self.path()?)
.args(version.arg.split(' '))
.output()?;
anyhow::ensure!(output.status.success(), "failed to run {}", self.name);
Expand Down
40 changes: 38 additions & 2 deletions xbuild/src/devices/adb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,44 @@ use std::time::Duration;
pub(crate) struct Adb(PathBuf);

impl Adb {
pub fn which() -> Result<Self> {
Ok(Self(which::which(exe!("adb"))?))
pub fn which() -> Result<PathBuf> {
const ADB: &str = exe!("adb");

match which::which(ADB) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I thought for a second about using which::which_in_global(ADB, paths) but that requires paths to be a single concatenated string of env("PATH"):env("ANDROID_HOME") in the event that these env vars exist, and that overall looks more messy without including any context about PATH vs ANDROID_HOME vs ANDROID_NDK_ROOT providing this exe.

Err(which::Error::CannotFindBinaryPath) => {
let sdk_path = {
let sdk_path = std::env::var("ANDROID_SDK_ROOT").ok();
if sdk_path.is_some() {
eprintln!(
"Warning: Environment variable ANDROID_SDK_ROOT is deprecated \
(https://developer.android.com/studio/command-line/variables#envar). \
It will be used until it is unset and replaced by ANDROID_HOME."
);
}

PathBuf::from(
sdk_path
.or_else(|| std::env::var("ANDROID_HOME").ok())
.context(
"Cannot find `adb` on in PATH nor is ANDROID_HOME/ANDROID_SDK_ROOT set",
)?,
)
};

let adb_path = sdk_path.join("platform-tools").join(ADB);
anyhow::ensure!(
adb_path.exists(),
"Expected `adb` at `{}`",
adb_path.display()
);
Ok(adb_path)
}
r => r.context("Could not find `adb` in PATH"),
}
}

pub fn new() -> Result<Self> {
Ok(Self(Self::which()?))
}

fn adb(&self, device: &str) -> Command {
Expand Down
10 changes: 5 additions & 5 deletions xbuild/src/devices/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use crate::{Arch, BuildEnv, Platform};
use anyhow::Result;
use std::path::Path;

mod adb;
mod host;
mod imd;
pub(crate) mod adb;
pub(crate) mod host;
pub(crate) mod imd;

#[derive(Clone, Debug)]
enum Backend {
Expand All @@ -31,7 +31,7 @@ impl std::str::FromStr for Device {
}
if let Some((backend, id)) = device.split_once(':') {
let backend = match backend {
"adb" => Backend::Adb(Adb::which()?),
"adb" => Backend::Adb(Adb::new()?),
"imd" => Backend::Imd(IMobileDevice::which()?),
_ => anyhow::bail!("unsupported backend {}", backend),
};
Expand All @@ -58,7 +58,7 @@ impl std::fmt::Display for Device {
impl Device {
pub fn list() -> Result<Vec<Self>> {
let mut devices = vec![Self::host()];
if let Ok(adb) = Adb::which() {
if let Ok(adb) = Adb::new() {
adb.devices(&mut devices)?;
}
if let Ok(imd) = IMobileDevice::which() {
Expand Down