On Wed, 17 Jun 2026 16:14:38 GMT, Chen Liang <[email protected]> wrote:
>> Introduce a `PathResolution` to `ExplodedImage` to remove a lot of ad-hoc >> treatments of preview path discovery. Also some general comments and test >> additions. >> >> One interesting thing is the mainline jimage is not mapping >> `META-INF/preview/file` to `file` when preview is enabled; it does map >> `META-INF/preview/dir/file` to `dir/file`. Don't know if this is intended; >> exploded image performs the first mapping, which differs from jimage. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Chen Liang 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 five additional commits since > the last revision: > > - Review comments > - Merge branch 'lworld' of https://github.com/openjdk/valhalla into > cleanup/lw-jrtfs > - Review remarks > - Merge branch 'lworld' of https://github.com/openjdk/valhalla into > cleanup/lw-jrtfs > - Cleanup ExplodedImage and enhance SystemImageTest I see the value of ResolvedPaths as a bundle for resolving a logical /modules/... name into regular/preview/selected physical paths. It makes the resolution result explicit and avoids reconstructing preview paths in multiple places. However, I wonder if we need ResolvedPaths as a persistent layer on PathNode. The regular-vs-preview policy already seems centralized in resolvePathInModules()/candidatePath(), with createPathInModulesNodeIfValid() handling node creation. The ResolvedPaths is mostly a tuple. For file/resource nodes, only selected is used after construction: content and size come from selected, and the node never re-resolves between regular and preview. For directory nodes, regular/preview are only needed to lazily initialize childNames; after childNames is initialized, later listings use the cached childNames and no longer need those paths for enumeration. So I think the important cleanup is the centralized resolution logic itself, not necessarily carrying regular/preview/selected on every real /modules PathNode. Could this be simplified so that the resolved paths stay local to node creation and/or directory completion, instead of becoming a long-lived PathNode field? That said, I have not touched this part of the code for 10+ years :-) so please take my comment with the appropriate grain of salt. This is more of a design question than an objection. ------------- PR Comment: https://git.openjdk.org/valhalla/pull/2545#issuecomment-4737322363
