-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Restore exec file compatibility after upgrade of ASM to version 9.5 #1492
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
Restore exec file compatibility after upgrade of ASM to version 9.5 #1492
Conversation
ae7ac80 to
0c7757a
Compare
0c7757a to
cb8a3b4
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
|
@Godin it worked with snapshot version. We are using jacoco jenkins plugin to publish the report so i had to build that plugin locally with the snapshot version provided by you and things worked after that. |
|
@Godin sorry to asking question again. As we verified and working fine, when can we expect the release for the same? |
cb8a3b4 to
42e6233
Compare
| assertEquals(1, nextProbeId); | ||
| // workaround for zero line number can be removed if needed | ||
| // during change of exec file version | ||
| assertEquals(0x1007, ExecutionDataWriter.FORMAT_VERSION); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love this idea! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marchof I have a strong feeling of déjà vu... oh no, actually it indeed happened - #636 (comment) 😆
42e6233 to
9f007ba
Compare
JaCoCo version
0.8.8was removing fromLineNumberTableentries with zero line numbers (see stacktrace ofRuntimeExceptionandclassinfooutput below) due to a bug in ASM (https://gitlab.ow2.org/asm/asm/-/issues/317989), this was fixed by an upgrade of ASM to9.5in JaCoCo version0.8.9(#1416), which unfortunately also lead to the insertion of more probes (seeexecinfooutput below) and so broke exec-file compatibility (seeArrayIndexOutOfBoundsExceptionandIllegalStateExceptionbelow).Using the following
Generator.javawhich generates class file with zero line number for a method invocation instructionexecution of
produces
Before this change
Usage of JaCoCo version
0.8.8for instrumentation and versions0.8.9or0.8.10for analysisleads to
ArrayIndexOutOfBoundsException:Usage of JaCoCo version
0.8.8together with versions0.8.9or0.8.10for instrumentationleads to
IllegalStateExceptionat analysis timeUsage of JaCoCo versions
0.8.9or0.8.10for instrumentation and analysisproduces
whereas usage of JaCoCo versions
0.8.9or0.8.10for instrumentation and version0.8.8for analysisleads to a misleading report
After this change
Usage of JaCoCo version
0.8.8for instrumentation and version with this change for analysisdoes not lead to
ArrayIndexOutOfBoundsException:however
usage of JaCoCo versions
0.8.9or0.8.10for instrumentation and version with this change for analysiswill still lead to a misleading report
and usage of JaCoCo version with this change together with versions
0.8.9or0.8.10for instrumentationwill still lead to
IllegalStateExceptionat analysis time.Fixes #1471