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

Provide musl libc compatibility #696

Merged
merged 9 commits into from Apr 24, 2023

Conversation

sanmai-NL
Copy link
Contributor

@sanmai-NL sanmai-NL commented Apr 19, 2023

Musl libc is a popular alternative to glibc. It was designed for simplicity, correctness and efficiency. It has some slight incompatibilities with glibc.

Musl libc is used by e.g., Alpine Linux and Gentoo as overlay. Alpine Linux is a popular OS for container images.

This change also allows for compilation to statically linked binaries.

@codecov
Copy link

codecov bot commented Apr 24, 2023

Codecov Report

Merging #696 (f941252) into main (744f8e8) will increase coverage by 0.00%.
The diff coverage is 73.17%.

@@           Coverage Diff           @@
##             main     #696   +/-   ##
=======================================
  Coverage   82.93%   82.94%           
=======================================
  Files          62       62           
  Lines       15926    15950   +24     
=======================================
+ Hits        13209    13229   +20     
- Misses       2717     2721    +4     
Impacted Files Coverage Δ
ntp-daemon/src/spawn/standard.rs 90.22% <0.00%> (-0.77%) ⬇️
ntp-udp/src/hwtimestamp.rs 42.50% <50.00%> (ø)
ntp-os-clock/src/unix.rs 29.70% <71.42%> (+1.96%) ⬆️
ntp-udp/src/raw_socket.rs 75.36% <90.47%> (+0.92%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@folkertdev folkertdev merged commit bee2101 into pendulum-project:main Apr 24, 2023
27 checks passed
Copy link
Contributor Author

@sanmai-NL sanmai-NL left a comment

Choose a reason for hiding this comment

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

@folkertdev Maybe you want to revert the merge pending a review on the safety aspect.

timespec.tv_sec += offset_secs as libc::time_t;
timespec.tv_sec += {
// this looks a little strange. it is to work around the `libc::time_t` type being
// deprectated for musl in rust's libc.
Copy link
Contributor Author

@sanmai-NL sanmai-NL Apr 24, 2023

Choose a reason for hiding this comment

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

I think it's better to explicitly silence this warning instead. There's no recommended migration path. It's questionable whether the deprecation will be upheld. See: rust-lang/libc#1848

(Typo: deprecated.)

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah I read the issue, and ... it's really unclear what they actually want to happen there. it must eventually be changed because that is just what musl expects now, right? I'll amend this to ignore the warning, and link to the issue

ntp-udp/src/raw_socket.rs Show resolved Hide resolved
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.

None yet

2 participants