Skip to content

Handle the case of a Multi-Release Jar not actually containing Multi-Release Classes#42

Merged
marchermans merged 2 commits intoMcModLauncher:mainfrom
AterAnimAvis:multirelease
Feb 14, 2024
Merged

Handle the case of a Multi-Release Jar not actually containing Multi-Release Classes#42
marchermans merged 2 commits intoMcModLauncher:mainfrom
AterAnimAvis:multirelease

Conversation

@AterAnimAvis
Copy link
Copy Markdown
Contributor

Seen a couple of people encounter this in mod dev (when adding dependencies):

SJH considers a Jar to be Multi-Release based on the Manifest

this.isMultiRelease = Boolean.parseBoolean(getManifest().getMainAttributes().getValue("Multi-Release"));

If it is then it resolves a path to /META-INF/versions

var vers = filesystem.getRoot().resolve("META-INF/versions");

And tries to walk it

try (var walk = Files.walk(vers)){

This works for most Jars, however it does not consider the case where a Jar is erroneously marked as Multi-Release, with /META-INF/versions not existing.

If this is the case, Files.walk will throw a UnionFileSystem$NoSuchFileException which is then wrapped and rethrown as a UncheckedIOException

throw new UncheckedIOException(ioe);

Attached Example Stacktrace, https://gist.github.com/AterAnimAvis/35ded98d52a78c2f257d59173c86366c


Current implementation of a fix is from a bit of spitballing in Discord

@AterAnimAvis AterAnimAvis added the bug Something isn't working label Jul 14, 2023
@heisluft
Copy link
Copy Markdown
Contributor

Why not use Files.isDirectory() instead of relying on an exception being thrown?

@AterAnimAvis
Copy link
Copy Markdown
Contributor Author

That's definitely an option (and probably the superior one), A few of us went through several options over on Discord fairly quickly, but I don't think that was raised.

It would require to change the code flow slightly around the setting of the final this.nameOverrides which I was personally hoping to avoid, but may be worth it.

@Technici4n
Copy link
Copy Markdown
Contributor

This needs to be updated after the recent refactor of this code to JarContentsImpl.

@marchermans marchermans merged commit 8d3b608 into McModLauncher:main Feb 14, 2024
@AterAnimAvis AterAnimAvis deleted the multirelease branch February 14, 2024 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants