Gradle migration

[HEdingfield]

Once completed, this fixes or makes progress on #239, #168, #110, and #270.

I need help on this! I've spent many hours on it and pushed it as far as I can for now. You'll probably need to download JDK 12 here and point the IntelliJ to that. Then use the Gradle menu along the right side of the screen, then Tasks => build => build.

The problem I'm running into is one of transitive dependencies, e.g. 100 errors like this:

error: module com.fasterxml.jackson.databind reads package org.apache.poi from both poi.ooxml and poi

I think it's a matter of tweaking things in the gradle.build and module-info.java files to get this right. Here are the most useful posts I've found on the subject so far:

• https://discuss.gradle.org/t/how-do-i-exclude-specific-transitive-dependencies-of-something-i-depend-on/17991/9
• https://blog.codefx.org/java/java-9-migration-guide/#Split-Packages
• https://stackoverflow.com/questions/42275516/excluding-tomcat-dependencies-from-spring-boot-in-gradle

[moldover]

This is cool! Did as you suggested (got JDK 12) and gradle downloaded all the packages and I'm able to re-pro the same compilation errors. Will dig in more!

[HEdingfield]

Seven blessings to you 🙏

[tarheel]

I'll throw in an eighth blessing if you figure out the errors.

[moldover]

@HEdingfield I pushed a couple commits which get the project building and creating .zip distribution. Unzipping and running the launcher script on my computer does launch the app, but it immediately shuts down:

2019-05-26 15:27:00 PDT INFO: RCV Tabulator logging execution to: /Users/Jon/Documents/rcv/build/distributions/rcv/bin/rcv_%g.log
2019-05-26 15:27:00 PDT INFO: Tabulator is being used via the CLI.
2019-05-26 15:27:00 PDT WARNING: Too many arguments! Max is 2 but got: 3
2019-05-26 15:27:00 PDT SEVERE: Error opening file: -classpath
java.io.FileNotFoundException: -classpath (No such file or directory)
2019-05-26 15:27:00 PDT SEVERE: Check file path and permissions and make sure they are correct!
2019-05-26 15:27:00 PDT SEVERE: Failed to load contest config: -classpath
2019-05-26 15:27:00 PDT SEVERE: Aborting because contest config is invalid!

Seem to be making some progress :)

However I'm not convinced this .zip file contains the .jdk files which jlink purports to include, since it seems to only contain 3rd-party jar files and the rcv jar file--i don't see any platform libs.

Other side notes:

• gradle build fails near the end, when it tries to execute a test task. Seems like it's not set up to run against our TabulatorTests yet, but this doesn't impact creating the distribution .zip file.
• running "build" and run/debugging test configurations from intelliJ doe not work

[moldover]

I think passing classpath is what jlink does for "default JVM arguments" and can be disabled in the jlink piece of gradle build file.

[HEdingfield]

Nice! I'm without a computer until 6/3, but I'll take a look soon after I get back if this still isn't resolved by then.

[tarheel]

You haven't downloaded the IntelliJ iPhone app yet?

[HEdingfield]

iPhone more like cryPhone cause it means you paid twice as much for inferior technology am I rite guys??

[moldover]

A bit more info on this as I learn more (so many opportunities for growth lol) The poi module compilation errors are actually due to poi being a "split" package. I did get it to build yes, but we are not including the org.apache.poi:poi package (several other packages were removed as well?) so it will fail at runtime.

I think it is actually going to take some fancy jar repackaging in order to make this work correctly.

See https://poi.apache.org/help/faq.html#faq-N102B0 they are "working" on automatic module support for poi.

More chatter for context: http://apache-poi.1045710.n5.nabble.com/Bug-62355-New-Unsplit-packages-to-conform-Jigsaw-Java-9-module-standard-td5730659.html

[tarheel]

I haven't spent any time looking at this, but I have a quick thought: any chance that it would be easier to use something other than poi instead of jumping through hoops to get poi to compile?

[moldover]

There are other xlsx options: but I think this would take much more time than tweaking our build.

Merging a split package doesn't seem that hard (it's essentially merging two directory trees and then repackaging as a jar) but after still more reading I think the better solution is just to not modularize our application yet - see this article which seems to be exactly our case. (you will need this background to understand what he is talking about)

TL;DR: one should probably wait till ALL one's dependencies have modular releases before modularizing your overall app.

However, I think/hope we should be able to do everything we want (use gradle, create platform-specific builds on JDK 12) and still release a self-contained (but non-modular) app.

@HEdingfield do you know how gradle build steps are defined? For example, it creates a .tar file, and tries to run tests which are not currently configured. We will want to tailor these.

[moldover]

@HEdingfield & @tarheel I think I have finally fixed this issue. Turns out apache poi in March released version 4.0.1 which addresses this split packages issue. I just pulled the newer version, added back the other required packages and it now complies and I'm able to run and tabulate an election. There are some things to be fixed:
gradle: There were failing tests. See the report at: file:///Users/Jon/Documents/rcv/build/reports/tests/test/index.html
On app launch: 2019-05-29 23:01:14 PDT WARNING: Resource "GuiStyles.css" not found.

