Opened 3 years ago

Closed 3 years ago

#19253 closed task (fixed)

replace submodule with released dependency of metrics-lib

Reported by: iwakeh Owned by:
Priority: Low Milestone:
Component: Metrics/Onionoo Version:
Severity: Minor Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

I just noticed that Onionoo is not yet using the released descriptor.jar and still has the git submodule.

Child Tickets

Attachments (1)

0001-Reordered-path-definitions-exclude-all-META-INF-file.patch (7.3 KB) - added by iwakeh 3 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 3 years ago by karsten

Status: newneeds_review

Right, let's fix that. Please review my branch task-19253 which also contains a fix to #19260.

comment:2 Changed 3 years ago by iwakeh

Status: needs_reviewmerge_ready

Looks fine!

Maybe add a hint about where to obtain the descriptor release to the INSTALL doc?

comment:3 Changed 3 years ago by iwakeh

One more thing: isn't hamcrest-core suffient for the tests?

comment:4 Changed 3 years ago by karsten

Thanks for reviewing! Do you mind adding such a hint to the doc and trying out whether hamcrest-core is sufficient and possibly sending a patch? Both changes sound useful to me.

Also, I have been trying to run the .jar produced by the changed build file and ran into problems with metrics-lib formerly signed .jar. Here's what I had to change after doing a quick search:

diff --git a/build.xml b/build.xml
index c92fc3a..8e2c73c 100644
--- a/build.xml
+++ b/build.xml
@@ -169,9 +169,16 @@
         <include name="logback-core-1.0.4.jar"/>
         <include name="slf4j-api-1.6.5.jar"/>
       </zipgroupfileset>
-      <zipgroupfileset dir="lib">
-        <include name="descriptor-1.2.0.jar"/>
-      </zipgroupfileset>
+      <restrict>
+        <not>
+          <name name="META-INF/*.SF" />
+        </not>
+        <archives>
+          <zips>
+            <fileset dir="lib" includes="descriptor-1.2.0.jar"/>
+          </zips>
+        </archives>
+      </restrict>
       <fileset dir="${classes}"
                excludes="org/torproject/onionoo/server/" />
       <fileset dir="${config}" includes="logback.xml" />

Is there a cleaner way to fix this?

comment:5 Changed 3 years ago by iwakeh

Status: merge_readyneeds_review

Good catch! I usually don't download descriptor.jar

The method for excluding the superfluous files is fine.
I excluded even the entire contents of META-INF from external jars; mostly the pom.xml and pom.properties from apache-commons jars.

I also restructured the paths a little to have lib names only in one place.
The we named property lib actually libs in collector; that's why I changed it here.
And, I removed the references to /usr/share/java. All third party jars should come from the libs-dir.

hamcrest-core is sufficient for passing the tests.

I don't have an onionoo data structure at hand, so I only could start the war part and look at the static pages; this needs a closer look when reviewing.

I added a small note to INSTALL.

Patch attached. Please review.

comment:6 Changed 3 years ago by karsten

Resolution: fixed
Status: needs_reviewclosed

Looks great, thanks for making those changes. Pushed to master and closing.

Note: See TracTickets for help on using tickets.