search

benedekfazekas / mranderson

Dependency inlining and shadowing
Eclipse Public License 1.0
151 stars 14 forks source link

Error inlining cljfmt 0.7 #47

Closed bbatsov closed 3 years ago

bbatsov commented 3 years ago

I ran into the following problem when trying to release cider-nrepl 0.25.6:

lein with-profile +1.10,+plugin.mranderson/config deploy clojars
OpenJDK 64-Bit Server VM warning: Options -Xverify:none and -noverify were deprecated in JDK 13 and will likely be removed in a future release.
Compiling cider.nrepl.inlined-deps.cljfmt.v0v7v0.cljfmt.main
Syntax error macroexpanding at (diff.clj:1:1).
Execution error (ClassNotFoundException) at java.net.URLClassLoader/findClass (URLClassLoader.java:435).
cidernrepl0256.difflib.DiffUtils

It doesn't seem to be a regression in the most recent mranderson, as I got the same problem with the previous release as well. I've reverted to cljfmt 0.6.8, so I can release something but ideally we should fix this problem somehow.

You can easily reproduce this by running on the current master of cider-nrepl. I'm using JDK 14, but I'm not sure if that's relevant or the problem's there for every JDK.

benedekfazekas commented 3 years ago

thanks Bug for flagging, will have a look.

bbatsov commented 3 years ago

Thanks! I looked quckly what's changed in cljfmt and this commit is the likely cause of the breakage IMO https://github.com/weavejester/cljfmt/commit/7571de0760a9a26f5167349f8811f62597cb1c3d Probably something to do with the genclass.

benedekfazekas commented 3 years ago

no idea how the above commit caused this, there was an obviously wrong assumption in the function in mranderson when calculating all the java class prefixes to create the jarjar rule to rename with. assumption was that all packages at least two tokens deep so foo.bar.Baz for any Baz class. that is not true for cljfmt's difflib depedency causing some classes with only the difflib package not being inlined properly by the jarjar step.

hope this makes sense. tested this with cider-nrepl (install and smoketest targets) and refactor-nrepl (test target). please give it a spin, available as 0.5.3-SNAPSHOT on clojars

bbatsov commented 3 years ago

I can confirm that the fix works. Thanks!

benedekfazekas commented 3 years ago

cool, will release 0.5.3 shortly

bbatsov commented 3 years ago

@benedekfazekas Don't forget about this new release. :-)

benedekfazekas commented 3 years ago

and I did. shame on me as @liquidz reminded me too. released now, sry folks!

bbatsov commented 3 years ago

No worries!