On Mon, 4 Dec 2023 15:34:34 GMT, Eirik Bjorsnos <d...@openjdk.org> wrote:

> Please consider this PR which suggests we rename `ZipEntry.extraAttributes` 
> to `ZipEntry.externalAttributes`.
> 
> This field was introduced in 
> [JDK-8218021](https://bugs.openjdk.org/browse/JDK-8218021), originally under 
> the name `ZipEntry.posixPerms`. 
> [JDK-8250968](https://bugs.openjdk.org/browse/JDK-8250968) later renamed the 
> field to `ZipEntry.extraAttributes` and extended its semantics to hold the 
> full two-byte value of the `external file attributes` field, as defined by 
> `APPNOTE.TXT`
> 
> The name `extraAttributes` is misleading. It has nothing to do with the 
> `extra field` (an unrelated structure defined in `APPNOTE.TXT`), although the 
> name indicates it does.
> 
> To prevent confusion and make life easier for future maintainers, I suggest 
> we rename this field to `ZipEntry.externalAttributes` and update related 
> methods, parameters and comments accordingly.
> 
> While this change is a straightforward renaming, reviewers should consider 
> whether it carries its weight, especially considering it might complicate 
> future backports. 
> 
> As a note to reviewers, this PR includes the following intended updates:
> 
> - Rename `ZipEntry.extraAttributes` and any references to this field to 
> `ZipEntry.externalAttributes`
> - Update `JavaUtilZipFileAccess` to similarly rename methods to 
> `setExternalAttributes` and `getExternalAttributes` 
> - Rename the field `JarSigner.extraAttrsDetected` to 
> `JarSigner.externalAttrsDetected`
> - Rename a local variable in `JarSigner.writeEntry` to `externalAttrs`
> - Rename `s.s.t.jarsigner.Main.extraAttrsDetected` to `externalAttrsDetected`
> - Rename resource string key names in `s.s.t.jarsigner.Resources` from 
> `extra.attributes.detected` to `external.attributes.detected`
> - Rename method `SymlinkTest.verifyExtraAttrs` to `verifyExternalAttrs`, also 
> updated two references to 'extra attributes' in this method
> - Updated copyright in all affected files
> 
> If the resource file changes should be dropped and instead handled via 
> `msgdop` updates, let me know and I can revert the non-default files.
> 
> I did a search across the code base to find 'extraAttrs', 'extra.attr' after 
> these updates, and found nothing related to zip/jar. The `zip` and `jar` 
> tests run clean:
> 
> 
> make test TEST="test/jdk/java/util/jar"
> make test TEST="test/jdk/java/util/zip"

Thank you for the contribution Eirik.  I will take a peek at this once we are 
able to push changes for JDK 23

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

PR Comment: https://git.openjdk.org/jdk/pull/16952#issuecomment-1839198963

Reply via email to