Extend std::fs::Permissions on Windows

[E-Mans-Application]

Proposal

Problem statement

I need to set file attributes on Windows, and I found it's exactly what std::fs::set_permissions does. However, struct std::fs::Permissions only exposes attribute readonly. I would also need to set other attributes, such as FILE_ATTRIBUTE_SYSTEM or FILE_ATTRIBUTE_TEMPORARY. On Linux, there is extension trait std::os::unix::fs::PermissionsExt, but it seems Windows has no such thing.

The attributes given to OpenOptions are only applied to existing files if the files are open with .create(true).truncate(true), which truncates the files. I need to get and set the attributes of an existing file, without changing its contents.

Solution sketch

An extension trait could be added on Windows to expose other attributes. This trait could also provide methods to convert Permissions from and to u32.

In std::sys::pal::windows::fs, extend impl FilePermissions:

impl FilePermissions {
    pub fn readonly(&self) -> bool {
        self.attrs & c::FILE_ATTRIBUTE_READONLY != 0
    }

    pub fn set_readonly(&mut self, readonly: bool) {
        if readonly {
            self.attrs |= c::FILE_ATTRIBUTE_READONLY;
        } else {
            self.attrs &= !c::FILE_ATTRIBUTE_READONLY;
        }
    }

    pub fn system(&self) -> bool {
        self.attrs & c::FILE_ATTRIBUTE_SYSTEM != 0
    }

    pub fn set_system(&mut self, system: bool) {
        if readonly {
            self.attrs |= c::FILE_ATTRIBUTE_SYSTEM;
        } else {
            self.attrs &= !c::FILE_ATTRIBUTE_SYSTEM;
        }
    }

    // ...
   // A macro could be used since all the functions will have the same structure

    pub fn to_u32(&self) -> u32 {
        self.attrs
    }
    fn from_u32(mask: u32) -> Self {
         // You may want to check the mask is correct and return an `Option` here, although `SetFileAttributes` seems to ignore invalid attributes.
         Self { attrs: mask }
    }
}

In std::os::windows::fs:

pub trait PermissionsExt {
    #[must_use]
    fn system(&self) -> bool;
    fn set_system(&mut self);

    #[must_use]
    fn temporary(&self) -> bool;
    fn set_temporary(&mut self);

   // Same for other attributes

    fn to_u32(&self) -> u32;
    fn from_u32(mask: u32) -> Self;
}

impl PermissionsExt for Permissions {
    // Delegates all to `as_inner` or `from_inner`, just like `PermissionsExt` is implemented on Unix.
}

Alternatives

Currently, I'm calling system API SetFileAttributesW directly, but this forces me to resort to unsafe code and to re-implement conversion from Path to Vec<u16> (with verbatim support for long paths), in a way that's probably less efficient than the standard library.

Another possibility would be to compute the mask manually and to transmute the result to Permissions (which is basically a u32), but I don't like this solution because there is no guarantee that Permissions will remain equivalent to a u32 in the future.

Links and related work

Initial discussion: https://internals.rust-lang.org/t/extend-std-permissions-on-windows/20943

[ChrisDenton]

I'm not the biggest fan of call attributes "permissions" but std does already so maybe that ship has sailed. Though I guess a possible alternative would be to have windows only FileAttributes struct.

[pitaj]

Yeah calling these all permissions feels wrong. readonly feels a lot more like a permission than those other two attributes.

What about a std::os::windows::fs::FileExt?

trait FileExt {
    fn attributes(&mut self) -> FileAttributes;
}
impl FileExt for File {
    ...
}

Where FileAttributes would be essentially the FilePermissions struct you have above.

[ChrisDenton]

What about a std::os::windows::fs::FileExt?

Another alternative is to simplify this further and just have

trait FileExt {
    fn attributes(&self) -> u32;
    fn set_attributes(&mut self, attrs: u32);
}
// plus equivalent functions in `std::os::windows::fs` that work on paths.

This is less ergonomic of course but it more or less matches OpenOptionsExt::attributes and side-steps quibbling over which attributes are important enough to have in std.

[E-Mans-Application]

I agree that permissions is not a very appropriate name to refer to attributes.

I suggested to extend Permissions because, despite its name, it exactly represents the file's attributes. (The unique field of fs_impl::FilePermissions is named attrs for a reason.)

I don't think fn attributes should belong to File because it only contains a HANDLE, so this would require an additional system call to get the attributes.

Actually, MetadataExt already has fn file_attributes (which returns a u32), so only "fn set_attributes" would be necessary.

trait FileExt {
    fn set_attributes(&self, attrs: u32) -> io::Result<()>;  // Would call `SetFileInformationByHandle` (or delegate to `File::set_permissions`)
}

// Optionally, in `std::fs`:
fn set_attributes<P: AsRef<Path>>(path: P, attributes: u32) -> io::Result<()> {
    // Would call `SetFileAttributes` (or delegate to `std::fs::set_permissions`)
} 

[ChrisDenton]

That sounds good to me.

[ChrisDenton]

This was discussed in today's libs-api meeting. The decision was to accept the original proposal to extend Permissions (using PermissionsExt) because it already does set attributes and this could lead to conflicts where setting readonly effectively undoes other attribute changes.

There was also discussion about naming. It was decided that we should use an explicit name for the to/from u32 functions so it's more clear what it's doing. E.g.:

pub trait PermissionsExt: Sealed {
    fn file_attributes(&self) -> u32;
    fn set_file_attributes(mask: u32) -> Self;
}

There wasn't a conclusion on which specific attributes should or shouldn't have a special method but that can be decided on a case-by-case basis before stabilization.