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

Support deterministic build of eclair-core artifact on ubuntu #1295

Merged
merged 6 commits into from Jan 30, 2020

Conversation

@araspitzu
Copy link
Member

araspitzu commented Jan 28, 2020

This PR adds support for deterministic builds of the eclair-core artifact by updating the necessary maven plugins (maven-source-plugin and maven-jar-plugin) to version 3.2.0 which support deterministic builds and hardcoding the property project.build.outputTimestamp to a fixed date (this can be updated at each release) . The deterministic build only works with the submodule eclair-core due to some quirks with the maven-capsule plugin, also the build is not deterministic across different OSs because of the way maven packages files with system-dependent newline markers. The SHA256 of eclair core at commit 93c2683 is: db3064fc4fc9e20c141770294186fbb82b50190ffc13f6bcc09379db2fb33b2b.

This is a first step toward deterministic builds for the mobile apps.

@t-bast

This comment has been minimized.

Copy link
Member

t-bast commented Jan 28, 2020

Ave you tried that: https://maven.apache.org/guides/mini/guide-reproducible-builds.html
It seems that maven 3.6.3 has almost everything needed for reproducible builds out of the box (which would be better than another plugin). But I'm not sure it works :)

@araspitzu

This comment has been minimized.

Copy link
Member Author

araspitzu commented Jan 28, 2020

It didn't work because there are some other non-reproducible bits in the MANIFEST file inside the generated jar, it might work by changing and/or updating other plugins like the maven-jar-plugin but i couldn't get it working and it felt like a rabbit hole, on the other hand maven-reproducible-build works out of the box :) (but unfortunately is yet another plugin to our build)

@araspitzu

This comment has been minimized.

Copy link
Member Author

araspitzu commented Jan 28, 2020

Not sure if we want to add a section in the README.md for the reproducible builds or we just add it to the release page.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 28, 2020

Codecov Report

Merging #1295 into master will increase coverage by 2.19%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1295      +/-   ##
==========================================
+ Coverage   77.18%   79.37%   +2.19%     
==========================================
  Files         143      144       +1     
  Lines        9976    11100    +1124     
  Branches      408      445      +37     
==========================================
+ Hits         7700     8811    +1111     
- Misses       2276     2289      +13
Impacted Files Coverage Δ
...ir-core/src/main/scala/fr/acinq/eclair/Setup.scala 72.76% <0%> (-0.95%) ⬇️
...r/acinq/eclair/payment/send/PaymentLifecycle.scala 91.97% <0%> (-0.83%) ⬇️
...scala/fr/acinq/eclair/payment/PaymentRequest.scala 91.79% <0%> (-0.76%) ⬇️
...clair/payment/send/MultiPartPaymentLifecycle.scala 97.65% <0%> (-0.57%) ⬇️
.../eclair/payment/relay/PostRestartHtlcCleaner.scala 90.06% <0%> (-0.57%) ⬇️
.../fr/acinq/eclair/wire/LightningMessageCodecs.scala 100% <0%> (ø) ⬆️
...core/src/main/scala/fr/acinq/eclair/Features.scala 100% <0%> (ø) ⬆️
...c/main/scala/fr/acinq/eclair/channel/Channel.scala 83.56% <0%> (ø) ⬆️
.../scala/fr/acinq/eclair/payment/PaymentEvents.scala 100% <0%> (ø) ⬆️
.../src/main/scala/fr/acinq/eclair/wire/OpenTlv.scala 100% <0%> (ø)
... and 15 more
@t-bast

This comment has been minimized.

Copy link
Member

t-bast commented Jan 28, 2020

Not sure if we want to add a section in the README.md for the reproducible builds or we just add it to the release page.

I think it's worth adding to the main README so that people find it easily.
We need to specify that it only works on linux, and maybe other querks too.

@pm47

This comment has been minimized.

Copy link
Member

pm47 commented Jan 28, 2020

It didn't work because there are some other non-reproducible bits in the MANIFEST file inside the generated jar, it might work by changing and/or updating other plugins like the maven-jar-plugin

The doc says to upgrade your plugins to reproducible version, particularly maven-source-plugin, maven-jar-plugin and maven-assembly-plugin to version 3.2.0 minimum, have you tried that? Agree with the rabbit hole risk, I'd just like to make sure that we have fully tried the simpler solution.

Not sure if we want to add a section in the README.md for the reproducible builds or we just add it to the release page.

I think it's worth adding to the main README so that people find it easily.
We need to specify that it only works on linux, and maybe other querks too.

How about BUILD.md? And a standard paragraph in each release description (like the part on signature)?

@pm47

This comment has been minimized.

Copy link
Member

pm47 commented Jan 28, 2020

NB, seems the last pain point (update of project.build.outputTimestamp) has been fixed two months ago: https://issues.apache.org/jira/browse/MRELEASE-1029. Seems really tempting. @sstone are you using the release plugin for this project btw?

@araspitzu

This comment has been minimized.

Copy link
Member Author

araspitzu commented Jan 28, 2020

It didn't work because there are some other non-reproducible bits in the MANIFEST file inside the generated jar, it might work by changing and/or updating other plugins like the maven-jar-plugin

The doc says to upgrade your plugins to reproducible version, particularly maven-source-plugin, maven-jar-plugin and maven-assembly-plugin to version 3.2.0 minimum, have you tried that? Agree with the rabbit hole risk, I'd just like to make sure that we have fully tried the simpler solution.

Not sure if we want to add a section in the README.md for the reproducible builds or we just add it to the release page.

I think it's worth adding to the main README so that people find it easily.
We need to specify that it only works on linux, and maybe other querks too.

How about BUILD.md? And a standard paragraph in each release description (like the part on signature)?

Alright, so i just gave another try to the non-plugin based approach and..it worked, i had to update both the maven-source-plugin and maven-jar-plugin to 3.2.0, i think in my earlier attempt i missed the maven-source-plugin and deemed the approach unfeasible too early. As you point out the project.build.outputTimestamp could be updated by the release phase of maven but i'm not sure if we use it during our release process, for the moment i'm hardcoding it to a fixed date. I'm also adding a small paragraph in BUILD.md to mention the eclair-core artifact build is deterministic.

araspitzu added 2 commits Jan 28, 2020
…achieve deterministic builds, remove reproducible-build-maven-plugin.
@araspitzu araspitzu force-pushed the deterministic_build branch from 9f8fd2a to 01663f2 Jan 28, 2020
@sstone

This comment has been minimized.

Copy link
Member

sstone commented Jan 28, 2020

We don't use the maven release plugin yet (testing and tagging the release is done manually)

@sstone

This comment has been minimized.

Copy link
Member

sstone commented Jan 29, 2020

@araspitzu Did you run tests with different versions of JDK11 and different Linux distros ?
I think that it's fine to restrict deterministic builds to specific operating systems and JDKs but it should be documented too.

@araspitzu

This comment has been minimized.

Copy link
Member Author

araspitzu commented Jan 29, 2020

@araspitzu Did you run tests with different versions of JDK11 and different Linux distros ?
I think that it's fine to restrict deterministic builds to specific operating systems and JDKs but it should be documented too.

I didn't try with different distros but I tried with different versions of ubuntu (18.10 vs 17.10) and different minor versions of JDK (JDK11.0.4 vs JDK11.0.3), the resulting artifact was identical. If we're okay restricting the deterministic build to the latest ubuntu version i'll add this to the paragraph in the BUILD.md.

@araspitzu araspitzu changed the title Support deterministic build of eclair-core artifact on unix Support deterministic build of eclair-core artifact on ubuntu Jan 29, 2020
BUILD.md Outdated Show resolved Hide resolved
BUILD.md Outdated Show resolved Hide resolved
@sstone

This comment has been minimized.

Copy link
Member

sstone commented Jan 30, 2020

Can you please update the hash of the eclair-core jar that should be built ? thanks

…e deterministic build
@pm47
pm47 approved these changes Jan 30, 2020
@sstone
sstone approved these changes Jan 30, 2020
@araspitzu araspitzu merged commit 78e6cdb into master Jan 30, 2020
4 checks passed
4 checks passed
ci/semaphoreci/pr: Build & Test The build passed on Semaphore 2.0.
Details
ci/semaphoreci/push: Build & Test The build passed on Semaphore 2.0.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@araspitzu araspitzu deleted the deterministic_build branch Jan 30, 2020
@n1bor

This comment has been minimized.

Copy link
Contributor

n1bor commented Feb 5, 2020

This one builds to the same hash:
eb3091401b8f86ed7c23328acb069b17a0805a4aadeb852bef86c3ab08f591a1 eclair-core_2.11-0.3.3.jar

But as mentioned somewhere node and node-gui jar is not.. which is very annoying as that is the one I actually run...
Was run with:
Ubuntu 19.10 (Vm on Hyper-v)
Apache Maven 3.6.3
openjdk version "11.0.6" 2020-01-14

@sstone

This comment has been minimized.

Copy link
Member

sstone commented Feb 6, 2020

But as mentioned somewhere node and node-gui jar is not.. which is very annoying as that is the one I actually run...

Yes it's mentioned in the PR's description :)
You can manually unzip the eclair-node jar (or eclair-node-gui) and verify the checksum of the included eclair-core jar, but it's tedious. We're working on finding another way of packaging our server apps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.