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

Pure nix builds with patched git revision #962

Merged
merged 3 commits into from
Jul 12, 2023

Conversation

ch1bo
Copy link
Member

@ch1bo ch1bo commented Jun 30, 2023

  • 🍉 Instead of compiling the revision directly into the haskell binary, we keep the actual binary derivation independent of the git index and only depend on the sources needed to build the hydra-node/hydra-tui binaries.

  • 🍉 Then, in a second step, patch the git revision into the binary built by haskell.nix. This allows to re-use the already built binaries when no source changed, but only something unrelated in the repository, e.g. the README.md

  • 🍉 Ultimately this should lead to less rebuilds of the actual binaries in our CI as we can share those derivations.

    A sneak peeks of a rebuild on this branch:

    image


  • CHANGELOG updated or not needed
  • Documentation updated or not needed
  • Haddocks updated or not needed
  • No new TODOs introduced or explained herafter

Original problem description:

Since we are depending on the git revision (via the nix flake), the hydra-node derivation needs to be rebuilt on ANY change to the git index.

This is also the reason why the cardano-node for example does not compile the git revision into the binary, but does patch the string in the final binary:

On a side note, this broke the cardano-node build at least once.

@github-actions
Copy link

github-actions bot commented Jun 30, 2023

Test Results

344 tests  ±0   338 ✔️ ±0   21m 8s ⏱️ - 2m 39s
114 suites ±0       6 💤 ±0 
    6 files   ±0       0 ±0 

Results for commit 912095e. ± Comparison against base commit ab71c58.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Jun 30, 2023

Transactions Costs

Sizes and execution budgets for Hydra protocol transactions. Note that unlisted parameters are currently using arbitrary values and results are not fully deterministic and comparable to previous runs.

Metadata
Generated at 2023-07-12 10:31:13.166283502 UTC
Max. memory units 14000000
Max. CPU units 10000000000
Max. tx size (kB) 16384

Script summary

Name Hash Size (Bytes)
νInitial 2212a4ee618434b9b2f366d7c330dbdfb5c7072e793a850fd0de6ddd 4294
νCommit 69e1ccf9ad73dc6d37a5bc8de5aec86f3c4c1710fe5fd334e0e16b18 2124
νHead 8ae095dca4d14a1b8edffb37faa6c84ec60340fbf389a62f027e0b76 9355
μHead 33642a45c7fbb955ce1704ef09229bb211bf9af9980530db28c6aafe* 4148
  • The minting policy hash is only usable for comparison. As the script is parameterized, the actual script is unique per Head.

Cost of Init Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 4741 14.39 5.66 0.52
2 4950 14.52 5.63 0.53
3 5152 19.60 7.65 0.59
5 5562 23.45 9.07 0.65
10 6586 33.95 12.98 0.81
37 12123 96.84 36.66 1.73

Cost of Commit Transaction

This is using ada-only outputs for better comparability.

UTxO Tx size % max Mem % max CPU Min fee ₳
1 596 14.98 5.74 0.34
2 787 19.57 7.70 0.40
3 975 24.66 9.84 0.46
5 1342 36.15 14.59 0.61
10 2279 71.81 28.88 1.04
13 2838 98.03 39.15 1.35

Cost of CollectCom Transaction

Parties UTxO (bytes) Tx size % max Mem % max CPU Min fee ₳
1 57 815 27.81 10.80 0.49
2 114 1136 43.68 17.10 0.68
3 171 1456 61.48 24.22 0.89
4 227 1776 82.29 32.56 1.13

Cost of Close Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 682 19.37 8.79 0.40
2 671 17.50 7.35 0.37
3 965 21.37 10.82 0.44
5 1295 24.46 13.45 0.50
10 2124 31.42 19.73 0.64
50 8725 87.17 69.99 1.74

Cost of Contest Transaction

Parties Tx size % max Mem % max CPU Min fee ₳
1 676 24.34 10.47 0.45
2 840 26.49 12.01 0.49
3 1006 28.20 13.38 0.52
5 1336 31.20 15.95 0.58
10 2161 40.20 22.97 0.74
45 7929 99.74 70.74 1.81

Cost of Abort Transaction

Some variation because of random mixture of still initial and already committed outputs.

Parties Tx size % max Mem % max CPU Min fee ₳
1 4714 19.24 7.80 0.57
2 5176 36.67 15.56 0.79
3 5496 53.73 22.99 0.99
4 5822 73.10 31.46 1.22
5 6141 95.51 41.26 1.49

