POC: Use jrt:/ file system as fallback for missing jmods#504
POC: Use jrt:/ file system as fallback for missing jmods#504Marcono1234 wants to merge 1 commit intoGuardsquare:masterfrom
jrt:/ file system as fallback for missing jmods#504Conversation
|
| System.out.println("=== MISSING ==="); | ||
| missing.forEach(System.out::println); | ||
|
|
||
| System.out.println("=== UNEXPECTED ==="); | ||
| unexpected.forEach(System.out::println); |
There was a problem hiding this comment.
I noticed some differences here for non-.class files. Not sure if that is a problem. For exampe the jmods seem to have many of them nested within classes/, whereas for jrt:/ they seem to be at the top level of a module.
There was a problem hiding this comment.
Don't think it's super relevant for library jars purposes but would be good to check.
| DataEntryReader reader; | ||
| DataEntrySource source = maybeGetJrtFallback(classPathFile, classPathEntry.isJmod()); | ||
| if (source != null) { | ||
| reader = dataEntryReader; |
There was a problem hiding this comment.
Not sure if dataEntryReader can really be used directly here or if some of the logic of DataEntryReaderFactory#createDataEntryReader needs to applied here too.
| @Override | ||
| public void close() throws IOException { | ||
| jrtFileSystem.close(); | ||
| // Note: Closing the class loader might not be thread-safe because by default the JDK caches the underlying | ||
| // JarFile, so closing it implicitly here might affect other users of that jar | ||
| fallbackClassLoader.close(); | ||
| } |
There was a problem hiding this comment.
Not sure if JarFile caching can really be a problem here. But PMD seems to have encountered issues with this (?), see pmd/pmd@62f2159.
Though actually JrtDataEntrySource#close() is currently not called at all because the created JrtDataEntry objects still need access to the file system for reading the content.
So maybe there is no proper place to call close() for the current ProGuard implementation?
| * Data entry created by {@link JrtDataEntrySource}. | ||
| */ | ||
| public class JrtDataEntry implements DataEntry { | ||
| private final Path jrtPath; |
There was a problem hiding this comment.
Mmh interesting, in my POC I was using the Path and computing the size and name from it indirectly, I'd need to check if both approaches have the same behaviour?
There was a problem hiding this comment.
Do you mean using Files#size?
Maybe it is a bit more performant to use the BasicFileAttributes which Files#walkFileTree obtains anyway (I think for all the other file walking approaches it has to obtain them internally as well?). It seems Files#size internally relies on the file attributes as well.
Regarding the name: Wouldn't Path#getFileName only include the last part, that is, it would be missing the package name?
|
Nice! I did try something similar in the past, but without any of the fallback class loader mechanism: package org.example;
import proguard.classfile.ClassPool;
import proguard.classfile.visitor.ClassPoolFiller;
import proguard.classfile.visitor.ClassPrinter;
import proguard.io.*;
import java.io.*;
import java.net.URI;
import java.nio.file.FileSystem;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.stream.Stream;
class Main {
static class JrtDataEntry implements DataEntry{
private final Path path;
private InputStream stream;
public JrtDataEntry(Path path) {
this.path = path;
}
@Override
public String getName() {
return this.path.getFileName().toString();
}
@Override
public String getOriginalName() {
return this.path.getFileName().toString();
}
@Override
public long getSize() {
try {
return Files.size(path);
} catch (IOException e) {
throw new RuntimeException(e);
}
}
@Override
public boolean isDirectory() {
return false;
}
@Override
public InputStream getInputStream() throws IOException {
if (stream == null) {
this.stream = Files.newInputStream(this.path);
}
return stream;
}
@Override
public void closeInputStream() throws IOException {
if (stream != null) {
stream.close();
}
}
@Override
public DataEntry getParent() {
return null;
}
}
static class JrtDataEntrySource implements DataEntrySource{
@Override
public void pumpDataEntries(DataEntryReader dataEntryReader) throws IOException {
FileSystem fs = FileSystems.getFileSystem(URI.create("jrt:/"));
Path modules = fs.getPath("modules");
try(Stream<Path> walk = Files.walk(modules)) {
walk.forEach(p -> {
try {
dataEntryReader.read(new JrtDataEntry(p));
} catch (IOException e) {
throw new RuntimeException(e);
}
});
}
}
}
public static void main(String[] args) throws Throwable {
ClassPool libraryClassPool = new ClassPool();
new JrtDataEntrySource().pumpDataEntries(new ClassFilter(
new ClassReader(
true,
true,
true,
true,
null,
new ClassPoolFiller(libraryClassPool))));
libraryClassPool.classesAccept(new ClassPrinter());
// try(DataEntryWriter writer = new JarWriter(new ZipWriter(new FixedFileWriter(new File("rt.jar"))))){
// libraryClassPool.classesAccept(new DataEntryClassWriter(writer));
// }
}
}I think this is a good starting point to go from. |



This is a proof-of-concept for using the
jrt:/file system if a JDK does not have thejmodsdirectory, see #473 (comment).In theory the
jrt:/file system could be used always, without having to rely onjmodsfiles, but for now it is only implemented as fallback.The classes
JrtDataEntrySource.javaandJrtDataEntry.javashould probably be in proguard-core, but for convenience of this POC, I have included them here.This is only a proof-of-concept, which seems to work, but I might have overlooked aspects of ProGuard, therefore this is probably not something which can be integrated as is. But feel free to take this as reference in case you want to implement this.