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

Dev version installation fails on node v12 #661

Closed
1 of 2 tasks
rumkin opened this issue Feb 23, 2020 · 6 comments
Closed
1 of 2 tasks

Dev version installation fails on node v12 #661

rumkin opened this issue Feb 23, 2020 · 6 comments

Comments

@rumkin
Copy link
Contributor

rumkin commented Feb 23, 2020

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

  • Update level to v6.0.0.
  • Update level-mem to v6.0.0 here and into ethereumjs-blockchain package, after monorepo migration.
@rumkin rumkin changed the title NPM installation failed on node v12 Dev version installation fails on node v12 Feb 23, 2020
@holgerd77
Copy link
Member

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
Copy link
Contributor Author

rumkin commented Feb 24, 2020

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
Copy link
Member

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

@evertonfraga
Copy link
Contributor

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

@rumkin
Copy link
Contributor Author

rumkin commented Feb 28, 2020

@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
Copy link
Member

Fixed by #662

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

No branches or pull requests

3 participants