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

Embed / pre-download adb and LLVM (or don't strip the NDK as harshly) #124

Open
MarijnS95 opened this issue Aug 3, 2023 · 4 comments
Open

Comments

@MarijnS95
Copy link
Member

MarijnS95 commented Aug 3, 2023

As previously established xbuild really likes to run on a development machine where tools like adb and LLVM/clang are provided by the host (already installed).

This is easier to expect or satisfy on Linux machines than Windows, where setup takes many more steps. Especially if the developer experience is tailored around Rust which requires nothing more than rustup. Our desire is to have xbuild work similarly out-of-the-box after a cargo install xbuild, and we're already half the way there by no longer having to install and set up the env vars for the SDK/NDK/build-tools, nor require Java. Though adb and LLVM are still a point of contention.
The solution I'm eyeing is downloading and unpacking these similar to how the stripped NDK is handled currently.

As a long-term solution we'd like to leverage LLVM within the rustup toolchain but we're not there yet.


Another caveat is that usernames with spaces are not currently supported. X puts the unpacked stripped-down NDK somewhere within the home directory (in an appdata cache folder IIRC?), which requires --sysroot to be passed to the C(XX)FLAGS for cc-rs and they clearly state that spaces are not supported in here, not even with quotes, escape characters or the \x1f Unit Separator ASCII character as used in CARGO_ENCODED_RUSTFLAGS: https://github.com/rust-lang/cc-rs/#external-configuration-via-environment-variables.

This is implicitly not a problem in cargo-apk by not using a stripped-down NDK, because the compiler already lives at ./bin/clang next to ./sysroot which is automatically detected:

https://github.com/llvm/llvm-project/blob/d82f0b74dec133cb90767edb032f987930d53772/clang/lib/Driver/ToolChains/Linux.cpp#L373-L380


@dvc94ch (and maybe @rib) what are your thoughts on this? If we can reach an agreement I'd like to build this in such a way that it can be merged into the main xbuild repo, rather than living on our fork as we'll inevitably build this to solve our immediate need of having an out-of-the box experience without skewed host setups.

@dvc94ch
Copy link
Contributor

dvc94ch commented Aug 3, 2023

It's a different problem and does not fit within scope of xbuild. You can build a script or tool to manage installation of these things on windows, which seems to be your main goal. But turning xbuild into a package manager for windows is not the goal. And there are plenty of options already, like chocolatey for example.

@MarijnS95
Copy link
Member Author

I checked out winget but it doesn't even support the most basic of things (because the LLVM installer doesn't expose it): enable "add to PATH".

I don't want to build a tool to manage the installation. I want to strip less out of the NDK build so that the user doesn't have to go and install a compiler by hand and add it to PATH. And download adb.exe and also add it to PATH.

@duckinator
Copy link

Regarding adding LLVM to your path on Windows:

LLVM has an open issue about adding a flag for that: llvm/llvm-project#54724

This is a prerequisite for winget install LLVM.LLVM working.

However, someone in that issue mentions a workaround: winget install -e -i --id LLVM.LLVM will show the GUI for the installer, which lets you select the option to add it to the PATH.

@MarijnS95
Copy link
Member Author

Correct, that issue is the main blocker why our internal docs (where we utilize xbuild) is not recommending to use winget yet. The -i flag is just a workaround but still forces developers to manually click through the installer and make the correct choices.

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

No branches or pull requests

3 participants