Fix duplicated eabihf suffix in ARMv7 platform triples#19139
Fix duplicated eabihf suffix in ARMv7 platform triples#19139nooscraft wants to merge 2 commits intoastral-sh:mainfrom
eabihf suffix in ARMv7 platform triples#19139Conversation
There was a problem hiding this comment.
One nit.
The approach here looks fine to me, but I'm not sure we actually need the {env}eabihf fallback anymore if the detected Environment consistently contains it. @nooscraft did you look into whether this is still sporadically possible?
Actually, the fact that we're doing this for multiple hf variants makes me think it'd be best to fully understand why we added this eabihf hack in the first place. I'll be looking at that more.
| let env_str = env.to_string(); | ||
| // Only append "eabihf" if the environment doesn't already have it | ||
| // (e.g., `Gnueabihf` already ends with "eabihf", but `Gnu` doesn't) | ||
| if env_str.ends_with("eabihf") { | ||
| env_str | ||
| } else { | ||
| format!("{env}eabihf") | ||
| } |
There was a problem hiding this comment.
Can we push this up? The condition here should be if matches!(...) && matches!(env, target_lexicon::Environment::Eabihf | ...) rather than a nested one.
|
Okay, I looked into this a bit more and I'm pretty there's a larger and nontrivial concern here:
In the happy path, this means we can probably just remove the I think the fix to the above is to properly emit This will still be wrong for soft-float ARMv7 hosts that fail FP detection, but that's the existing behavior anyways (and is probably okay given how uncommon those are in 2026). It'll also be more correct for any unicorns out there running ARMv7 softfloat + musl, since they'll get an explicit matching failure rather than silently being given a hardfloat binary that'll One wrinkle with the above though: fixing it at the TL;DR: I think this PR acts as a hotfix, but there's a larger (and more general) way we should probably handle this at the libc detection layer/in the |
|
I saw #19157 and agree that handling this at the I’ve force-pushed the inline-comment refactor here using the typed match guard for |
Fixes uv self update on ARMv7 Linux hosts where the generated platform string ended up with eabihf twice:
armv7-unknown-linux-gnueabihfeabihfThe issue was that
Platform::as_cargo_dist_triple()always appended eabihf for ARMv7 Linux, even when libc detection already returned Gnueabihf.This adds a small guard so we only append eabihf when it is not already present. The existing soft-float Gnu path still gets the suffix as expected.
Added regression coverage for both cases.
Closes #19137.