Skip to content

Define mkostemp(s) and <spawn.h> bindings on DragonFly #4291

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

Closed
wants to merge 3 commits into from

Conversation

jrmarino
Copy link
Contributor

@jrmarino jrmarino commented Mar 2, 2025

Description

This changeset defines bindings for mkostemp, mkostemps, and the <spawn.h> functions on DragonFly.
These are required to build fish 4.0.0 (https://github.com/fish-shell/fish-shell)

Sources

<spawn.h>
<stdlib.h>

Checklist

  • Relevant tests in libc-test/semver have been updated
  • No placeholder or unstable values like *LAST or *MAX are
    included (see #3131)
  • Tested locally (cd libc-test && cargo test --target mytarget);
    especially relevant for platforms that may not be checked in CI

jrmarino added 3 commits March 2, 2025 20:48
define functions (identical to FreeBSD):
   - posix_spawn
   - posix_spawnp
   - posix_spawnattr_init
   - posix_spawnattr_destroy
   - posix_spawnattr_getsigdefault
   - posix_spawnattr_setsigdefault
   - posix_spawnattr_getsigmask
   - posix_spawnattr_setsigmask
   - posix_spawnattr_getflags
   - posix_spawnattr_setflags
   - posix_spawnattr_getpgroup
   - posix_spawnattr_setpgroup
   - posix_spawnattr_getschedpolicy
   - posix_spawnattr_setschedpolicy
   - posix_spawnattr_getschedparam
   - posix_spawnattr_setschedparam
   - posix_spawn_file_actions_init
   - posix_spawn_file_actions_destroy
   - posix_spawn_file_actions_addopen
   - posix_spawn_file_actions_addclose
   - posix_spawn_file_actions_adddup2

define constants (identical to FreeBSD):
   - POSIX_SPAWN_RESETIDS
   - POSIX_SPAWN_SETPGROUP
   - POSIX_SPAWN_SETSCHEDPARAM
   - POSIX_SPAWN_SETSCHEDULER
   - POSIX_SPAWN_SETSIGDEF
   - POSIX_SPAWN_SETSIGMASK
@rustbot
Copy link
Collaborator

rustbot commented Mar 2, 2025

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@tgross35
Copy link
Contributor

tgross35 commented Mar 2, 2025

The signatures look correct to me, but a lot of these are defined in freebsdlike/freebsd/mod.rs. Instead of adding new definitions, could you move these existing functions up to freebsdlike/mod.rs so they are shared between Dragonfly and FreeBSD? As long as they share the same signature.

<spawn.h>
<stdlib.h>

Are permalinks to Dragonfly's libc headers available anywhere?

Cc @asomers for the FreeBSD family, I'm not sure if we have a dedicated person for Dragonfly.

@tgross35 tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Mar 2, 2025
@jrmarino
Copy link
Contributor Author

jrmarino commented Mar 3, 2025

actually all the changes are shared with FreeBSD.
Why is there a notice that the branch has conflicts? I just created it from the head today.

So I didn't realize the "mod.rs" in the parent directory was "shared" between FreeBSD and DragonFly. I was attempted to fix DragonFly without touching FreeBSD.

However, I can certainly relocate the definitions from Freebsd/mod.rs to the parent mod.rs, and drop the changes to dragonfly/mod.rs, if that's what is desired.

@tgross35
Copy link
Contributor

tgross35 commented Mar 3, 2025

However, I can certainly relocate the definitions from Freebsd/mod.rs to the parent mod.rs, and drop the changes to dragonfly/mod.rs, if that's what is desired.

That would be preferable if they work for both, we try to keep definitions deduplicated where possible.

Why is there a notice that the branch has conflicts? I just created it from the head today.

#4290 merged a few hours ago which added a function at the end of the same block, not much of a conflict but it can't merge automatically.

@jrmarino
Copy link
Contributor Author

jrmarino commented Mar 3, 2025

I'm probably going to make another branch, push another PR, and close this one with a reference.
I don't know if merges are flat or not, but it would be a bit cleaner if you don't.

@jrmarino
Copy link
Contributor Author

jrmarino commented Mar 3, 2025

replacement PR: #4294
Closing this one.

@jrmarino jrmarino closed this Mar 3, 2025
@tgross35
Copy link
Contributor

tgross35 commented Mar 3, 2025

I don't know if merges are flat or not, but it would be a bit cleaner if you don't.

They are as-is with a merge commit (GH doesn't allow much configuration with merge queues), I ask the author to squash/clean up history if needed. Atomic commits are fine as long as it builds between (since I do a lot of cherry picks).

@jrmarino jrmarino deleted the dfly-spawn-mkostemp branch March 4, 2025 00:17
@tgross35 tgross35 removed the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants