ethereumjs / ethereumjs-monorepo

Monorepo for the Ethereum VM TypeScript Implementation
2.6k stars 755 forks source link

Dev version installation fails on node v12 #661

Closed rumkin closed 4 years ago

rumkin commented 4 years ago

Development version of VM installation fails on node v12. The problem is caused by levedown native module compilation failure. Leveldown is a dependency of level package and according to it readme it isn't support current node version.

Reproduce

  1. Install node v12.
  2. Clone repo.
  3. Run npm i . inside of repository directory.

Solution

Upgrade level package to the latest version.

Checklist

holgerd77 commented 4 years ago

The level dependency is used in the blockchain test runner which passes on an instance to the Blockchain object as its DB.

ethereumjs-blockchain is using level-mem with a v3.0.1 dependency (see package.json), so also an older version.

I think it would make sense to keep this (relatively) in sync rather than update on one side and do an update both on the VM and Blockchain in one PR once we have both libraries together in the monorepo, what do you think @evertonfraga @ryanio?

rumkin commented 4 years ago

Level-family packages has the same interface and AFAIK backward compatible interfaces, so it seems safe and reliable to update only a single package. This update is required to fix node native package installation issue and as you can see in PR #662 all tests passed well even with the current level-mem. Current version of level breaks the installation so it seems reasonable to fix this and add an issue for mono-repo to update the rest of level related things.

holgerd77 commented 4 years ago

Ok, that makes sense. Not that much of a level expert and always unsure on how conservative to be here in updates.

evertonfraga commented 4 years ago

Seems sensible to me to pack changes together in the monorepo.

rumkin commented 4 years ago

@evertonfraga There should be a label package: blockchain too, could you add this? I'm going to update package level-mem for it. Can I do it now, or there is still migration in progress?

holgerd77 commented 4 years ago

Fixed by #662