But mainly I think the way we are invoking jlink is not actually adding any java framework classes, so I suspect it won't yet run if user does not have java 10+. What are you guys on? Hylton, do you know anything about how jlink knows what files to add?

@gngilbert
I attached the .zip our gradle build creates. can you try launching the app by:

1. download
2. unzip
3. open bin folder
4. double-clicking rcv.bat

rcv.zip

[moldover]

@HEdingfield @tarheel I have continued to chip away at this and I've got it to a point where it seems to be working. It builds, debugs, passes regression tests, and my local run of the distribution works.

There are some smaller things which can be improved, but I think the main task is complete and should be reviewed and tested. I may need some help on any needed changes / additions as I am really burnt out on this. Thank you.

[HEdingfield]

Thanks for all the work, @moldover! It's a big step ahead. I spent a few hours on this and basically got it working, but recorded a bunch of outstanding issues. I think we can consider merging it now (and I'll just file them as issues), with the following caveats:

• I could only get it to run using JDK 11+ now (I tried with both 11 and 12 and they worked, but 10 didn't).
• The new distribution (i.e. the one created using Tasks > distribution > distZip in IntelliJ) is platform-specific! The one I generate works in Windows, but the one Jon generated and attached above does NOT work for me, probably because it uses *mac JavaFX libraries (whereas mine includes *win versions).
• It's also currently not possible to use the CLI with the new distribution method.
• I think we could make a truly cross-platform version that doesn't even require Java to be installed if we could get Tasks > build > jlink to work, but it's currently failing for me with the following:

Task :createMergedModule
Cannot derive uses clause from service loader invocation in: com/fasterxml/jackson/databind/ObjectMapper$2.run().
Cannot derive uses clause from service loader invocation in: com/fasterxml/jackson/databind/ObjectMapper.secureGetServiceLoader().
Cannot derive uses clause from service loader invocation in: org/apache/commons/compress/utils/ServiceLoaderIterator.().
C:\Users\Hylton\IdeaProjects\rcv\build\jlinkbase\tmpjars\rcv.merged.module\module-info.java:392: error: the service implementation does not have a default constructor: XBeansXPath
provides org.apache.xmlbeans.impl.store.PathDelegate.SelectPathInterface with org.apache.xmlbeans.impl.xpath.saxon.XBeansXPath;
C:\Users\Hylton\IdeaProjects\rcv\build\jlinkbase\tmpjars\rcv.merged.module\module-info.java:393: error: the service implementation does not have a default constructor: XBeansXQuery
provides org.apache.xmlbeans.impl.store.QueryDelegate.QueryInterface with org.apache.xmlbeans.impl.xquery.saxon.XBeansXQuery;
2 errors

@tarheel I leave it up to you to make the final call on whether or not it's ok to merge in like this. Or, even better, to take up the torch and try to resolve these jlink errors.

[gngilbert]

I think I would be worried if I knew what you guys were talking about. Since I don't, however, sounds to me like you've got it all under control! 🤗

…

On Wed, Jun 5, 2019, 5:27 PM HEdingfield ***@***.***> wrote: Thanks for all the work, @moldover <https://github.com/moldover>! It's a big step ahead. I spent a few hours on this and basically got it working, but recorded a bunch of outstanding issues. I *think* we can consider merging it now (and I'll just file them as issues), with the following caveats: - I could only get it to run using JDK 11+ now (I tried with both 11 and 12 and they worked, but 10 didn't). - The new distribution (i.e. the one created using Tasks > distribution > distZip in IntelliJ) is platform-specific! The one I generate works in Windows, but the one Jon generated and attached above does NOT work for me, probably because it uses *mac JavaFX libraries (whereas mine includes *win versions). - It's also currently not possible to use the CLI with the new distribution method. - I think we could make a truly cross-platform version that doesn't even require Java to be installed if we could get Tasks > build > jlink to work, but it's currently failing for me with the following: Task :createMergedModule Cannot derive uses clause from service loader invocation in: com/fasterxml/jackson/databind/ObjectMapper$2.run(). Cannot derive uses clause from service loader invocation in: com/fasterxml/jackson/databind/ObjectMapper.secureGetServiceLoader(). Cannot derive uses clause from service loader invocation in: org/apache/commons/compress/utils/ServiceLoaderIterator.(). C:\Users\Hylton\IdeaProjects\rcv\build\jlinkbase\tmpjars\rcv.merged.module\module-info.java:392: error: the service implementation does not have a default constructor: XBeansXPath provides org.apache.xmlbeans.impl.store.PathDelegate.SelectPathInterface with org.apache.xmlbeans.impl.xpath.saxon.XBeansXPath; C:\Users\Hylton\IdeaProjects\rcv\build\jlinkbase\tmpjars\rcv.merged.module\module-info.java:393: error: the service implementation does not have a default constructor: XBeansXQuery provides org.apache.xmlbeans.impl.store.QueryDelegate.QueryInterface with org.apache.xmlbeans.impl.xquery.saxon.XBeansXQuery; 2 errors @tarheel <https://github.com/tarheel> I leave it up to you to make the final call on whether or not it's ok to merge in like this. Or, even better, to take up the torch and try to resolve these jlink errors. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#272?email_source=notifications&email_token=AJODE7X4JMNNV3MT3WMXQVLPZAVT3A5CNFSM4HN5OI4KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXBCE5I#issuecomment-499262069>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJODE7XKHD7AQH6NI2SKQZDPZAVT3ANCNFSM4HN5OI4A> .

[HEdingfield]

Ok, I spent 9 more hours on this shit and I feel like it's good to push out now. There are still a few major things I'll want to fix ASAP, but at least I got the tests and the CLI working, and I confirmed that it's possible to get a Linux-friendly distribution generated.

Major outstanding issues (which I'll post after this is merged in):

• Building distributions using distZip is platform-specific due to JavaFX graphics libraries getting included (separate for Windows, Mac, Linux); you currently have to build the dist on an actual system using that OS, though hopefully we can figure out a way around that
• jlink fails due to some annoying errors; if we could get this working, we could probably make a single version that works on all platforms and doesn't even require Java to be installed!
• distZip doesn't include any of the test files, which we may want to include as part of the distribution
• I tried updating beyond Gradle 4.10.3 (i.e. a number of versions all the way up to 5.4.1), and test2013MinneapolisMayorScale() would crap out around 500k votes with the message "TestEngine with ID 'junit-jupiter' failed to execute tests"; sticking with 4.10.3 (the IntelliJ default) for now...
• There are a couple of warnings during the build about unchecked exceptions
• I also discovered a minor weird CLI bug when running with Gradle rcv.bat (and possibly otherwise, and possibly only in Windows): directories specified for CVRs (and output) inject \null\ before... workaround requires using a path like "..\2013_minneapolis_mayor_cvr.xlsx" instead of "2013_minneapolis_mayor_cvr.xlsx"

@tarheel: I'll merge after you approve.

[moldover]

Nice work. I agree with your assessments and have been trying to find info on this jlink error and have so far failed. It's unclear what the message really means as there is no requirement for service implementations to have default ctors so I don't have a theory on it. Maybe just ask them.

[HEdingfield]

Cool, thanks for working on this too. Yeah, I don't really have any leads on the jlink issue; I do on the others, and I'll file issues with that info as soon as @tarheel approves this PR.

In the meantime, I just posted this question on SO.

[moldover]

Well played @HEdingfield. Your SO question got such a quick response and it works! At least, the entire build + tests completed for me with no errors so I pushed the fix, although I don't entirely understand why.

[HEdingfield]

Hell yeah! That Spaniard is a champion!

Confirmed that the jlink image works without needing Java installed (the zip is 48 MB vs. 26 MB with distZip), but unfortunately it looks like it's still packaging Windows-specific libraries. Still, a nice step up and a good option to have.

Once this is submitted, I'll file an issue to look into using jpackage and/or special arguments for jlink or distZip to be able to do builds for all 3 OSes on one system.

@tarheel: here are the things you'll want to verify on your end for your review:

Using the IntelliJ Gradle panel

• Run:
  Tasks => application => run

• Unit tests:
  Tasks => verification => test

• Distribute:
  Test both after extracting and navigating to the bin directory:
  ** Tasks => distribution => distZip (outputs to build/distributions/rcv.zip; requires that Java 11+ SDK is already installed on your system)
  ** Tasks => build => jlinkZip (outputs to build/image.zip; doesn't require Java to be installed)

[tarheel]

[tarheel]

Extrapolating from our trajectory to date, I predict that our test files are 60 levels deep by the time we officially release this app.

[HEdingfield]

We did it!!

[gngilbert]

Congratulations and great work!! All you have to do now is explain to me what it was you accomplished.?

…

On Sun, Jun 9, 2019, 10:27 PM HEdingfield ***@***.***> wrote: We did it!! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#272?email_source=notifications&email_token=AJODE7V5WTJ2DMNN23TOM73PZW3YVA5CNFSM4HN5OI4KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXIYTUA#issuecomment-500271568>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJODE7WY4WUOINLC54OSLUTPZW3YVANCNFSM4HN5OI4A> .

[HEdingfield]

It basically fixes all 4 of the issues mentioned at the very top. I'll update them and add my last outstanding issues shortly.

[gngilbert]

I am getting a new laptop tomorrow so you guys will have to tell me how to get our latest work loaded and running.

…

On Sun, Jun 9, 2019, 10:36 PM HEdingfield ***@***.***> wrote: It basically fixes all 4 of the issues mentioned at the very top. I'll update them and add my last outstanding issues shortly. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#272?email_source=notifications&email_token=AJODE7URPB2SJNYQ3TAVDZTPZW44PA5CNFSM4HN5OI4KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXIY6XA#issuecomment-500272988>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJODE7TKA7CE5I6NIISH4X3PZW44PANCNFSM4HN5OI4A> .