Fix Lerna issues? Migrate to Yarn workspaces

[frangio]

Attempts to fix #665.

There is some context in #665, but I would like to have a clearer picture of what are the issues that people have been having with our current Lerna setup. It's clear to me that there's general unhappiness about it in the team but I haven't had enough experience with it to know the concrete issues.

The things I've run into are: 1) running npm ci in a package subdirectory doesn't work, and 2) npm installing any new dependency breaks the Lerna "links".

I've been experimenting with ways of fixing this but Lerna has basically zero documentation, specially around the lerna link functionality.

I've tried lerna link convert, which automatically converts all repository-local dependencies into file: dependencies, and at the same time hoists all dev dependencies to the root. However, the automatic conversion had some bugs, and I also didn't see the reason to do the hoisting at the same time.

I then tried manually converting to file: dependencies and I ran into some npm bugs where it simply fails to install (and I'm pretty sure this would have happened with Lerna's automatic conversion too). I have run into these bugs before in different projects. I just don't think npm handles file: dependencies well.

I then migrated to Yarn workspaces, and this seems to work more robustly. I've configured Lerna to work with Yarn workspaces. We can still use it for publishing, etc. This is what's included in this PR.

The downside is that this implies we have to migrate to Yarn rather than npm.

[frangio]

I have no idea if the CI is going to work.

[frangio]

The only thing pending is fixing the starter kits integration test, but I don't want to step on @ylv-io here because I think he was working on that this week. Let's coordinate.

[ylv-io]

Hey @frangio. Thanks for all the efforts to move SDK to workspaces 🚀
Starter kit CI issue I fixed was on starter kit repo itself so have nothing to do with SDK.
Regarding kits integration test at SDK. It works just fine at master branch but It is not going to work on this PR because it depends on lerna bootstrap command to create symlinks for local oz command.

    run(`${findUp.sync('node_modules/.bin/lerna')} bootstrap --scope=tests-cli-kits --scope="@openzeppelin/*"`);

I am not familiar with yarn workspaces and how they manage local package dependencies. Do you have any idea what would the equivalent to the prior line in terms of yarn workspaces? Does lerna bootstrap still works under yarn workspaces?

[ylv-io]

Yet. There might be an issue with kits tests at SDK as well. They need some refactor as well anyway. I'll make a refactor/fix PR for kits.

[ylv-io]

#1269 updated kits tests but I am pretty certain we still have to deal with lerna bootstrap

[frangio]

lerna bootstrap now uses yarn under the hood, so it's okay to keep it.

Thanks! I'll review it in a bit.

[spalladino]

Looks great @frangio! I particularly like how the CI script looks like now. And I see you used the opportunity to fix or clean up a bunch of other things as well.

Could you add a paragraph on the README to explain how to bootstrap the developer environment now? Assume a setup from scratch, no need to explain how to migrate from the old one.

[spalladino]

Great usage of workspaces! The CI script is much cleaner now, and without any loss of performance 👏

[frangio]

@spalladino I updated the existing HACKING.md file with the information about development environment setup. Let me know if you think that information should go in the README.

[abcoathup]

@spalladino @frangio should the environment setup go in CONTRIBUTING.md, otherwise change CONTRIBUTING to use setup in HACKING?

To run tests in packages/cli
I run yarn in the root
I still have to compile some contracts (I don't know if I am missing a step, or if there is still some config missing)

packages/cli$ npx truffle compile

Then I can run the tests

packages/cli$ yarn test

I get two tests failing, I don't know if this is as expected or if I am missing a step.
For the second failing test, I assume I am missing a step to run ganache-cli on a specific port:

  1) link command
       should call link script with a list of dependencies and no install:
     Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/mnt/c/Users/andre/Documents/projects/forum/openzeppelin-sdk/packages/cli/test/commands/link.test.js)


  2) Contract: set-admin script
       on application contract
         "before each" hook: setup for "does not attempt to change admin of unowned proxy":
     Error:
Could not connect to your Ethereum client with the following parameters:
    - host       > localhost
    - port       > 9555
    - network_id > 4447
Please check that your Ethereum client:
    - is running
    - is accepting RPC connections (i.e., "--rpc" option is used in geth)
    - is accessible over the network
    - is properly configured in your Truffle configuration file (truffle-config.js)

packages/lib$ yarn test

I get three tests failing, I assume I am missing a step to run ganache-cli on a specific port:

     Error:
Could not connect to your Ethereum client with the following parameters:
    - host       > localhost
    - port       > 9555
    - network_id > 4447

---

Also may need to mention that this process replaces global installs (which feels obvious but I didn't know this until I deleted my local copy of the repo 😄)

$ npm list -g --depth=0
/home/abcoathup/.nvm/versions/node/v10.16.0/lib
├── @openzeppelin/cli@2.5.3 -> /mnt/c/Users/andre/Documents/projects/forum/openzeppelin-sdk/packages/cli
├── @openzeppelin/upgrades@2.5.3 -> /mnt/c/Users/andre/Documents/projects/forum/openzeppelin-sdk/packages/lib

[frangio]

I still have to compile some contracts (I don't know if I am missing a step, or if there is still some config missing)

It shouldn't be necessary to compile the contracts in packages/cli because they're only used for testing. They should be compiled automatically by Truffle when you run yarn test.

It should also not be necessary to run Ganache.

This is weird, because the tests are passing in CI, but some tests are also failing in my computer so there is something wrong here. There may also be some flakiness. Something crashed the first time I ran the CLI tests but didn't the second time. I'm gonna look into it.

---

Also may need to mention that this process replaces global installs (which feels obvious but I didn't know this until I deleted my local copy of the repo smile)

Wow, that should not be happening. Are you sure it was Yarn? @spalladino can you reproduce this?

[spalladino]

Wow, that should not be happening. Are you sure it was Yarn? @spalladino can you reproduce this?

Could not reproduce. @abcoathup maybe you had run npm link earlier at some point...?

[abcoathup]

@frangio

maybe you had run npm link earlier at some point...?

Sorry, I did run npm link during a previous attempt to run the tests using CONTRIBUTING

npm install
npx lerna bootstrap
openzeppelin-sdk/packages/cli$ npm link
openzeppelin-sdk/packages/lib$ npm link

Then

openzeppelin-sdk/packages/cli$ npm test
openzeppelin-sdk/packages/lib$ npm test

[abcoathup]

Getting the following issues still:
#1268 (comment)