Reorganize the project as a nimble package?

[kaushalmodi]

Hello,

Can you please reorganize the project contents so that the below Just Works^TM:

git clone https://github.com/c-blake/cligen
cd cligen
nimble build

At the moment, doing so gives:

   Warning: Package 'cligen' has an incorrect structure. The top level of the package source directory should contain at most one module, named 'cligen.nim', but a file named 'argcvt.nim' was found. This will be an error in the future.
      Hint: If this is the primary source file in the package, rename it to 'cligen.nim'. If it's a source file required by the main module, or if it is one of several modules exposed by 'cligen', then move it into a 'cligen/' subdirectory. If it's a test file or otherwise not required to build the the package 'cligen.nim', prevent its installation by adding `skipFiles = @["argcvt.nim"]` to the .nimble file. See https://github.com/nim-lang/nimble#libraries for more info.
  Verifying dependencies for cligen@0.9.9
       Tip: 2 messages have been suppressed, use --verbose to show them.
     Error: Nothing to build. Did you specify a module to build using the `bin` key in your .nimble file?

---

Nimble Project Structure

[c-blake]

I've been aware that the file/directory layout is non-canonical since I began the project, and I read that #libraries bit. I think I disagree with nimble's views on this where two names are somehow much worse than one (my original hope was to get parseopt3 into stdlib and only have argcvt and cligen). Also, there is nothing to build - your nimble build invocation is pointless. This is a source library. All you do is clone it/whatever and make sure you add wherever you clone it to to your path in your nim.cfg. I suspect one gets some similar warning message just doing nimble install, too, though. All that said, I have been considering creating a subdirectory just to conform to "normal", but there seems to be a lot of diversity..some people do pkg/src/, others, pkg/pkg/, etc., and to be honest I don't use nimble myself (it actually crashed on me just now when I tried to use it...) and am unsure what setup is least onerous on downstream users.

[kaushalmodi]

I don't use nimble myself (it actually crashed on me just now when I tried to use it...)

That's odd. Note that I am just learning to code in Nim, but using nimble has been fine. For example, all I needed to create a binary for the nistow project was to clone it, cd to it, and nimble build. And I have done that for many other projects.

your nimble build invocation is pointless

Yes, I realized that, once I reorganized your project locally into a nimble approved structure :P. As I said, I am not experienced in Nim, but may be organizing it in nimble-friendly way would allow easy "nimble install" for folks who want to use this even as a library? Copying @dom96 for more insight.

[kaushalmodi]

This is all I did (ofcourse, would need to refactor your tests, etc. too): kaushalmodi@78d23a9 kaushalmodi@3805ef5

Now doing nimble check gives:

Success: cligen is valid!

And nimble install works too! See this other comment of mine.

[dom96]

I think I disagree with nimble's views on this where two names are somehow much worse than one

I'm always open to discussions about this. But you've got to realise that Nimble's enforcement of these rules is there to ensure packages are not polluting each other's namespaces. This is important and I don't see any better way to achieve this short of enforcing these package structures (like I said, I'm open to suggestions and discussions around this).

(it actually crashed on me just now when I tried to use it...)

How did it crash? :/

This is all I did (ofcourse, would need to refactor your tests, etc. too): kaushalmodi/cligen@78d23a9

That looks perfect to me.

[c-blake]

Right now it crashes with a "SEGV/attempt to read from nil" on most any invocation other than --help..or before long, anyway - I can get a download going and then it crashes out before any install happens. This is just the version in the current devel branch of Nim with gcc-8.1. So, I tried a clone of git://github.com/nim-lang/nimble, but doing a nimble build there just SEGVs, too. running nim c -r nimble.nim install neo also SEGVs at the same stage. I am working on getting a debug build going so this can be a less pointless report.

Is it really necessary to have two whole new levels (src/ and src/cligen)? That seems really excessive. neo doesn't do that, for example. Also, why should we have to skip test/ to be "valid" when that subdir functions as both example code/documentation and test code (and nothing will put the /test/ in the path and accidentally import anything there anyway)? And why should I refactor my tests? So nimble test might work? I have a test.sh. Surely nimble doesn't require you to use its test framework.

[kaushalmodi]

@dom96 can answer the nimble questions.

But regarding:

why should I refactor my tests?

I mentioned it as a disclaimer as I did not run your tests after that re-organization. So if you are importing just parseopt3 and not cligen/parseopt3 in your tests, they would fail (I haven't checked).

After the re-org, nimble install works nicely. So people won't need to clone this repo for each project where they want to use it. They can just clone this repo once, cd to it, and do nimble install.

That will create this:

km²~/.nimble/:pkgs> ls --tree cligen-0.9.9/
cligen-0.9.9/
├── cligen/
│  ├── argcvt.nim
│  ├── parseopt3.nim
│  └── textUt.nim
├── cligen.nim
├── cligen.nimble
└── nimblemeta.json

Folks can then just do import cligen in their projects without having to clone a local copy for each project.

why should we have to skip test/

I might be wrong, but why would you want to ship (as part of nimble installs) the tests to cligen users who would never run those tests.

[kaushalmodi]

btw, I needed refactoring few more imports.. here's the force-pushed commit: kaushalmodi@3805ef5

After that, and nimble install, this works without having to clone cligen in that project:

# Time-stamp: <2018-05-09 12:54:48 kmodi>

proc foobar(foo=1, bar=2.0, baz="hi", verb=false, paths: seq[string]): int =
  ##Some existing API call
  result = 1          # Of course, real code would have real logic here

when isMainModule: import cligen; dispatch(foobar)    # Whoa...Just one line??

If I want to package that code as a nimble package and want to auto-install cligen for users building the project, I would need to add something like this to that package's .nimble file:

requires "nim >= 0.18.0", "cligen >= 0.9.9"

[c-blake]

Ok. I re-orged it a bunch myself (but thanks for the PR suggestions). nimble check passes now and nimble install works without complaint. Closing this issue.

As for the nimble crashes, I believe something in my ~/.config/nim.cfg was somehow causing a buggy build of nimble. I have a working version now.

[kaushalmodi]

@c-blake Thanks! I am already using it here: kaushalmodi-forks/nistow@e6c0cfb :D

Now I only need to figure out how to replicate the old behavior below using cligen:

if paramCount() == 0:
  writeHelp()
  quit(0)

.. and so I opened #27.

[dom96]

As for the nimble crashes, I believe something in my ~/.config/nim.cfg was somehow causing a buggy build of nimble. I have a working version now.

Interesting. If you can reproduce it then please report an issue :)

[c-blake]

FWIW, it seems to come and go just based on whether I have --experimental active anywhere in my ~/.config/nim.cfg - just edited at run-time with no recompiles necessary. rm -rf ~/.nimble, add --experimental to your ~/.config/nim.cfg and try to install any package. It asks for a download confirm and then you answer "y" and then it gets the package list fine and right after the "Downloading...using git" message it SEGVs. strace suggests it faults during the parsing of that ~/.config/nim.cfg.

Anyway, this is not some great use case since I shouldn't set experimental mode so globally, but I was lazy, and it's off now.

I'm not sure if you can reproduce the failure in your own environment from that description, but this is at least more to go on than my first report. I'm not quite sure why nimble even reads nim.cfg as part of nimble install.

[c-blake]

Also, here is a backtrace from gdb which supports my "crashes while reading nim.cfg" thesis:

#0  0x00005555556ad44f in processSwitch_t6SoLnwbYM6Na8RlwC43Gw ()
#1  0x00005555556c489e in readConfigFile_C6FgBgMhny6dlbrH9bYzedA ()
#2  0x00005555556c4af8 in loadConfigs_55BCj2M39b1j2LYY1ejJlfA ()
#3  0x00005555555ddf0e in execScript_cMbYcugsW5bWVzsrXmropQ ()
#4  0x00005555555def83 in readPackageInfoFromNims_lmGzJ0iUBNbMuUV0BU9bhcA ()
#5  0x00005555555da3c5 in readPackageInfo_IS0JOQTaRCrDU40vkjeflw ()
#6  0x00005555555da902 in getPkgInfoFromFile_CaH6q8DcOoVKnt6CDeWNQQ ()
#7  0x00005555555d5eff in downloadPkg_J9cfNWVZhskqA3zRbjrqGyA ()
#8  0x000055555556d1ed in install_T1NC9brVVA49a9cIiYLkT8e3Q ()
#9  0x00005555555747b9 in doAction_Nylx3M8pmcbTsc3Rkl5Bxg ()
#10 0x0000555555574dee in NimMainModule ()
#11 0x0000555555574ccf in NimMain ()
#12 0x000055555556873d in main ()