Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standalone Executable #660

Merged
merged 20 commits into from
Jun 14, 2023
Merged

Conversation

artoonie
Copy link
Collaborator

@artoonie artoonie commented Apr 25, 2023

Auto-create executables using jpackage.

Issues:

  1. The version number is now in three different places. We need an easy way to sync this across the project, but I couldn't find a straightforward way yet. We'll need a ticket to track this.
  2. We're now creating a ton of packages: the launchable binaries (jpackage), the java runtime image (jlinkZip), SHAs for each, x3 platforms --> the packages are around 350mb. When a release is created, this won't be an issue since people can just download the relevant file, but we should consider whether we want to support the jlinkZip image indefinitely, or if the jpackage will supercede it

@artoonie artoonie force-pushed the feature/issue-57_standalone-executable branch from ac70be4 to 189f692 Compare April 25, 2023 02:52
@artoonie artoonie force-pushed the feature/issue-57_standalone-executable branch from 189f692 to 61241bf Compare April 25, 2023 02:59
Base automatically changed from hotfix/auto-release to master April 25, 2023 16:21
@artoonie artoonie force-pushed the feature/issue-57_standalone-executable branch 10 times, most recently from 2678efa to 734667e Compare April 25, 2023 19:43
@artoonie artoonie force-pushed the feature/issue-57_standalone-executable branch from 734667e to bfd2b84 Compare April 25, 2023 20:24
@artoonie artoonie marked this pull request as ready for review April 25, 2023 20:29
@HEdingfield
Copy link
Contributor

Curious why we're merging this into master instead of develop? Just so we can test it out sooner?

MacOS packages aren't signed, and seem impossible to install on newer macs. They are painful to install on older macs. We probably need to sign the packages, which I believe require paying Apple $100/year?

My knee-jerk reaction to this is helllllllll no. But I guess we can see what @chughes297 thinks.

Also, I'm happy to test the bins for Linux and Windows if you can point me to where they are.

The version number is now in three different places. We need an easy way to sync this across the project, but I couldn't find a straightforward way yet. We'll need a ticket to track this.

We should be able to get rid of it in release.yml, right, defaulting to the GitHub tag variable instead? Looks like there are some merge conflicts to resolve with release.yml anyway.

We're now creating a ton of packages: the launchable binaries (jpackage), the java runtime image (jlinkZip), SHAs for each, x3 platforms --> the packages are around 350mb. When a release is created, this won't be an issue since people can just download the relevant file, but we should consider whether we want to support the jlinkZip image indefinitely, or if the jpackage will supercede it

Yeah, I think jpackage will replace jlinkZip. However, we should discuss the ramifications of this with @chughes297 to make sure it doesn't cause any additional security issues having them use a compiled version. Also wondering if we need to provide SHA512 checksums for the bins too.

@chughes297
Copy link
Collaborator

MacOS packages aren't signed, and seem impossible to install on newer macs. They are painful to install on older macs. We probably need to sign the packages, which I believe require paying Apple $100/year?

My knee-jerk reaction to this is helllllllll no. But I guess we can see what @chughes297 thinks.

Why hell no? I need to confirm this but I assume RCVRC can cover those costs, especially if it makes install that much easier.

Also, I'm happy to test the bins for Linux and Windows if you can point me to where they are.

We're now creating a ton of packages: the launchable binaries (jpackage), the java runtime image (jlinkZip), SHAs for each, x3 platforms --> the packages are around 350mb. When a release is created, this won't be an issue since people can just download the relevant file, but we should consider whether we want to support the jlinkZip image indefinitely, or if the jpackage will supercede it

Yeah, I think jpackage will replace jlinkZip. However, we should discuss the ramifications of this with @chughes297 to make sure it doesn't cause any additional security issues having them use a compiled version. Also wondering if we need to provide SHA512 checksums for the bins too.

I need a bit more info about what is being discussed - not sure I understand what the potential security issues are if using a compiled version? And yes I believe we'll need to continue providing checksums for each release (if that's what's being asked lol).

src/main/java/network/brightspots/rcv/Main.java Outdated Show resolved Hide resolved
build.gradle Outdated Show resolved Hide resolved
build.gradle Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/actions/gradle-and-sha/action.yml Outdated Show resolved Hide resolved
.github/actions/gradle-and-sha/action.yml Outdated Show resolved Hide resolved
.github/actions/gradle-and-sha/action.yml Outdated Show resolved Hide resolved
@HEdingfield
Copy link
Contributor

@chughes297: Why hell no? I need to confirm this but I assume RCVRC can cover those costs, especially if it makes install that much easier.

I guess because I'm allergic to costs associated with open source software (especially recurring ones) but, more importantly, I thought all the election tabulation stations ran Windows anyway?

@chughes297: I need a bit more info about what is being discussed - not sure I understand what the potential security issues are if using a compiled version? And yes I believe we'll need to continue providing checksums for each release (if that's what's being asked lol).

I guess I was thinking because it's just an extra level of opacity, which probably also creates a registry entry on their machine (correct me if I'm wrong, @artoonie).

@artoonie artoonie changed the base branch from master to develop April 26, 2023 14:07
@artoonie
Copy link
Collaborator Author

Curious why we're merging this into master instead of develop? Just so we can test it out sooner?

This depended on the hotfix/auto-release branch, and when that was merged into master, github automatically redirected this PR to point at master. Now that that branch is merged into develop, I've updated this PR to also point at develop.

Also, I'm happy to test the bins for Linux and Windows if you can point me to where they are.

