nimble icon indicating copy to clipboard operation
nimble copied to clipboard

Frictionless Nimble directory structures

Open Araq opened this issue 5 years ago • 12 comments

Currently Nimble is quite picky about the valid project structures, see https://github.com/nim-lang/nimble#project-structure. This is often annoying for Nimble users and even if it weren't, it's something people have to learn and remember and is hardly justified. "Namespacing" is often used as the justification but that is not necessarily Nimble's job. If package A and package B both offer utils.nim in their top level directories, it could still be possible to disambiguate manually like import A / utils. And soon enough the package's users will demand sane names anyway.

A slightly different, but more pressing problem is that Nimble only copies the .nim files from srcDir over to the "installation" destination. In other words, the installation environment differs from the devel environment. That is asking for trouble and even if it works, it means the users of a package do not see the example programs or the documentation because it is not "installed" at all!

Nimble should simply git clone a package followed by a git checkout to select the required version/commit. This would also simplify the creation of eventual pull requests that the package's user might want to do. But even if Nimble continues to download/copy the package's files without the hidden .git directory, it should not touch the directory structure and it should not "install" only a selected set of files.

So what previously was $nimbleDir/p-1.0.0/p.nim becomes $nimbleDir/p-1.0.0/p/src/p.nim, for example. For backwards compat the Nim compiler can then resolve an import like import p to import p/src/p.

Advantages

  • cd $nimbleDir/foo && nimble docs or any other build target works out of the box without any special logic in the project.nimble file.
  • Nimble's source code becomes simpler, fewer project structure rules to enforce.
  • Nimble becomes easier to use.
  • It seems to be completely backwards compatible.

Disadvantages

The Nim compiler must do a bit more work to resolve imports. However, already it needs to be aware of the $nimbleDir, versions and "nimble develop" redirections. It is not a big deal to add slightly more logic on top of this mess if Nimble becomes easier to use and understand as a result.

Open Questions

How to handle git repos that contain multiple nimble packages?

Araq avatar May 09 '19 22:05 Araq

I really like how NPM does some things. Mainly installing packages in your local directory. I like how you can just edit and look at the source of the included package. You can easily search the files in your editor tree. The benefits are many!

Installing packages globally saves on space (if you have many copies of the package) but that is kilobytes compared to our terabyte drives.

I find that I hardly use nimble anymore and prefer to just git clone packages for this reason.

That also means I need to add new path to nim.cfg example:

--path:"/p/thingy/jwt/src/"
--path:"/p/thingy/redis/src/"
--path:"/p/thingy/ws/src/"
--path:"/p/thingy/pg/src/"

Because nim can import from "foo" but not "foo/src" which nimble requires all packages to have. (nimble develop does something similar but again it’s global!)

It also requires to editing my .nimble file to make versions to be in sync.

My preference would be for nimble simply:

  • git clone the package into current directory (or nim_modules like NPM does)
  • edit nim.cfg in the current directory (or make nim look into "foo/src" as well as "foo")
  • edit .nimble file to include the depency (with the exact git hash)

treeform avatar May 10 '19 00:05 treeform

My other unrelated preference would be to make packages grow complex not start out complex.

  • It could be cool if package could start out as a single file package maybe with like a “nimble” comment at top. And people could use it from like a gist or a url. No git required.
  • Then maybe as it grows a dedicated .nimble file is added and the whole thing is placed into a git repo. You know it’s growing!
  • Then as it grows it can move more files into the “src” directory.
  • And grows docs and test directories after that. Look a mature package!

treeform avatar May 10 '19 00:05 treeform

I also agree with krux02 on https://github.com/nim-lang/packages/issues/1012 . Cant we just look up stuff in usual places instead of having a central json file?

treeform avatar May 10 '19 00:05 treeform

@treeform I agree with all of your points, but I hope the next reactions are a bit more on topic. ;-) This is a specific RFC, there should be more to follow.

Araq avatar May 10 '19 07:05 Araq

To elaborate a bit more on the requirements:

  • On install, Nimble will copy package contents to $nimbleDir/pkgs/package-x.y.z/package instead of $nimbleDir/pkgs/package-x.y.z. This will ensure every package gets added to a unique namespace. Nimble should continue to call Nim the same as today: nim c --path:$nimbleDir/pkgs/package-x.y.z. The net effect is that a package that used to be import package will now need to be imported as import package/package.

  • Nimble will no longer scan package contents - does this mean skipDirs, skipFiles, skipExt, installDirs, installFiles and installExt should be deprecated or are we saying Nimble still allows package maintainer to decide what should and should not be installed? Retaining this functionality means some scanning of packages but not having an opinion on structure.

  • $srcDir today is location where .nim files are and by default, it is the root dir. This is why Nimble today allows importing nim files from the root. In order for src to be treated as the root, srcDir needs to be explicitly set. Does this RFC mean srcDir is set to src by default? Then there's still an opinion on package structure and it breaks every package that has nim files in the root or somewhere else. E.g. import nimterop/cimport because $nimbleDir/pkgs/nimterop-x.y.z/nimterop/cimport.nim and there's no $srcDir defined.

  • If it instead means $srcDir is no longer of interest to Nimble, backwards compatibility will be more challenging since now Nim needs to know where to look instead and will need to scan the .nimble file to redirect import package to import package/$srcDir/package.

Regarding using git clone, it works for regular packages but not for repos that have multiple packages. It is possible to do a sparse git clone but then package structure will be a bit different and might need some extra work to fix the namespace or backwards compatibility. I think this is best left to a separate RFC.

Additional advantages

  • Nimble can be enhanced to build docs by default and make it easy to open docs
  • Nimble can allow running tests on installed packages to verify they still work with local compiler

Another item worth discussion is whether the Nim redirection to resolve imports should be temporary and deprecreted eventually.

genotrance avatar May 10 '19 14:05 genotrance

So I've hidden some comments that have gone off-topic, please create separate RFCs for those.

You might be surprised, but I do actually love this idea. There are still some concerns that I have though:

Semantics of the new Nim compiler package resolution

For backwards compat the Nim compiler can then resolve an import like import p to import p/src/p.

@Araq, you say this is for backwards compatibility, this implies that you do not want future code to use this shortcut. Are you expecting people to write import p/src/p in new code?

I also want us to be very specific about the semantics of this, can you come up with an implementation? Even in pseudo-code would be helpful to make sure we're on the same page.

Nimble's srcDir

As you may know, and @genotrance alluded to, the defaults for Nimble aren't that the srcDir is set to "src". We would need to change this as well. I'm happy with doing this but just so we're clear this will be a breaking change.

Semantics of how packages are installed that this RFC proposes

Just to be clear, let me specify concretely how Nimble will install packages if this RFC is implemented:

  • nimble install jester

Package structure will look like this:

.~/.nimble/pkgs/jester-1.0.0/jester/
├── changelog.markdown
├── src
│   ├── jester.nim
│   ├── jester
│   │   ├── patterns.nim
│   │   ├── private
│   │   │   ├── errorpages.nim
│   │   │   └── utils.nim
│   │   └── request.nim
├── jester.nimble
├── license.txt
├── optimisation.md
├── profile_results.txt
├── readme.markdown

Here is how I envision imports to work:

  • import jester -> ~/.nimble/pkgs/jester-1.0.0/jester/src/jester.nim
  • import jester/patterns -> ~/.nimble/pkgs/jester-1.0.0/jester/src/jester/patterns.nim

Note that I left some crap that I have in my repo (optimisation.md, profile_results.txt), there was a lot more of this. If you want Nimble to install all of this then you'll get a lot of generated files and binaries installed to your .nimble directory. IMO that shouldn't happen but that can be a discussion for another time.

Multiple packages in a single repo

How to handle git repos that contain multiple nimble packages?

