Skip to content

Conversation

TibboddiT
Copy link
Contributor

Fix usages of std.c.versionCheck, where .linux os tag should also be checked against android libc version.

closes #24460

Fix occurences of potentially errorneous libc version checks due to the
differences between glibc and bionic version numbers.
https://android.googlesource.com/platform/bionic/+/HEAD/docs/status.md
was used as the main reference.
@alexrp alexrp self-assigned this Jul 17, 2025
Co-authored-by: Alex Rønne Petersen <alex@alexrp.com>
Copy link
Member

@alexrp alexrp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM other than the style nit above.

@alexrp alexrp enabled auto-merge (squash) August 7, 2025 15:05
@TibboddiT
Copy link
Contributor Author

@alexrp I was rechecking changes and this one doesn't seem correct:

--- a/lib/std/posix.zig
+++ b/lib/std/posix.zig
@@ -610,8 +610,8 @@ pub fn getrandom(buffer: []u8) GetRandomError!void {
     if (@TypeOf(system.getrandom) != void) {
         var buf = buffer;
         const use_c = native_os != .linux or
-            (!builtin.abi.isAndroid() and std.c.versionCheck(std.SemanticVersion{ .major = 2, .minor = 25, .patch = 0 })) or
-            (builtin.abi.isAndroid() and std.c.versionCheck(std.SemanticVersion{ .major = 28, .minor = 0, .patch = 0 }));
+            (builtin.abi.isAndroid() and std.c.versionCheck(.{ .major = 28, .minor = 0, .patch = 0 })) or
+            std.c.versionCheck(.{ .major = 2, .minor = 25, .patch = 0 });
 
         while (buf.len != 0) {
             const num_read: usize, const err = if (use_c) res: {

If we are on android 24, use_c will be true, but getrandom will not be available.
I confess I just clicked "Commit suggestion" on your first suggested change, sorry about that.

Next commit will fix the check, but maybe you should disable auto-merge for now ?

auto-merge was automatically disabled August 7, 2025 15:25

Head branch was pushed to by a user without write access

@alexrp
Copy link
Member

alexrp commented Aug 7, 2025

Oh, you're right of course, that was a silly suggestion.

That std.c.checkVersion function really needs an overhaul so that you have to explicitly pass the version of each relevant libc in a struct or something. But that's a problem for another day.

@alexrp alexrp enabled auto-merge (squash) August 7, 2025 15:33
@alexrp alexrp merged commit 2cf15be into ziglang:master Aug 8, 2025
12 checks passed
@TibboddiT TibboddiT deleted the fix-libc-version-checks branch August 12, 2025 09:08
@alexrp alexrp removed their assignment Aug 13, 2025
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.

copy_file_range not available on android bionic before 34
2 participants