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

Use own ASM classes instead of depending on external ones #103

Open
aaime opened this issue Oct 22, 2016 · 10 comments
Open

Use own ASM classes instead of depending on external ones #103

aaime opened this issue Oct 22, 2016 · 10 comments

Comments

@aaime
Copy link

aaime commented Oct 22, 2016

Hi,
I'm working in a project where parboiled is used along with other libraries that also use ASM, but depend on a different version of it.
The ASM FAQ suggest that tools using ASM should repackage it:
http://asm.ow2.org/doc/faq.html#Q15

Could parboiled-java do that? I'm not familiar with sbt but I can try to learn and help with that. Is a 1.1.8 release planned anytime soon?

@aaime
Copy link
Author

aaime commented Oct 22, 2016

Found a sbt plugin that could maybe used to perform the repackaging: https://github.com/Tapad/sbt-jarjar

@aaime
Copy link
Author

aaime commented Nov 3, 2016

Bump ;-)

@sirthias
Copy link
Owner

sirthias commented Nov 3, 2016

Thanks Andrea, for opening this ticket.
However, as you've probably seen from the commit and release list, I have basically zero time for further work on parboiled and will therefore not get this change.
I'd have to rely on someone to take over maintenance of this library...

@aaime
Copy link
Author

aaime commented Nov 3, 2016

Eh, while I'm willing to help for this particular task, I don't understand parsers well enough to help with a full blown maintenance (beside already leading on two mamooth sized projects, geotools and geoserver). Thanks for letting me know anyways, I'll check my options... might end doing a tiny fork just to fix this particular issue, parboiled works well for me as-is besides this packaging issue.

@wenerme
Copy link

wenerme commented Feb 22, 2017

When will parboiled use own ASM classes ?

java.lang.RuntimeException: Error creating extended parser class: INVOKESPECIAL/STATIC on interfaces require ASM 5

	at org.parboiled.Parboiled.createParser(Parboiled.java:58)
	at com.my.ExpParserTest.test(ExpParserTest.java:18)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:497)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:51)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:237)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:497)
	at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)
Caused by: java.lang.IllegalArgumentException: INVOKESPECIAL/STATIC on interfaces require ASM 5
	at org.objectweb.asm.MethodVisitor.visitMethodInsn(Unknown Source)
	at org.objectweb.asm.tree.MethodNode.visitMethodInsn(Unknown Source)
	at org.objectweb.asm.ClassReader.a(Unknown Source)
	at org.objectweb.asm.ClassReader.b(Unknown Source)
	at org.objectweb.asm.ClassReader.accept(Unknown Source)
	at org.objectweb.asm.ClassReader.accept(Unknown Source)
	at org.parboiled.transform.ClassNodeInitializer.process(ClassNodeInitializer.java:63)
	at org.parboiled.transform.ParserTransformer.extendParserClass(ParserTransformer.java:44)
	at org.parboiled.transform.ParserTransformer.transformParser(ParserTransformer.java:39)
	at org.parboiled.Parboiled.createParser(Parboiled.java:54)
	... 28 more

@aaime
Copy link
Author

aaime commented Jun 5, 2017

I see there has been a release in January 2017 (which was unexpected to me).
If I make a pull request to embed ASM maybe it can be considered for a future release? :-)

@ben-manes
Copy link

This plays poorly with other dependencies and, since Gradle 4 changed the classpath ordering, I see breaks depending on the OS being run on. This is being used by jtwig and shading ASM is a well known best practice. It really shouldn't be a major issue to address and release, especially since @aaime offered to help.

@TuomasKiviaho
Copy link

There is nowadays an ASM6 available that is needed for jdk9/10 support.

@sirthias I tried to switch to it but RuleMethodInterpreter seems to be inheriting the wrong constructor. By declaring ASM5 support like below one can assure this lib will continue to work with the newer ASM versions.

    public RuleMethodInterpreter(RuleMethod method) {
    	super(ASM5);
        this.method = method;
    }

This was the only problem that prevented me from running with ASM6 out-of-the-box just by bumping up the version.

@TuomasKiviaho
Copy link

There seems to be even a pull request available for what I mentioned before #117

@aaime
Copy link
Author

aaime commented May 31, 2019

I reiterate my interest in helping out with this one, and shading the asm dependency.
I'm reaching a point where I'll have to either fork the project and shade, or switch to some other parser.

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