search

status-im / nimbus-eth2

Nim implementation of the Ethereum Beacon Chain
https://nimbus.guide
Other
535 stars 231 forks source link

Building environment problems. #1397

Open cheatfate opened 4 years ago

cheatfate commented 4 years ago

Because we are promoting our current development environment to other teams in Status, i think we should stop using config.nims file in root directory, i'm talking about https://github.com/status-im/nim-beacon-chain/blob/master/config.nims .

Because of its placement it got applied for every possible package inside of vendor directory, so even if you want to work on nim-libp2p or nim-stew options from this config.nims will still be used.

For example because of this config.nims file it is impossible to use vcc compiler and test features like it was done in nim-stew CI.

cl : Command line error D8021 : invalid numeric argument '/Wl,--no-insert-timestamp'

This happens because of https://github.com/status-im/nim-beacon-chain/blob/master/config.nims#L8

You can say that we are using only mingw compiler, but some of the packages was made not only for mingw compiler and sometimes usage of different compiler could help to investigate problem.

Proposal: We should move this config.nims file inside of beacon_chain directory so it will be applied to beacon_chain only, or we should embed options in this config.nims file into Makefile, so it will not affect all other packages inside of vendor.

stefantalpalaru commented 4 years ago

We should move this config.nims file inside of beacon_chain directory

Unfortunately, we have Nim files in other 7 top-level directories besides "beacon_chain".

we should embed options in this config.nims file into Makefile

Not a good idea, because it has NimScript logic in it, and the more we mix it with the Makefile, the harder it is to maintain.

How about passing --skipParentCfg to the Nim compiler when you want to work on a "vendor/..." project without being affected by the top-level "config.nims"? This parameter can even be added by default to "vendor/foo/foo.nimble" without affecting NBC, in most cases.

cheatfate commented 4 years ago

We should move this config.nims file inside of beacon_chain directory

Unfortunately, we have Nim files in other 7 top-level directories besides "beacon_chain".

Then, we should add this config.nims file to all this 7 top-level directories.

zah commented 4 years ago

I've ran into this problem myself and I've considered creating a new status-workplace repository that will have just enough automation to create the following directory structure:

status-workspace/
├── nim-beacon-chain/
│   ├── config.nims
│   ├── beacon_chain/
│   ...
├── nim-chronos/
│   ...
├── nim-stew/
.
.
.
└── Makefile

In other words, the vendor directory becomes the top-level one. This will prepare us well for a future where we switch to lockfile-enabled version of Nimble.

cheatfate commented 4 years ago

I vote for everything which replaces Makefiles 👍