-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
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.8
was removing fromLineNumberTable
entries with zero line numbers (see stacktrace ofRuntimeException
andclassinfo
output below) due to a bug in ASM (https://gitlab.ow2.org/asm/asm/-/issues/317989), this was fixed by an upgrade of ASM to9.5
in JaCoCo version0.8.9
(#1416), which unfortunately also lead to the insertion of more probes (seeexecinfo
output below) and so broke exec-file compatibility (seeArrayIndexOutOfBoundsException
andIllegalStateException
below).Using the following
Generator.java
which generates class file with zero line number for a method invocation instructionexecution of
produces
Before this change
Usage of JaCoCo version
0.8.8
for instrumentation and versions0.8.9
or0.8.10
for analysisleads to
ArrayIndexOutOfBoundsException
:Usage of JaCoCo version
0.8.8
together with versions0.8.9
or0.8.10
for instrumentationleads to
IllegalStateException
at analysis timeUsage of JaCoCo versions
0.8.9
or0.8.10
for instrumentation and analysisproduces
whereas usage of JaCoCo versions
0.8.9
or0.8.10
for instrumentation and version0.8.8
for analysisleads to a misleading report
After this change
Usage of JaCoCo version
0.8.8
for instrumentation and version with this change for analysisdoes not lead to
ArrayIndexOutOfBoundsException
:however
usage of JaCoCo versions
0.8.9
or0.8.10
for 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.9
or0.8.10
for instrumentationwill still lead to
IllegalStateException
at analysis time.Fixes #1471