RFR: 8322565 (zipfs) Files.setPosixPermissions should preserve 'external file attributes' bits [v2]

Eirik Bjørsnøs eirbjo at openjdk.org
Wed Jan 10 18:11:40 UTC 2024


On Wed, 10 Jan 2024 17:37:11 GMT, Lance Andersen <lancea at openjdk.org> wrote:

> Thank you for the PR. Overall it looks good a few couple nit comments.

Thanks Lance, see e4a505fa073874824f247c20b76c3531a068ee32 for the latest update following your review.

> test/jdk/jdk/nio/zipfs/TestPosix.java line 761:
> 
>> 759:         Files.write(zipFile, zip);
>> 760: 
>> 761:         // Verify that a read/synch roundtrip preserves the external file attributes
> 
> typo 'synch'

This comment was replaced with "Run the provided action on the ZipFileSystem" following the rewrite described below.

> test/jdk/jdk/nio/zipfs/TestPosix.java line 770:
> 
>> 768: 
>> 769:         // Verify calling Files.setPosixFilePermissions with current permission set
>> 770:         try (FileSystem fs = FileSystems.newFileSystem(zipFile, ENV_POSIX)) {
> 
> This should be a separate test IMHO

I've separated two tests `setPermissionsShouldPreserveRemainingBits` and `setLastModifiedTimeShouldNotChangeExternalFileAttribute`, both using the support method `assertExternalFileAttributeUnchanged`.

I agree this was much cleaner, since these are in fact separate and independent (but perhaps overlapping) issues.

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

PR Comment: https://git.openjdk.org/jdk/pull/17170#issuecomment-1885367252
PR Review Comment: https://git.openjdk.org/jdk/pull/17170#discussion_r1447745607
PR Review Comment: https://git.openjdk.org/jdk/pull/17170#discussion_r1447747366


More information about the core-libs-dev mailing list