Turning the ethereumjs-vm into a monorepo #666

evertonfraga · 2020-02-25T19:04:56Z

This PR introduces the Monorepo, bringing the libraries ethereumjs-account, ethereumjs-block, ethereumjs-blockchain, ethereumjs-common and ethereumjs-tx to this repository.

How to review this PR

I advise looking directly to my fork's commits for a better understanding of the changes, as the comparison view in this PR is huge.

Every commit was generated by this script I made, so I could work on changes and improve the migration for several iterations. I made my best efforts to keep the git history clean, despite the huge changes around.

A brief explanation of the commits involved in this PR:

1. `monorepo: moving common`

Moving each library source code down to ./packages

2. `Merge remote-tracking branch 'account/master' into monorepo`

These commits merges the whole history of each package to this repo, maintaining the commit hashes, authors and dates. Mind this issue, though #657.

3. `Implementing Lerna`

Adds Lerna config and package.json files

4. `Lerna config: publishConfig.directory`

This is needed for integrated module testing/development with TypeScript, linked by Lerna. Instead of using files from dist/, it will look for files in src/.

5. `monorepo: Moving .github files to root; deleting other CI files`

Unify all packages' .github workflow files to the root's .github/
Same with other GitHub files (auto-labeler), contributing.md
Codecov does not offer support for coverage for external PRs just yet, so here we delete all GitHub coverage files. Coverage is handled by a CircleCI workflow, added in a following commit.

6. `monorepo: Adding test cascade directives`

Injects paths-ignore directives to github actions so that it will run only tests from affected packages in that particular change, instead of always running all tests. More info: #561 (comment)

7. `monorepo: Fixing execution paths for github actions`

Injects cwd directives to run each packages' tests in their own directory.

8. `Generating docs with updated urls`

Regenerating all documentation, minding the updated monorepo URLs.

9. `Updating tag comparison links in CHANGELOG`

Updates all links in CHANGELOG to account for the new tag format (eg: @ethereumjs/account@1.0.3)

10. `Adding peripheral files`

Adding important files to the new monorepo structure:

README.md -> has a nice explainer of the monorepo and a ton of badges.
.circleci/config.yml -> used only for coverage reporting to Codecov.
greenkeeper.json -> Greenkeeper configured for the monorepo. Tested in my fork and works nicely 👌
scripts/ -> preparation for the integrated TS testing with Lerna (not included in this PR)
package.json

Closes #561.

Org links and git hooks

…ooks Org links and git hooks

TypeScript migration

…transactions Fix signing eip155 transactions

chore(package): update husky to version 2.1.0

…ooks Org links and git hooks

…12.0 Update util to the latest version 🚀

Add node 11 to travis.yml

evertonfraga · 2020-02-25T19:24:02Z

GitHub might have had a hard time detecting the new test files, but they are all passing:

https://github.com/evertonfraga/ethereumjs-vm/actions?query=branch%3Amaster

holgerd77 · 2020-02-26T12:37:55Z

Ok, did some practical walkthrough on various things. some questions, eventually improvements, but at least on this round no blockers along the way. 😄 I would generally suggest that we try to avoid to regenerate this PR on improvement suggestions if things are not grave to gain further confidence in the respective PR submitted and rather do subsequent changes on follow-up PRs.

I would also suggest to do the merge rather quickly (but also: with no rush) - eventually already tomorrow - to get back to some consistent new state. This update is so huge that I think it is better to not allow a too long period for unexpected events to occur somewhat drifting away from a stable state.

Nevertheless there should be 2-3 more voices here before a merge.

EthereumJS VM Monorepo Transition - Practical Experience Report

Experience Report

