lodestar/cli: overhaul util/version and gitdata #2958

q9f · 2021-08-13T17:03:07Z

Motivation

Allow for more information in the version string; also clean up util/version and gitdata and reduce duplicated code.

Description

Refactored git data file code and removed some duplicated structures and unused code.
util/gitData/* no longer generates version strings, it just handles all version information (semver, branch, commit, ...)
util/version now generates a proper version string from available data (either from json or from git)
- example: v0.28.1/q9-yarn-semver/+14/b0616bfd (unstable) (semver/branch/number of commits since last release/commit hash/release track if any)
util/version now defines a default release track (stability), for now: alpha
cli/cli improved top banner of the --help output and also used same information for --version

~/.opt/chainsafe/lodestar q9-yarn-semver
❯ ./lodestar --version
🌟 Lodestar: TypeScript Implementation of the Ethereum 2.0 Beacon Chain.
  * Version: v0.28.1/q9-yarn-semver/+14/b0616bfd (unstable)
  * by ChainSafe Systems, 2018-2021

~/.opt/chainsafe/lodestar q9-yarn-semver
❯ ./lodestar --help   
🌟 Lodestar: TypeScript Implementation of the Ethereum 2.0 Beacon Chain.
  * Version: v0.28.1/q9-yarn-semver/+14/b0616bfd (unstable)
  * by ChainSafe Systems, 2018-2021

Commands:
  beacon             Run a beacon chain node
  validator          Run one or multiple validator clients
  account <command>  Utilities for generating and managing Ethereum 2.0 accounts
  init               Initialize Lodestar directories and files necessary to run
                     a beacon chain node. This step is not required, and should
                     only be used to prepare special configurations
  dev                Quickly bootstrap a beacon node and multiple validators.
                     Use for development and testing

Options:
      --rootDir     Lodestar root directory                             [string]
      --network     Name of the Eth2 chain network to join
           [string] [choices: "mainnet", "pyrmont", "prater", "altair-devnet-3"]
                                                            [default: "mainnet"]
      --preset      Specifies the default eth2 spec type
                   [string] [choices: "mainnet", "minimal"] [default: "mainnet"]
      --paramsFile  Network configuration file
                                        [string] [default: $rootDir/config.yaml]
  -h, --help        Show help                                          [boolean]
  -v, --version     Show version number                                [boolean]

📖 For more information, check the CLI reference:
  * https://chainsafe.github.io/lodestar/reference/cli

✍️ Give feedback and report issues on GitHub:
  * https://github.com/ChainSafe/lodestar

The code also handles the logic of releases and dirty/nightly builds. A release version string is simply v0.28.1 (alpha) whereas dirty builds have all info v0.28.1/q9-yarn-semver/+14/b0616bfd (unstable). If git data is not available, it will fall back to lerna.json/package.json and just use 0.28.1.

This is my first typescript contribution. Review with care. 🙏🏼

Fixes #2950

CLAassistant · 2021-08-13T17:03:11Z

All committers have signed the CLA.

codeclimate · 2021-08-13T17:03:46Z

Code Climate has analyzed commit 9a54076 and detected 0 issues on this pull request.

View more on Code Climate.

lgtm-com · 2021-08-13T17:12:12Z

This pull request introduces 3 alerts when merging d43112e into 80c248b - view on LGTM.com

new alerts:

2 for Invocation of non-function
1 for Unused variable, import, function or class

q9f · 2021-08-16T11:24:57Z

I would appreciate if we can make a test point release (v0.28.2) if this is good to go.

github-actions · 2021-08-16T11:32:58Z

Performance Report

✔️ no performance regression detected

Full benchmark results

Benchmark suite	Current: `56bce48`	Previous: `c664482`	Ratio
getCommitteeAssignments - req 1000 vs - 250000 vc	7.6097 ms/op	7.9364 ms/op	0.96
processBlock altair 250000 vs - 7PWei - maxed ops	448.28 ms/op	418.20 ms/op	1.07
processBlock altair 250000 vs - 7PWei - average	173.55 ms/op	144.77 ms/op	1.20
epoch altair - 250000 vs - 7PWei - processInactivityUpdates	2.4972 s/op	2.3585 s/op	1.06
epoch altair - 250000 vs - 7PWei - processRewardsAndPenalties	1.1121 s/op	1.0776 s/op	1.03
epoch altair - 250000 vs - 7PWei - processParticipationFlagUpdates	464.95 ms/op	437.87 ms/op	1.06
processBlock phase0 250000 vs - 7PWei - maxed ops	206.86 ms/op	184.29 ms/op	1.12
processBlock phase0 250000 vs - 7PWei - average	29.164 ms/op	24.133 ms/op	1.21
epoch phase0 - 250000 vs - 7PWei - beforeProcessEpoch	1.6319 s/op	1.5556 s/op	1.05
epoch phase0 - 250000 vs - 7PWei - processRewardsAndPenalties	718.14 ms/op	656.81 ms/op	1.09
epoch phase0 - 250000 vs - 7PWei - processEffectiveBalanceUpdates	119.06 ms/op	106.76 ms/op	1.12
getAttestationDeltas - 250000 vs - 7PWei	114.68 ms/op	129.11 ms/op	0.89
processSlots - 250000 vs - 7PWei - 32 empty slots	3.2020 s/op	3.1690 s/op	1.01
shuffle list - 16384 els	15.508 ms/op	24.883 ms/op	0.62
shuffle list - 250000 els	221.03 ms/op	365.94 ms/op	0.60
getPubkeys - persistent - req 1000 vs - 250000 vc	17.630 us/op	15.847 us/op	1.11
BLS verify - blst-native	2.0893 ms/op	1.8577 ms/op	1.12
BLS verifyMultipleSignatures 3 - blst-native	4.2726 ms/op	3.8112 ms/op	1.12
BLS verifyMultipleSignatures 8 - blst-native	8.9437 ms/op	8.2021 ms/op	1.09
BLS verifyMultipleSignatures 32 - blst-native	32.862 ms/op	29.852 ms/op	1.10
BLS aggregatePubkeys 32 - blst-native	45.171 us/op	40.787 us/op	1.11
BLS aggregatePubkeys 128 - blst-native	170.96 us/op	155.89 us/op	1.10
getAttestationsForBlock	242.22 ms/op	222.66 ms/op	1.09
validate gossip signedAggregateAndProof - struct	8.2628 ms/op	4.5075 ms/op	1.83
validate gossip signedAggregateAndProof - treeBacked	5.1218 ms/op	4.4777 ms/op	1.14
validate gossip attestation - struct	4.8099 ms/op	4.4260 ms/op	1.09
validate gossip attestation - treeBacked	2.6174 ms/op	2.2206 ms/op	1.18

by benchmarkbot/action

wemeetagain

LGTM!

I tested locally against this branch and with a mock v0.28.2 release.
This shouldn't effect our npm releases, it will just make lodestar --version more accurate.

q9f · 2021-08-17T11:02:55Z

Ok, fixed the linter issues, this should be good to go.

q9f · 2021-08-17T14:53:50Z

Sorry, fixed one minor logic issue. Now it's good to go.

dapplion

Looks great! Much more accurate and polished than before.

The purpose of the gitData file was to distribute the commit at which that version is built to NPM. Since (package.json).version only includes the semver, that information is not available and you need to go to git to check the commit associated to that tag. That has been very useful for nightly releases since we show the "version" tag (version + branch + commit) in the metrics panel, so very quickly you can check what that node is running from Grafana.

Do you think we achieve a similar behavior is more clean and polished way?

q9f · 2021-08-17T17:10:20Z

Thanks for the review. The logic around gitData was not changed and should be still available. Do you mean, we should expose branch and commit on the metrics even though it's usually a tag release? Metrics is the only place where I changed it for simplicity but happy to add back more details.

dapplion · 2021-08-17T17:35:03Z

@q9f Another better option would be to use nightly versioning as 0.27.1-dev.a6b280 instead of what we have right now that's just a counter 0.27.1-dev.31 👍

q9f · 2021-08-18T11:34:14Z

@q9f Another better option would be to use nightly versioning as 0.27.1-dev.a6b280 instead of what we have right now that's just a counter 0.27.1-dev.31 +1

I just checked, the nighly release version is 0.28.2-dev.3+022659b5fc0 according to Github actions (appending the --canary git sha option) but the NPM registry just truncates everything after the + to strict (?) semver and it just remains 0.28.2-dev.3. I found this: lerna/lerna#1893

Association with the current NPM versions is very hard. If the published version is foo@0.1.5-alpha.124, I have to count 124 commits up from 0.1.4 or go through CI logs to figure out which commit that version is tied to. If it were foo@0.1.5-alpha.124.3d6bee55 or foo@0.1.5-alpha.3d6bee55 it would be much easier.

It looks like Lerna is publishing with the +${sha} suffix, but it seems NPM is stripping it out. If that's the case, why is this format not configurable?

At first glance, there seems no easy way to force lerna to do that properly.

dapplion · 2021-08-22T11:35:21Z

@q9f Another better option would be to use nightly versioning as 0.27.1-dev.a6b280 instead of what we have right now that's just a counter 0.27.1-dev.31 +1

I just checked, the nighly release version is 0.28.2-dev.3+022659b5fc0 according to Github actions (appending the --canary git sha option) but the NPM registry just truncates everything after the + to strict (?) semver and it just remains 0.28.2-dev.3. I found this: lerna/lerna#1893

Association with the current NPM versions is very hard. If the published version is foo@0.1.5-alpha.124, I have to count 124 commits up from 0.1.4 or go through CI logs to figure out which commit that version is tied to. If it were foo@0.1.5-alpha.124.3d6bee55 or foo@0.1.5-alpha.3d6bee55 it would be much easier.
It looks like Lerna is publishing with the +${sha} suffix, but it seems NPM is stripping it out. If that's the case, why is this format not configurable?

At first glance, there seems no easy way to force lerna to do that properly.

What if we write our own version format? Not use any counter at all and only the commit hash:

0.28.2-dev.022659b5fc0

q9f added 6 commits August 13, 2021 17:42

lodestar/cli: add version string to top banner

9ec6e91

lodestar/cli: fix graffiti default package info

29c47fa

lodestar/cli: update cmds handlers to use version instead of git data

a5230b4

lodestar/cli: clean up util/gitdata logic

230ab75

lodestar/cli: allow for release probing

c1dca1a

lodestar/cli: allow version util to be more verbose about dirty versions

d43112e

github-actions bot added the CLI label Aug 13, 2021

q9f added 2 commits August 16, 2021 12:44

lodestar/cli: reenable git data file persistence

7201397

lodestar/cli: add top banner and proper version string to -v

929d03f

q9f added cosmetic labels Aug 16, 2021

q9f requested a review from dapplion August 16, 2021 11:23

q9f marked this pull request as ready for review August 16, 2021 11:23

q9f requested review from mpetrunic, twoeths and wemeetagain as code owners August 16, 2021 11:23

lodestar/cli: restore git data file comments

21eb15c

lodestar/cli: address eslint concerns

2dea0ac

wemeetagain previously approved these changes Aug 16, 2021

View reviewed changes

q9f added 3 commits August 17, 2021 12:33

chore: merge branch origin/master into q9-yarn-semver

e81fb3f

lodestar/cli: define a default release track

b29660a

chore: fix linter issues

b0616bf

q9f dismissed wemeetagain’s stale review via b0616bf August 17, 2021 10:57

lodestar/cli: prefix lerna json version string with v

8666615

wemeetagain previously approved these changes Aug 17, 2021

View reviewed changes

lodestar/cli: fall back to persistent git data if no cli avail

9a54076

q9f dismissed wemeetagain’s stale review via 9a54076 August 17, 2021 14:52

wemeetagain approved these changes Aug 17, 2021

View reviewed changes

dapplion approved these changes Aug 17, 2021

View reviewed changes

dapplion merged commit b9a7e6a into master Aug 17, 2021

dapplion deleted the q9-yarn-semver branch August 17, 2021 17:35

q9f mentioned this pull request Aug 18, 2021

cli/cmds: restore raw git data version info in metrics #2970

Merged

q9f mentioned this pull request Aug 23, 2021

review release tracks and versioning scheme #2990

Closed

q9f added status5-mergeready and removed status3-needsreview labels Sep 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lodestar/cli: overhaul util/version and gitdata #2958

lodestar/cli: overhaul util/version and gitdata #2958

q9f commented Aug 13, 2021 •

edited

Loading

CLAassistant commented Aug 13, 2021 •

edited

Loading

codeclimate bot commented Aug 13, 2021 •

edited

Loading

lgtm-com bot commented Aug 13, 2021

q9f commented Aug 16, 2021

github-actions bot commented Aug 16, 2021 •

edited

Loading

wemeetagain left a comment

q9f commented Aug 17, 2021

q9f commented Aug 17, 2021

dapplion left a comment

q9f commented Aug 17, 2021

dapplion commented Aug 17, 2021

q9f commented Aug 18, 2021

dapplion commented Aug 22, 2021

lodestar/cli: overhaul util/version and gitdata #2958

lodestar/cli: overhaul util/version and gitdata #2958

Conversation

q9f commented Aug 13, 2021 • edited Loading

CLAassistant commented Aug 13, 2021 • edited Loading

codeclimate bot commented Aug 13, 2021 • edited Loading

lgtm-com bot commented Aug 13, 2021

q9f commented Aug 16, 2021

github-actions bot commented Aug 16, 2021 • edited Loading

Performance Report

wemeetagain left a comment

Choose a reason for hiding this comment

q9f commented Aug 17, 2021

q9f commented Aug 17, 2021

dapplion left a comment

Choose a reason for hiding this comment

q9f commented Aug 17, 2021

dapplion commented Aug 17, 2021

q9f commented Aug 18, 2021

dapplion commented Aug 22, 2021

q9f commented Aug 13, 2021 •

edited

Loading

CLAassistant commented Aug 13, 2021 •

edited

Loading

codeclimate bot commented Aug 13, 2021 •

edited

Loading

github-actions bot commented Aug 16, 2021 •

edited

Loading