-
Notifications
You must be signed in to change notification settings - Fork 459
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
Fix UnsatisfiedLinkError we've been seeing in nodejs formatters #586
Conversation
…al cache, which is pretty complicated.
…tless-nodejs-cache`.
@simschla I did some hacking to fix the UnsatisfiedLinkError, and also to keep the dll's out of the userhome. In particular, we used to do
which I removed in c5ae0fb. I'm on a windows machine, and it was throwing console errors, and after I removed it everything still worked fine (unit tests passed, and it worked for my projects). Did I make a mistake and I should have left it in? |
@nedtwigg Thank you for that (not so simple) fix 👍 I wasn’t able to fix it in a way that it would not throw console errors. Your solution looks good to me! thank you! |
@nedtwigg Nice one (even if I'm not a fan of that reflection based code, but I understand there no better solution right now). Is this only for the gradle plugin? |
The bug only showed up if J2V8 was loaded by multiple classloaders within one JVM. Spotless caches these classloaders (to allow JIT warmup), so the bug is only triggered if some other daemon loads Spotless more than once. So if you don't use the build daemon, you would never see this bug in Gradle. I don't know maven well, but I expect that this would not affect maven users, but maybe it does? Regardless, it is in the master branch of maven, and will be in the next release. |
@nedtwigg the directory where the J2V8 was installed is an issue in Maven plugin as well, also the "setFlags", "-color=false" That's why I was asking if this is only for the gradle plugin? As only the gradle changes file was amended I guess this has no effect in Maven plugin? Thx |
I believe the maven changes was also updated, with the J2V8 install change mentioned. I didn't mention the setFlags thing, happy to take a PR to improve the changelog if there's anything missing or confusing :) https://github.com/diffplug/spotless/pull/586/files#diff-dba632fdc53b52d0b261faf5130512c7 |
@nedtwigg ah, seems I somehow missed that. But yes, now I see the maven part was changed as well. Good stuff 👍 |
Fixed in |
Fixes #302, and also keeps the J2V8 dll out of the userhome directory.