Convert `Unix{Datagram,Stream}::{set_}passcred()` to per-OS traits #117156

jmillikin · 2023-10-25T08:25:40Z

These methods are the pre-stabilized API for obtaining peer credentials from an AF_UNIX socket, part of the unix_socket_ancillary_data feature.

Their current behavior is to get/set one of the SO_PASSCRED (Linux), LOCAL_CREDS_PERSISTENT (FreeBSD), or LOCAL_CREDS (NetBSD) socket options. On other targets the {set_}passcred() methods do not exist.

There are two problems with this approach:

Having public methods only exist for certain targets isn't permitted in a stable std API.
These options have generally similar purposes, but they are non-POSIX and their details can differ in subtle and surprising ways (such as whether they continue to be set after the next call to recvmsg()).

Splitting into OS-specific extension traits is the preferred solution to both problems.

These methods are the pre-stabilized API for obtaining peer credentials from an `AF_UNIX` socket, part of the `unix_socket_ancillary_data` feature. Their current behavior is to get/set one of the `SO_PASSCRED` (Linux), `LOCAL_CREDS_PERSISTENT` (FreeBSD), or `LOCAL_CREDS` (NetBSD) socket options. On other targets the `{set_}passcred()` methods do not exist. There are two problems with this approach: 1. Having public methods only exist for certain targets isn't permitted in a stable `std` API. 2. These options have generally similar purposes, but they are non-POSIX and their details can differ in subtle and surprising ways (such as whether they continue to be set after the next call to `recvmsg()`). Splitting into OS-specific extension traits is the preferred solution to both problems.

rustbot · 2023-10-25T08:25:48Z

r? @joshtriplett

(rustbot has picked a reviewer for you, use r? to override)

jmillikin · 2023-10-25T08:28:46Z

Additional context: #76915 (comment)

This part of the unix_socket_ancillary_data feature is not affected by proposed API changes in RFC 3430.

jmillikin · 2023-10-26T02:05:40Z

@rustbot label +T-libs-api -T-libs

bors · 2024-01-13T16:44:54Z

