Skip to content

Cygwin: Add PTY and group API #4309

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

Merged
merged 1 commit into from
Mar 10, 2025
Merged

Conversation

Berrysoft
Copy link
Contributor

@Berrysoft Berrysoft commented Mar 8, 2025

Description

Some methods required by nix crate.

Sources

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

@rustbot
Copy link
Collaborator

rustbot commented Mar 8, 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 9, 2025

I see forkpty and openpty defined at https://github.com/cygwin/cygwin/blob/a3863bfeb73f3c3832038685498c7c4148a06890/winsup/cygwin/include/pty.h#L11-L14 and getgrouplist at https://github.com/cygwin/cygwin/blob/a3863bfeb73f3c3832038685498c7c4148a06890/winsup/cygwin/include/cygwin/grp.h#L19, but don't see getgrgid_r and the others in the directory you linked. Is https://github.com/cygwin/cygwin/blob/a3863bfeb73f3c3832038685498c7c4148a06890/newlib/libc/include/grp.h#L67-L78 the correct source?

Assuming so, lgtm. Please update the description / commit message to say what actually changed (Nix requires a lot of things)

@Berrysoft
Copy link
Contributor Author

Yes, it's the correct source. I'll update the commit message later.

I tested nix by enabling --all-features.

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

Thanks! I updated the links in the description.

@rustbot label +stable-nominated

@rustbot rustbot added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Mar 10, 2025
@tgross35 tgross35 changed the title Add methods required by nix Cygwin: Add PTY and group API Mar 10, 2025
@tgross35 tgross35 enabled auto-merge March 10, 2025 05:28
* `forkpty` & `openpty`: for nix::pty
* `getgrgid_r`, `getgrouplist`, getgrnam_r`, `initgroups`: for `user` feature of `nix::unistd`
@tgross35 tgross35 added this pull request to the merge queue Mar 10, 2025
Merged via the queue into rust-lang:main with commit d8b216d Mar 10, 2025
44 checks passed
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Mar 10, 2025
* `forkpty` & `openpty`: for nix::pty
* `getgrgid_r`, `getgrouplist`, getgrnam_r`, `initgroups`: for `user` feature of `nix::unistd`

(backport <rust-lang#4309>)
[ update message to mention Cygwin - Trevor ]
(cherry picked from commit 3dd7092)
@tgross35 tgross35 mentioned this pull request Mar 10, 2025
tgross35 pushed a commit to tgross35/rust-libc that referenced this pull request Mar 10, 2025
* `forkpty` & `openpty`: for nix::pty
* `getgrgid_r`, `getgrouplist`, getgrnam_r`, `initgroups`: for `user` feature of `nix::unistd`

(backport <rust-lang#4309>)
[ update message to mention Cygwin - Trevor ]
(cherry picked from commit 3dd7092)
@tgross35 tgross35 added stable-applied This PR has been cherry-picked to libc's stable release branch and removed stable-nominated This PR should be considered for cherry-pick to libc's stable release branch labels Mar 10, 2025
@Berrysoft Berrysoft deleted the cygwin-nix branch March 19, 2025 13:08
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
O-unix S-waiting-on-review stable-applied This PR has been cherry-picked to libc's stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants