RFR: JDK-8260966 (fs) Consolidate Linux and macOS implementations of UserDefinedFileAttributeView

Alan Bateman alanb at openjdk.java.net
Wed Feb 17 09:56:50 UTC 2021


On Wed, 17 Feb 2021 09:22:08 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Deduplicated code in these classes:
>> - `LinuxUserDefinedAttributeView` + `BsdUserDefinedAttributeView` → `UnixUserDefinedAttributeView`. Due to different supported length of attribute names, I added an abstract method `UnixUserDefinedAttributeView.maxNameLength()`.
>> - `LinuxNativeDispatcher` + `BsdNativeDispatcher` → `UnixNativeDispatcher`. I basically just moved the Linux implementation up to Unix and added preprocessor directives to distinguish Linux/BSD/Other. Others will throw ENOTSUP UnixExceptions.
>> - `LinuxFileStore.isExtendedAttributesEnabled()` + `BsdFileStore.isExtendedAttributesEnabled()` → `UnixFileStore.isExtendedAttributesEnabled()`
>> 
>> For the latter I introduced `UnixConstants.XATTR_NOT_FOUND`, which is `ENODATA` on Linux and `ENOATTR` on BSD. Note that `UnixConstants.ENODATA` is still present, as @AlanBateman "would prefer if we left ENODATA so that it can be used in Linux specific code" (Quote from https://github.com/openjdk/jdk/pull/2363#issuecomment-772534587)
>> 
>> This also fixes `Files.copy(src, dst, StandardCopyOption.COPY_ATTRIBUTES)` on macOS/BSD. Previosly an [invocation was missing](https://github.com/openjdk/jdk/blob/jdk-17%2B7/src/java.base/macosx/classes/sun/nio/fs/BsdFileSystem.java#L70-L72).
>> 
>> I plan to add further commits to clean up this code, once the deduplication is reviewed.
>
> Thanks for spitting up the steps in the refactoring as it makes it easier to review. One other thing that is still creating noise in the patch is that I think your IDE or editor has changed the formatting to 8 space indent in several places where the original code in LinuxUserDefinedFileAttributeView.java was 4 space indent. We've typically used 4 space indent in this code. If you revert those indentations then I think it will reduce the size of the patch and make it easy for us to see where the real changes are.

> _Mailing list message from [Sebastian Stenzel](mailto:sebastian.stenzel at gmail.com) on [nio-dev](mailto:nio-dev at openjdk.java.net):_
> 
> Can you show me an example, where you see an 8 space indentation?

UnixUserDefinedFileAttributeView L41, L58, L73, L125-126, L151-152, L207, ...  so looks like changes from the original code in LinuxUserDefinedFileAttributeView, might be the IDE did it. It's not a big deal, just noise in the patch.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2604


More information about the nio-dev mailing list