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

std.Target.Query: fix compilation error #21669

Merged
merged 6 commits into from
Oct 12, 2024
Merged

Conversation

dravenk
Copy link
Contributor

@dravenk dravenk commented Oct 11, 2024

closes #21668

@dravenk dravenk changed the title Bug fixed. #21668 Bug fixed. #21668 Oct 11, 2024
lib/std/Target/Query.zig Outdated Show resolved Hide resolved
@alexrp alexrp enabled auto-merge (squash) October 12, 2024 09:19
lib/std/Target/Query.zig Outdated Show resolved Hide resolved
@alexrp alexrp disabled auto-merge October 12, 2024 09:20
@alexrp alexrp linked an issue Oct 12, 2024 that may be closed by this pull request
@alexrp
Copy link
Member

alexrp commented Oct 12, 2024

This isn't strictly related to this PR as it's just restoring existing behavior, but I don't understand why Android actually gets special-cased here. The idea seems to be that you can do CC=/path/to/android/sdk/cc zig libc -target arm-linux-androideabi or similar. But why is Android special in this regard? Surely I could just as well do CC=mips64-linux-gnuabi64-gcc zig libc -target mips64-linux-gnuabi64? cc @mikdusan

@dravenk dravenk requested a review from andrewrk October 12, 2024 09:32
@andrewrk andrewrk enabled auto-merge (squash) October 12, 2024 18:00
@andrewrk andrewrk changed the title Bug fixed. #21668 std.Target.Query: fix compilation error Oct 12, 2024
@mikdusan
Copy link
Member

mikdusan commented Oct 12, 2024

@alexrp I added this a while back leaning towards a curated approach of non-native libc detection and so checks were added for the only 2 platforms I tested against: various apple SDKs and android NDK. Also this impl doesn't explicitly allow NDK on macos or windows hosts, and although rare, the NDKs are available.

What I was happy about is that a garbage libc.txt was avoided. But if we take out canDetectLibC() (non-native logic), everything just falls through to LibCInstallation.findNative() and useless libc.txt is generated, or a very unhelpful error is given such as LibCRuntimeNotFound in case of zig libc -target aarch64-linux-android when NOT setting CC to the correct NDK pathname. We were getting issues raised against this.

In a better impl, what I'd like to see is some tech where an auto-generated libc.txt paths are fuzzy-validated.

To be clear, I think it is important to support exactly the use case pattern you listed (general cross compiler pathname) and it could very well be the curated approach is at odds with that goal.

@andrewrk andrewrk merged commit ba13310 into ziglang:master Oct 12, 2024
10 checks passed
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

Successfully merging this pull request may close these issues.

canDetectLibC should check target Target.abi
4 participants