On Wed, 27 Aug 2025 13:39:17 GMT, David Beaumont <[email protected]> wrote:
>> Changes jimage creation to sort any non-empty packages first in the list of >> package entries. >> >> Changes imageFile.cpp to read only the lowest bit in the flag word so that >> other bits can later be used for preview mode. >> >> Reviewer notes: >> ---- >> Unfortunately I cannot be 100% sure that this change has covered all the >> cases where the flag word is read. I only found imageFile.cpp looking at it, >> and all the other jimage tools I know about, such as ImageReader, ignore it >> completely. >> >> The PR is "safe" in the sense that the value being written to the flag word >> is unchanged, and the response in imageFile.cpp to a value is unchanged. >> However getting this PR in does not guarantee there is no more code that >> does a zero/non-zero test on the flag word. >> >> To check this as best as I can heuristically, I ran a version of the code >> through CI where the flag value was OR'ed with 0x80000000, meaning it was >> never zero. If other code were doing a simple zero/non-zero test on the flag >> it is hoped the this would catch it out. >> >> I have also left any code which iterates the package entries alone (for now) >> because they might be given older jimage files. In order to usefully process >> the knowledge that "if there's a non-empty package it's listed first", it >> might be necessary to make a minor bump to the version number of the file >> jimage file so this feature can be detected. >> >> --------- >> ### Progress >> - [x] Change must not contain extraneous whitespace >> >> ### Error >> ⚠️ The pull request body must not be empty. >> >> >> >> ### Reviewing >> <details><summary>Using <code>git</code></summary> >> >> Checkout this PR locally: \ >> `$ git fetch https://git.openjdk.org/valhalla.git pull/1534/head:pull/1534` \ >> `$ git checkout pull/1534` >> >> Update a local copy of the PR: \ >> `$ git checkout pull/1534` \ >> `$ git pull https://git.openjdk.org/valhalla.git pull/1534/head` >> >> </details> >> <details><summary>Using Skara CLI tools</summary> >> >> Checkout this PR locally: \ >> `$ git pr checkout 1534` >> >> View PR using the GUI difftool: \ >> `$ git pr show -t 1534` >> >> </details> >> <details><summary>Using diff file</summary> >> >> Download this PR as a diff file: \ >> <a >> href="https://git.openjdk.org/valhalla/pull/1534.diff";>https://git.openjdk.org/valhalla/pull/1534.diff</a> >> >> </details> > > David Beaumont has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains two new > commits since the last revision: > > - 8366200: Modify existing use of jimage fields to make space for preview > mode flags > - remove dead code looks ok. src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java line 161: > 159: } > 160: > 161: Stream<PackageReference> sortedRefs() { Method name: "Refs" is pretty generic, in a class named ResourceTree. How about sortedPackageRefs(). ------------- Marked as reviewed by rriggs (Committer). PR Review: https://git.openjdk.org/valhalla/pull/1534#pullrequestreview-3160746477 PR Review Comment: https://git.openjdk.org/valhalla/pull/1534#discussion_r2304623892
