Include JaCoCo version in instrumentation/analysis exception messages #1217
Conversation
marchof
force-pushed
the
error-with-version
branch
from
140b97a
to
7091240
Compare
marchof
force-pushed
the
error-with-version
branch
from
7091240
to
95a2b3e
Compare
|
Another very convincing example - #1224 (comment) |
Godin
changed the title
This will hopefully help people to understand which JaCoCo version they are actually using in their build setups.
marchof
force-pushed
the
error-with-version
branch
from
95a2b3e
to
9e736bf
Compare
| @@ -19,9 +19,17 @@ | |||
| */ | |||
| public final class JaCoCo { | |||
|
|
|||
| /** Qualified build version of the JaCoCo core library. */ | |||
| /** Version of JaCoCo core. */ | |||
There was a problem hiding this comment.
@marchof this is indeed qualified version, i.e. in https://repo1.maven.org/maven2/org/jacoco/org.jacoco.core/0.8.7/org.jacoco.core-0.8.7.jar
VERSION=0.8.7.202105040129
So I'm wondering whether change of javadoc here implies that you expect(ed) to see
Error while analyzing Example.class with JaCoCo 0.8.8/95a2b3e2.
instead of
Error while analyzing Example.class with JaCoCo 0.8.8.202202010859/95a2b3e2.
?
There was a problem hiding this comment.
Ups my bad. I was wrongly assuming we have the plain version here. From my point of view the plain version would be enough here. What do you think, can we change this to project.version?
If we don't want to change this in this PR the qualified version is also good for me.
There was a problem hiding this comment.
What do you think, can we change this to
project.version?
Well I have mixed feelings:
Maybe not a big deal to change it, however strictly speaking it is part of our public API on it's own and a return value of Agent API org.jacoco.agent.rt.IAgent.getVersion, also it is shown by version command in CLI and always displayed in footer (right-bottom corner) of html reports.
And on one side, since qualifier is build timestamp, maybe nice that people will see how old version they are using. On the other side, since their configuration files use unqualified version, presence of qualifier might be confusing.
We can introduce VERSION_SHORT by analogy with COMMITID_SHORT, but maybe this is an overkill.
If we don't want to change this in this PR the qualified version is also good for me.
So let's keep it as is for now.
I was wrongly assuming we have the plain version here.
I'll add back word "Qualified" so that it can again remind us next time 😉
| @@ -1,3 +1,4 @@ | |||
| VERSION=${qualified.bundle.version} | |||
| COMMITID=${buildNumber} | |||
There was a problem hiding this comment.
@marchof hope people won't be faking this value in forks 😉
org.jacoco.agent.rt.test/src/org/jacoco/agent/rt/internal/CoverageTransformerTest.java
Outdated
Show resolved
Hide resolved
org.jacoco.agent.rt.test/src/org/jacoco/agent/rt/internal/CoverageTransformerTest.java
Outdated
Show resolved
Hide resolved
org.jacoco.agent.rt.test/src/org/jacoco/agent/rt/internal/ExceptionRecorder.java
Outdated
Show resolved
Hide resolved
Godin
force-pushed
the
error-with-version
branch
from
8f77b1a
to
2bd5a8f
Compare
Co-authored-by: Evgeny Mandrikov <138671+Godin@users.noreply.github.com>
Godin
force-pushed
the
error-with-version
branch
from
2bd5a8f
to
f9e49cf
Compare
| COMMITID_SHORT = COMMITID.substring(0, 7); | ||
| HOMEURL = bundle.getString("HOMEURL"); | ||
| RUNTIMEPACKAGE = bundle.getString("RUNTIMEPACKAGE"); |
There was a problem hiding this comment.
@marchof BTW fun fact:
COMMITID_SHORT is the first 7 digits of COMMITID computed at runtime,
RUNTIMEPACKAGE is the last 7 digits of COMMITID computed at build time
😆
| @@ -19,9 +19,17 @@ | |||
| */ | |||
| public final class JaCoCo { | |||
|
|
|||
| /** Qualified build version of the JaCoCo core library. */ | |||
| /** Version of JaCoCo core. */ | |||
There was a problem hiding this comment.
What do you think, can we change this to
project.version?
Well I have mixed feelings:
Maybe not a big deal to change it, however strictly speaking it is part of our public API on it's own and a return value of Agent API org.jacoco.agent.rt.IAgent.getVersion, also it is shown by version command in CLI and always displayed in footer (right-bottom corner) of html reports.
And on one side, since qualifier is build timestamp, maybe nice that people will see how old version they are using. On the other side, since their configuration files use unqualified version, presence of qualifier might be confusing.
We can introduce VERSION_SHORT by analogy with COMMITID_SHORT, but maybe this is an overkill.
If we don't want to change this in this PR the qualified version is also good for me.
So let's keep it as is for now.
I was wrongly assuming we have the plain version here.
I'll add back word "Qualified" so that it can again remind us next time 😉
This should help in situations where users cannot see which version is actually used, like in AGP builds.