Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants