Conversation
| case "MD:": | ||
| case "FD:": | ||
| return MappingFormat.SRG; | ||
| case "PK\u0003": |
There was a problem hiding this comment.
Is this correct when the jar is read as if it's utf-8?
There was a problem hiding this comment.
This correctly identifies jar files when using something like
MappingReader.read(Path.of("minecraft-1.18.2-client.jar"), mappingTree);|
|
||
| @Override | ||
| public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { | ||
| buffer = processFile(file, buffer, visitor); |
There was a problem hiding this comment.
In jars, some directories aren't valid package names, like ones with -, also the META_INF and the version-specific class files.
There was a problem hiding this comment.
processFile ignores any files not ending with ".jar" or ".class", is that enough to skip non valid files?
There was a problem hiding this comment.
That doesn't handle say classes in doc-files or multi-release classes in META-INF. Checking and skipping invalid directories may be needed.
There was a problem hiding this comment.
Shouldn't all classes be processed regardless of the directory they're in? processClass only uses the file content, not the file name or directory.
There was a problem hiding this comment.
No, this isn't determinate for Multi-release jars, also doc files etc should be ignored
There was a problem hiding this comment.
I see, are there any directories besides "META-INF" and "doc-files" that need to be filtered out?
There was a problem hiding this comment.
I recommend skipping any class in any folder with - in name, as those are invalid java package names. However, I don't know how you want to deal with nested jars, and you might still consider jars in those directories.
There was a problem hiding this comment.
It now skips any classes in folders with names containing "-". Jars are not affected by this but they should probably be skipped if they are in "doc-files" folders.
There was a problem hiding this comment.
Jars in "doc-files" folders are now skipped.
|
I wanted to do this in a more layered fashion with mapping packaging as its own encompassing facility, not merged with mapping formats |
Could you elaborate on this please so I can see if this is something I might be able to implement? |
|
JarReader now uses ZipFile which allows it to read zip files that contain extra data at the beginning of the file. This makes it possible to read the old Minecraft windows_server executables. Nested jars are ignored at the moment |
| public static void read(Path path, MappingVisitor mappingVisitor) throws IOException { | ||
| AnalyzingVisitor analyzingVisitor = new AnalyzingVisitor(mappingVisitor); | ||
|
|
||
| ZipFile zipFile = new ZipFile(path.toFile()); |
There was a problem hiding this comment.
You should probably use a jar file system instead,
There was a problem hiding this comment.
Hm, it's really convenient to pass a ZipEntry to ClassReader, what's the benefit of a jar file system?
There was a problem hiding this comment.
For it's more modern, and you can skip invalid directory with its children (like in a DirectoryStream) than having to step into it like when you are iterating with a zip file.
3 participants
This PR adds support for reading an empty mapping (source only) from a jar file. The result can be written out as Enigma and Tiny v2, Tiny v1 skips everything without a destination so it can't be used.