jdk icon indicating copy to clipboard operation
jdk copied to clipboard

8373436: Cleanup JRTIndex usages

Open lahodaj opened this issue 3 weeks ago • 4 comments

The com.sun.tools.javac.file.JRTIndex class serves two purposes: a) the keeps a package-oriented cache of directories and files inside the JRT FileSystem; b) allows to get the additional flags for classes in packages on the default classpath. Sadly, it is not easy to completely disentangle these two purposes, as the directories cache also holds the additional flags.

The JRTIndex is used on two places, the ClassFinder, and in JavacFileManager, but the former is only using the additional flags, and the latter is using the cache/index. The logic determining whether the JRTIndex is used in ClassFinder is also not trivial.

The goal of this PR is to attempt to clean this a bit:

  • there's a separate abstraction for getting the additional flags (LegacyCtPropertiesAccess), with the non-trivial implementation residing in JavacFileManager. This allows to concentrate the complex almost completely inside the file manager.
  • the JRTIndex itself is now a package-private class inside the ....file package.

It is not a goal of this PR to change the behavior, just make the implementation more understandable.

Note this removal from ClassFinder:

-        if (fm instanceof DelegatingJavaFileManager delegatingJavaFileManager) {
-            fm = delegatingJavaFileManager.getBaseFileManager();
-        }

The DelegatingJavaFileManager is used when --release is specified, so this allows to get the underlying base file manager. But, regardless of the underlying base manager, the system classes should always come from the --release data, which are not in the underlying base manager, and hence (ultimately), the JRTIndex data should not be used by ClassFinder when --release is used. The removal makes this more obvious.


Progress

  • [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • [x] Change must not contain extraneous whitespace
  • [x] Commit message must refer to an issue

Issue

Reviewers

Reviewers without OpenJDK IDs

  • @david-beaumont (no known openjdk.org user name / role) 🔄 Re-review required (review applies to dc275831)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28761/head:pull/28761
$ git checkout pull/28761

Update a local copy of the PR:
$ git checkout pull/28761
$ git pull https://git.openjdk.org/jdk.git pull/28761/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 28761

View PR using the GUI difftool:
$ git pr show -t 28761

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28761.diff

Using Webrev

Link to Webrev Comment

lahodaj avatar Dec 11 '25 08:12 lahodaj

:wave: Welcome back jlahoda! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

bridgekeeper[bot] avatar Dec 11 '25 08:12 bridgekeeper[bot]

@lahodaj This change is no longer ready for integration - check the PR body for details.

openjdk[bot] avatar Dec 11 '25 08:12 openjdk[bot]

@lahodaj The following label will be automatically applied to this pull request:

  • compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

openjdk[bot] avatar Dec 11 '25 08:12 openjdk[bot]