https://github.com/BrightSpots/rcv/actions/runs/4801866529
image

The version number is now in three different places. We need an easy way to sync this across the project, but I couldn't find a straightforward way yet. We'll need a ticket to track this.

We should be able to get rid of it in release.yml, right, defaulting to the GitHub tag variable instead? Looks like there are some merge conflicts to resolve with release.yml anyway.

Not anymore -- the version number generated by jpackage is based on the version number in build.gradle, which can differ from the tag or ref_name. I tried this solution, but it doesn't work for development builds, so I'll need to find another solution that works in all cases.

I guess I was thinking because it's just an extra level of opacity, which probably also creates a registry entry on their machine (correct me if I'm wrong, @artoonie).

Yes, that is true -- when installing the software, you cede some control to the operating system which could be seen as reducing transparency. For example: If you try installing 1.3.0 but you previously had 1.4.0 installed, will your operating system decline to downgrade, but show you that it succeeded? The answer is No today, but that's the type of thing we lose control over.

(This may not be the best example of what can go wrong, but it's the only thing I can think of right now.)

However, assuming other election software follows the same model (which I expect they do), I think we can feel safe following the same best practices?

@chughes297
Copy link
Collaborator

chughes297 commented Apr 26, 2023

@chughes297: Why hell no? I need to confirm this but I assume RCVRC can cover those costs, especially if it makes install that much easier.

I guess because I'm allergic to costs associated with open source software (especially recurring ones) but, more importantly, I thought all the election tabulation stations ran Windows anyway?

Most do. So far in my experience, though, some offices have a Linux machine handy and others may have a Mac workstation they can use. I lean towards being as comprehensive as possible with OSes so we can be flexible and jurisdictions can ideally use a piece of hardware they already have sitting around instead of having to buy a new one.

I guess I was thinking because it's just an extra level of opacity, which probably also creates a registry entry on their machine (correct me if I'm wrong, @artoonie).

Yes, that is true -- when installing the software, you cede some control to the operating system which could be seen as reducing transparency. For example: If you try installing 1.3.0 but you previously had 1.4.0 installed, will your operating system decline to downgrade, but show you that it succeeded? The answer is No today, but that's the type of thing we lose control over.

(This may not be the best example of what can go wrong, but it's the only thing I can think of right now.)

However, assuming other election software follows the same model (which I expect they do), I think we can feel safe following the same best practices?

I'm curious to hear more about the security/opacity argument (maybe easier to talk out on the phone?). And I would need to ask vendors what their standard practice is but I imagine it is similar to what Armin described.

@artoonie artoonie force-pushed the feature/issue-57_standalone-executable branch 4 times, most recently from e020825 to eba5ad9 Compare June 8, 2023 05:25
@artoonie artoonie force-pushed the feature/issue-57_standalone-executable branch 4 times, most recently from c95c946 to a527b91 Compare June 8, 2023 06:19
@artoonie artoonie force-pushed the feature/issue-57_standalone-executable branch 2 times, most recently from df2636a to 162175c Compare June 8, 2023 15:42
@artoonie artoonie force-pushed the feature/issue-57_standalone-executable branch from 162175c to 48ddb22 Compare June 8, 2023 15:47
@artoonie artoonie removed the WIP label Jun 8, 2023
@artoonie artoonie changed the title WIP: Test package Standalone Executable Jun 8, 2023
@artoonie
Copy link
Collaborator Author

artoonie commented Jun 8, 2023

@HEdingfield this is ready for review. I've tested on mac, windows, and linux, and the binaries all work. The mac version is signed to avoid warnings with the strictest operating systems -- it's "notarized" which, among other things, sends a hash Apple when building, and that hash is verified by the user's machine before opening.

Copy link
Contributor

@HEdingfield HEdingfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some light changes for you here to review too, per our discussion.

build.gradle Outdated
name = "rcv"
// TODO Sync version number with release.yml and Main.java (github.com/BrightSpots/rcv/issues/662)
name = "RCTab"
version = "1.3.1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume it was 1.3.1 just for testing purposes? Can we set to 1.4.0 now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little afraid to set it to 1.4.0 prematurely, as we'd have fake but signed 1.4.0 versions floating around. I guess the same is true with 1.3.1?

The issue stems from the fact that Mac won't allow a bundle to have a -alpha at the end, so we need some way of signifying a fake/development version number. See: https://stackoverflow.com/questions/70066384/javafx-gradle-jlink-plugin-jpackageimage-fails-because-of-1-0-snapshot-conta

Perhaps we should use 0.0.0 unless we are building a release? Or maybe this just isn't a real issue, and we're okay having "false" 1.4.0s around?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dumb Apple. That may be a blessing in disguise in a small way, though, because we don't want to be creating these builds / releasing alpha versions. I'd say we set it to 1.4.0-alpha here now so the find-replace is easier later (plus I guess it'll block building).

build.gradle Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
@artoonie artoonie force-pushed the feature/issue-57_standalone-executable branch from 72a5ecd to 8d7a87c Compare June 13, 2023 00:31
@artoonie artoonie force-pushed the feature/issue-57_standalone-executable branch from 8d7a87c to 72cd660 Compare June 13, 2023 00:35
Copy link
Contributor

@HEdingfield HEdingfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic job! This would have taken me forever to get going.

.github/workflows/release.yml Show resolved Hide resolved
@HEdingfield HEdingfield merged commit 477994d into develop Jun 14, 2023
@HEdingfield HEdingfield deleted the feature/issue-57_standalone-executable branch June 14, 2023 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants