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

Remove duplicate libraries #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Remove duplicate libraries #41

wants to merge 1 commit into from

Conversation

zpin
Copy link

@zpin zpin commented Aug 14, 2023

The current method of replacing "org.quiltmc:hashed" with "net.fabricmc:intermediary" will result in duplicate entries for "net.fabricmc:intermediary" if the JSON already contains such an entry.
These libraries then get downloaded in parallel, resulting in an "AccessDeniedException" (tested on Windows Server 2019) because the two processes will attempt to write to the same file simultaneously:

Downloading library at: https://maven.fabricmc.net/net/fabricmc/intermediary/1.20.1/intermediary-1.20.1.jar
Downloading library at: https://maven.fabricmc.net/net/fabricmc/intermediary/1.20.1/intermediary-1.20.1.jar
java.util.concurrent.CompletionException: java.io.UncheckedIOException: java.nio.file.AccessDeniedException: .\libraries\net\fabricmc\intermediary\1.20.1\intermediary-1.20.1.jar
        at java.base/java.util.concurrent.CompletableFuture.encodeThrowable(Unknown Source)
        at java.base/java.util.concurrent.CompletableFuture.completeThrowable(Unknown Source)
        at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(Unknown Source)
        at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.exec(Unknown Source)
        at java.base/java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
        at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(Unknown Source)
        at java.base/java.util.concurrent.ForkJoinPool.scan(Unknown Source)
        at java.base/java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
        at java.base/java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)
Caused by: java.io.UncheckedIOException: java.nio.file.AccessDeniedException: .\libraries\net\fabricmc\intermediary\1.20.1\intermediary-1.20.1.jar
        at org.quiltmc.installer.action.InstallServer.lambda$downloadLibrary$8(InstallServer.java:272)
        ... 7 more
Caused by: java.nio.file.AccessDeniedException: .\libraries\net\fabricmc\intermediary\1.20.1\intermediary-1.20.1.jar
        at java.base/sun.nio.fs.WindowsException.translateToIOException(Unknown Source)
        at java.base/sun.nio.fs.WindowsException.rethrowAsIOException(Unknown Source)
        at java.base/sun.nio.fs.WindowsException.rethrowAsIOException(Unknown Source)
        at java.base/sun.nio.fs.WindowsFileSystemProvider.newByteChannel(Unknown Source)
        at java.base/java.nio.file.spi.FileSystemProvider.newOutputStream(Unknown Source)
        at java.base/java.nio.file.Files.newOutputStream(Unknown Source)
        at java.base/java.nio.file.Files.copy(Unknown Source)
        at org.quiltmc.installer.action.InstallServer.lambda$downloadLibrary$8(InstallServer.java:267)
        ... 7 more

The suggested change removes duplicate entries from the library list, which solves the issue on my test system.

Make sure no duplicates result from injecting intermediary
Copy link
Contributor

@TheGlitch76 TheGlitch76 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! There's actually a bit of history behind this hack, and there's a better fix for your issue:
When this hack was written, Quilt Meta ONLY provided Hashed, not Intermediary. Since then, Quilt Meta provides both Hashed and Intermediary, but Quilt Loader versions below 0.17.7 will crash if Hashed is present.

For now, the best solution would be to remove this hack entirely, and I can come in later and add a different hack to remove Hashed entirely if the selected loader version is below 0.17.7 (which will require semver parsing and much more work)

@zpin
Copy link
Author

zpin commented Aug 14, 2023

Thank you for the quick reply! That does sound better, yeah.
I'd suggest incorporating this change until the version-specific workaround is in place. This way, it will work for all versions in the meantime.

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