#20596 closed enhancement (implemented)

streamline build.xml and metrics_checkstyle.xml throughout all java projects

Reported by: iwakeh Owned by: iwakeh
Priority: Medium Milestone:
Component: Metrics Version:
Severity: Normal Keywords:
Cc: hiro Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description (last modified by iwakeh)

This concerns all Metrics Java projects.

An additional attribute tag needs to be added:

	<attribute name="JDK-Version"
		   value="${java.runtime.version}"/>

and all topics added below.

Child Tickets

Change History (47)

comment:1 Changed 14 months ago by karsten

Status: newneeds_review

Like this?

diff --git a/build.xml b/build.xml
index 794188f..e36b780 100644
--- a/build.xml
+++ b/build.xml
@@ -199,6 +199,7 @@
         <attribute name="Implementation-Title" value="DescripTor"/>
         <attribute name="Implementation-Version"
                    value="${release.version}-${git.revision}"/>
+        <attribute name="JDK-Version" value="${java.runtime.version}"/>
       </manifest>
     </jar>
     <jar destfile="${jarsourcesfile}">
@@ -209,6 +210,7 @@
         <attribute name="Implementation-Title" value="DescripTor"/>
         <attribute name="Implementation-Version"
                    value="${release.version}-${git.revision}"/>
+        <attribute name="JDK-Version" value="${java.runtime.version}"/>
       </manifest>
     </jar>
     <jar destfile="${jardocsfile}">
@@ -218,6 +220,7 @@
         <attribute name="Implementation-Title" value="DescripTor"/>
         <attribute name="Implementation-Version"
                    value="${release.version}-${git.revision}"/>
+        <attribute name="JDK-Version" value="${java.runtime.version}"/>
       </manifest>
     </jar>
   </target>

Result is as follows:

diff -Nur x/META-INF/MANIFEST.MF y/META-INF/MANIFEST.MF
--- x/META-INF/MANIFEST.MF	2016-11-08 14:12:16.000000000 +0100
+++ y/META-INF/MANIFEST.MF	2016-11-08 14:13:30.000000000 +0100
@@ -3,4 +3,5 @@
 Created-By: The Tor Project
 Implementation-Title: DescripTor
 Implementation-Version: 1.5.0-dev-dd4b395
+JDK-Version: 1.8.0_102-b14

If this looks good, I'll make this change in metrics-lib, CollecTor, and Onionoo.

comment:2 in reply to:  1 Changed 14 months ago by iwakeh

That looks fine!

The attribute is only necessary for the class file jar, but I didn't write that.

Do you mind adding two more attributes?

In total these lines:

<attribute name="JDK-Version" value="${java.runtime.version}"/>
<attribute name="JVM-Name" value="${java.vm.name}"/>
<attribute name="JVM-Vendor" value="${java.vm.vendor}"/>

i.e.

          <attribute name="Implementation-Title" value="DescripTor"/>
          <attribute name="Implementation-Version"
                     value="${release.version}-${git.revision}"/>
 +        <attribute name="JDK-Version" value="${java.runtime.version}"/>
 +        <attribute name="JVM-Name" value="${java.vm.name}"/>
 +        <attribute name="JVM-Vendor" value="${java.vm.vendor}"/>
        </manifest>
      </jar>
      <jar destfile="${jarsourcesfile}">

jvm vendor and name might be useful, too.

comment:3 Changed 13 months ago by karsten

Wait, to be clear: these three attributes should only be added to the .jar file with class files and not to the .jar files with sources and with javadocs?

Somewhat off-topic: does this change mean that I should build future releases in a Debian stable VM, not in macOS, for maximum reproducibility?

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

Replying to karsten:

Wait, to be clear: these three attributes should only be added to the .jar file with class files and not to the .jar files with sources and with javadocs?

It's more important for the class-jar, but if you think it is more consistent, just add them all over.

Somewhat off-topic: does this change mean that I should build future releases in a Debian stable VM, not in macOS, for maximum reproducibility?

