ACP Implementation of PermissionsExt for Windows #152995

asder8215 · 2026-02-23T00:28:17Z

This PR implements the PermissionsExt for Windows ACP and adds file attribute methods in FilePermissions struct (to be decided whether we use them or not). See this tracking issue for further detail and links.

I also added some comments in the code for clarifications about the ACP (e.g. whether we should have a set_file_attributes() + from_file_attributes() method to mirror what unix's PermissionsExt is doing).

Also, some relevant links on this:

Note: Apologies for the multiple forced push. I haven't set up my Windows VM up yet to compile and check the code, so I've been using the CI to help me with that.

r? @ChrisDenton

rustbot · 2026-02-23T00:28:21Z

ChrisDenton is currently at their maximum review capacity.
They may take a while to respond.

asder8215 · 2026-02-23T07:18:30Z

library/std/src/os/windows/fs.rs

+/// let mut permissions = Permissions::set_file_attributes(my_file_attr);
+/// assert_eq!(permissions.mode(), my_file_attr);
+/// ```
+pub trait PermissionsExt {


Permissions struct doesn't have a Sealed trait bound (the Unix version of PermissionsExt doesn't use a Sealed trait bound either), so I'm unsure if this should be done here.

Any new structs like this should be sealed. The reason the Unix version of PermissionExt doesn't is an historical artefact and one we'd love to fix. Otherwise it makes adding any functions a breaking change (this can be worked around by adding PermissionExt2 but we'd rather avoid that if possible).

library/std/src/os/windows/fs.rs

ChrisDenton · 2026-03-03T12:32:17Z

library/std/src/os/windows/fs.rs

+///
+///     // readonly and hidden file attributes that we
+///     // want to add to existing file
+///     let my_file_attr = 0x1 | 0x2;


Given this is a public example, this should use constants instead of magic values, even if that means defining the constants right about this line.

ChrisDenton · 2026-03-03T12:37:56Z

library/std/src/os/windows/fs.rs

+///     );
+///     Ok(())
+/// }
+/// ```


Actually this whole example seems much more like a test than an example. I think it's better for users if examples are kept short and to the point.

library/std/src/os/windows/fs.rs

ChrisDenton · 2026-03-03T12:45:16Z

library/std/src/sys/fs/windows.rs

+            // According to SetFileAttributes, any other values
+            // passed to this should override FILE_ATTRIBUTE_NORMAL


Windows itself will override FILE_ATTRIBUTE_NORMAL if another attribute is present so I'm not sure it's necessary for us to do it. But if we do do it then I'd rather use a bitmask rather than repeating if self.normal() each time. Or maybe an internal-only helper function.

asder8215 · 2026-03-03T14:41:51Z

I'll get to updating this PR later today!

To note, the doctest/example was inspired from the Unix version. I mirrored writing it in the way Unix doc for PermissionsExt did. I do agree with you that I should use const value with specific names than just directly using magic values and clarify the docs a bit more on file attributes. I'm open to bitmasking the normal file attribute out through using an XOR operation and then ORing the result with respective file attributes (and if this actually ends up being unnecessary, I can remove it later down the line).

I'll try and see if I can apply the Sealed trait bound on PermissionsExt, but I also recall receiving a compiler error when I tried that earlier saying that Permissions doesn't implement Sealed trait so I can't implement PermissionsExt on Permissions (I may be misremembering though, will have to check later today).

Thanks for taking a look at this PR @ChrisDenton!

ChrisDenton · 2026-03-03T15:11:35Z

I'll try and see if I can apply the Sealed trait bound on PermissionsExt, but I also recall receiving a compiler error when I tried that earlier saying that Permissions doesn't implement Sealed trait so I can't implement PermissionsExt on Permissions (I may be misremembering though, will have to check later today).

You should just be able implement Sealed for PermissionExt.

To note, the doctest/example was inspired from the Unix version. I mirrored writing it in the way Unix doc for PermissionsExt did.

Yeah, after looking at the Unix example I do find that a bit verbose. In either case, considering the example is no_run I do think there should be a test somewhere that actually runs and tests this works.

asder8215 · 2026-03-04T04:48:13Z

If there are no compilation errors, then yea I've added your feedback. Let me know if the doc comments for PermissionsExt is okay! I took off the larger no_run example and kept the smaller one in.

On a side note, I've been looking at FilePermissions/Permissions methods and I kind of wished the set_* methods returned itself so that it would be possible to just chain the methods. If the other Windows file attributes have the okay to be implemented on PermissionsExt, do you think it would be fine to just return Self instead of bool for its setter methods?

asder8215 · 2026-03-04T04:53:00Z

Yeah, after looking at the Unix example I do find that a bit verbose. In either case, considering the example is no_run I do think there should be a test somewhere that actually runs and tests this works.

I'm down to update the Unix example in a separate PR to reduce verbosity. As for testing the functions here, where would be the appropriate place to do that for this PR?

Moreover, does CI here run Windows tests? I've been wondering that a bit because in my Reverse Ancestor PR, I have Windows tests there for paths with Prefix components, but I didn't see anything in CI that mentions that it ran my Windows test (and passes).

asder8215 · 2026-03-04T04:55:05Z

Also, yea the Sealed trait bound gives me a compilation error for some reason:

  error[E0277]: the trait bound `Permissions: Sealed` is not satisfied
     --> library/std/src/os/windows/fs.rs:399:25
      |
  399 | impl PermissionsExt for Permissions {
      |                         ^^^^^^^^^^^ unsatisfied trait bound
      |
  help: the trait `Sealed` is not implemented for `Permissions`
     --> library/std/src/fs.rs:294:1
      |
  294 | pub struct Permissions(fs_imp::FilePermissions);
      | ^^^^^^^^^^^^^^^^^^^^^^
      = help: the following other types implement trait `Sealed`:
                Child
                OsStr
                OsString
                Simd<f32, N>
                Simd<f64, N>
                StderrLock<'_>
                StdinLock<'_>
                StdoutLock<'_>
              and 12 others
  note: required by a bound in `PermissionsExt`
     --> library/std/src/os/windows/fs.rs:383:27
      |
  383 | pub trait PermissionsExt: Sealed {
      |                           ^^^^^^ required by this bound in `PermissionsExt`

ChrisDenton · 2026-03-04T08:54:30Z

You'll need to impl Sealed for Permissions. Take a look at FileTypeExt, it should be doing something similar.

asder8215 · 2026-03-04T11:57:31Z

You'll need to impl Sealed for Permissions. Take a look at FileTypeExt, it should be doing something similar.

Oh okay, I could do that. I was worried that I may not be allowed to do that if it's a breaking change.

asder8215 · 2026-03-12T13:24:31Z

@ChrisDenton Apologies for the ping, is there anything else I should do or take care of for this PR?

asder8215 · 2026-03-28T16:20:06Z

r? libs

asder8215 · 2026-04-08T15:26:12Z

r? libs

Mark-Simulacrum · 2026-04-11T14:14:34Z

library/std/src/os/windows/fs.rs

+    // &mut self like how unix uses `set_mode()`?
+    // (Note to self: if we do the above, I need to make changes to
+    // the doc comments).
+    /// Sets the file attribute bits.


View changes since the review

I would mirror the unix PermissionsExt (file_attributes/set_file_attributes/from_file_attributes).

Mark-Simulacrum · 2026-04-11T14:15:07Z

library/std/src/sys/fs/windows.rs

@@ -1155,18 +1155,231 @@ fn to_u64(ft: &c::FILETIME) -> u64 {
    (ft.dwLowDateTime as u64) | ((ft.dwHighDateTime as u64) << 32)
 }

+#[allow(dead_code)]


View changes since the review

I think we should, for this PR, drop all the dead code here. It sounds like that's for potential future exposure:

to be decided whether we use them or not

and since that seems like it ought to have its own ACP(s) I'd suggest deleting it from this PR, merging it (with the other comment on methods in the extension trait fixed), and then if you want the additional helpers that can be its own, likely separate, proposal.

Alright, I've removed the dead code. I'll likely create an ACP or discussion in the tracking issue on if we should have additional helper functions to set/check for specific file attributes.

Mark-Simulacrum · 2026-04-11T14:22:55Z

library/std/src/sys/fs/windows.rs

+
+    fn set_hidden(&mut self, hidden: bool) {
+        // According to SetFileAttributes, any other values
+        // passed to this should override FILE_ATTRIBUTE_NORMAL


View changes since the review

I'm confused by the phrasing here. Why are we toggling the FILE_ATTRIBUTE_NORMAL bit? If we want to reset them, shouldn't that be &= !c::FILE_ATTRIBUTE_NORMAL?

The docs in https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-setfileattributesa suggest that other values will override normal (which makes sense, since it's actually the absence of other bits...) but do we need to do that in user code? Even if yes, it seems much clearer to do it by removing normal with &= !... as I outlined above.

If it's illegal to call SetFileAttributes with an all-zero attributes mask, then perhaps we should determine whether FILE_ATTRIBUTE_NORMAL is set or not just before the call, and internally store just the non-normal bits? It's not very clear to me why that's a special bit rather than just being part of the general mask.

I'm confused by the phrasing here. Why are we toggling the FILE_ATTRIBUTE_NORMAL bit? If we want to reset them, shouldn't that be &= !c::FILE_ATTRIBUTE_NORMAL?

Yeah, that's my mistake. I think I tunnel visioned myself on how xor could 0 out values that are both 1, but I forgot that this sets values to 1 if we had 0 ^ 1. I've gotta remove all these functions besides readonly/set_readonly/file_attributes functions. &= !c::FILE_ATTRIBUTE_NORMAL is much clearer.

The docs in https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-setfileattributesa suggest that other values will override normal (which makes sense, since it's actually the absence of other bits...) but do we need to do that in user code? Even if yes, it seems much clearer to do it by removing normal with &= !... as I outlined above.

I think I was unsure if what they meant by override FILE_ATTRIBUTE_NORMAL, that our set functions should clear out FILE_ATTRIBUTE_NORMAL always and then set the accordingly flag, or that the presence and behavior of FILE_ATTRIBUTE_NORMAL is not acknowledged if other file attributes are set. I opted for the first one out of caution, but I think what you say here:

since it's actually the absence of other bits...

Makes sense to me and we may not need to do this set FILE_ATTRIBUTE_NORMAL in user code. I would also assume that in the SetFileAttributesA file attributes chart, the meaning behind FILE_ATTRIBUTE_NORMAL:

A file that does not have other attributes set. This attribute is valid only when used alone.

Implies that if we were to set a file with FILE_ATTRIBUTE_NORMAL attribute, it doesn't necessarily mean that the other file attributes are zeroed out.

rustbot · 2026-04-11T14:27:36Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

rustbot · 2026-04-11T20:18:34Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

…tilities to observe, set, and create a Permissions struct with certain file attributes

rustbot assigned ChrisDenton Feb 23, 2026

rustbot added O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 23, 2026

asder8215 mentioned this pull request Feb 23, 2026

Tracking Issue for extending std::fs::Permissions on Windows #152956

Open

4 tasks

This comment has been minimized.

Sign in to view

asder8215 force-pushed the windows_permissions_ext branch from 00739a8 to 2daf248 Compare February 23, 2026 01:04

This comment has been minimized.

Sign in to view

asder8215 force-pushed the windows_permissions_ext branch from 2daf248 to 29689a6 Compare February 23, 2026 05:47

This comment has been minimized.

Sign in to view

asder8215 force-pushed the windows_permissions_ext branch from 29689a6 to b752775 Compare February 23, 2026 06:54

This comment has been minimized.

Sign in to view

asder8215 force-pushed the windows_permissions_ext branch from b752775 to 23a096f Compare February 23, 2026 07:15

asder8215 commented Feb 23, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

asder8215 force-pushed the windows_permissions_ext branch from 23a096f to 3ed985e Compare February 23, 2026 07:36

This comment has been minimized.

Sign in to view

asder8215 force-pushed the windows_permissions_ext branch from 3ed985e to 696ce60 Compare February 23, 2026 07:58

ChrisDenton reviewed Mar 3, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

asder8215 requested a review from ChrisDenton March 4, 2026 04:55

asder8215 force-pushed the windows_permissions_ext branch from b53af07 to 09f0506 Compare March 4, 2026 22:24

This comment has been minimized.

Sign in to view

asder8215 force-pushed the windows_permissions_ext branch from 09f0506 to 31b3b10 Compare March 4, 2026 23:08

rustbot assigned joboet and unassigned ChrisDenton Mar 28, 2026

rustbot assigned Mark-Simulacrum and unassigned joboet Apr 8, 2026

Mark-Simulacrum requested changes Apr 11, 2026

View reviewed changes

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 11, 2026

rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 11, 2026

asder8215 force-pushed the windows_permissions_ext branch from 31b3b10 to 57010d7 Compare April 11, 2026 20:16

This comment has been minimized.

Sign in to view

asder8215 force-pushed the windows_permissions_ext branch from 57010d7 to bd53742 Compare April 11, 2026 20:18

asder8215 force-pushed the windows_permissions_ext branch from bd53742 to 22199d8 Compare April 11, 2026 20:20

asder8215 requested a review from Mark-Simulacrum April 11, 2026 20:23

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 Apr 11, 2026

Implemented PermissionsExt ACP on Windows, which provides functions/u…

782a214

…tilities to observe, set, and create a Permissions struct with certain file attributes

asder8215 force-pushed the windows_permissions_ext branch from 22199d8 to 782a214 Compare April 11, 2026 20:26

		// According to SetFileAttributes, any other values
		// passed to this should override FILE_ATTRIBUTE_NORMAL

Uh oh!

Conversation

asder8215 commented Feb 23, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Feb 23, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

asder8215 Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

ChrisDenton Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

ChrisDenton Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

ChrisDenton Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ChrisDenton Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asder8215 commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChrisDenton commented Mar 3, 2026

Uh oh!

asder8215 commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

asder8215 commented Mar 4, 2026

Uh oh!

asder8215 commented Mar 4, 2026

Uh oh!

ChrisDenton commented Mar 4, 2026

Uh oh!

asder8215 commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

asder8215 commented Mar 12, 2026

Uh oh!

asder8215 commented Mar 28, 2026

Uh oh!

asder8215 commented Apr 8, 2026

Uh oh!

Mark-Simulacrum Apr 11, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mark-Simulacrum Apr 11, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asder8215 Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Mark-Simulacrum Apr 11, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asder8215 Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

rustbot commented Apr 11, 2026

Uh oh!

This comment has been minimized.

rustbot commented Apr 11, 2026

Uh oh!

Reviewers

asder8215 commented Feb 23, 2026 •

edited by rustbot

Loading

ChrisDenton Mar 3, 2026 •

edited

Loading

asder8215 commented Mar 3, 2026 •

edited

Loading

asder8215 commented Mar 4, 2026 •

edited

Loading

asder8215 commented Mar 4, 2026 •

edited

Loading

Mark-Simulacrum Apr 11, 2026 •

edited by rustbot

Loading

Mark-Simulacrum Apr 11, 2026 •

edited by rustbot

Loading

Mark-Simulacrum Apr 11, 2026 •

edited by rustbot

Loading