MT-hardening builder.ClasspathJar.knownPackageNames #3250 #3255
Conversation
| @@ -71,13 +67,6 @@ public ClasspathJrtWithReleaseOption(String zipFilename, AccessRuleSet accessRul | |||
| this.externalAnnotationPath = externalAnnotationPath.toString(); | |||
| } | |||
| this.release = getReleaseOptionFromCompliance(release); | |||
| JrtFileSystem systemForRelease = null; | |||
There was a problem hiding this comment.
Why is this code removed? How it relates to the MT cleanup?
There was a problem hiding this comment.
Compiler complained the code is unused after refactoring (to compute only modules not packages)
|
I'm away from keyboard till next Monday, but looking on the touched code & failed test cases I would recommend to be careful, the module/JRT code evolved a lot over time with all the extra cases for every new Java version / feature added to the undocumented JRT file system. @jarthana and @stephan-herrmann should know what I mean and should be able to help with the review, but first the tests should be green again :-) |
* use Set<String> * cache a unmodifiable copy * simplify PackageCacheEntry to record * add missing final/volatile modifiers * remove ClasspathJrt.PackageCache (only module names had been used) * don't calculate unused location of module.info * remove closeZipFileAtEnd (was always true when zipfile!=null) eclipse-jdt#3250
jukzi
force-pushed
the
knownPackageNames
branch
from
0df4c20
to
e5f93b9
Compare
tests failed because i used |
And remove duplicate code in ClasspathJrtWithReleaseOption.getModuleNames()
|
If anybody wants to review, please do it by Monday or leave a message. |
| }); | ||
|
|
||
| return entry.packageSet; | ||
| } | ||
| protected String readJarContent(final SimpleSet packageSet) { | ||
| String modInfo = null; | ||
| /** overloaded */ |
There was a problem hiding this comment.
@jukzi this comment looks plain wrong.
Additionally a reference to the issue / PR is missing from the commit comment.
#3250