No not at all. I think building it anywhere given the correct jdk and dependencies ought to be fine. This information could just be helpful when people build elsewhere and then report bugs (cf. around 19720#comment:30). This manifest entry would give us a hint where things were running.

comment:5 Changed 13 months ago by iwakeh

For preventing the jdk version issue (cf. #20521 comments 4 following) a version check could be added to warn on the command line:

--- a/build.xml
+++ b/build.xml
@@ -89,7 +89,19 @@
     <delete file="${cobertura.ser.file}" quiet="true" />
   </target>
 
-  <target name="init">
+  <condition property="wrong-jvm">
+    <not>
+      <contains substring="${source-and-target-java-version}"
+               string="${java.version}" />
+    </not>
+  </condition>
+
+  <target name="jvmversioncheck" if="wrong-jvm">
+      <echo message="WARNING!" />
+      <echo message="Expected ${source-and-target-java-version}, but have ${java.version}." />
+  </target>
+
+  <target name="init" depends="jvmversioncheck">
     <mkdir dir="${classes}"/>
     <mkdir dir="${testclasses}"/>
     <mkdir dir="${docs}"/>

This will result in a reminder warning message, if the jdk is not correct:

jvmversioncheck:
     [echo] WARNING!
     [echo] Expected 1.7, but have 1.8.0_102.

comment:6 Changed 13 months ago by iwakeh

In order not to forget anything before committing the following task could be added:

  <target name="pre-commit" depends="test,checks,coverage" >
    <fail if="wrong-jvm"
	  message="Please run with Java ${source-and-target-java-version} instead of ${java.version}.">
    </fail>
  </target>

With this one 'only' needs to run one task before committing.

comment:7 Changed 13 months ago by iwakeh

unify all checkstyle rules across Metrics' projects (cf. #20668 comment:5)

comment:8 in reply to:  7 Changed 13 months ago by iwakeh

Replying to iwakeh:

unify all checkstyle rules across Metrics' projects (cf. #20668 comment:5)

in addition the "unused imports" rule, which we do not have in all projects yet.

comment:9 Changed 13 months ago by karsten

Hmmmm, I think I agree with the changes above, but would you mind preparing a patch that I can review and try out? I'm worried that I'll otherwise incorporate those changes in another way than intended. (Thanks!)

comment:10 Changed 13 months ago by iwakeh

The collector patch, the metrics-lib patch.

comment:11 Changed 13 months ago by karsten

Ah, I didn't mean only the checkstyle parts but also the build.xml parts above. Anyway, let's focus on checkstyle for the moment.

Looking at the checkstyle commits I noticed that even those two don't contain the exact same output. And we're still missing three more (Onionoo, Metrics website, and ExoneraTor). How about we work on one checkstyle file for one code base, and once that's good, we copy that over to the others?

Quick question: Why add two modules SuppressWarningsFilter and SuppressWarningsHolder? Those are not explained in the changes section. Do we need both, and why?

comment:12 in reply to:  11 Changed 13 months ago by iwakeh

Owner: changed from metrics-team to iwakeh
Status: needs_reviewassigned

Replying to karsten:

Ah, I didn't mean only the checkstyle parts but also the build.xml parts above. [snip]

That makes sense. I'll provide these patches. (Maybe there is some better way to avoid the code-duplication ...)

Could you just go through the suggestions and point out those which don't make immediate sense?

Last edited 13 months ago by iwakeh (previous) (diff)

comment:13 Changed 13 months ago by karsten

The remaining parts that don't make immediate sense are:

  • Why add two modules SuppressWarningsFilter and SuppressWarningsHolder? Those are not explained in the changes section. Do we need both, and why?
  • Can you prepare a patch for, say, metrics-lib's metrics_checks.xml as complete version with all changes and documentation why we're making those changes and which checkstyle issues are affected in brackets, that we can use to discuss and later copy over to the other four code bases?
  • Can you submit a patch for metrics-lib's build.xml with the various suggestions in comments 2 to 6 above? I'm afraid I lost track which of them should go where.

Thanks!

comment:14 Changed 13 months ago by iwakeh

In addition to the tasks in comment:13 verify the signing mechanism and add the workaround from #20712 comment:3.

comment:15 Changed 13 months ago by iwakeh

Description: modified (diff)
Summary: add jdk version to manifest filesstreamline build.xml and metrics_checkstyle.xml throughout all java projects

Change title and description to match the wider scope of this ticket.

comment:16 Changed 12 months ago by iwakeh

I'd propose having another git repository metrics-base or metrics-java-base for providing the standard build elements, which currently would be a base-build.xml and metrics' checkstyle rules.
This can be referenced as a sub-module and thus avoid code duplication.

Thoughts?

comment:17 in reply to:  16 ; Changed 12 months ago by karsten

Replying to iwakeh:

I'd propose having another git repository metrics-base or metrics-java-base for providing the standard build elements, which currently would be a base-build.xml and metrics' checkstyle rules.
This can be referenced as a sub-module and thus avoid code duplication.

Plausible!

Thoughts?

First thought is that we should try to avoid confusing new contributors. Basically, we're adding yet another hurdle for them to get up to speed. Though I admit that this hurdle is worth considering to avoid duplicating code.

What we'd have to do is fail with a useful error message in case this submodule has not yet been initialized. That message should contain the exact command that is necessary to initialize the missing submodule. Wording is important here, I think.

Another thought is that we should make sure that this won't break any future Jenkins tasks. I believe initializing a Git submodule from the same repository is okay, but we should check. Copying hiro on this ticket for that reason.

comment:18 Changed 12 months ago by karsten

Cc: hiro added

comment:19 in reply to:  17 ; Changed 12 months ago by iwakeh

Replying to karsten:

Replying to iwakeh:

I'd propose having another git repository metrics-base or metrics-java-base for providing the standard build elements, which currently would be a base-build.xml and metrics' checkstyle rules.
This can be referenced as a sub-module and thus avoid code duplication.

Plausible!

Which name?
metrics-base or metrics-java-base or something else?

Thoughts?

First thought is that we should try to avoid confusing new contributors. Basically, we're adding yet another hurdle for them to get up to speed. Though I admit that this hurdle is worth considering to avoid duplicating code.

What we'd have to do is fail with a useful error message in case this submodule has not yet been initialized. That message should contain the exact command that is necessary to initialize the missing submodule. Wording is important here, I think.

Yes, the error message should be clear.
Actually the submodule update could be perfomed by ant.

Another thought is that we should make sure that this won't break any future Jenkins tasks.

The submodule setup only needs ant and git, which are available in a Jenkins build.

Last edited 12 months ago by iwakeh (previous) (diff)

comment:20 in reply to:  19 ; Changed 12 months ago by karsten

Replying to iwakeh:

Replying to karsten:

Replying to iwakeh:

I'd propose having another git repository metrics-base or metrics-java-base for providing the standard build elements, which currently would be a base-build.xml and metrics' checkstyle rules.
This can be referenced as a sub-module and thus avoid code duplication.

Plausible!

Which name?
metrics-base or metrics-java-base or something else?

Hmm, how about metrics-base and we add any shared resources regardless of language? In the beginning there'd be only Java files, but would it hurt if we'd add files that are only relevant for other languages?

Thoughts?

First thought is that we should try to avoid confusing new contributors. Basically, we're adding yet another hurdle for them to get up to speed. Though I admit that this hurdle is worth considering to avoid duplicating code.

What we'd have to do is fail with a useful error message in case this submodule has not yet been initialized. That message should contain the exact command that is necessary to initialize the missing submodule. Wording is important here, I think.

Yes, the error message should be clear.
Actually the submodule update could be perfomed by ant.

Sounds good!

Another thought is that we should make sure that this won't break any future Jenkins tasks.

The submodule setup only needs ant and git, which are available in a Jenkins build.

Including the network access to fetch the dependent repository from Tor's Git server? If so, great!

comment:21 in reply to:  20 ; Changed 12 months ago by iwakeh

Replying to karsten:

..
Hmm, how about metrics-base and we add any shared resources regardless of language? In the beginning there'd be only Java files, but would it hurt if we'd add files that are only relevant for other languages?

Yes, metrics-base is the best choice. And, sure, should other languages be included when needed.
How to proceed? I'll continue with the streamlining. Who will request the new repo etc.?

comment:22 in reply to:  21 Changed 12 months ago by karsten

Replying to iwakeh:

Yes, metrics-base is the best choice. And, sure, should other languages be included when needed.
How to proceed? I'll continue with the streamlining. Who will request the new repo etc.?

There, I just created #21040 for the repository.

comment:23 Changed 12 months ago by karsten

Oh, and that repository already exists. So fast! Let me know when you have something to put there, and I'll push that.

comment:24 Changed 12 months ago by iwakeh

Although our projects seem generic, there are quite some more or less subtle differences.

I guess, I generated the common part for metrics-base now. It should be sufficient to fill the repo and a non-empty one is necessary for initializing the git sub-modules correctly. The other changes will follow once metrics-base contains this patch.

The bootstrapping by executing the correct git commands will have to be performed one time and after that an ant task will update the submodule without user interference.

Changed 12 months ago by iwakeh

init patch

comment:25 Changed 12 months ago by iwakeh

Status: assignedneeds_review

set to review, please set to assigned once metrics-base is filled.

comment:26 Changed 12 months ago by karsten

Status: needs_reviewassigned

Pushed to the repository, though without review, so that you can continue working on this with the now-existing remote repository.

comment:27 Changed 12 months ago by iwakeh

Please review the changes in:
Onionoo one commit,
metrics-lib (three commits, the first unrelated test problems, the third a suggestion for documentation),
CollecTor also only one commit.

Before that the attached patch for metrics-base needs to be applied.

Test steps:
clone using --recursive; if checkout happened without that use src/main/resources/bootstrap-development.sh.

metrics-web and exonerator are not included yet, b/c those are not yet released and quite different.

Changed 12 months ago by iwakeh

comment:28 Changed 12 months ago by iwakeh

Status: assignedneeds_review

comment:29 Changed 12 months ago by karsten

Status: needs_reviewneeds_revision

Whee, quite some code to review here. Sure, a lot of code has just moved around, but that only makes it trickier to spot the little differences. I finished a first (tool-supported) read of the changes but didn't finish trying it all out. But hopefully this initial feedback is useful anyway:

https://gitweb.torproject.org/metrics-base.git/commit/?id=7a01d68f813cb5ae2904e73dbc81999ce0622eca

  • The two properties "libs" and "source-and-target-java-version" are listed twice.
  • Speaking of "source-and-target-java-version", we should probably keep that property project-specific. Otherwise we'll have to raise the Java version for all projects at once, including metrics-lib and projects depending on metrics-lib.
  • The Checkstyle configuration file java/metrics_checks.xml does not contain modules SuppressWarningsFilter and SuppressWarningsHolder that we added in metrics-lib's version nor module UnusedImports in Onionoo's version. It also still defines a maximum line length of 100 rather than 80. Let's combine the latest settings from all three code bases here.
  • This is certainly a minor issue, but the usage instructions in the usage target should all use the same capitalization and sentence structure, like "'name' does XY", not "'name' Does XY" or "'name' XY".
  • It's already time to update the copyright year in the docs target! Happy 2017!

https://trac.torproject.org/projects/tor/attachment/ticket/20596/0001-Implements-part-of-task-20596.patch

  • Typo in java/build.xml.template: "jarpatterprop" is missing an "n".
  • By the way, I already pushed this commit to facilitate testing. We can fix things in subsequent commits.

https://gitweb.torproject.org/user/iwakeh/onionoo.git/commit/?h=task-20596-submod&id=0442032f24c8764d6c055ed3b9f4e047fbaa886e

  • Why did you change the "jetty.version" property to "" rather than "-8.1.16.v20140903"?
  • The new generic tar target does not include the DESIGN file anymore. I'd argue that we should remove that file anyway, but if you want to keep it, we'll have to include it in the release tarball, too.
  • The script file src/main/resources/bootstrap-development.sh should be checked in with the executable flag.
  • This commit or a subsequent commit should remove src/test/resources/metrics_checks.xml which is now obsolete.

https://gitweb.torproject.org/user/iwakeh/metrics-lib.git/commit/?h=task-20596-submod&id=60d20b85c4dac3d9d0905a185b7bc928736f7fcf

  • Which tests did not pass prior to this commit? They run through just fine here.
  • It shouldn't be necessary to initialize attributes with null or 0, which is the default anyway. All new no-args constructors should be empty (except for a comment saying that they're empty on purpose).

https://gitweb.torproject.org/user/iwakeh/metrics-lib.git/commit/?h=task-20596-submod&id=946c403c357661a2290b424ecb1a1c70d70c1334

  • The script file src/main/resources/bootstrap-development.sh should be checked in with the executable flag.
  • This commit or a subsequent commit should remove src/test/resources/metrics_checks.xml which is now obsolete.

https://gitweb.torproject.org/user/iwakeh/metrics-lib.git/commit/?h=task-20596-submod&id=7903d6a55504801dd03c75867ce79ff8f3799f11

  • Looks good!

https://gitweb.torproject.org/user/iwakeh/collector.git/commit/?h=task-20596-submod&id=277be1b5802f8a7408fad27748ca00dab802d54f

  • The script file src/main/resources/bootstrap-development.sh should be checked in with the executable flag.
  • This commit or a subsequent commit should remove src/test/resources/junittest.policy and src/test/resources/metrics_checks.xml which are now obsolete.

I guess I'll wait for the revision before trying out these changes altogether. My plan was to create release tarballs and compare them with the latest releases.

Thanks again for making build processes better!

comment:30 in reply to:  29 Changed 12 months ago by iwakeh

Status: needs_revisionneeds_review

Replying to karsten:

Whee, quite some code to review here. Sure, a lot of code has just moved around, but that only makes it trickier to spot the little differences. ...

Yes, that is how it feels to code this stuff ;-)
I updated all branches and will attach the patches for metrics-base here.
More in-line.

... But hopefully this initial feedback is useful anyway:

Very useful! Such a tasks just needs lots of testing/reviewing.

https://gitweb.torproject.org/metrics-base.git/commit/?id=7a01d68f813cb5ae2904e73dbc81999ce0622eca

  • The two properties "libs" and "source-and-target-java-version" are listed twice.

Removed.

  • Speaking of "source-and-target-java-version", we should probably keep that property project-specific. Otherwise we'll have to raise the Java version for all projects at once, including metrics-lib and projects depending on metrics-lib.

If a project deviates from the base java version, only the property needs to be added to build.xml and takes precedence. (Try setting it to java 1.4 and see how the compile complains about generics.)

I added a comment to the build.template.xml

  • The Checkstyle configuration file java/metrics_checks.xml does not contain modules SuppressWarningsFilter and SuppressWarningsHolder that we added in metrics-lib's version nor module UnusedImports in Onionoo's version. It also still defines a maximum line length of 100 rather than 80. Let's combine the latest settings from all three code bases here.

Changed the line length check; all others were already part of the initial check-in.

  • This is certainly a minor issue, but the usage instructions in the usage target should all use the same capitalization and sentence structure, like "'name' does XY", not "'name' Does XY" or "'name' XY".

Streamlined.

  • It's already time to update the copyright year in the docs target! Happy 2017!

