devices/adb: Find adb executable in `$ANDROID_HOME`/`$ANDROID_SDK_ROOT` by MarijnS95 · Pull Request #116 · rust-mobile/xbuild

MarijnS95 · 2023-05-03T18:16:33Z

adb is not always available on PATH, sometimes it is installed only via the SDK. Make sure we find it there too - after checking PATH - via well-known SDK variables.

I tried but thus far failed to easily add Adb::which() (Result) to x doctor, so that the result is accurate.

…nment Environment variables can be set and optionally override the process environment through `.cargo/config.toml`'s `[env]` section: https://doc.rust-lang.org/cargo/reference/config.html#env These config variables have specific precedence rules with regards to overriding the environment set in the process, and can optionally represent paths relative to the parent of the containing `.cargo/` folder. Besides exposing variables to all other processes called by `xbuild`, this also allows `xbuild` itself to be driven by variables set in `.cargo/config.toml`, such as `$ANDROID_HOME` needed for #116. rust-mobile/cargo-subcommand#12 rust-mobile/cargo-subcommand#16

MarijnS95 · 2023-05-03T19:05:26Z

xbuild/src/devices/adb.rs

+    pub fn which() -> Result<PathBuf> {
+        const ADB: &str = exe!("adb");
+
+        match which::which(ADB) {


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.

dvc94ch · 2023-05-03T20:00:10Z

I'm not a big fan of adding a bunch of hacks for Android. Having adb in the path seems like a reasonable assumption. Package managers on Linux, macos, windows easily let you install adb and add it to path.

MarijnS95 · 2023-05-03T20:02:39Z

I've been told by my colleagues that there's no adb via winget yet.

…nment Environment variables can be set and optionally override the process environment through `.cargo/config.toml`'s `[env]` section: https://doc.rust-lang.org/cargo/reference/config.html#env These config variables have specific precedence rules with regards to overriding the environment set in the process, and can optionally represent paths relative to the parent of the containing `.cargo/` folder. Besides exposing variables to all other processes called by `xbuild`, this also allows `xbuild` itself to be driven by variables set in `.cargo/config.toml`, such as `$ANDROID_HOME` needed for #116. rust-mobile/cargo-subcommand#12 rust-mobile/cargo-subcommand#16

dvc94ch · 2023-08-15T13:07:53Z

looks like this issue is being tracked here: microsoft/winget-pkgs#4082

dvc94ch · 2023-08-15T13:10:21Z

would be cool to track which packages are missing in winget that x doctor requires. Maybe contributing to winget pkgs might be the best option.

MarijnS95 · 2023-08-15T13:27:13Z

Adding xbuild as a winget buildable package with proper dependencies?

dvc94ch · 2023-08-15T16:47:23Z

Once all the dependencies are on Winget, we could maintain a Winget/brew package. Probably not worth it for Linux as everyone knows what they're doing and use their own package manager

…nment Environment variables can be set and optionally override the process environment through `.cargo/config.toml`'s `[env]` section: https://doc.rust-lang.org/cargo/reference/config.html#env These config variables have specific precedence rules with regards to overriding the environment set in the process, and can optionally represent paths relative to the parent of the containing `.cargo/` folder. Besides exposing variables to all other processes called by `xbuild`, this also allows `xbuild` itself to be driven by variables set in `.cargo/config.toml`, such as `$ANDROID_HOME` needed for #116. rust-mobile/cargo-subcommand#12 rust-mobile/cargo-subcommand#16

MarijnS95 · 2023-08-17T15:53:31Z

xbuild/src/command/doctor.rs

-            if let Some(line) = output.split('\n').nth(version.row as _) {
+            if let Some(line) = output.lines().nth(version.row as _) {


Drive-by fix (that I can PR separately): on Windows adb --version returns \r\n, so the \r remains in the version() string and clears the line (adb 1.0.41\r) that was printed before it, leaving only path() 🤭

…nment (#117) * Propagate `.cargo/config.toml` `[env]` settings to the process environment Environment variables can be set and optionally override the process environment through `.cargo/config.toml`'s `[env]` section: https://doc.rust-lang.org/cargo/reference/config.html#env These config variables have specific precedence rules with regards to overriding the environment set in the process, and can optionally represent paths relative to the parent of the containing `.cargo/` folder. Besides exposing variables to all other processes called by `xbuild`, this also allows `xbuild` itself to be driven by variables set in `.cargo/config.toml`, such as `$ANDROID_HOME` needed for #116. rust-mobile/cargo-subcommand#12 rust-mobile/cargo-subcommand#16 * cargo/config: Don't canonicalize joined `[env]` `relative` paths Cargo doesn't do this either, and canonicalization requires the path to exist which it does not have to.

`adb` is not always available on `PATH`, sometimes it is installed only via the SDK. Make sure we find it there too - after checking `PATH` - via well-known SDK variables.

MarijnS95 mentioned this pull request May 3, 2023

Propagate .cargo/config.toml [env] settings to the process environment #117

Merged

MarijnS95 commented May 3, 2023

View reviewed changes

MarijnS95 force-pushed the adb-from-sdk branch from 3b9a745 to 6d068c4 Compare August 17, 2023 15:49

MarijnS95 commented Aug 17, 2023

View reviewed changes

MarijnS95 mentioned this pull request Aug 26, 2023

doctor: Split version str with .lines() to account for Windows' \r\n #130

Merged

MarijnS95 force-pushed the adb-from-sdk branch from 6d068c4 to 8fd72e5 Compare August 26, 2023 15:15

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

2c5f25c

`adb` is not always available on `PATH`, sometimes it is installed only via the SDK. Make sure we find it there too - after checking `PATH` - via well-known SDK variables.

MarijnS95 force-pushed the adb-from-sdk branch from 8fd72e5 to 2c5f25c Compare August 26, 2023 20:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

devices/adb: Find adb executable in `$ANDROID_HOME`/`$ANDROID_SDK_ROOT`#116
MarijnS95 wants to merge 1 commit intomasterfrom
adb-from-sdk

MarijnS95 commented May 3, 2023

Uh oh!

MarijnS95 May 3, 2023

Uh oh!

dvc94ch commented May 3, 2023

Uh oh!

MarijnS95 commented May 3, 2023

Uh oh!

dvc94ch commented Aug 15, 2023

Uh oh!

dvc94ch commented Aug 15, 2023

Uh oh!

MarijnS95 commented Aug 15, 2023

Uh oh!

dvc94ch commented Aug 15, 2023

Uh oh!

MarijnS95 Aug 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		if let Some(line) = output.split('\n').nth(version.row as _) {
		if let Some(line) = output.lines().nth(version.row as _) {

Uh oh!

Conversation

MarijnS95 commented May 3, 2023

Uh oh!

MarijnS95 May 3, 2023

Choose a reason for hiding this comment

Uh oh!

dvc94ch commented May 3, 2023

Uh oh!

MarijnS95 commented May 3, 2023

Uh oh!

dvc94ch commented Aug 15, 2023

Uh oh!

dvc94ch commented Aug 15, 2023

Uh oh!

MarijnS95 commented Aug 15, 2023

Uh oh!

dvc94ch commented Aug 15, 2023

Uh oh!

MarijnS95 Aug 17, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants