Col-E / Recaf

The modern Java bytecode editor
https://recaf.coley.software
MIT License
6.03k stars 464 forks source link

2.0 does not patch itself #197

Closed blshkv closed 4 years ago

blshkv commented 4 years ago

openjdk-bin-11

bash$ java -jar recaf-1.15.10.jar 
Picked up _JAVA_OPTIONS: -Dawt.useSystemAAFontSettings=on
Detected JDK 11+ without OpenJFX dependencies
Dependencies will be downloaded and Recaf will restart...
Rerunning

bash $ java -jar recaf-2.0.0-J8-jar-with-dependencies.jar 
Picked up _JAVA_OPTIONS: -Dawt.useSystemAAFontSettings=on
INFO: Recaf-2.0.0
Exception in thread "main" java.lang.NoClassDefFoundError: javafx/concurrent/Task
        at me.coley.recaf.command.impl.Initializer.run(Initializer.java:43)
        at picocli.CommandLine.executeUserObject(CommandLine.java:1769)
        at picocli.CommandLine.access$900(CommandLine.java:145)
        at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2150)
        at picocli.CommandLine$RunLast.handle(CommandLine.java:2144)
        at picocli.CommandLine$RunLast.handle(CommandLine.java:2108)
        at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:1975)
        at picocli.CommandLine.execute(CommandLine.java:1904)
        at me.coley.recaf.Recaf.main(Recaf.java:41)
Caused by: java.lang.ClassNotFoundException: javafx.concurrent.Task
        at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
        at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
        at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
        ... 9 more
blshkv commented 4 years ago

I realised that J8 means java 1.8. So I have tried with this version:

bash$ java -jar recaf-2.0.0.jar 
Picked up _JAVA_OPTIONS: -Dawt.useSystemAAFontSettings=on
no main manifest attribute, in recaf-2.0.0.jar

bash$ java -jar recaf-2.0.0-J8-jar-with-dependencies.jar 
Picked up _JAVA_OPTIONS: -Dawt.useSystemAAFontSettings=on
INFO: Recaf-2.0.0
Exception in thread "main" java.lang.NoClassDefFoundError: javafx/concurrent/Task
        at me.coley.recaf.command.impl.Initializer.run(Initializer.java:43)
        at picocli.CommandLine.executeUserObject(CommandLine.java:1769)
        at picocli.CommandLine.access$900(CommandLine.java:145)
        at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2150)
        at picocli.CommandLine$RunLast.handle(CommandLine.java:2144)
        at picocli.CommandLine$RunLast.handle(CommandLine.java:2108)
        at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:1975)
        at picocli.CommandLine.execute(CommandLine.java:1904)
        at me.coley.recaf.Recaf.main(Recaf.java:41)
Caused by: java.lang.ClassNotFoundException: javafx.concurrent.Task
        at java.net.URLClassLoader.findClass(URLClassLoader.java:382)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:419)
        at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:352)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:352)
        ... 9 more

bash$  java -version
Picked up _JAVA_OPTIONS: -Dawt.useSystemAAFontSettings=on
openjdk version "1.8.0_242"
OpenJDK Runtime Environment (AdoptOpenJDK)(build 1.8.0_242-b08)
OpenJDK 64-Bit Server VM (AdoptOpenJDK)(build 25.242-b08, mixed mode)
Col-E commented 4 years ago

I just haven't gotten around to porting the functionality over.

However the 2.0.0 release can be downloaded/cloned and built easily against Java 11 with build then following the prompts.

Col-E commented 4 years ago

Added in ca0db6611fad80d2b7c0f7a548564f05d66e0c71

image image

Grab the latest pre-release and you should be good to go!

blshkv commented 4 years ago

sorry, not fixed:

bash$ java -version
Picked up _JAVA_OPTIONS: -Dawt.useSystemAAFontSettings=on
openjdk version "1.8.0_242"
OpenJDK Runtime Environment (AdoptOpenJDK)(build 1.8.0_242-b08)
OpenJDK 64-Bit Server VM (AdoptOpenJDK)(build 25.242-b08, mixed mode)

bash$ ~/Downloads $ java -jar recaf-2.0.0-J8-jar-with-dependencies.jar
Picked up _JAVA_OPTIONS: -Dawt.useSystemAAFontSettings=on
LOGGER WARN: 'recaf.home' could not be found in system properties
INFO: Missing JavaFX dependencies, attempting to patch in missing classes
INFO:  - Local cache found!
INFO:  - Loading dependencies...
INFO:  - Disabling access system
ERROR: Failed to add dependencies to classpath!: java.lang.ClassNotFoundException: jdk.internal.loader.ClassLoaders
        at java.net.URLClassLoader.findClass(URLClassLoader.java:382)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:419)
        at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:352)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:352)
        at java.lang.Class.forName0(Native Method)
        at java.lang.Class.forName(Class.java:264)
        at me.coley.recaf.util.self.SelfPatcher.loadFromCache(SelfPatcher.java:94)
        at me.coley.recaf.util.self.SelfPatcher.patch(SelfPatcher.java:59)
        at me.coley.recaf.Recaf.init(Recaf.java:87)
        at me.coley.recaf.Recaf.main(Recaf.java:41)
Col-E commented 4 years ago

Oh. I didn't account for Java 8 not bundling JFX...

Col-E commented 4 years ago

Looks like xxDark resolved my mistake.

Col-E commented 4 years ago

Added enhancement since it wasn't there in 2.0 before, added bug because it not working in 2.0 was my bad. Should be fixed. LMK if I need to open this again 👍

blshkv commented 4 years ago

still not fixed :( Again, I tried with both

  [1]   openjdk-bin-8
  [2]   openjdk-bin-11

java -jar ./recaf-2.0.0.4-J8-jar-with-dependencies.jar 
Picked up _JAVA_OPTIONS: -Dawt.useSystemAAFontSettings=on
INFO: Missing JavaFX dependencies, attempting to patch in missing classes
INFO:  - Local cache found!
INFO:  - Loading dependencies...
INFO:  - Disabling access system
INFO:  - Done!
INFO: Recaf-2.0.0
Exception in thread "main" java.lang.NoClassDefFoundError: javafx/concurrent/Task
        at me.coley.recaf.command.impl.Initializer.run(Initializer.java:43)
        at picocli.CommandLine.executeUserObject(CommandLine.java:1769)
        at picocli.CommandLine.access$900(CommandLine.java:145)
        at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2150)
        at picocli.CommandLine$RunLast.handle(CommandLine.java:2144)
        at picocli.CommandLine$RunLast.handle(CommandLine.java:2108)
        at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:1975)
        at picocli.CommandLine.execute(CommandLine.java:1904)
        at me.coley.recaf.Recaf.main(Recaf.java:42)
Caused by: java.lang.ClassNotFoundException: javafx.concurrent.Task
        at java.net.URLClassLoader.findClass(URLClassLoader.java:382)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:419)
        at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:352)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:352)
        ... 9 more

java -jar recaf-2.0.0.4.jar 
Picked up _JAVA_OPTIONS: -Dawt.useSystemAAFontSettings=on
no main manifest attribute, in recaf-2.0.0.4.jar
Col-E commented 4 years ago

Damn

blshkv commented 4 years ago

I'm not sure if it would help, but it works if I compile it from the source. I'm getting recaf-2.0.0-J11-jar-with-dependencies.jar binary

Col-E commented 4 years ago

Ideally it would work without having to compile it. That was the smaller J8 can be distributed and everyone works off of that when receiving updates.

blshkv commented 4 years ago

The patching itself idea seems very hackish. I know nothing about JavaFX, but shouldn't it be possible to drop its binaries next to the rest dependencies?

Col-E commented 4 years ago

The patching itself idea is seems very hackish.

To be fair, it sorta is. But the implementation in 2.0 is much better than 1.X. The new way just forcefully extends the classpath. The old way repackaged recaf and re-ran itself.

The problem is most distributions of Java 8 (Not OpenJDK) bundle JavaFX. I'm trying to reduce bloat for those who don't need to download these files by not including them in the release binary.

Plus, if this system works well enough, most or all dependencies can be injected this way. Currently, dependencies account for 95% of Recaf's distribution size. If this system can be perfected then Recaf's initial distribution size could be drastically reduced. Recaf's own classes & resources are just a bit over a megabyte. The dependencies are 24 Mb, or 34 Mb if you count JavaFX.

Or maybe I'm thinking way too into the whole optimization thing...

but shouldn't it be possible to drop its binaries next to the rest dependencies?

Not if you want it to be a double-click runnable jar. They'd have to be inside the jar. The goal of this system is to effectively do exactly this. Dynamically load existing jar files of dependencies.

blshkv commented 4 years ago

Can I suggest to forget about Java 8 and focus on 11? https://www.infoworld.com/article/3305073/removed-from-jdk-11-javafx-11-arrives-as-a-standalone-module.html (2018)

