Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error reporting and dynamically skip files already loaded on module path #17

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

shartte
Copy link
Contributor

@shartte shartte commented Jun 2, 2024

This PR will look up which modules are already loaded on the JVM boot layer and record every module with a valid filesystem location. It will then use that information for two purposes:

  • Implement a dynamic ignorelist which does not attempt to make modules out of (legacy-)class-path entries if the same filesystem location is already a module on the boot layer
  • If the module name of a file on the class-path is already used by a module on the boot layer, but that module has a different filesystem location, report both paths and the module name in an error message

Especially the latter point should provide a much better error experience in case of version conflicts:

Exception in thread "main" java.lang.IllegalStateException: Module named JarJarFileSystems was already on the JVMs module path loaded from 
C:\Gradle Home\caches\modules-2\files-2.1\net.neoforged\JarJarFileSystems\0.4.0\ef7e5716525bbe50c784a362f9393457a33e6daf\JarJarFileSystems-0.4.0.jar but class-path contains it at location 
C:\Gradle Home\caches\modules-2\files-2.1\net.neoforged\JarJarFileSystems\0.4.1\78f59f89defcd032ed788b151ca6a0d40ace796a\JarJarFileSystems-0.4.1.jar

@shartte shartte requested a review from Matyrobbrt June 2, 2024 16:31
@shartte shartte merged commit 863489e into McModLauncher:main Jun 3, 2024
1 check passed
@shartte shartte deleted the ignore-loaded-modules branch June 3, 2024 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants