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

backport current buildscripts to 1.12.2 #3856

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

wagyourtail
Copy link
Collaborator

@wagyourtail wagyourtail commented Feb 28, 2023

Backports current buildscripts to 1.12.2, and adds legacy fabric support... ( also a bump to unimined version to fix some issues 🙃 )

So basically, no more setupDecompWorkspace, and stuff is under unimined instead of forgegradle, genIntellijRuns may require you to set the java version to 8 afterwards for launching in dev if you have 17 set as the sdk in idea...

TODO

  • update setup.md

supersedes #3855

@wagyourtail
Copy link
Collaborator Author

wagyourtail commented Feb 28, 2023

when porting up, until 1.17, no build files should change except gradle.properties, so if something changes one of these files it'll probably be wrong... also ci only needs gradle build not the ones for forge/fabric now.

at 1.17, there will be a mapping change to official mojang mappings...
this uses mappings "net.minecraft:minecraft:${project.minecraft_version}:client-mappings"
you can also enable parchment there. see 1.19.3's build.gradle for details

@leijurv
Copy link
Member

leijurv commented Mar 1, 2023

will this work easily with #3855? such as for ImpactDevelopment/maven@155f713 as used by lambda-client/lambda@90d5575

currently i can just do ./gradlew jar to get a mcp deobf jar, is that still the case?

@leijurv
Copy link
Member

leijurv commented Mar 1, 2023

when porting up, until 1.17, no build files should change

sounds like a great use case for the git fake merge (example: git reset --hard $(git commit-tree 1.13.2^{tree} -p 1.13.2 -p master -m "fake merge of master into 1.13.2")) :trollface:

@wagyourtail
Copy link
Collaborator Author

wagyourtail commented Mar 1, 2023

will this work easily with #3855? such as for ImpactDevelopment/maven@155f713 as used by lambda-client/lambda@90d5575

this is set up for maven publish to publish artifacts similarly to arch-loom's output... also srg is the proper target so they dont need the exact same mcp mappings and fg.deobf will work. maven publish local it and see the output... because it is slightly different than the artifacts baritone previously published, but yes they should be able to handle the outputs

Copy link
Collaborator

@ZacSharp ZacSharp left a comment

Choose a reason for hiding this comment

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

Dist jars have the appropriate mappings for their platform, tweaker:runClient and fabric:runClient seem to work, forge:runClient is broken and :runClient surprisingly existed and started fabric.

What's the purpose of the "-dev" and "-shadow" jars? E.g. the fabric "-dev" jar doesn't contain any classes.

Is there any way to run proguard less often? Running it six times per build isn't quite ideal.

build.gradle Outdated Show resolved Hide resolved
Comment on lines +200 to +209
//TODO: fix for j16
// final JavaVersion javaVersion = new DefaultJvmVersionDetector(new DefaultExecActionFactory(new IdentityFileResolver())).getJavaVersion(java);
//
// if (!javaVersion.getMajorVersion().equals("8")) {
// System.out.println("Failed to validate Java version " + javaVersion.toString() + " [" + java + "] for ProGuard libraryjars");
// // throw new RuntimeException("Java version incorrect: " + javaVersion.getMajorVersion() + " for " + java);
// return false;
// }
//
// System.out.println("Validated Java version " + javaVersion.toString() + " [" + java + "] for ProGuard libraryjars");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any update hints/notes on this one? Or will you resolve it before merging or can I just ignore it when merging up?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ignore, it doesn't matter, actually that's probably been resolved now that we've upgraded proguard, I also put a little if statement in place to make java8 and java17 both work at

if (JavaVersion.current().isJava9Compatible()) {
template.add(2, "-libraryjars <java.home>/jmods/java.base.jmod(!**.jar;!module-info.class)");
template.add(3, "-libraryjars <java.home>/jmods/java.desktop.jmod(!**.jar;!module-info.class)");
template.add(4, "-libraryjars <java.home>/jmods/jdk.unsupported.jmod(!**.jar;!module-info.class)");
template.add(5, "-libraryjars <java.home>/jmods/java.compiler.jmod(!**.jar;!module-info.class)");
} else {
template.add(2, "-libraryjars <java.home>/lib/rt.jar");
}

fabric/src/main/resources/fabric.mod.json Outdated Show resolved Hide resolved
forge/build.gradle Outdated Show resolved Hide resolved
@ZacSharp
Copy link
Collaborator

ZacSharp commented Mar 1, 2023

sounds like a great use case for the git fake merge (example: git reset --hard $(git commit-tree 1.13.2^{tree} -p 1.13.2 -p master -m "fake merge of master into 1.13.2")) :trollface:

There's other changes so I'll do a normal merge and then checkout the old file.
Also thanks @wagyourtail for the detailed update instructions.

also srg is the proper target so they dont need the exact same mcp mappings and fg.deobf will work

just gotta repeat that. Mcp/yarn mapped is almost as bad as notch mapped because it forces you to use the same mappings version as Baritone.

Could just drop this into my libs folder and it worked

2023-03-02_00 28 07
(I was using a srg mapped Baritone build all the time though so not the same situation as lambda)

@wagyourtail
Copy link
Collaborator Author

Dist jars have the appropriate mappings for their platform, tweaker:runClient and fabric:runClient seem to work, forge:runClient is broken and :runClient surprisingly existed and started fabric.

