Retain suid/sgid/sticky bits in Metadata.permissions #44624
Conversation
Most users would expect set_permissions(Metadata.permissions()) to be non-destructive. While we can't guarantee this, we can at least pass the needed info to chmod. Also update the PermissionsExt documentation to disambiguate what it contains, and to refer to the underlying value as `st_mode` rather than its type `mode_t`. Closes rust-lang#44147
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
@alexcrichton do you remember why we're masking at all here? It seems kind of weird. |
|
This line of code as added in 2015 back in the days of @tmerr can you add a test for this? |
|
I don't feel super strongly, but it might be nice to just drop the mask entirely - it doesn't seem like it's really buying us anything. |
|
OK sounds reasonable @sfackler. For the test I guess I could set a directory's permisisons to 0, then to 0o1777, then read them back. (ignoring suid/sgid here since whether they're honored is apparently implementation defined?) |
|
Sure, that seems reasonable. |
|
OK, removed mask and added test case |
carols10cents
added
the
S-waiting-on-review
|
ping @sfackler, mind taking another look? |
|
@bors r+ |
|
📌 Commit 6ae9fc2 has been approved by |
Retain suid/sgid/sticky bits in Metadata.permissions Most users would expect set_permissions(Metadata.permissions()) to be non-destructive. While we can't guarantee this, we can at least pass the needed info to chmod. Also update the PermissionsExt documentation to disambiguate what it contains, and to refer to the underlying value as `st_mode` rather than its type `mode_t`. Closes #44147
|
☀️ Test successful - status-appveyor, status-travis |
Most users would expect set_permissions(Metadata.permissions()) to be
non-destructive. While we can't guarantee this, we can at least pass
the needed info to chmod.
Also update the PermissionsExt documentation to disambiguate what it
contains, and to refer to the underlying value as
st_moderather thanits type
mode_t.Closes #44147