And I really don't care if the version with dependencies is 20 or 30Mb. Well, alternatively, you can add one more jar (with javafx) as an option

Correct me if I'm wrong but the future way would be:

Col-E commented 4 years ago

Correct me if I'm wrong but the future way would be ...

You don't need the 2nd step since they'd be referenced as a dependency. Just moving to Java 11 as a baseline would make everything so much simpler.

Can I suggest to forget about Java 8 and focus on 11?

I would love to focus on Java 11, but that simply isn't backed by statistics as far as I'm aware: https://snyk.io/blog/developers-dont-want-to-leave-java-8-as-64-hold-firm-on-their-preferred-release/ (TLDR: Java 8 = 64%, Java 11+=30%)

I don't want to lock Recaf behind a "go update Java" prompt. Especially if I have 1.15.X auto-update to 2.0.0 only for it to stop working.

And I really don't care if the version with dependencies is 20 or 30Mb.

Fair enough. If JavaFX 11 is bundled within the releases (even targeting Java 8 bytecode) I believe it still runs just fine. These classes just go unused assuming JavaFX is already on the system. If its not, then there's a "invalid class" error because the Java 8 VM won't support the Java 11 compiled JavaFX dependencies.

blshkv commented 4 years ago

I see .. so only systems with old Java 8 and no JavaFx will be affected. As you said, most distributions of Java 8 bundle JavaFX so it won't be many. For the rest ... "go update Java" ;-) .

Also, not sure about other distros, but Gentoo allows to install both java and switch between easily. So users don't need to worry about old setup, just install one more Java in parallel.

blshkv commented 4 years ago

Another point which I forgot to stress. The patch hack will not work if the package is installed properly, i.e. via a package installer as a system wide file. A regular user suppose to install it using pkg install recaf and update later pkg update. The self modification is malicious behaviour and against any distro linux policy.

P.S. I package Recaf for Pentoo Linux. So I have to run the tool, get a patch version and distribute it from our website which is a hack of your hack and unnecessary work for me

Col-E commented 4 years ago

And as you said, most distributions of Java 8 bundle JavaFX so it won't be many

Fair point again. The last point to beat is that there are natives that are unique per system for JavaFX. Recaf fetches the proper ones.

The self modification is malicious behaviour and against any distro linux policy. I package Recaf for Pentoo Linux. So I have to run the tool, get a patch version and distribute it from our website which is a hack of your hack and unnecessary work for me

Yeaaahhh, sorry about that.... Most windows users have no concept of updating software that isn't internal to the software. Hence the "we'll just do that for you" approach here.


Got any ideas for fitting all these goals? Because to me it seems near impossible to satisfy everything at once:

  1. Multi-platform (win/linux/mac)
    • Expected behavior on windows with auto-updates since there is no internal update system with the OS
  2. Multi-version (8/11+)
  3. Support automatic CI integration (See new drone configuration)
  4. Easy to use. java -jar ... or click-to-run
andylizi commented 4 years ago

Not if you want it to be a double-click runnable jar. They'd have to be inside the jar.

That's actually not necessary. MANIFEST.MF has a Class-Path option that does what its name says, so you can use relative paths like Class-Path: lib/javafx.jar. It's a rarely used feature as most of the time it's just easier to shade the dependencies in.

I had always thought the reason you didn't utilize this feature is because... you didn't want lots of files/folder cluttering the working directory? Or something like that? I seem to recall a similar topic we discussed a long time ago, but maybe I misremembered.

Col-E commented 4 years ago

It's a rarely used feature as most of the time it's just easier to shade the dependencies in.

Oh my god I forgot that was even possible. I think its been 4-5 years since I've done that.

you didn't want lots of files/folder cluttering the working directory?

I was actually for having everything be local until recently when I was convinced to use a standard home directory (system property/configurable).

Col-E commented 4 years ago

Ok, this time for real. It should be fixed. This was due to improper path handling.

The exception is when you have a Java 8 install without JavaFX (OpenJDK). In that case, you'll just have to update to Java 11. I or somebody else should probably add a prompt/note that indicates this when running Recaf.

Regarding discussion about the entire concept of this system, and other related ideas, lets move that to either the pinned discussion issue or the discord server.

blshkv commented 4 years ago

it is fixed indeed. Thanks!

ViRb3 commented 4 years ago

Just a side note, Amazon Corretto 8 is an OpenJDK 8 distribution with JavaFX bundled.