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

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

Closed
zah opened this issue Aug 20, 2019 · 17 comments
Closed

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

zah opened this issue Aug 20, 2019 · 17 comments

Comments

@zah
Copy link
Member

zah commented Aug 20, 2019

We need to pin our dependencies precisely through lockfiles:
nim-lang/nimble#127

Uncategorized yet:
nim-lang/nimble#542
nim-lang/nimble#543
nim-lang/nimble#495
nim-lang/nimble#589
nim-lang/nimble#432

@stefantalpalaru
Copy link
Contributor

stefantalpalaru commented Aug 20, 2019

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.

@zah
Copy link
Member Author

zah commented Aug 20, 2019

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.

@stefantalpalaru
Copy link
Contributor

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.

@zah
Copy link
Member Author

zah commented Aug 20, 2019

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.

@arnetheduck
Copy link
Member

arnetheduck commented Aug 20, 2019

  • 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

@zah
Copy link
Member Author

zah commented Aug 21, 2019

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

zah commented Aug 21, 2019

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

@arnetheduck
Copy link
Member

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)

@zah
Copy link
Member Author

zah commented Aug 21, 2019

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.

@stefantalpalaru
Copy link
Contributor

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.

@arnetheduck
Copy link
Member

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)

@zah
Copy link
Member Author

zah commented Aug 21, 2019

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.

@stefantalpalaru
Copy link
Contributor

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.

@yglukhov
Copy link
Contributor

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 :)

@arnetheduck
Copy link
Member

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

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

+1

@tersec
Copy link
Contributor

tersec commented Jun 5, 2024

This is not a nimbus-eth1 issue, and has been extensively discussed/developed elsewhere in the 5 years since.

If someone looks at the current situation at some point and believes Nimble is ready for nimbus-eth1 usage and that nimbus-eth1 should use it, that can be an issue of its own. But this issue isn't resolvable by any action in this repository.

@tersec tersec closed this as completed Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants