Centralize JAR handling - #26331
Conversation
| for | ||
| file <- expandPath(path, expandStar = false) | ||
| dir <- Option(AbstractFile.getDirectory(file)) | ||
| dir <- Option(AbstractFile.getDirectory(file, ctx.settings.javaOutputVersion.value)) |
There was a problem hiding this comment.
theme 1: it's no longer optional to pass a version when (potentially) dealing with JARs through this API
(and yes it's weird that getDirectory also handles ZIPs and JARs... but I'm not sure how easy it would be to avoid that)
| file.exists && (file.ext.isTasty || (file.ext.isClass && !file.hasSiblingTasty)) | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
theme 2: delete lots of dead code
| val jar = JarArchive.open(Path(arg), create = false) | ||
| def tastyFiles(file: AbstractFile): Iterator[AbstractFile] = | ||
| if file.isDirectory then file.iterator.flatMap(tastyFiles) | ||
| else if file.ext.isTasty then Iterator.single(file) else Iterator.empty |
There was a problem hiding this comment.
TASTYRun also needed this so I centralized deepIterator into AbstractFile
| if (is == null) throw new PluginLoadException(jarp.path, s"Missing $PluginFile in $jarp") | ||
| else fromFile(is, jarp) | ||
|
|
||
| val fileEntry = new java.util.jar.JarEntry(PluginFile) |
There was a problem hiding this comment.
theme 3: go through our abstractions for JARs (so we can enforce the use of a version... JarArchive doesn't do that yet)
| new JarArchive(path, Directory(root)) | ||
| } | ||
|
|
||
| // See http://download.java.net/jdk7/docs/api/java/nio/file/Path.html |
There was a problem hiding this comment.
this stuff here is moved from deleted classes
| try Files.isDirectory(jpath) | ||
| catch { case ex: SecurityException => jpath.toString == "." } | ||
| def isAbsolute: Boolean = jpath.isAbsolute() | ||
| def isEmpty: Boolean = path.length == 0 |
There was a problem hiding this comment.
this one was just evil... directory.isEmpty doesn't check what one would assume it does
| def jpath: JPath | Null = null | ||
| def input: InputStream = throw UnsupportedOperationException("NoAbstractFile.input") | ||
| def isDirectory: Boolean = false | ||
| override def absolute: AbstractFile = this |
There was a problem hiding this comment.
theme 4: add overrides so we can detect dead code in subclasses easily
SolalPirelli
added
the
needs-squashing
|
|
||
| def readRun(using ctx: Context): ReadOnlyRun = new: | ||
| val suspendedAtTyperPhase = ctx.run.nn.suspendedAtTyperPhase | ||
| val suspendedAtTyperPhase = ctx.run != null && ctx.run.nn.suspendedAtTyperPhase |
There was a problem hiding this comment.
so it can be used in scripting where we don't have a Run but need to eagerly evaluate this; this pointless abstraction is going away in #26236 anyway
This comment was marked as outdated.
This comment was marked as outdated.
|
|
||
| def create(zipFile: AbstractFile)(using Context): ClassPath = | ||
| val release = Option(ctx.settings.javaOutputVersion.value).filter(_.nonEmpty) | ||
| val release = ctx.settings.javaOutputVersion.value |
There was a problem hiding this comment.
If -java-output-version is not set this is now "", it used to be None. So we pass Some("") to FileZipArchive and we end up in the other branch here
release match {
case Some(r) if file.nn.getName.endsWith(".jar") =>
new JarFile(file, true, ZipFile.OPEN_READ, if r == "" then Runtime.version() else Runtime.Version.parse(r))
case _ =>
new ZipFile(file)
}
The new ZipFile branch is no longer used for jars.
Multi-release Jars are now used by default, and the version the compiler sees depends on the JDK version being used in compilation.
Is all of that intended?
There was a problem hiding this comment.
Yes, for #26232 and more generally, I don't think we should ever not use a version-specific file if we can?
In my view "not specifying a target release" == "using the current JDK".
Also, I can imagine library authors getting very confusing bug reports if someone uses their library in a way that javac doesn't and that doesn't immediately make sense (using JDK N but a classfile meant for JDK <N).
4 participants
Prep for #26232
Part of #26492
Delete lots of dead code, remove some of our JAR "abstractions".
This leaves
JarArchiveandZipArchive, the latter of which handles multi-release JARs. I'll move JAR handling to the former in a future PR.How much have you relied on LLM-based tools in this contribution?
Not at all
How was the solution tested?
Covered by existing tests (this is a refactoring)