RFR: 8321274: Rename ZipEntry.extraAttributes to ZipEntry.externalAttributes [v2]
Eirik Bjørsnøs
eirbjo at openjdk.org
Tue Jan 30 16:17:13 UTC 2024
> 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"
Eirik Bjørsnøs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
- Update copyright years for 2024
- Merge branch 'master' into zipentry-external-attributes
- Rename ZipEntry.extraAttributes to ZipEntry.externalAttributes
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/16952/files
- new: https://git.openjdk.org/jdk/pull/16952/files/87db8646..481ae754
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=16952&range=01
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=16952&range=00-01
Stats: 155690 lines in 2884 files changed: 86094 ins; 58372 del; 11224 mod
Patch: https://git.openjdk.org/jdk/pull/16952.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/16952/head:pull/16952
PR: https://git.openjdk.org/jdk/pull/16952
More information about the core-libs-dev
mailing list