-
-
Notifications
You must be signed in to change notification settings - Fork 42
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
base: master
Are you sure you want to change the base?
Conversation
…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
pub fn which() -> Result<PathBuf> { | ||
const ADB: &str = exe!("adb"); | ||
|
||
match which::which(ADB) { |
There was a problem hiding this comment.
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.
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. |
I've been told by my colleagues that there's no |
…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
looks like this issue is being tracked here: microsoft/winget-pkgs#4082 |
would be cool to track which packages are missing in winget that |
Adding |
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
3b9a745
to
6d068c4
Compare
xbuild/src/command/doctor.rs
Outdated
if let Some(line) = output.split('\n').nth(version.row as _) { | ||
if let Some(line) = output.lines().nth(version.row as _) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
6d068c4
to
8fd72e5
Compare
`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.
8fd72e5
to
2c5f25c
Compare
adb
is not always available onPATH
, sometimes it is installed only via the SDK. Make sure we find it there too - after checkingPATH
- via well-known SDK variables.I tried but thus far failed to easily add
Adb::which()
(Result
) tox doctor
, so that the result is accurate.