On Thu Dec 1 10:46:41 2022 +0000, Zebediah Figura wrote:
Sorry for getting to review this so late; any particular reason this was closed?
Hi Zeb,
Thanks for taking a look. I closed this MR because after more testing I found some problems and a couple of other things I wanted to improve about it, but I'm still experimenting with the concept.
At the moment we are storing DOS attributes (`FILE_ATTRIBUTE_HIDDEN` and `FILE_ATTRIBUTE_SYSTEM`) in the Samba-style extended attributes only, however depending on the OS and filesystem type these may not be available, or other methods of storage may be available instead.
The full list as I currently understand it is:
* Samba-style user extended attributes - works on Linux and MacOS with modern filesystems. * The `system.dos_attrib` extended attribute for the Linux ntfs3 driver * The `FAT_IOCTL_SET_ATTRIBUTES` `ioctl` for the Linux FAT filesystem driver. * `fchflags`/`st_flags` on MacOs, FreeBSD and NetBSD can represent `FILE_ATTRIBUTE_READONLY` with `UF_IMMUTABLE` and `FILE_ATTRIBUTE_HIDDEN` with `UF_HIDDEN`. The show up in the UI on MacOS. (See Wine Bug [#39366](https://bugs.winehq.org/show_bug.cgi?id=39366))
I'm working on patches that take an opportunitistic approach of storing the attributes in any place that might be available. When reloading the attributes, my plan was to load them from all the places that are available and OR them together.
I also think we should revisit `FILE_ATTRIBUTE_READONLY`. Currently this is excluded from the set of attributes that is stored in the Samba attribute. However, I think this design might have issues with interoperability - it would strip off the attribute if one was already present on a file.
As you know `FILE_ATTRIBUTE_READONLY` is currently stored in the file's access mod flags. However, I think it would be better to add this to the mix for the `READONLY` attribute along with the other backends listed above.
At the same time I want to polish up this patch: [server: Allow to open files without any permission bits.](https://github.com/wine-staging/wine-staging/blob/master/patches/server-File...). We spoke about this one previously - it temporarily `chmod`s a read-only file to allow it to be openned. (This is fixes Wine Bug [#50771](https://bugs.winehq.org/show_bug.cgi?id=50771))
The biggest catch I can see with my concept for the `READONLY` attribute is that some of the backends would add additional layers of locking to a file, so to open read-only file, we would need to try to unlock it via `chmod`, `fchflags` and `FAT_IOCTL_SET_ATTRIBUTES` - just because we don't know how the attribute might have been set. Forunately, `FAT_IOCTL_SET_ATTRIBUTES` is Linux-only, and `fchflags` is BSD-only, but still it would add to code complexity.
Even though what I'm proposing is fairly complex, the problem is that the current implementation is limited to Linux and MacOS, and will only work on modern filesystems, and won't properly interopate with FAT and NTFS filesystems.
I would appreciate your thoughts about this design. If you like the concept, I can finish it off and submit it with all the backends.
Thanks Joel