Restore exec file compatibility after upgrade of ASM to version 9.5#1492
Merged
marchof merged 1 commit intojacoco:masterfrom Oct 10, 2023
Merged
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
marchof
reviewed
Oct 9, 2023
| assertEquals(1, nextProbeId); | ||
| // workaround for zero line number can be removed if needed | ||
| // during change of exec file version | ||
| assertEquals(0x1007, ExecutionDataWriter.FORMAT_VERSION); |
Member
Author
There was a problem hiding this comment.
@marchof I have a strong feeling of déjà vu... oh no, actually it indeed happened - #636 (comment) 😆
42e6233 to
9f007ba
Compare
marchof
approved these changes
Oct 10, 2023
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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