Good that this only needs a change in one place now :-)
The year is now computed from the current time UTC.

https://trac.torproject.org/projects/tor/attachment/ticket/20596/0001-Implements-part-of-task-20596.patch

  • Typo in java/build.xml.template: "jarpatterprop" is missing an "n".

Fixed.

  • By the way, I already pushed this commit to facilitate testing. We can fix things in subsequent commits.

Very useful.

https://gitweb.torproject.org/user/iwakeh/onionoo.git/commit/?h=task-20596-submod&id=0442032f24c8764d6c055ed3b9f4e047fbaa886e

  • Why did you change the "jetty.version" property to "" rather than "-8.1.16.v20140903"?

Fixed.

  • The new generic tar target does not include the DESIGN file anymore. I'd argue that we should remove that file anyway, but if you want to keep it, we'll have to include it in the release tarball, too.

Removed. Designs should be elsewhere, for example Tech-Reports.

  • The script file src/main/resources/bootstrap-development.sh should be checked in with the executable flag.

Done.

  • This commit or a subsequent commit should remove src/test/resources/metrics_checks.xml which is now obsolete.

Done.

https://gitweb.torproject.org/user/iwakeh/metrics-lib.git/commit/?h=task-20596-submod&id=60d20b85c4dac3d9d0905a185b7bc928736f7fcf

  • Which tests did not pass prior to this commit? They run through just fine here.

