nimbus-eth1 icon indicating copy to clipboard operation
nimbus-eth1 copied to clipboard

Nimble has a number of show-stopper issues preventing us from using it.

Open zah opened this issue 4 years ago • 16 comments

We need to pin our dependencies precisely through lockfiles: https://github.com/nim-lang/nimble/issues/127

Uncategorized yet: https://github.com/nim-lang/nimble/issues/542 https://github.com/nim-lang/nimble/issues/543 https://github.com/nim-lang/nimble/issues/495 https://github.com/nim-lang/nimble/issues/589 https://github.com/nim-lang/nimble/issues/432

zah avatar Aug 20 '19 15:08 zah

Nimble would also need some sort of virtual environments, to avoid clashes between global dependencies pointing at the same package at different versions. The NIMBLE_DIR env var would need to be set to the virtual env's ".nimble" package repo dir, for the Nim compiler to find them.

This might also be required by lockfile support.

stefantalpalaru avatar Aug 20 '19 16:08 stefantalpalaru

The global cache can store multiple versions of the same package and the lockfile specifies which version should be used by your particular project.

While the lockfile is usually kept in Git, there is also a user-specific nimble.paths file which can specify alternative paths for certain packages. The intention is that you would check out the repos you want to work on in a folder such as ~/status and then your ~/status/nim-beacon-chain/nimble.paths with specify that nim-chronos is located at ~/status/nim-chronos. This effect is local only to the directory where the paths file appear.

The lockfile can also specify local relative paths for certain packages. This usage is intended when your repo contains the packages in-tree or in submodules.

zah avatar Aug 20 '19 16:08 zah

The global cache can store multiple versions of the same package and the lockfile specifies which version should be used by your particular project.

From what I remember, the Nim compiler adds all subdirectories in NIMBLE_DIR/pkgs (defaults to ~/.nimble/pkgs) to its import path, so the first foobar-x.y.z to appear in there is the first one used by a package without lockfiles. That's a problem.

stefantalpalaru avatar Aug 20 '19 16:08 stefantalpalaru

We'll look into this. Perhaps NIMBLE_DIR shouldn't be used at all and instead nimble should specify the individual library paths of the correct packages.

zah avatar Aug 20 '19 16:08 zah

  • multiple libraries / applications per repo (monorepo support so that libraries can be published individually)
  • build c-and-other stuff - {.compile.} inside nim doesn't work beyond the most simple cases (ie compile and link with mixed c and c++ when nim c is used)
  • detect host feature support (like gcc version etc) and pass to nim (autoconf style)
  • sane directory structure with support for local out folder where binaries and temporaries etc go, and src dir for sources
  • global nimble install mentality prevents side-by-side versions - including but not limited to binaries - also leads to poor user experience and upstream interaction after the first install - should default to per-project first so that everything is project-local

eventually:

  • side-by-side versioning support for diamond libraries
  • multiple package repo support for decentralized package publishing
  • full git cloning for backup purposes in case upstream disappears
  • full git clone in package repo also for backup purposes
  • alternate methods for source identification
  • build environment that manages rebuilds smartly including dependencies, environment, used tool versions etc (think

generally the global cache is a red herring - there's nothing about it that really matters in today's world. a sane starting point would be to remove it completely and maybe add it back if it's ever actually needed (ie compare to git), for an overall simplification

arnetheduck avatar Aug 20 '19 19:08 arnetheduck

@arnetheduck, I wouldn't classify any of your points as showstoppers - they don't prevent us from switching to Nimble.

Few of the items can be addressed directly in Nim (more .compile. features, autoconf-style feature detection), others are better suited for a build-system wrapping Nimble (e.g. Nix can be used to set up your host environment precisely and to make C dependencies available).

You have personal opinions and preferences regarding the global cache, but I think we can agree that the presence of a global cache does not prevent us from using Nimble. We can use the rough consensus criteria to move forward.

zah avatar Aug 21 '19 11:08 zah

Better article on rough consensus: https://doist.com/blog/decision-making-flat-organization/

zah avatar Aug 21 '19 11:08 zah

but I think we can agree that the presence of a global cache does not prevent us from using Nimble.

the cache, as it's implemented today, can't be used - it has to go away or be replaced by something that is indistinguishable from it not being there, turning it into a cache in the true sense: if you remove a cache, the system continues functioning without any observable differences.

Removing it from nimble is simply a practical way of ensuring that this is really the case, and given the quality of implementation, likely a prudent one as well because it guarantees there are no "leaks" - but how that goal is reached doesn't really matter.

most crucially, nimble install of binaries presents a similar problem that lock files do not solve, really. this makes it harder than it has to be to use tools like nimterop etc - these are part of the build just as much as anything else.

e.g. Nix can be used to set up your host environment precisely and to make C dependencies available

... or just keep using the Makefiles + submodules which also solve this problem, better than Nix from a cross-platform perspective.

Crucially, the Makefiles already build dependencies so we can't replace Makefiles with nimble without that feature covered somehow.

The other points are really trivially basic stuff that PM's offer and that we've discussed in the past as limitations to us being able to maintain a sane workflow and at the same time make our code available for the community to enjoy (eg monorepo)

arnetheduck avatar Aug 21 '19 12:08 arnetheduck

I've outlined how the cache is going to work. I don't see any problem with binary packages used in nimble-initiated builds. Nimble will select the correct package path derived from the contents of the Lockfile and the binary will be launched from there. For user convenience, a command like nimble exec nimterop ... can be used in the shell to launch the correct executable as well. You can get further with a command like nimble shell or a PATH-altering script, but these are non-critical extras.

It's a bit of an overstatement to say that the Makefiles are currently used to manage the build dependencies. We are still installing in a manual way things like RocksDB, PCRE, Go, etc. I'm not quite sure we are using the Makefiles for actual C builds at the moment. Our dependencies on libraries like Milagro are handled though the limited .compile. features of Nim.

In any case, it's quite different experience to run a script from time to time to install dependencies than to pay the price in terms of reduced convenience when using git submodules all the time. The submodules/makefiles approach raises the cost of creating new stand-alone tools. I remember many discussions where you've pointed out that raising the cost of something is an important consideration.

zah avatar Aug 21 '19 13:08 zah

I'm not quite sure we are using the Makefiles for actual C builds at the moment.

We do, for two C libraries inside nim-nat-traversal submodules.

stefantalpalaru avatar Aug 21 '19 13:08 stefantalpalaru

and for p2pd - and ideally, we'd do it for rocksdb as well (rPI instructions include the necessary magic, but we're still vulnerable to distros compiling rocks with different options, so we can't use rocks snappy support for example)

arnetheduck avatar Aug 21 '19 13:08 arnetheduck

And as long as we are listing non-critical deficiencies, there are many for submodules/makefiles as well:

  • More difficulty when testing the effect of a library change in both beacon_node and nimbus (the price is paid all the time)
  • More manual work when installing and updating transitive dependencies (paid rarely)
  • No long term viability (we see ourselves as a library-first project and our users will need a working Nimble eventually)
  • Steeper learning curve when you are getting started with the build system. We have team members who have refused to pay the price and still avoid using the build system. The lack of familiarity means that tweaking things is harder.

zah avatar Aug 21 '19 14:08 zah

We have team members who have refused to pay the price and still avoid using the build system.

Makefiles and shell scripts will continue until morale improves.

stefantalpalaru avatar Aug 21 '19 17:08 stefantalpalaru

One reason I see the global cache useful and pretty crucial is the CI use case. You don't want to do full nimble install every time for the ci builds.

One way of "fixing" the globally installed binaries is to make them aliases like myTool -> nimble run myTool, which then might behave differently depending on your CWD and launch the binary which is local to the developed repo.

Another thing is nim ideally has to be more aware of the lockfiles, and for that instead of knowing about $NIMBLE_DIR it can call out to nimble to get the search paths. Nimble itself will rely on the lockfiles, update the dependencies if needed. This is the performance critical part though, so there needs to be some project-local nimble cache which nimble will feed to nim directly in most scenarios. For nim I find it more beneficial as it's "package finding" logic will be completely stripped out from it. After all having $NIMBLE_DIR usually implies that we already have nimble installed :)

yglukhov avatar Aug 22 '19 09:08 yglukhov

One reason I see the global cache useful and pretty crucial is the CI use case. You don't want to do full nimble install every time for the ci builds.

this works the same with a project/directory-local cache, no?

arnetheduck avatar Aug 23 '19 09:08 arnetheduck

For nim I find it more beneficial as it's "package finding" logic will be completely stripped out from it.

+1

arnetheduck avatar Aug 23 '19 09:08 arnetheduck