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

Decompile Failure #26

Closed
russjr08 opened this issue Dec 13, 2013 · 30 comments
Closed

Decompile Failure #26

russjr08 opened this issue Dec 13, 2013 · 30 comments

Comments

@russjr08
Copy link

setupDecompWorkspace seems to be crashing and burning right now:
http://pastebin.com/QJnvujYH

EVERYONE! SEE THE LAST COMMENT FOR THE SOLUTION

-- Abrar

@AbrarSyed
Copy link
Member

Like 3 people have had this error now... And I have no clue why... I cant
reproduce at all, when after clearing cache and everything...
On Dec 13, 2013 11:39 AM, "Russell" notifications@github.com wrote:

setupDecompWorkspace seems to be crashing and burning right now:
http://pastebin.com/QJnvujYH


Reply to this email directly or view it on GitHubhttps://github.com//issues/26
.

@russjr08
Copy link
Author

Is there any kind of log files that may help diagnose the issue that I can provide? Not too familiar with gradle yet.
Also, how do I clear Gradle's caches? I've tried running the command with the --refresh-dependencies flag and that doesn't seem to be helping either.

@AbrarSyed
Copy link
Member

gradlew cleanCache clean setupDecompWorkspace -d --refresh-dependencies
then give me the log file in projectDir/.gradle/gradle.log

@russjr08
Copy link
Author

Just booted into my Windows partition (Was on OS X) and tried it. Works perfectly... It's probably an upstream issue then with whatever tool does the patches unfortunately. For now I'll just have to do the decomp on Windows and possibly copy it back to OS X if that's where I'm going to work.

Here's the gradle.log file (Had to upload it to mediafire, pastebin wouldn't accept it.)

https://www.mediafire.com/?eo31641bho61qnc

@LexManos
Copy link
Member

You problem need to alloacate more ram to the default java process, those two classes are LARGE and can sometimes OOM when decompiling. Not sure how to do that as I dont have a mac, but im sure google, or your experience will tell you.

@russjr08
Copy link
Author

I looked around and apparently if you open the gradle wrapper file, there's a default JVM flags option that you can set. I tried allocating more ram, and it did speed up the process considerably, however it still seems to be crashing at the same step.
I'm at a loss as to what's going on here.

@AbrarSyed
Copy link
Member

you can use this to set the gradle memory stuff...
http://www.gradle.org/docs/current/userguide/build_environment.html

@chrixian
Copy link

Short answer: @russjr08 to fix on your end with no changes to ForgeGradle-- the User-Agent of the HTTP requests in your log indicate you're using Java 6, upgrade to Java 7 and you won't encounter this error. @AbrarSyed to fix on ForgeGradle's end for people still using Java 6 for whatever reason, add exec.setMaxHeapSize("512m") to the DecompileTask.

Long answer: With Minecraft 1.6 fernflower started having a memory issue on the Mac when using Java 6... you could workaround this by prepending -Xxm512m to fernflower's configuration options in the conf/mcp.cfg file, which FML would do for you, so the end user didn't have to worry about it.

Modifying the Gradle wrapper like you tried, or setting JAVA_OPTS or GRADLE_OPTS or any of the daemon information contained in "the gradle memory stuff" link above, won't have an effect here because you'd only be setting options in the context the build script is running and not any forked child processes created by exec tasks as they determine their own JVM arguments at creation.

When Gradle is used as designed this would be trivial to change for the end user as it would just involve finding the JavaExec task in question and adding/modifying its maxHeapSize property. Unfortunately, for whatever reason, Forge has opted to implement nearly 100% of the build as a buildScript dependency in an external, precompiled library written in Java thus making it monolithic to the end user and requiring an entire redeployment for even the most minor of tweaks.

I'll bite my tongue and not go on since this isn't the place for any constructive discussion anyway-- in any case for anyone using Java 6 and encountering this bug you can't do anything other than upgrade to Java 7 because the JVM arguments of Fernflower's execution are beyond your reach, so take it as a sign it's time to move into 2011 and update your JVM.

@AbrarSyed
Copy link
Member

Indeed, this is not the place for this discussion, however, I see a point in your arguments.

Regarding FernFlower, that is indeed going to be moved to its own task very soon, and when that happens it will be editable.

Regadring a monolithic library, you are always free to put it in the BuildSrc yourself and have it build that way, but having it they way we have it right now allows us to seamlessly push updates, a feature which I have been using tirelessly.

Come on iRC sometime, we can talk about all of this.

Also.. can you verify that your fix is working? my analysis has pointed at something entirely unrelated to FernFlower being the culprit, and the issue is not isolated to Java 6 as far as I have seen, this guy here isnt the only one having it, but it isnt an epidemic either..

@chrixian
Copy link

Ya, the actual problem is the heap size and Fernflower and not necessarily Java 6 related so setting maxHeapSize in the task will correct.. Fernflower doesn't throw an exception or complain in any way when it happens it just produces code with missing values.

@chrixian
Copy link

Scratch that, using setMaxHeapSize didn't fix the problem to my surprise.