Hmm, now these tests pass here, too. I'll go hunting for the jdk/environment setting responsible, but this shouldn't affect this ticket. Please, just cherry pick.

  • It shouldn't be necessary to initialize attributes with null or 0, which is the default anyway. All new no-args constructors should be empty (except for a comment saying that they're empty on purpose).

The compiler complains about uninitialized fields (those are final in the respective classes). Otherwise, same as the previous.

https://gitweb.torproject.org/user/iwakeh/metrics-lib.git/commit/?h=task-20596-submod&id=946c403c357661a2290b424ecb1a1c70d70c1334

  • The script file src/main/resources/bootstrap-development.sh should be checked in with the executable flag.

Done.

  • This commit or a subsequent commit should remove src/test/resources/metrics_checks.xml which is now obsolete.

Done.

https://gitweb.torproject.org/user/iwakeh/metrics-lib.git/commit/?h=task-20596-submod&id=7903d6a55504801dd03c75867ce79ff8f3799f11

  • Looks good!

Yeah!

https://gitweb.torproject.org/user/iwakeh/collector.git/commit/?h=task-20596-submod&id=277be1b5802f8a7408fad27748ca00dab802d54f

  • The script file src/main/resources/bootstrap-development.sh should be checked in with the executable flag.

Done.

  • This commit or a subsequent commit should remove src/test/resources/junittest.policy and src/test/resources/metrics_checks.xml which are now obsolete.