Cost of FanOut Transaction

Involves spending head output and burning head tokens. Uses ada-only UTxO for better comparability.

Parties UTxO UTxO (bytes) Tx size % max Mem % max CPU Min fee ₳
5 0 0 4761 8.66 3.57 0.46
5 1 57 4800 10.06 4.39 0.47
5 5 285 4940 15.64 7.69 0.55
5 10 568 5119 22.61 11.82 0.64
5 20 1140 5486 36.56 20.07 0.83
5 30 1707 5843 50.52 28.33 1.02
5 40 2276 6206 64.49 36.60 1.21
5 50 2845 6561 78.46 44.87 1.40
5 65 3698 7100 99.42 57.28 1.68

@ch1bo ch1bo requested review from pgrange, a user, angerman, v0d1ch and ffakenz June 30, 2023 15:42
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I am slightly annoyed this adds a lot of nix code which we'll need to maintain, and we know for a fact @ch1bo is the only nix expert we have in the team. Could not we find a way to have the same functionality implemented outside of nix and only have nix call it?

@ghost ghost force-pushed the ch1bo/pure-build-patched-gitrev branch from 72e9cbd to eb4c247 Compare July 4, 2023 06:39
@ffakenz ffakenz force-pushed the ch1bo/pure-build-patched-gitrev branch from eb4c247 to 1d9b6bd Compare July 4, 2023 14:35
@ghost
Copy link

ghost commented Jul 5, 2023

Should we merge this? I saw noone approved but I guess it's an improvement over what we have ?

@v0d1ch
Copy link
Contributor

v0d1ch commented Jul 6, 2023

yes, I would merge it.

@v0d1ch v0d1ch force-pushed the ch1bo/pure-build-patched-gitrev branch from 1d9b6bd to 8866081 Compare July 6, 2023 09:40
@ch1bo
Copy link
Member Author

ch1bo commented Jul 10, 2023

I am slightly annoyed this adds a lot of nix code which we'll need to maintain, and we know for a fact @ch1bo is the only nix expert we have in the team. Could not we find a way to have the same functionality implemented outside of nix and only have nix call it?

We could move a bit more logic into bash (e.g. the padSuffix function) with the only drawback of needing to write bash instead of nix + build-time errors (while the nix function fails on evaluation should someone ever break it)

@ch1bo ch1bo force-pushed the ch1bo/pure-build-patched-gitrev branch from 8866081 to 6d5b93d Compare July 11, 2023 13:57
@ch1bo
Copy link
Member Author

ch1bo commented Jul 11, 2023

@pgrange I would like to move forward with this. I could not reproduce locally why there was a rebuild after modifying docs/README.md. However, I do not think this is worse than currently on master and I think we could improve the local caching next with something like https://determinate.systems/posts/magic-nix-cache or https://github.com/rikhuijzer/cache-install/blob/master/action.yml#L42 (where this is the key there)

ch1bo added 3 commits July 12, 2023 12:09
This would make the derivation of the executable depend directly on the
git revision of the repository and lead to unnecessary rebuilds.
This allows to patch the binary after build with the git revision.
This is using the embedding at a known placeholder (40 zeros) to patch
the hydra-node and hydra-tui binaries (dynamic and static).

This way, the basic derivation is only depending on the sources and only
the last "step" of patching depends on the git revision.
@pgrange pgrange force-pushed the ch1bo/pure-build-patched-gitrev branch from 6d5b93d to 912095e Compare July 12, 2023 10:09
@pgrange
Copy link
Contributor

pgrange commented Jul 12, 2023

@ch1bo Can you just take a look at the failing CI before merging?
Maybe it's irrelevant but I just noticed this one is failing and don't know what do to with it: https://ci.iog.io/build/248229

@ch1bo
Copy link
Member Author

ch1bo commented Jul 12, 2023

@ch1bo Can you just take a look at the failing CI before merging?
Maybe it's irrelevant but I just noticed this one is failing and don't know what do to with it: https://ci.iog.io/build/248229

Seems to have been a random failure on the build host? Although not reported correctly, it's green now. merging this

@ch1bo ch1bo merged commit 4878d57 into master Jul 12, 2023
@ch1bo ch1bo deleted the ch1bo/pure-build-patched-gitrev branch July 12, 2023 16:48
@ch1bo ch1bo added this to the 0.12.0 milestone Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants