ACP Implementation of PermissionsExt for Windows by asder8215 · Pull Request #152995 · rust-lang/rust

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).

…rivate methods in FilePermissions struct (to be decided in PR whether to use or remove)

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.

…d out FILE_ATTRIBUTE_NORMAL in private methods within FilePermissions

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?

rust-log-analyzer · 2026-03-04T04:52:36Z

The job pr-check-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

/dev/sda15      105M  6.2M   99M   6% /boot/efi
tmpfs           1.6G   12K  1.6G   1% /run/user/1001
================================================================================

Sufficient disk space available (95818016KB >= 52428800KB). Skipping cleanup.
##[group]Run echo "[CI_PR_NUMBER=$num]"
echo "[CI_PR_NUMBER=$num]"
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}

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.

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