On Sat, 29 Jun 2024 18:24:46 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
>> Please consider this PR which suggests we rename `ZipEntry.extraAttributes` >> to `ZipEntry.externalFileAttributes`. >> >> 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.externalFileAttributes` 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.externalFileAttributes` >> - Update `JavaUtilZipFileAccess` to similarly rename methods to >> `setExternalFileAttributes` and `getExternalFileAttributes` >> - Rename the field `JarSigner.extraAttrsDetected` to >> `JarSigner.externalFileAttrsDetected` >> - Rename a local variable in `JarSigner.writeEntry` to >> `externalFileAttributes` >> - Rename `s.s.t.jarsigner.Main.extraAttrsDetected` to >> `externalFileAttributesDetected` >> - Rename resource string key names in `s.s.t.jarsigner.Resources` from >> `extra.attributes.detected` to `external.file.attributes.detected` >> - Rename method `SymlinkTest.verifyExtraAttrs` to >> `verifyExternalFileAttributes`, 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" > > Eirik Bjørsnøs has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains eight commits: > > - Merge branch 'master' into zipentry-external-attributes > - Rename SymlinkTest.verifyExternalAttrs to verifyExternalFileAttributes > - Rename ZipFileSystem.Entry.posixPerms to externalFileAttributes > - For clarity, use "externalFileAttributes" instead of "externalAttributes" > - Merge branch 'master' into zipentry-external-attributes > - Update copyright years for 2024 > - Merge branch 'master' into zipentry-external-attributes > - Rename ZipEntry.extraAttributes to ZipEntry.externalAttributes Hello Eirik, I'll run some tests and review this PR this coming week. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16952#issuecomment-2198432408