Did a fresh fork from the original EthereumJS repo to https://github.com/holgerd77/ethereumjs-vm and the fork from Everton with master containing the monorepo changes to https://github.com/vyper-online/ethereumjs-vm
Opened a monorepo PR simulation PR Monorepo add devp2p holgerd77/ethereumjs-vm#1 and merged, everything went well (side consistency check: number of merged commits both on 1,320 commits)
Did a commit hash search on one of the integrated hashes 42bbb5f2ddbad44ccf264b5c689d3a6471bfe039, found the original commit on the block library
Did an npm install on an exemplary package package account and run the tests with npm run test, worked fine, also did a negative check by letting one of the tests fail, this was reported correctly
GitHub Actions CI runs triggered by the merge actually also passed without problems
Did both a positive and a negative (adding some space characters) linting test (npm run lint) on the account libraries, passed and failed as expected
Did a test PR on the account package updating a random dev dependency https://github.com/holgerd77/ethereumjs-vm/pull/2, this triggered the account and common (not sure on this combination TBH) GitHub lint and test workflows respectively actions (using some cached results though)
Side quest: checked the API docs from the account package, both internal documentation linking and code links worked well
Side note: also after 7. still not able to see the new workflows in the branch protection rule setup settings, not sure if this needs some more time, no blocker though
Opened a second test PR with a code change on the VM https://github.com/holgerd77/ethereumjs-vm/pull/3, this time VM, blockchain and block linting + tests are triggered (still not sure here, at least the package affected is triggered as well). This time CI runs are not cached, passing after some time, doing the merge.
I had a short but not too extensive look on the release side. Do I get this right that we can do all eventual release republishing, package renaming and additional lerna configuration in a separate step?

Remarks/questions/reassurances along the way

R1: package.json files (e.g. for the account package) still contain the old package name and URL formats, this is intended I suppose?

R2: Stumbled upon this scripts folder with a generic script execution structure (nice!) and actually executing on one script modifying the StateManager.ts file, script is triggered by the test:prepare command included in the main package.json file. Is this a one time thing? What is this for? (Forgive me if stuff like this already has been answered, since scope of this is so huge I might have lost oversight here and there. Then just point me to the comment already made)

R3: Various URLs in the main README.md still contain fork references (to Evertons fork), this is likely also a post-monorepo-PR-merge new PR change?

Some follow-up items

I1: For pre-monorepo-fork submitted PRs on the VM I would have a tendency towards a policy that rebases are not allowed and PRs have to be resubmitted to be accepted. Does this make sense?

holgerd77 · 2020-02-26T14:15:56Z

Will do a commit review by commenting on the numbered list items below since many changes are not applicable for direct comments.

1. monorepo: moving common

Moving each library source code down to ./packages

✅ 6 commits for each library moved, did some simple checks on completeness, looks good

2. Merge remote-tracking branch 'account/master' into monorepo

These commits merges the whole history of each package to this repo, maintaining the commit hashes, authors and dates. Mind this issue, though #657.

✅ 5 commits for each of the external libraries

Comparing the "Changes from x commits number on each merge commit summary page (see screenshot below, e.g. account merge commit) with the number of commits on the library itself.

account, commit: 136, library: 134
block, commit: 250, library: 274
blockchain, commit: 246, library: 244
tx, commit: 250, library: 483
common, commit: 168, library: 166

=> consistent pattern (2 more on the commit side) except for block and tx

🚫 @evertonfraga Do you have an idea where those missing commits are respectively is there some reasoning for the differences in the numbers?

3. Implementing Lerna

Adds Lerna config and package.json files

✅ Kind of a stub, can be added/updated later

4. Lerna config: publishConfig.directory

This is needed for integrated module testing/development with TypeScript, linked by Lerna. Instead of using files from dist/, it will look for files in src/.

✅ This is referring to this lerna directive, associated commit here, simple change, added to all six packages

5. monorepo: Moving .github files to root; deleting other CI files

Unify all packages' .github workflow files to the root's .github/

Same with other GitHub files (auto-labeler), contributing.md

Codecov does not offer support for coverage for external PRs just yet, so here we delete all GitHub coverage files. Coverage is handled by a CircleCI workflow, added in a following commit.

