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

Make changes needed to build on musl #198

Closed

Conversation

andrewliebenow
Copy link
Contributor

No description provided.

@andrewliebenow
Copy link
Contributor Author

I only checked that the project builds on "x86_64-unknown-linux-musl". This is pretty ugly, but maybe someone will want to use this as a starting point re: musl support.

@jgarzik
Copy link
Contributor

jgarzik commented Sep 10, 2024

Neat! I'll think a bit about the best way to merge this, specifically libc_aliases

Initial reaction: This PR serves to highlight the need to isolate libc functions in plib crate, and remove any need to touch individual utils with libc_aliases related changes directly.

@andrewliebenow
Copy link
Contributor Author

👍🏻. Some of the hacky special casing for musl will be unnecessary when rust-lang/libc#3190 is resolved, so one option would be to just wait on that. I made a PR to add some missing constants (rust-lang/libc#3908), but I would assume it's not easy/quick to get things merged into the libc crate.

@andrewliebenow andrewliebenow force-pushed the fix-musl-build branch 2 times, most recently from 2af5f08 to 94f3272 Compare September 10, 2024 23:00
@jgarzik
Copy link
Contributor

jgarzik commented Sep 11, 2024

High level guidance thoughts:

  • We do want to support platforms such as musl
  • Our current set of unsafe libc calls in utils is distasteful... unwanted but sometimes necessary.
  • Adding conditional compilation to utils due to this only compounds the ugliness
  • The proper solution is to have a plib (posixutils' internal library crate) or another crate provide a nice, clean, safe wrapper for the class of libc library function. Example: tiny plib module for get/setpriority POSIX syscalls and constants
  • restrict musl ifdefs to plib, unless there is an incredibly strong reason (c.f. getconf)

@andrewliebenow andrewliebenow force-pushed the fix-musl-build branch 2 times, most recently from 7ca27ef to 5340b68 Compare September 12, 2024 10:16
@jgarzik
Copy link
Contributor

jgarzik commented Sep 13, 2024

On-going comments:

  • The platform:XXX direction is good. This removes libc references from non-plib code; an improvement.
  • Dislike PPriorityWhichT -- it is better to promote xgetpriority and xsetpriority to a new plib/priority.rs module, remove the 'x' prefix, and bury all the glibc/musl/macos ifdefs in plib/priority.rs. Making the cast portable just extends the ugliness, which we would rather leave renice.rs altogether.
  • Happy to entertain cleanups, as separate PRs, to keep this PR smaller and self-contained.

@andrewliebenow
Copy link
Contributor Author

Closed in favor of #198

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.

2 participants