Skip to content

fix: support glibc < 2.30 - #6

Merged
markovejnovic merged 4 commits into
mainfrom
atuin/fix/compile-on-old-glibc
Oct 24, 2026
Merged

markovejnovic merged 4 commits into
mainfrom
atuin/fix/compile-on-old-glibc

Conversation

@markovejnovic

Copy link
Copy Markdown
Member

Mirrors EmbarkStudios/tracing-logfmt#23 and adds one commit that isn't upstream. Fixes EmbarkStudios/tracing-logfmt#22: on glibc < 2.30 (RHEL 8) the crate fails to link because glibc doesn't export gettid there. gettid is now looked up once with dlsym, and syscall(SYS_gettid) is used when it is missing.

  • ci: test on rockylinux:8 ... is fork-only. It adds a cargo test --all-features job in a rockylinux:8 container (glibc 2.28). Run locally in that image, main fails with undefined reference to `gettid' and this branch passes all tests. Written by Claude; drop it or send it upstream as you prefer.
  • This conflicts with fix: escape quoted values per RFC 8259, not escape_debug #4 and feat: support tracing-log #5 in Cargo.toml/Cargo.lock, since each adds a dependency. Merge it last, and main gets merged into this branch to resolve.

cargo-deny stays red until #1 lands.

Without the fix, cargo test on rockylinux:8 (glibc 2.28) fails to link
with "undefined reference to `gettid'"; with it, all tests pass. Both
were checked locally in the rockylinux:8 image on aarch64.
@socket-security

socket-security Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedlibc@​0.2.1917810093100100

View full report

@greptile-apps

greptile-apps Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The change affects thread-ID fields in logs for users of the formatter.

The PR is not ready to merge because Apple builds still fail.

Findings

  1. P1 macOS builds fail ▶

Summary

The PR replaces the direct Linux gettid call with a cached lookup and a syscall fallback.

  • Linux log rows can include thread IDs without requiring glibc 2.30.

Reviews (2) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." · Reviewed by Greptile

Comment thread src/os/apple.rs
pub(super) fn gettid() -> u64 {
let mut id: u64 = 0;
// SAFETY: id is stack-allocated and guaranteed to outlast the call.
let res = unsafe { libc::pthread_threadid_np(0, &mut id) };

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 macOS builds fail

libc::pthread_threadid_np takes a pointer-typed pthread_t on Apple, but this call passes the integer 0. Rust does not convert that integer into a null pointer, so macOS builds fail with a type mismatch. Pass std::ptr::null_mut() instead.

Suggested change
let res = unsafe { libc::pthread_threadid_np(0, &mut id) };
let res = unsafe { libc::pthread_threadid_np(std::ptr::null_mut(), &mut id) };

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not true: on Apple, libc defines pthread_t = crate::uintptr_t (src/unix/bsd/mod.rs:8 in libc 0.2.191), so 0 is a usize and Test (macos-14) passes. std::ptr::null_mut() is what would fail to compile.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You're right—my comment was incorrect. With the locked libc 0.2.191 binding, Apple’s pthread_t is uintptr_t (usize), so 0 is type-correct here, while std::ptr::null_mut() would fail to compile. The macOS CI pass is consistent with that. Please disregard my suggested change; the existing call is correct.

…old-glibc

# Conflicts:
#	Cargo.lock
#	Cargo.toml
@markovejnovic
markovejnovic merged commit 14edc5c into main Oct 24, 2026
13 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.

1 participant