nix
nix copied to clipboard
Add wrappers for posix_spawn related functions.
Hello! This is my first PR to nix so I'd appreciate some guidance/patience.
This PR adds support for posix_spawn* related functions. All functions are added with the exception of posix_spawnattr_{get, set}schedpolicy and posix_spawnattr_{get, set}schedparam, since those require full sched.h support not yet in nix.
Implemented functions use I/O safe versions of file descriptors.
Blockers/Notes
- I'd like to know how, if necessary, I can test the
posix_spawn-related functions. - Most functions return a
Result, as to my understanding even though the safe wrapper will prevent most failures listed by POSIX (eg.EINVALforposix_spawnattr_setflags), implementations may return an error for other reasons not specified by the standard. Is that a correct assumption? - I am uncertain as to how to correctly determine targets to support (with
cfg), and if there is a way to determine that locally. posix_spawn*currently take theargsandenvparguments similar toexec*functions, with slight modification due to https://github.com/rust-lang/libc/issues/1272. Can this be fixed together with #555?
In the meantime, existing code is ready for review.
Closes #1740.
CI failures seem to indicates that libc::posix_spawn does not exist on some platforms, so this needs to be gated to specific targets.
Hi @kosayoda, thanks for your interest in contributing to Nix! I am sorry that this PR hasn't been reviewed for such a long time!
I plan to take a look at this PR in the near future (possibly 3 days later) :)
- I'd like to know how, if necessary, I can test the
posix_spawn-related functions.
I think we can give it a test, perhaps simply spawning a child process and waiting for it would be good.
- Most functions return a
Result, as to my understanding even though the safe wrapper will prevent most failures listed by POSIX (eg.EINVALfor [posix_spawnattr_setflags]
That's exactly what Nix is for! Even though some error cases won't happen with Nix interface, we don't do any thing special about the error handling and the return type:>
(https://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_spawnattr_setflags.html)), implementations may return an error for other reasons not specified by the standard. Is that a correct assumption?
I guess yes? Standards are just standards, implementations can always do extra stuff. Though I think we don't need to do extra job here either as Errno::result(res) will handle everything for us.
- I am uncertain as to how to correctly determine targets to support (with
cfg), and if there is a way to determine that locally.
You can test this locally if you can cross-compile for those targets. It is also ok if you don't, since our CI gets you covered
posix_spawn*currently take theargsandenvparguments similar toexec*functions, with slight modification due to https://github.com/rust-lang/libc/issues/1272. Can this be fixed together with #555?
Emm, seems that the posix_spawn() and posix_spawnp() exposed by the libc crate are actually correct? I think the interface proposed in this PR looks good.
As for that allocation issue, do posix_spawn suffer from this? Seems that the allocation is done before invoking fork()
BTW, a CHANGELOG entry is needed, please see this on how to add one:)
@kosayoda are you still working on this / would you mind if I tried picking it up?