Done.

I guess I'll wait for the revision before trying out these changes altogether. My plan was to create release tarballs and compare them with the latest releases.

Be aware, that Onionoo does not include the protocol version in the manifest anymore, because the protocol version is part of the release version now.

In addition, also simulate development tasks.

Thanks again for making build processes better!

Thanks for plowing through all these changes!

Last edited 12 months ago by iwakeh (previous) (diff)

comment:31 Changed 12 months ago by karsten

Status: needs_reviewneeds_revision

Okay, here's what I did and where I ran into issues:

  • Tried running ant tar in your onionoo task-20596-submod branch, but it keeps telling me that signing of at least one of the .jars failed. I didn't investigate further yet. Does that work for you?
  • Rebased your metrics-lib task-20596-submod branch and pushed it to branch task-20596 in my repository. (Though I'm planning to squash your squash commit into "Implements task-20596" rather than "Added development description" when merging to master.) Realized that Gson is indeed unhappy, so we should keep your commit, but without explicit null-initializations, if possible. However, I ran into two issues:
    • The sources jar contains sources in a different place than previous tarballs.
    • The executable jar contains other classes than just our own, which it shouldn't (as opposed to the other executable jars).
  • Rebased your collector task-20596-submod branch and pushed it to branch task-20596 in my repository. Building seems to work fine, but I found this warning: "[javadoc] javadoc: error - Error while reading file /Users/karsten/src/collector/src/main/resources/overview.html". I didn't compare .jar file contents yet but expect to find similar differences as I found in the metrics-lib .jar files. Same goes for Onionoo above.

Would you want to look into some of these issues? I can do another round of tests and checks tomorrow. Thanks!

comment:32 in reply to:  31 Changed 12 months ago by iwakeh

Replying to karsten:

Okay, here's what I did and where I ran into issues:

  • Tried running ant tar in your onionoo task-20596-submod branch, but it keeps telling me that signing of at least one of the .jars failed. I didn't investigate further yet. Does that work for you?

That worked fine in the test setting I had, well. I'll look into it.

  • Rebased your metrics-lib task-20596-submod branch and pushed it to branch task-20596 in my repository. (Though I'm planning to squash your squash commit into "Implements task-20596" rather than "Added development description" when merging to master.) Realized that Gson is indeed unhappy, so we should keep your commit, but without explicit null-initializations, if possible. However, I ran into two issues:
    • The sources jar contains sources in a different place than previous tarballs.
    • The executable jar contains other classes than just our own, which it shouldn't (as opposed to the other executable jars).
  • Rebased your collector task-20596-submod branch and pushed it to branch task-20596 in my repository. Building seems to work fine, but I found this warning: "[javadoc] javadoc: error - Error while reading file /Users/karsten/src/collector/src/main/resources/overview.html". I didn't compare .jar file contents yet but expect to find similar differences as I found in the metrics-lib .jar files. Same goes for Onionoo above.

Would you want to look into some of these issues? I can do another round of tests and checks tomorrow. Thanks!

No problem, I'll take a look at all the issues. Thanks for checking quickly!

Could you specify the source location differences?
And, (without knowing how weird the paths might be) is the new path structure problematic?

comment:33 Changed 12 months ago by karsten

The old sources .jar file contained paths like org/torproject/descriptor/BandwidthHistory.java, whereas the new file contains that file in main/java/org/torproject/descriptor/BandwidthHistory.java. The new file also contains other sources and resources that the old file did not contain. I'm not using sources .jar files anywhere, but I could imagine that some tools depend on the old directory structure, so it might be good to keep it. But I don't know for sure.

Thanks!

comment:34 Changed 12 months ago by iwakeh

All things should be resolved now except for the javadoc complaint about overview.html.
This doesn't hinder javadoc creation so fine for this ticket. But, as we ship javadoc with all products it would be a better style to provide an overview page with useful contents for each.
New ticket?

The following was changed:

comment:35 Changed 12 months ago by karsten

Hmm, I'm having trouble building all three now. :/

I pushed your metrics-base commit to the official metrics-base repository. Here's what I get when trying to build release tarballs:

Onionoo:

signall:
  [signjar] Signing JAR: /Users/karsten/src/onionoo/generated/dist/onionoo-3.1-1.0.1-dev-javadoc.jar to /Users/karsten/src/onionoo/generated/dist/signed/onionoo-3.1-1.0.1-dev-javadoc.jar as karsten
  [signjar] jarsigner: unable to sign jar: java.util.zip.ZipException: duplicate entry: allclasses-frame.html

BUILD FAILED
/Users/karsten/src/onionoo/src/build/java/base.xml:310: jarsigner returned: 1

Total time: 20 seconds

metrics-lib:

other:

BUILD FAILED
/Users/karsten/src/metrics-lib/src/build/java/base.xml:337: target attribute must not be empty

Total time: 0 seconds

CollecTor:

other:

BUILD FAILED
/Users/karsten/src/collector/src/build/java/base.xml:337: target attribute must not be empty

Total time: 0 seconds

Oh, and yes, new ticket for the overview page sounds good.

comment:36 Changed 12 months ago by iwakeh

Status: needs_revisionneeds_review

Here's the tiny fix.

(It's just problematic keeping all in sync without committing to the actual metrics-base repo. I'll look for a way to switch sub-module origins.)

comment:37 Changed 12 months ago by iwakeh

The new tickets: #21138 and #21139.

comment:38 Changed 12 months ago by karsten

Pushed that fix to the official metrics-base repository.

Onionoo works fine now, except for one tiny change: The MANIFEST.MF file in the .jar file now says Created-By: 1.8.0_102-b14 (Oracle Corporation) rather than Created-By: The Tor Project. The .war file still says the latter. Any ideas?

Note that metrics-lib and CollecTor still don't work. See the problem above.

comment:39 Changed 12 months ago by iwakeh

So, three more commits on my branch of metrics-base. (And, what is good, nowhere else!)

The JDK version was supposed to be included in the manifest (making reproducing builds easier). Thus, I added Implemented-By: The Tor Project and left Created-By: ... to refer to the JDK as it usually does. Hope, this is fine.

Other changes: make all three tar tasks work and only add Main-Class to the appropriate executable jar.

I tested by brute-force sub-repo replacement and it worked.

comment:40 Changed 12 months ago by karsten

I merged your commits to the official metrics-base repository.

The latest branches I'm working with are:

Remaining issues are:

  • Neither CollecTor's *-sources.jar file nor its *-javadoc.jar file should contain the two files collector.properties and logback.xml. Can you remove those (but leave them in the main *.jar)?
  • Onionoo's .war file still contains manifest entry Created-By: The Tor Project which I guess should also be Implemented-By: The Tor Project as in the .jar file.
  • There should be change log entries in all three code bases on medium or minor level saying something like: "Replaced several build files or parts thereof with their equivalents from the metrics-base repository to unify the build process among metrics code bases." Feel free to tweak that sentence. I can add it while merging.

We're getting closer! :)

comment:41 in reply to:  40 ; Changed 12 months ago by iwakeh

Replying to karsten:

I merged your commits to the official metrics-base repository.

The latest branches I'm working with are:

I just noticed that you use your own branches for collector and metrics-lib and assume there were no changes I should have looked at.

Remaining issues are:

  • Neither CollecTor's *-sources.jar file nor its *-javadoc.jar file should contain the two files collector.properties and logback.xml. Can you remove those (but leave them in the main *.jar)?

Fixed in metrics-base.

  • Onionoo's .war file still contains manifest entry Created-By: The Tor Project which I guess should also be Implemented-By: The Tor Project as in the .jar file.

Fixed in Onionoo.

  • There should be change log entries in all three code bases on medium or minor level saying something like: "Replaced several build files or parts thereof with their equivalents from the metrics-base repository to unify the build process among metrics code bases." Feel free to tweak that sentence. I can add it while merging.

Added changelog entries to onionoo, collector, descriptor ;-)

All changes on the known branches in my git repos.

We're getting closer! :)

Yes! This all might seem tedious, but while making the fixes it is already very much more efficient; and, I guess trying to keep all projects in sync without metrics-base would just cause exponentially more iterations if not being an impossible task in the long run.

comment:42 in reply to:  41 Changed 12 months ago by karsten

Alright, I think we're there. I pushed to metrics-base and to the three other repositories. Maybe take a look if I messed up something. Thank you!

comment:43 Changed 12 months ago by iwakeh

Resolution: implemented
Status: needs_reviewclosed

Clean check-outs from all three projects (Onionoo, metrics-lib, CollecTor):

  • ./src/main/resources/bootstrap-development.sh performed as intended, and
  • all ant tasks worked, i.e.,
    • clean
    • compile
    • test
    • docs
    • checks (see new tickets)
    • tar
    • coverage

Creating two new tickets for ExoneraTor and MetricsWeb.

Closing.

Thanks!

Note: See TracTickets for help on using tickets.