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

Add 1.13.2 support #11

Open
wants to merge 4 commits into
base: dev/1.6
Choose a base branch
from

Conversation

ThexXTURBOXx
Copy link

First of all: Thank you very much for this fork of architectury-loom!
However, I have found a few issues related to 1.13.2 - it just did not want to work properly.
After some tinkering around with various configurations, I found the culprits:

  • The MCP repo patch needs to be applied to 1.13.2 configurations as well, however not the rest of the fixes related to FG2
  • Intermediary and mojmap are not available for 1.13.2 -> handle those like in 1.12.2
  • A few tiny things here and there

For this to work, the new implementation relies on the spec version (for some older Forge 1.13.2 versions we need to hack it to 2, though).

Furthermore, during this whole process I found that the LZMA files that are being generated by legacy configurations are "damaged" - this is due to ning/jvm-compressor-benchmark#1, which is referenced in jponge/lzma-java#9.
Since org.tukaani:xz:1.8 was already a (probably runtime (?)) dependency, I removed lzma-java and migrated the code to xz, which does not have any issues with silenced OOMs anymore.

@ThexXTURBOXx
Copy link
Author

I have even added partial 1.7.10 support now: https://github.com/ThexXTURBOXx/architectury-loom/tree/dev/1.6-1.7.10
The only that does not work yet is: It cannot resolve https://maven.minecraftforge.net/net/minecraftforge/forge/1.7.10-10.13.4.1614-1.7.10/forge-1.7.10-10.13.4.1614-1.7.10.pom
And indeed, that POM does not exist for 1.7.10 and below - I have no clue on how to fix this as of now.
So, any tips/feedback would be greatly appreciated!

As a workaround (to test the current 1.7.10 configuration), one can add https://femtopedia.de/maven/ as a maven repository.
I have added the necessary files there: https://femtopedia.de/maven/net/minecraftforge/forge/1.7.10-10.13.4.1614-1.7.10/
The POM is just some 1.8.9 POM that has been modified to look like it was a legitimate 1.7.10 POM.
Just by adding this repository, it indeed works fine on my machine at least - will test in my CI/CD workflow soon.
If it also works there, the only issue that still needs fixing is the missing POM - after that I will merge the changes into this PR

@ThexXTURBOXx
Copy link
Author

ThexXTURBOXx commented Jun 20, 2024

I have finally fixed the issue, but the approach became way more hacky than I would like it to be: ThexXTURBOXx/architectury-loom@dev/1.6...ThexXTURBOXx:architectury-loom:dev/1.6-1.7.10
Basically, by enabling sources.artifact() for the Forge maven repo, I can bypass the POM download.
However, still, Loom needs the POM to set the ForgeVersion correctly.
I skip past this by setting the resolved version manually before the provider resolves the dependencies on its own.
In order for all of this to work properly, I added an extra dependency configuration forgeLegacy, which can be used to tell the system that resolving the version won't work.
Yes, all of this is very hacky. Because of this, i won't push my 1.7.10 branch in here yet.
If someone has any suggestions, feel free to let me know!

Edit: At least I can verify that my branch also works fine in my CI/CD configuration!

@ThexXTURBOXx
Copy link
Author

Rebased on upstream

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.

1 participant