✅ Ok, transfers look good, also indicated to work by the triggered GitHub action runs on the test forks

6. monorepo: Adding test cascade directives

Injects paths-ignore directives to github actions so that it will run only tests from affected packages in that particular change, instead of always running all tests. More info: #561 (comment)

⚠️ Not completely getting the selection of packages to run (related to my first impression description on CI runs) (e.g. why is common not ignoring the account package), here is the associated commit, otherwise ok

7. monorepo: Fixing execution paths for github actions

Injects cwd directives to run each packages' tests in their own directory.

✅ Ok, done for all six packages

8. Generating docs with updated urls

Regenerating all documentation, minding the updated monorepo URLs.

⚠️ This is removing a lot of descriptive information (see e.g. here respectively further scroll through the commit diff), eventually not a blocker since this is generated stuff and the information is not lost, but minimally something which needs to get a closer look later on (if you haven't got a clear instant explanation on what is happening)

9. Updating tag comparison links in CHANGELOG

Updates all links in CHANGELOG to account for the new tag format (eg: @ethereumjs/account@1.0.3)

🚫 I think there is the package name missing in the second part of the URL comparison, see e.g.:

@ethereumjs/account@2.0.1...@ethereumjs/2.0.2

10. Adding peripheral files

Adding important files to the new monorepo structure:

README.md -> has a nice explainer of the monorepo and a ton of badges.

.circleci/config.yml -> used only for coverage reporting to Codecov.

greenkeeper.json -> Greenkeeper configured for the monorepo. Tested in my fork and works nicely 👌

scripts/ -> preparation for the integrated TS testing with Lerna (not included in this PR)

package.json

⚠️ Here is this question on the URLs with fork references in the README.md file for the badges.

Summary:
Likely too many things to address for a possible merge tomorrow, so definitely to be postponed to - minimally - sometime during Friday.

Thanks Everton, this is really outstanding work, totally admire all the discipline you put up in preparing and documenting all this stuff! 👍 🙏 😄

evertonfraga · 2020-02-26T14:35:33Z

Thanks for the thorough review, as always @holgerd77. I will respond to each topic acknowledging/reasoning/fixing as I go.

evertonfraga · 2020-02-26T15:25:57Z

Experience Report

Did a test PR on the account package updating a random dev dependency https://github.com/holgerd77/ethereumjs-vm/pull/2, this triggered the account and common (not sure on this combination TBH) GitHub lint and test workflows respectively actions (using some cached results though)

OK I will take a look. the combination really does not look correct. (script)

I had a short but not too extensive look on the release side. Do I get this right that we can do all eventual release republishing, package renaming and additional lerna configuration in a separate step?

Correct. Releases are external to the source code (and can't have PRs for validation), they will be handled separately. I believe it would be an unnecessary risk to migrate to scoped packages in the same PR. I wanted to keep this PR straightforward, with as few changes to the source code as possible, to keep reviewability at a good level.

Please take a look at the post-migration steps on the monorepo board.

Remarks/questions/reassurances along the way

R1: package.json files (e.g. for the account package) still contain the old package name and URL formats, this is intended I suppose?

Correct, I will handle those along with the scoped package publishing.

R2: Stumbled upon this scripts folder with a generic script execution structure (nice!) and actually executing on one script modifying the StateManager.ts file, script is triggered by the test:prepare command included in the main package.json file. Is this a one time thing? What is this for? (Forgive me if stuff like this already has been answered, since scope of this is so huge I might have lost oversight here and there. Then just point me to the comment already made)

ethereumjs-common is not exporting any genesis files to dist/index.js, to keep index.js at a manageable size. So StateManager.ts imports files from dist/. In the context of Lerna package linking + TypeScript, we can't be certain that dist/ files will be available, so that patch (npm run test:prepare) performs the change to the import, ideally in the CI/testing scope.

Mind that testing with linked packages is not yet deployed to this PR, and the scripts/ directory is for now pretty much "dead code".

R3: Various URLs in the main README.md still contain fork references (to Evertons fork), this is likely also a post-monorepo-PR-merge new PR change?

I kept my own fork URLs just so you can refer back to the fork page and see all the badges working (especially codecov and github actions). Now that you validated them, I will send a commit pointing to ethereumjs-vm :)

Some follow-up items

I1: For pre-monorepo-fork submitted PRs on the VM I would have a tendency towards a policy that rebases are not allowed and PRs have to be resubmitted to be accepted. Does this make sense?

That makes sense. There are only a few of them and it's pretty easy to reapply commits.

evertonfraga · 2020-02-26T15:57:53Z

Commit review

🚫 @evertonfraga Do you have an idea where those missing commits are respectively is there some reasoning for the differences in the numbers?

GitHub poses a hard cap of 250 commits they display on the website in PR comparisons:

But doing the math, we can achieve the exact number this PR aims to merge (1,321 commits)

⚠️ This is removing a lot of descriptive information (see e.g. here respectively further scroll through the commit diff)…

Right! That slipped my mind. Will take a look.

🚫 I think there is the package name missing in the second part of the URL comparison, see e.g.:

Great catch! I'll tweak the RegExp and run only this part of the script again. Will send a commit soon.

⚠️ Here is this question on the URLs with fork references in the README.md file for the badges.

Sent the permanent commit already (9633415).

Thanks Everton, this is really outstanding work, totally admire all the discipline you put up in preparing and documenting all this stuff! 👍 🙏 😄

Thanks for the kind words, @holgerd77! My pleasure.

holgerd77 · 2020-02-26T16:58:15Z

@evertonfraga ah, this all sounds more manageable than I had hoped for, maybe we can even still merge tomorrow during the day, we'll see.

Let me/us explicitly know when you think this is ready to be merged.

evertonfraga · 2020-02-26T18:22:55Z

Remaining tasks for this PR:

Point README badges to ethereumjs repo
Fix changelog links
GitHub actions exclusion rules
Fix docs:build content dropping (@ryanio is taking that one)

* upgrade typedoc and use `library` mode * generate docs * run lint:fix * remove vm state/promisified from docs * common: remove tests/* and other unnecessary files from typedoc * blockchain: remove unnecessary files from docs * run lint:fix

evertonfraga · 2020-02-27T02:05:04Z

This PR is ready for review.

holgerd77 · 2020-02-27T09:48:36Z

Thanks @ryanio and @evertonfraga!

Point README badges to ethereumjs repo

This looks good, seems that some badges will only be generated after the first corresponding CI run, so not displaying yet. Anyhow, even if there would be some broken badge links this wouldn't be a blocker though.

Fix changelog links

Looks good now.

GitHub actions exclusion rules

Ok, commit looks good.

Fix docs:build content dropping (@ryanio is taking that one)

Ok, nice. 😄

Great.

I will give this another 3+ hours waiting period for some last minute chance to raise a voice here and otherwise merge.

Intended Merge Time between 2-3pm CET (Thursday)

holgerd77

LGTM

holgerd77 · 2020-02-27T14:11:07Z

Monorepo. 😄

evertonfraga · 2020-02-27T14:20:04Z

Yay!

holgerd77 and others added 30 commits April 26, 2019 12:55

Added EthereumJS footer with organization docs linking

e5c8ebc

Added husky (Git hooks) dependency and pre-push hook to package.json

69ebd28

Added contributing file to .github folder

3147aa7

Added EthereumJS footer with organization docs linking

0360166

Added husky (Git hooks) dependency and pre-push hook to package.json

540f59f

Merge pull request ethereumjs#70 from ethereumjs/org-links-and-git-hooks

2c3908e

Org links and git hooks

Added contributing file to .github folder

90bbfd5

Added EthereumJS footer with organization docs linking

1f073b8

Added husky (Git hooks) dependency and pre-push hook to package.json

d2da7f9

Merge pull request ethereumjs#50 from ethereumjs/org-links-and-git-hooks

0a0fcb1

Org links and git hooks

Merge pull request ethereumjs#107 from ethereumjs/org-links-and-git-h…

0d60ae1

…ooks Org links and git hooks

Add files field to package.json

cdc8db1

Remove .babelrc

3cbfb5e

Merge pull request ethereumjs#145 from alcuadrado/typescript-migration

0d87acf

TypeScript migration

Merge pull request ethereumjs#147 from ethereumjs/fix-signing-eip155-…

4a665c2

…transactions Fix signing eip155 transactions

Added contributing file to .github folder

84a439c

Added EthereumJS footer with organization docs linking

46b7ca9

Added husky (Git hooks) dependency and pre-push hook to package.json

f4d68fb

Merge pull request ethereumjs#50 from ethereumjs/greenkeeper/husky-2.1.0

8305636

chore(package): update husky to version 2.1.0

Merge pull request ethereumjs#148 from ethereumjs/org-links-and-git-h…

f392be4

…ooks Org links and git hooks

fix(package): update util to version 0.12.0

4f877df

Merge pull request ethereumjs#108 from ethereumjs/greenkeeper/util-0.…

977b67d

…12.0 Update util to the latest version 🚀

Add node 11 to .travis.yml

280fce6

Merge pull request ethereumjs#146 from ethereumjs/travis-node11

3648da4

Add node 11 to travis.yml

Simplify EIP155 handling.

37d43fc

Update tests

d0c7375

Validate v in its setter

48e4f57

Fix bug when signing an already signed tx

1159315

Remove outdated comment

27b8876

Restore the Transaction's toJSON method

7f08267

evertonfraga requested review from holgerd77, ryanio, alcuadrado, axic and s1na and removed request for ryanio February 25, 2020 19:05

This was referenced Feb 26, 2020

Add initialization method to prevent race conditions #665

Merged

Refactor opcodes #664

Merged

Pointing badges to ethereumjs repos and services

9633415

evertonfraga marked this pull request as ready for review February 26, 2020 15:34

fix: changelog comparison urls

93b6028

evertonfraga and others added 2 commits February 26, 2020 14:31

Updating workflows path selective run

211882d

Upgrade typedoc (#11)

48748e6

* upgrade typedoc and use `library` mode * generate docs * run lint:fix * remove vm state/promisified from docs * common: remove tests/* and other unnecessary files from typedoc * blockchain: remove unnecessary files from docs * run lint:fix

holgerd77 approved these changes Feb 27, 2020

View reviewed changes

holgerd77 merged commit e2c191b into ethereumjs:master Feb 27, 2020

evertonfraga added the package: vm label Feb 27, 2020

evertonfraga added this to the VM v5 milestone Jun 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Turning the ethereumjs-vm into a monorepo #666

Turning the ethereumjs-vm into a monorepo #666

evertonfraga commented Feb 25, 2020 •

edited

Loading

evertonfraga commented Feb 25, 2020 •

edited

Loading

holgerd77 commented Feb 26, 2020 •

edited

Loading

holgerd77 commented Feb 26, 2020 •

edited

Loading

1. `monorepo: moving common`

2. `Merge remote-tracking branch 'account/master' into monorepo`

3. `Implementing Lerna`

4. `Lerna config: publishConfig.directory`

5. `monorepo: Moving .github files to root; deleting other CI files`

6. `monorepo: Adding test cascade directives`

7. `monorepo: Fixing execution paths for github actions`

8. `Generating docs with updated urls`

9. `Updating tag comparison links in CHANGELOG`

10. `Adding peripheral files`

evertonfraga commented Feb 26, 2020

evertonfraga commented Feb 26, 2020 •

edited

Loading

Experience Report

Remarks/questions/reassurances along the way

Some follow-up items

evertonfraga commented Feb 26, 2020

Commit review

holgerd77 commented Feb 26, 2020

evertonfraga commented Feb 26, 2020 •

edited

Loading

evertonfraga commented Feb 27, 2020

holgerd77 commented Feb 27, 2020

holgerd77 left a comment

holgerd77 commented Feb 27, 2020

evertonfraga commented Feb 27, 2020

Turning the ethereumjs-vm into a monorepo #666

Turning the ethereumjs-vm into a monorepo #666

Conversation

evertonfraga commented Feb 25, 2020 • edited Loading

How to review this PR

1. monorepo: moving common

2. Merge remote-tracking branch 'account/master' into monorepo

3. Implementing Lerna

4. Lerna config: publishConfig.directory

5. monorepo: Moving .github files to root; deleting other CI files

6. monorepo: Adding test cascade directives

7. monorepo: Fixing execution paths for github actions

8. Generating docs with updated urls

9. Updating tag comparison links in CHANGELOG

10. Adding peripheral files

evertonfraga commented Feb 25, 2020 • edited Loading

holgerd77 commented Feb 26, 2020 • edited Loading

EthereumJS VM Monorepo Transition - Practical Experience Report

Experience Report

Remarks/questions/reassurances along the way

Some follow-up items

holgerd77 commented Feb 26, 2020 • edited Loading

1. monorepo: moving common

2. Merge remote-tracking branch 'account/master' into monorepo

3. Implementing Lerna

4. Lerna config: publishConfig.directory

5. monorepo: Moving .github files to root; deleting other CI files

6. monorepo: Adding test cascade directives

7. monorepo: Fixing execution paths for github actions

8. Generating docs with updated urls

9. Updating tag comparison links in CHANGELOG

10. Adding peripheral files

evertonfraga commented Feb 26, 2020

evertonfraga commented Feb 26, 2020 • edited Loading

Experience Report

Remarks/questions/reassurances along the way

Some follow-up items

evertonfraga commented Feb 26, 2020

Commit review

holgerd77 commented Feb 26, 2020

evertonfraga commented Feb 26, 2020 • edited Loading

evertonfraga commented Feb 27, 2020

holgerd77 commented Feb 27, 2020

Intended Merge Time between 2-3pm CET (Thursday)

holgerd77 left a comment

Choose a reason for hiding this comment

holgerd77 commented Feb 27, 2020

evertonfraga commented Feb 27, 2020

evertonfraga commented Feb 25, 2020 •

edited

Loading

1. `monorepo: moving common`

2. `Merge remote-tracking branch 'account/master' into monorepo`

3. `Implementing Lerna`

4. `Lerna config: publishConfig.directory`

5. `monorepo: Moving .github files to root; deleting other CI files`

6. `monorepo: Adding test cascade directives`

7. `monorepo: Fixing execution paths for github actions`

8. `Generating docs with updated urls`

9. `Updating tag comparison links in CHANGELOG`

10. `Adding peripheral files`

evertonfraga commented Feb 25, 2020 •

edited

Loading

holgerd77 commented Feb 26, 2020 •

edited

Loading

holgerd77 commented Feb 26, 2020 •

edited

Loading

1. `monorepo: moving common`

2. `Merge remote-tracking branch 'account/master' into monorepo`

3. `Implementing Lerna`

4. `Lerna config: publishConfig.directory`

5. `monorepo: Moving .github files to root; deleting other CI files`

6. `monorepo: Adding test cascade directives`

7. `monorepo: Fixing execution paths for github actions`

8. `Generating docs with updated urls`

9. `Updating tag comparison links in CHANGELOG`

10. `Adding peripheral files`

evertonfraga commented Feb 26, 2020 •

edited

Loading

evertonfraga commented Feb 26, 2020 •

edited

Loading