This is actually simple. Nimble already gets a Git URL which includes the path to the package, all you have to do is effectively cd git-repo/pkgname && nimble install (let me know if that doesn't make sense).

dom96 avatar Sep 21 '19 11:09 dom96

I've just been going through the issues and wondered: while we're adopting this RFC could we also adopt scoped org packages? i.e. nimble install dom96/jester. Even if we don't adopt this now, it would be useful to think about for the future, I'm not sure how we would support this in the Nim compiler. @Araq any thoughts? (Here is one issue that this feature might alleviate: #574)

dom96 avatar Sep 21 '19 16:09 dom96

Re: using git clone

In order for this to be useful, we will need to add functionality in Nimble to convert a regular repo clone into a fork when the user wants to contribute to an installed package. It is easy enough to fork manually and nimble develop though.

Using git for updating a package isn't useful either since in theory, you want multiple versions of a package to be retained. Another complication is how nimble install from a directory will retain the git directory structure.

I vote for leaving git out of this RFC and dealing with that separately if even warranted.

Re: opinionated package structure

I'm not sure what the opinionated part is. Using srcDir is optional. The other defaults like using private or tests are simple enough.

I suggest leaving srcDir behavior as is.

I agree the entire package should be copied as is though. If anything, this seems like the only part of this RFC that is really warranted. The code changes required would be to resolve imports relative to srcDir if declared. This will require parsing the .nimble file and works with the ideas in https://github.com/nim-lang/nimble/issues/654.

Re: package namespacing

If we want to make changes to Nimble's package structure and resolution ($nimbleDir/pkgs/package-x.y.z/package instead of $nimbleDir/pkgs/package-x.y.z), we need to make sure it coexists with the ideas in https://github.com/nim-lang/nimble/issues/654 which wants to remove Nim's knowledge of Nimble.

While I understand the benefit of two packages not conflicting, I don't know how important this is and whether it needs to be mixed into the same RFC.

Re: multi-package repos

effectively cd git-repo/pkgname && nimble install

If we want the git repo retained in ~/.nimble/pkgs, this won't work since nimble install doesn't do that. Meanwhile, I've already suggested against retaining git.

genotrance avatar Sep 23 '19 05:09 genotrance

If you want Nimble to install all of this then you'll get a lot of generated files and binaries installed to your .nimble directory. IMO that shouldn't happen but that can be a discussion for another time.

If it's harmless enough for "git clone", it's harmless enough for Nimble. Simple.

Araq avatar Sep 23 '19 06:09 Araq

If it's harmless enough for "git clone", it's harmless enough for Nimble. Simple.

You might very well install local Nimble packages, and then you'll get all sorts of generated files. A solution might be to just parse .gitignore files.

dom96 avatar Sep 23 '19 12:09 dom96

@dom96 IMO https://github.com/nim-lang/RFCs/issues/291 gives the missing ingredient to allow this RFC.

Under this RFC, you'll have:

  • no scope pollution, which is a real problem
  • no need for srcDir
  • no ambiguity
  • more flexibility as to how paths are resolved, when the need arises
  • can be done without a breaking change

example

constantine package defines helpers/static_for.nim at top level (https://github.com/mratsim/constantine/blob/master/helpers/static_for.nim) however, it violates package namespacing (import helpers/static_for or import pkg/helpers/static_for) should not compile for 2 reasons:

  • it may clash with some other module's helper's directory
  • it doesn't convey that it belongs to constantine package

instead, with https://github.com/nim-lang/RFCs/issues/291 you'll have: nimble passes --path:constantine:constantineClonePath/constantine (instead of --path:constantineClonePath/src as done currently) and then these would work:

import constantine/primitives
import pkg/constantine/primitives
import pkg/constantine/../helpers/static_for # the `..` is resolved after pkg/constantine is resolved

links

  • https://github.com/mratsim/constantine/issues/111 which prompted this

timotheecour avatar Jan 11 '21 07:01 timotheecour

@timotheecour I love it. Let's do it :)

Edit: Actually, thinking about it some more. This will break packages like jester which allow you to just import jester, you'll need import jester/jester, or am I missing something?

dom96 avatar Jan 11 '21 16:01 dom96