Opened 20 months ago

Closed 2 months ago

#25217 closed enhancement (fixed)

metrics-base: Update checkstyle to debian stable

Reported by: iwakeh Owned by: karsten
Priority: Low Milestone:
Component: Metrics Version:
Severity: Minor Keywords:
Cc: metrics-team Actual Points:
Parent ID: Points:
Reviewer: irl Sponsor:

Description (last modified by iwakeh)

Verify that all works as expected (which should be the case) and update to the latest available and working version.

From Iain's mailing list post:

7.5 should be arriving in unstable soon and they are working on updating to 8.8

Child Tickets

TicketTypeStatusOwnerSummary
#25689taskclosedmetrics-teamProvide a backport to stretch for checkstyle in Debian

Change History (14)

comment:1 Changed 20 months ago by iwakeh

Description: modified (diff)

comment:2 Changed 20 months ago by iwakeh

Priority: MediumLow
Severity: NormalMinor

Setting to lesser prio and severity b/c we're waiting on Debian to upgrade to a version >= 6.17

comment:3 Changed 18 months ago by karsten

FWIW, Debian stable still has 6.15 (as expected), there is nothing in stretch-backports (also as expected), and both testing and unstable have 8.8: https://packages.debian.org/search?keywords=checkstyle

comment:4 in reply to:  3 Changed 18 months ago by iwakeh

Replying to karsten:

FWIW, Debian stable still has 6.15 (as expected), there is nothing in stretch-backports (also as expected), and both testing and unstable have 8.8: https://packages.debian.org/search?keywords=checkstyle

Yes, this is a ticket to not forget to change the non-debian dependency to a debian package when a working version is available.

comment:5 Changed 2 months ago by karsten

Owner: changed from metrics-team to karsten
Status: newassigned

I'm taking another look now, after having looked into updating the other libraries to buster.

comment:6 Changed 2 months ago by karsten

Status: assignedneeds_review

Alright, I got it working with Checkstyle 8.15 that comes with Debian buster.

This was not a one-line change, because the .jar file we used earlier contained all its dependencies whereas the one that comes with Debian does not (which makes sense). That's why we'll have to include a few more libraries than just the Checkstyle one. Here's the set of libraries we'll have to include, all of which come with buster, too:

diff --git a/java/base.xml b/java/base.xml
index f484dcd..d377798 100644
--- a/java/base.xml
+++ b/java/base.xml
@@ -58,7 +58,10 @@
 
   <path id="checkstyle.classpath" >
     <fileset dir="${libs}">
-      <include name="checkstyle-6.17-all.jar" />
+      <include name="antlr4-runtime-4.7.2.jar" />
+      <include name="checkstyle-8.15.jar" />
+      <include name="commons-collections3-3.2.2.jar" />
+      <include name="commons-logging-1.2.jar" />
     </fileset>
   </path>

After making this change to metrics-base, ant checks runs through, even though it prints out this warning that I don't know how to remove:

ANTLR Tool version 4.6 used for code generation does not match the current runtime version 4.7.2ANTLR Runtime version 4.6 used for parser compilation does not match the current runtime version 4.7.2ANTLR Tool version 4.6 used for code generation does not match the current runtime version 4.7.2ANTLR Runtime version 4.6 used for parser compilation does not match the current runtime version 4.7.2

I'd say this warning is acceptable and that we should make this change, in an effort to remove the last remaining non-Debian dependency.

irl, what do you think?

comment:7 Changed 2 months ago by karsten

Cc: metrics-team added
Status: needs_reviewassigned

comment:8 Changed 2 months ago by karsten

Reviewer: irl
Status: assignedneeds_review

comment:9 Changed 2 months ago by irl

Status: needs_reviewmerge_ready

Ah ok, should have looked at all the tickets first.

This looks good to me.

It is probable that the error is that the antlr version used to build the jar in debian was older, and then was later updated without the checkstyle jar being rebuilt.

I will ask that the jar is rebuilt.

comment:10 in reply to:  9 Changed 2 months ago by karsten

Status: merge_readyneeds_information

Replying to irl:

Ah ok, should have looked at all the tickets first.

This looks good to me.

Great!

It is probable that the error is that the antlr version used to build the jar in debian was older, and then was later updated without the checkstyle jar being rebuilt.

I will ask that the jar is rebuilt.

Oh, does it make sense to wait until this has happened, or should we just move ahead, accept the warning, and upgrade if there's a newer checkstyle jar around that removes the warning for us? What's your preference?

comment:11 Changed 2 months ago by irl

The version of the checkstyle jar would not change, so we should just move ahead.

comment:12 Changed 2 months ago by irl

Status: needs_informationmerge_ready

comment:13 Changed 2 months ago by karsten

Ah, good point. Alright, moving ahead.

comment:14 Changed 2 months ago by karsten

Resolution: fixed
Status: merge_readyclosed

Pushed to master as commit ca352d3. Resolving. Thanks!

Note: See TracTickets for help on using tickets.