hmm, weird forge:runClient didn't... genIntellijRuns might make a working forge run... I'll have to look into that at some point tho.

What's the purpose of the "-dev" and "-shadow" jars? E.g. the fabric "-dev" jar doesn't contain any classes.

-dev is just jaring the results of the current project, see in tweaker it'll just have the 2 classes defined there, and in forge same thing with the BaritoneForgeModXD class.
-shadow then shades in the "common" classes before remapping
and then finally it remaps to the final jar

Is there any way to run proguard less often? Running it six times per build isn't quite ideal.

I'm not a proguard expert. if there's a way to overlap it maybe... but there are technically 6 distinct builds. We could probably run proguard once, export the mappings and then do the others with tiny remapper, but the other optimiazations wouldn't be achieved that way

@wagyourtail
Copy link
Collaborator Author

I rolled high on a d20 so "baritoe" it is

@ZacSharp ZacSharp self-requested a review March 14, 2023 02:24
build.gradle Outdated Show resolved Hide resolved
Co-authored-by: ZacSharp <[email protected]>
@ZacSharp
Copy link
Collaborator

ZacSharp commented Mar 19, 2023

The server thread crashes when joining a world on tweaker:runClient.
java.lang.IncompatibleClassChangeError: Class com.google.common.base.Functions$ForMapWithDefault does not implement the requested interface java.util.function.Function
        at net.minecraft.advancements.Advancement$Builder.resolveParent(Advancement.java:157) ~[Advancement$Builder.class:?]
        at net.minecraft.advancements.AdvancementList.loadAdvancements(AdvancementList.java:64) ~[AdvancementList.class:?]
        at net.minecraft.advancements.AdvancementManager.reload(AdvancementManager.java:74) ~[AdvancementManager.class:?]
        at net.minecraft.advancements.AdvancementManager.(AdvancementManager.java:66) ~[AdvancementManager.class:?]
        at net.minecraft.world.WorldServer.init(WorldServer.java:164) ~[WorldServer.class:?]
        at net.minecraft.server.integrated.IntegratedServer.loadAllWorlds(IntegratedServer.java:98) ~[IntegratedServer.class:?]
        at net.minecraft.server.integrated.IntegratedServer.init(IntegratedServer.java:130) ~[IntegratedServer.class:?]
        at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:434) [MinecraftServer.class:?]
        at java.lang.Thread.run(Thread.java:750) [?:1.8.0_362]

@wagyourtail
Copy link
Collaborator Author

damn, thought I'd fixed that bug.... I'll look into it

@wagyourtail
Copy link
Collaborator Author

The server thread crashes when joining a world on tweaker:runClient.

fixed

Copy link
Collaborator

@ZacSharp ZacSharp left a comment

Choose a reason for hiding this comment

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

Not sure anymore whether this was the error from forge:runClient all the time so I'll include it here just in case.
[23:35:26] [main/ERROR] [LaunchWrapper]: Unable to launch
java.lang.reflect.InvocationTargetException: null
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_362]
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_362]
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_362]
        at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_362]
        at net.minecraft.launchwrapper.Launch.launch(Launch.java:135) [launchwrapper-1.12.jar:?]
        at net.minecraft.launchwrapper.Launch.main(Launch.java:28) [launchwrapper-1.12.jar:?]
Caused by: joptsimple.MissingRequiredOptionsException: Missing required option(s) [accessToken, version]
        at joptsimple.OptionParser.ensureRequiredOptions(OptionParser.java:426) ~[OptionParser.class:?]
        at joptsimple.OptionParser.parse(OptionParser.java:400) ~[OptionParser.class:?]
        at net.minecraft.client.main.Main.main(Main.java:51) ~[Main.class:?]
        ... 6 more

tweaker:runClient works now so everything which used to work is working and I can't complain 😄
(I did test a built jar as well)

@wagyourtail
Copy link
Collaborator Author

wagyourtail commented Mar 21, 2023

ok, well that's also fixed now... (the task from genIntellijRuns worked before that commit for testing forge, but now it's properly fixed)

@ZacSharp
Copy link
Collaborator

On the idea of running proguard only once:
If remapping is faster than proguard we could first build everything, put it into one big jar, run it through proguard together and then split the loader specific parts back out and remap the resulting 9 jars to notch/srg/intermediary.
Apart from this being a weird way of ordering things the only problem I see so far is Forge modifying the Minecraft code so we might get problems providing a minecraft library to proguard (because there's two) or convincing proguard everything is indeed fine.
The loader specific parts would need to be contained within their own package and proguard must not move code across those package boundaries, because otherwise we can't cleanly separate the code for the different loaders afterwards.

@wagyourtail
Copy link
Collaborator Author

wagyourtail commented Jul 24, 2023

since we don't depend on any forge patches, the vanilla jar would be fine...
one option could be to not obfucsate the loader specific class names/packages.

@ZacSharp
Copy link
Collaborator

Since the loader specific parts depend on main and api we have to run them through proguard, but explicitly marking them as kept should have the required effect.
If we want to not run them through proguard at all (and hence not merge them) we need a new internal api package / source set which is never obfuscated and used as a dependency by both main and the loader specific parts.

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.

3 participants