☔ The latest upstream changes (presumably #117285) made this pull request unmergeable. Please resolve the merge conflicts.

joshtriplett · 2024-02-11T02:44:34Z

r? libs

cuviper · 2024-02-11T05:17:45Z

r? libs-api

library/std/src/os/netbsd/net.rs

jmillikin · 2024-02-29T13:35:34Z

@rustbot label -S-waiting-on-author +S-waiting-on-review

dtolnay

Thanks!

dtolnay · 2024-03-01T11:47:56Z

@bors r=Amanieu,dtolnay

bors · 2024-03-01T11:47:59Z

📌 Commit 2ed49fc has been approved by Amanieu,dtolnay

It is now in the queue for this repository.

…anieu,dtolnay Convert `Unix{Datagram,Stream}::{set_}passcred()` to per-OS traits These methods are the pre-stabilized API for obtaining peer credentials from an `AF_UNIX` socket, part of the `unix_socket_ancillary_data` feature. Their current behavior is to get/set one of the `SO_PASSCRED` (Linux), `LOCAL_CREDS_PERSISTENT` (FreeBSD), or `LOCAL_CREDS` (NetBSD) socket options. On other targets the `{set_}passcred()` methods do not exist. There are two problems with this approach: 1. Having public methods only exist for certain targets isn't permitted in a stable `std` API. 2. These options have generally similar purposes, but they are non-POSIX and their details can differ in subtle and surprising ways (such as whether they continue to be set after the next call to `recvmsg()`). Splitting into OS-specific extension traits is the preferred solution to both problems.

Rollup of 9 pull requests Successful merges: - rust-lang#109263 (fix typo in documentation for std::fs::Permissions) - rust-lang#117156 (Convert `Unix{Datagram,Stream}::{set_}passcred()` to per-OS traits) - rust-lang#120684 (Update E0716.md for clarity) - rust-lang#121739 (Display short types for unimplemented trait) - rust-lang#121815 (Move `gather_comments`.) - rust-lang#121835 (Move `HandleStore` into `server.rs`.) - rust-lang#121847 (Remove hidden use of Global) - rust-lang#121861 (Use the guaranteed precision of a couple of float functions in docs) - rust-lang#121875 ( Account for unmet T: !Copy in E0277 message) r? `@ghost` `@rustbot` modify labels: rollup

Dylan-DPC · 2024-03-02T10:47:29Z

failed in rollup
@bors r-

@bors rollup=iffy

jmillikin · 2024-03-02T10:59:51Z

Is there any way to run the Android portion of CI before trying to merge?

I've tried to fix the Android test failure based on the failed merge's build logs, but I don't have an Android build environment or any Android devices capable of running the Rust test suite. Having to iterate on the build via the full bors merge queue is bothersome to other developers, and takes quite a long time.

jmillikin · 2024-03-02T12:50:08Z

@rustbot label -S-waiting-on-author +S-waiting-on-review

Amanieu · 2024-03-04T11:00:09Z

You can edit the CI configuration to run the Android builder for PR CI instead of just for bors. You can find the configuration in src/ci/github-actions/ci.yml.

r=me once you've checked that the Android CI passes.

dtolnay · 2024-03-11T05:52:57Z

Arm-android CI succeeded in #122324.

@bors r=Amanieu,dtolnay

bors · 2024-03-11T05:52:59Z

📌 Commit 93f2f2c has been approved by Amanieu,dtolnay

It is now in the queue for this repository.

bors · 2024-03-11T07:46:04Z

⌛ Testing commit 93f2f2c with merge 6639672...

bors · 2024-03-11T10:21:03Z

☀️ Test successful - checks-actions
Approved by: Amanieu,dtolnay
Pushing 6639672 to master...

rust-timer · 2024-03-11T11:36:37Z

Finished benchmarking commit (6639672): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.8%	[-3.1%, -0.4%]	2
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.9%	[-3.4%, -2.7%]	4
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 645.581s -> 645.914s (0.05%)
Artifact size: 309.97 MiB -> 309.95 MiB (-0.00%)

jmillikin · 2024-03-11T12:02:12Z

@dtolnay Thank you so much for handling the ARM testing! I had poked a bit at the CI config YAML but didn't have time last week to dig in deeper.

Should the same situation happen again, I'll use your commit ed48ec0 as a reference for platform-specific CI.

rustbot assigned joshtriplett Oct 25, 2023

jmillikin mentioned this pull request Oct 26, 2023

Tracking Issue for feature(unix_socket_ancillary_data) #76915

Open

rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 26, 2023

rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Feb 11, 2024

rustbot assigned cuviper and unassigned joshtriplett Feb 11, 2024

rustbot assigned dtolnay and unassigned cuviper Feb 11, 2024

Amanieu added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Feb 21, 2024

Amanieu reviewed Feb 27, 2024

View reviewed changes

library/std/src/os/netbsd/net.rs Outdated Show resolved Hide resolved

jmillikin force-pushed the os-unix-socket-ext branch from e53c955 to bea6f37 Compare February 29, 2024 12:44

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 29, 2024

dtolnay approved these changes Mar 1, 2024

View reviewed changes

Dylan-DPC mentioned this pull request Mar 2, 2024

Rollup of 9 pull requests #121887

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 2, 2024

jmillikin force-pushed the os-unix-socket-ext branch from 2ed49fc to 93f2f2c Compare March 2, 2024 10:58

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 2, 2024

dtolnay mentioned this pull request Mar 11, 2024

Test Unix{Datagram,Stream}::{set_}passcred() on Android #122324

Closed

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 11, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 11, 2024

bors merged commit 6639672 into rust-lang:master Mar 11, 2024
12 checks passed

rustbot added this to the 1.78.0 milestone Mar 11, 2024

bors mentioned this pull request Mar 11, 2024

std::net: adding acceptfilter feature for netbsd/freebsd. #121881

Merged

jmillikin deleted the os-unix-socket-ext branch March 11, 2024 11:59

matthiaskrgr mentioned this pull request Sep 15, 2024

ICE: asked to assemble auto trait candidates of unexpected type: FreshTy(0) #130411

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert `Unix{Datagram,Stream}::{set_}passcred()` to per-OS traits #117156

Convert `Unix{Datagram,Stream}::{set_}passcred()` to per-OS traits #117156

jmillikin commented Oct 25, 2023

rustbot commented Oct 25, 2023

jmillikin commented Oct 25, 2023

jmillikin commented Oct 26, 2023

bors commented Jan 13, 2024

joshtriplett commented Feb 11, 2024

cuviper commented Feb 11, 2024

jmillikin commented Feb 29, 2024

dtolnay left a comment

dtolnay commented Mar 1, 2024

bors commented Mar 1, 2024

Dylan-DPC commented Mar 2, 2024

jmillikin commented Mar 2, 2024 •

edited

Loading

jmillikin commented Mar 2, 2024

Amanieu commented Mar 4, 2024

dtolnay commented Mar 11, 2024

bors commented Mar 11, 2024

bors commented Mar 11, 2024

bors commented Mar 11, 2024

rust-timer commented Mar 11, 2024

jmillikin commented Mar 11, 2024

Convert Unix{Datagram,Stream}::{set_}passcred() to per-OS traits #117156

Convert Unix{Datagram,Stream}::{set_}passcred() to per-OS traits #117156

Conversation

jmillikin commented Oct 25, 2023

rustbot commented Oct 25, 2023

jmillikin commented Oct 25, 2023

jmillikin commented Oct 26, 2023

bors commented Jan 13, 2024

joshtriplett commented Feb 11, 2024

cuviper commented Feb 11, 2024

jmillikin commented Feb 29, 2024

dtolnay left a comment

Choose a reason for hiding this comment

dtolnay commented Mar 1, 2024

bors commented Mar 1, 2024

Dylan-DPC commented Mar 2, 2024

jmillikin commented Mar 2, 2024 • edited Loading

jmillikin commented Mar 2, 2024

Amanieu commented Mar 4, 2024

dtolnay commented Mar 11, 2024

bors commented Mar 11, 2024

bors commented Mar 11, 2024

bors commented Mar 11, 2024

rust-timer commented Mar 11, 2024

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

jmillikin commented Mar 11, 2024

Convert `Unix{Datagram,Stream}::{set_}passcred()` to per-OS traits #117156

Convert `Unix{Datagram,Stream}::{set_}passcred()` to per-OS traits #117156

jmillikin commented Mar 2, 2024 •

edited

Loading