Alright, I'm annoyed and will devout my morning to this and hit you up on IRC after I figure it out.

@chrixian
Copy link

Alright I got around to looking at it closer and it appears the culprit is in FFPatcher, specifically the TRAILINGZERO pattern being used here with replaceAll when the intention was probably to concat the two matching groups together rather than replace the whole thing with nothing.

AbrarSyed added a commit that referenced this issue Dec 14, 2013
@AbrarSyed
Copy link
Member

untested, but I hope this work. Il reoppeen it if I hear the problem persists.

SOLUTION

  • install java 7
  • gradlew cleanCache clean setupDecompWorkspace --refresh-dependencies

@chrixian
Copy link

That did fix the issue with the missing values but unfortunately there are at least 5 places I see where the output directly from FernFlower is just plain different when using Java 6 vs using Java 7 on my Mac ... here's an example

Java 7 works as expected, and should probably be a requirement for Mac usage.

@LexManos
Copy link
Member

That has nothing to do with the JVM/OS. Thats just the fact that FF is non-deterministic.
The two styles result in the same functional code.

@jeffreykog
Copy link

It IS the same functional code, but with these differences in it, it is actually able to derp up the source patcher.

@LexManos
Copy link
Member

Possible yes, but we don't patch those parts. And we have support in the 1.7 branch for multiple patch options.

@chrixian
Copy link

So what are you saying Lex? The entire point here is that in some conditions the code produced by FF is different than that which is expected and it's resulting in failure when attempting to patch RenderBlock.java and RenderItem.java.

The semantics of whether the code is functionally equivalent is irrelevant, my reference to RConThreadClient was merely an example showing a command in the same environment with only the JVM differing resulting in varying output. I can further post the full output with -verbose on showing FernFlower making use of different reflection classes between the two versions and then falling back to some regular expression matching in other. So ya, JVM has something to do with it.

@AbrarSyed
Copy link
Member

we know thats not the problem because we tried it with two identical JVMs, and one failed while the other did not.

@AbrarSyed AbrarSyed reopened this Dec 15, 2013
@LexManos
Copy link
Member

FF Decompile issues are NOT JVm specific, know how you can test this? Decompile minecraft ~10 times, you should at least get one that is decompiled differently in a few classes.
But as I said, we do not patch those cases.
And if you actually looked into any of the data from users who have this issue it's fairly easy to see what is going wrong. Because you can compare there source to the one the patches expect quite easily.

The issue here is something that AbrarSyed needs to work out, somehow, for certain users, the FmlCleanup code is grabbing wrong/unsorted names.
https://github.com/MinecraftForge/ForgeGradle/blob/master/src/main/java/net/minecraftforge/gradle/sourcemanip/FmlCleanup.java#L164

I'm leaving this up to Abrar to fix as it should be a simple issue. If someone else wants to take a crack at it go ahead. However, my time is better spent working on the 1.7 update.

@AbrarSyed
Copy link
Member

@russjr08 does this problem persist given the solution I posted a few days ago?

@russjr08
Copy link
Author

@AbrarSyed Nope! The solution works perfectly. On behalf of all of the people who had this issue, thank you!

@chrixian
Copy link

@russjr08 i was curious... was the solution to upgrade to java 7 or did the trailing zero patch allow you to continue using java 6?

@russjr08
Copy link
Author

@chrixian A combination of both. I upgraded to Java 7, yet it still didn't work until the Trailing Zero issue was fixed.

@chrixian
Copy link

Awesome, that is what I would have expected.. Thanks 👍

@russjr08
Copy link
Author

No problem. Closing this issue now that it's resolved. For anyone looking at this issue wondering how to fix the problem, view Abrar's solution here: #26 (comment)

@AbrarSyed
Copy link
Member

odd thing is.. other people are getting this error.. and its NOT fixed for them....

@chrixian
Copy link

Well I can't speak to the issue LexManos was referring to involving FmlCleanup but "this" issue here was different than any others involving failed patching... resulting in the trailing zero thing produced on Macs ...

I'd like to continue to help... not having all the information from those "other people" that reported to you elsewhere makes it difficult ... like earlier when you said "we know thats not the problem because we tried it with two identical JVMs, and one failed while the other did not" ... none of the other details had been reported in this thread so... had I known that I wouldn't have made assumptions about the JVM/Platform being suspect ...

So have people start providing information on github ... in a new issue perhaps ...

@AbrarSyed
Copy link
Member

I have done a lot of working with someone who has had the issue..
and have narrowed it down to the fac that it is indeed FernFlower.... I have no idea WHY this is the case though...

and its all surrounding a single change in variable order....

@AbrarSyed
Copy link
Member

so.. final fixing thing here.... this VERIFIED works...

SOLUTION

  • Install latest possible java
  • uninstall all older forms of java
  • set JAVA_HOME system variable (google this, its quite easy)
  • gradlew cleanCache clean setupDecompWorkspace --refresh-dependencies

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

No branches or pull requests

5 participants