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

Binstall musl and gnu builds not compatible due to derived target from build options #155

Closed
ryankurte opened this issue Jun 1, 2022 · 18 comments

Comments

@ryankurte
Copy link
Collaborator

ryankurte commented Jun 1, 2022

hey folks, thanks (particularly @somehowchris, @NobodyXu and @passcod) for all your excellent work on improvements and getting a release out!

the musl binaries are a neat addition, but not technically compatible with the gnu versions because by default we use the target (injected via build.rs) to determine what binaries we'd like to install...

i think the best approach to resolving this is probably to trim the gnu and musl parts of the target and perform a check for both, defaulting to the one that matches out build with user selection if we're running interactively.
(also it would be nice to still build the gnu binaries, the cursed nightmares related to armv7 and aarch64 is why we weren't initially using cross, maybe a mix of both approaches would do the trick).

i'll try to have a look at it sometime soon but, posting an issue in case one of y'all gets there first ^_^

@ryankurte ryankurte changed the title Binstall derives target from build options Binstall musl and gnu builds not compatible due to derived target from build options Jun 1, 2022
@passcod
Copy link
Member

passcod commented Jun 1, 2022

Ah, my bad, I completely missed that!

@ryankurte
Copy link
Collaborator Author

ryankurte commented Jun 1, 2022

all good! i haven't had the bandwidth to keep up with all the (great) progress / only thought of it because i've done the armv7/aarch64/gnu/musl dance too many times before ^_^

@NobodyXu
Copy link
Member

NobodyXu commented Jun 2, 2022

Can we instead detect the target at runtime by invoking rustc -vV and then grep the line starting with host: ?

@passcod
Copy link
Member

passcod commented Jun 2, 2022

I don't know if we necessarily want to assume that rustc is available, even though this is a cargo subcommand?

@NobodyXu
Copy link
Member

NobodyXu commented Jun 2, 2022

IMO it is perfectly reasonable to assume the existence of rustc, since cargo-binstall is mostly used in CI environments and they already have rustc installed.

For people using it on their own computer, they are extremely likely to also have rustc installed.

If rustc is not installed, we can always fallback to the target used in build time.

@passcod
Copy link
Member

passcod commented Jun 2, 2022

Sounds reasonable. Is the information available in a machine-readable format somewhere with rustc or is the format rustc -vV outputs stable or? That's probably more an upstream question if this is not documented somewhere.

@NobodyXu
Copy link
Member

NobodyXu commented Jun 2, 2022

There's a nightly cmdline option for machine readable output in rustc rust-lang/rust#14863, but when I tested it with -vV, I realized that it made no difference at all:

❯ rustc +nightly -Zparse-only -vV
rustc 1.63.0-nightly (ebbcbfc23 2022-05-27)
binary: rustc
commit-hash: ebbcbfc236ced21d5e6a92269edb704692ff26b8
commit-date: 2022-05-27
host: aarch64-apple-darwin
release: 1.63.0-nightly
LLVM version: 14.0.4
❯ rustc -vV
rustc 1.61.0 (fe5b13d68 2022-05-18)
binary: rustc
commit-hash: fe5b13d681f25ee6474be29d748c65adcd91f69e
commit-date: 2022-05-18
host: aarch64-apple-darwin
release: 1.61.0
LLVM version: 14.0.0

passcod added a commit to passcod/cargo-binstall that referenced this issue Jun 2, 2022
@passcod passcod mentioned this issue Jun 2, 2022
@passcod
Copy link
Member

passcod commented Jun 2, 2022

Cargo uses this command and parses it internally so we can likely assume it's pretty stable.

@ryankurte
Copy link
Collaborator Author

IMO it is perfectly reasonable to assume the existence of rustc, since cargo-binstall is mostly used in CI environments and they already have rustc installed.

one of my primary goals in starting this was to use on embedded devices without rust installed (which i frequently do), so, would definitely prefer not to depend on this... we could try the host-based detection with a fallback, but, the fallback would still need to resolve the musl vs. gnu thing.

@passcod
Copy link
Member

passcod commented Jun 2, 2022

That makes sense! A thought: rustup must figure out somehow the platform's target triple to be able to bootstrap/install the first rustc. I'll take a look and see if we can't adapt that for binstall. It's a big bash thing, but we can probably simplify a bit.

@NobodyXu
Copy link
Member

NobodyXu commented Jun 3, 2022

@passcod Take a look at target_lexicon, I found it on lib.rs and might be helpful.

@passcod
Copy link
Member

passcod commented Jun 3, 2022

That just does the same thing as us, it uses the TARGET at build time for the host

@NobodyXu
Copy link
Member

NobodyXu commented Jun 3, 2022

@passcod Yeah, I was looking at its build.rs and just found that out.
It's a shame that there is no virtually no crate on lib.rs that parses target at runtime and we have to reinvent the wheels ourselves.

@passcod
Copy link
Member

passcod commented Jun 3, 2022

There's this, but it doesn't differentiate between musl/gnu at the moment. Might need to fork/patch it.

@NobodyXu
Copy link
Member

NobodyXu commented Jun 3, 2022

Looking at this, it might better to just look for libc.so and check its prefix so that we won't have to launch ldd --version.

@passcod
Copy link
Member

passcod commented Jun 3, 2022

Also thinking through, this issue is for musl/gnu but there's actually more issues if we look wider:

  • apple x64 running on M1 with rosetta... if available, we probably want to install m1 builds if available and fallback to x64 otherwise
  • musl/gnu, we actually may want to go fetch gnu binaries with a musl binstall if we're on a gnu system, but fallback to musl if that's all the upstream has
  • 32-bit builds running on x86-64... or even x64 builds on x64, but only a 32-bit build is available from upstream (that should be pretty rare)

So there's both that we want to know the platform of the host at runtime, and also that we may want some flexibility in the fetching logic.

@NobodyXu
Copy link
Member

NobodyXu commented Jun 3, 2022

Maybe we can do the following best-effort detection of target:

  • If rustc is present, use rustc -vV to find the target
  • Otherwise, do the best-effort detection of:
    • If we are on linux, detect if libc.so is gnu libc. If failed to find libc.so in the predefined locations (/lib, /lib64, /usr/lib, /usr/lib64), then just assumes that glibc isn't used by this system.
    • If we are on mac, detect if it is arch64. If failed to run the cmds/functions used in detection, then just treat it as x86_64-apple-darwin.
  • Else, fallback to musl/x64

To archive the fallback @ryankurte proposed, we can change host_target to return Vec<Box<str>> in the order of preference so that

  • for linux, we can return something like ["x86_64-unknown-linux-gnu", "x86_64-unknown-linux-musl"] if glibc is present, else just ["x86_64-unknown-linux-musl"]
  • for mac, similarly, we can return ["aarch64-apple-darwin", "x86_64-apple-darwin"] or ["x86_64-apple-darwin"]

And then we can use tokio::spawn to launch tasks to search for multiple targets at the same time to speedup cargo-binstall and wait in the order of preference (could also add a newtype to JoinHandle that cancels on drop to avoid occupying too much resource).

Using this approach, we don't have to worry too much about our target detection code not working in some nik environment, like non-standard linux filesystem hierarchy and automatically fallback to something that works on all systems (since musl target is statically linked on rust, the binary would likely also statically link other C/C++ libraries and x86_64 binaries work on aarch64-apple-darwin through rosetta).

@NobodyXu
Copy link
Member

NobodyXu commented Jun 3, 2022

@passcod Actually, we might not need guess_host_triple for linux at all, as we already know most of the information it provides.

What we don't know is just libc information, the easiest way to obtain it is to run ldd --version (glibc version prints the version in stdout while the musl version in alpine prints it in stderr), which is not a good idea to run it in guess_host_triple as the maintainer probably wants it to be as simple as possible and avoid creation of process, which is quite expensive.

Here in cargo-binstall we already spawns rustc, we can afford to also spawn ldd in parallel by using tokio::spawn to hide the latency of spawning a process.

For MacOS, however, using guess_host_triple could help us in detecting whether aarch64-apple-darwin is available or not.

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