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

Add installExt to hldiff.nimble #1

Closed
sschwarzer opened this issue Aug 23, 2020 · 55 comments
Closed

Add installExt to hldiff.nimble #1

sschwarzer opened this issue Aug 23, 2020 · 55 comments

Comments

@sschwarzer
Copy link

Since hldiff is described as "A port of Python difflib to compute diffs and (re)highlight diff output intraline", I assume it should be a hybrid package.

Installing hldiff 0.1 or head with nimble install hldiff, however, installs only the binaries, not the .nim source files.

There was a change in Nimble a while ago that requires that hybrid packages need an installExt = @["nim"] line to install the Nim source files. See https://github.com/nim-lang/nimble/blob/master/changelog.markdown#090---19092018 and nim-lang/nimble#561 .

@c-blake
Copy link
Owner

c-blake commented Aug 23, 2020

I agree it should install the source, too. Sorry about that. I don't find nimble easy/nice to drive, personally. I think tools that mandate a specific directory layout to operate are lame. Case in point, if I just add that installExt then nimble check fails with "incorrect package structure".

So, It looks like from https://github.com/nim-lang/nimble#hybrids that I will have to rename src/ to hldiffpkg/ and hope it can still install both binaries from there. Unless that breaks some other aspect, like being able to install two binaries (edits & hldiff) in this case.

@c-blake
Copy link
Owner

c-blake commented Aug 23, 2020

I tried that src/ -> hldiffpkg/ and then also tried having the directory be named hldiff/ and also with hldiff.nim at the top level. They all fail to nimble install. I went ahead and commited & pushed those experiements so that you can try to nimble install --verbose hldiff@#githash if you want.

I'm happy to set it up however it can work and install both binaries and both sources, but this experience is only solidifying in my mind how brittle nimble tends to be. I can probably also provide an install.sh and install.bat scripts, but my guess is nimble is also not flexible enough to allow such delegation.

If we need to we can drop the edits binary and only keep edits.nim. This is not ideal since @sschwarzer had mentioned in private communication using edits as a Windows substitute for diff -u. But it's better than nothing. I'm mostly out of patience trying to coax nimble to work. Just tell me how to set it up and I'll do it. Or tell me it's impossible and file a nimble issue.

@c-blake
Copy link
Owner

c-blake commented Aug 23, 2020

I guess I figured it out. Let me know if this still doesn't work for you.

@c-blake c-blake closed this as completed Aug 23, 2020
@c-blake
Copy link
Owner

c-blake commented Aug 23, 2020

For what it's worth, I did get it to build (but not install) an hldiff.out with only an hldiff/ subdir name which in my opinion is very preferable to hldiffpkg/. I guess it can detect to rename the -o but then not during the install phase or some such.

@sschwarzer
Copy link
Author

I guess I figured it out. Let me know if this still doesn't work for you.

I cloned the Git repo ( commit 814175d ) . If I run nimble check in the working directory, I get

     Error: Package 'hldiff' has an incorrect structure. The top level of the package source directo
ry should contain at most one module, named 'hldiff.nim', but a file named 'edits.nim' was found. Th
is will be an error in the future.
      Hint: If this is the primary source file in the package, rename it to 'hldiff.nim'. If it's a 
source file required by the main module, or if it is one of several modules exposed by 'hldiff', the
n move it into a 'hldiffpkg/' subdirectory. If it's a test file or otherwise not required to build t
he the package 'hldiff.nim', prevent its installation by adding `skipFiles = @["edits.nim"]` to the 
.nimble file. See https://github.com/nim-lang/nimble#libraries for more info.
   Failure: Validation failed

The Nimble version is 0.11.4.

I tried different reorganizations but didn't get it to work. One of the approaches was running nimble init in a new directory and creating a hybrid package in the interactive dialog. But as soon as I tried to have two binaries, I couldn't satisfy nimble check.

However, to my surprise nimble build succeeded, as does nimble install https://github.com/c-blake/hldiff.

Correction: It only seems to succeed. The nimble install installs only the binaries in ~/.nimble/pkgs, not the .nim modules. I'll open a ticket for https://github.com/nim-lang/nimble to get advice.

@c-blake
Copy link
Owner

c-blake commented Aug 23, 2020

If I do nimble install 'hldiff@#head' then I get both binaries in $HOME/.config/nimble/bin as symlinks pointing to $HOME/.config/nimble/pkgs/hldiff-#head. edits.nim and hldiff.nim are both installed in that latter directory (though I have not tried to test-import them in terms of whatever nim path nimble sets up).

@sschwarzer
Copy link
Author

edits.nim and hldiff.nim are both installed in that latter directory (though I have not tried to test-import them in terms of whatever nim path nimble sets up).

Ok, I can confirm that this works for me, too. I had assumed that nimble install https://github.com/c-blake/hldiff would install head, but it installs the latest tag (the Nimble output actually mentions 0.1, but I guess I had overlooked it).

Anyway, here's the output of the Nimble package installation:

$ nimble install hldiff@#head
Downloading https://github.com/c-blake/hldiff using git
   Warning: Package 'hldiff' has an incorrect structure. The top level of the package source directory should contain at most one module, named 'hldiff.nim', but a file named 'edits.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 'hldiff.nim'. If it's a source file required by the main module, or if it is one of several modules exposed by 'hldiff', then move it into a 'hldiffpkg/' subdirectory. If it's a test file or otherwise not required to build the the package 'hldiff.nim', prevent its installation by adding `skipFiles = @["edits.nim"]` to the .nimble file. See https://github.com/nim-lang/nimble#libraries for more info.
  Verifying dependencies for hldiff@#head
   Warning: No nimblemeta.json file found in /home/schwa/.nimble/pkgs/vppdiff-0.1.0
      Info: Dependency on cligen@#head already satisfied
  Verifying dependencies for cligen@#head
 Installing hldiff@#head
   Building hldiff/hldiff using c backend
   Building hldiff/edits using c backend
   Success: hldiff installed successfully.

$ ls ~/.nimble/pkgs/hldiff-#head/
edits  edits.nim  hldiff  hldiff.nim  hldiff.nimble  nimblemeta.json

So it seems we have a setup that works at the moment, but with the Nimble warning and the messages from nimble check the current state doesn't look reliable. So the new Nimble ticket nim-lang/nimble#837 is still relevant from my point of view.

@dom96
Copy link

dom96 commented Aug 23, 2020

I'm quite disappointed that people find Nimble's package directory structure errors so confusing. I spent a lot of time making the errors as informative as possible. Let me now explain how I fix the current structure of this package, based on the errors that Nimble gives and hopefully we can gain some insight into the confusion:

  • So I cloned the hldiff repo and ran nimble check:

Error: Package 'hldiff' has an incorrect structure. The top level of the package source directory should contain at most one module, named 'hldiff.nim', but a file named 'edits.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 'hldiff.nim'. If it's a source file required by the main module, or if it is one of several modules exposed by 'hldiff', then move it into a 'hldiffpkg/' subdirectory. If it's a test file or otherwise not required to build the the package 'hldiff.nim', prevent its installation by adding skipFiles = @["edits.nim"] to the .nimble file. See https://github.com/nim-lang/nimble#libraries for more info.

Failure: Validation failed

  • The error's main explanation is: "The top level of the package source directory should contain at most one module, named 'hldiff.nim', but a file named 'edits.nim' was found."

  • So I know that edits.nim is not where it's supposed to be. But it's already inside a hldiffpkg directory, so how can that be? Ahh, well if you look at the .nimble file you see that it defines the "top level of the package source directory" to be hldiffpkg. So we would have to move edits.nim into yet another hldiffpkg directory...

  • The solution here is to remove the srcDir declaration in the .nimble file. Then nimble check passes.

  • But then we need to ensure that it builds (and actually nimble check should probably verify this in a preliminary fashion).

  • So we run nimble build:

Error: cannot open '/mnt/c/Users/morfe/other_projects/hldiff/hldiff.nim'

  • Now Nimble cannot find hldiff.nim because it's nested in a hldiffpkg directory. I can see why this might be confusing. But the solution is to move hldiff.nim to the root directory (hldiffpkg's parent).

  • Running nimble build again shows that now edits.nim doesn't build, that's because it's still in the hldiffpkg directory. We cannot move it to the root directory, but we can edit the .nimble file:

bin         = @[ "hldiff", "hldiffpkg/edits" ]
  • We're finished. The package builds successfully and passes nimble check. I also installed it to verify that it all works fine, ran the binaries etc. and it's all good.

You might ask why is this rigmarole necessary? Well, in the state that this package is before my changes above, the source code will be installed like so:

  • ~/.nimble/pkgs/hldiff-0.2.0/hldiff.nim
  • ~/.nimble/pkgs/hldiff-0.2.0/edits.nim

Which means that edits will pollute the namespace of other packages, i.e. you'll be able to import it via import edits. This should only be allowed for an edits package.

The reason this is how it works is a lot to do with the constraints that Nim has imposed on Nimble. Is it perfect? No. Will it stay like this? maybe not. Is it necessary currently to preserve semblance of sanity when many tens/hundreds/thousands of packages are installed/dependend on? Yes.

CC @genotrance (you may find the above explanation helpful, and it should be taken into consideration for future directions with how Nimble installs packages).

@c-blake
Copy link
Owner

c-blake commented Aug 24, 2020

Is it necessary currently to preserve semblance of sanity when many tens/hundreds/thousands of packages are installed/dependend on? Yes.

No. Just no. Before any of this issue being opened both were in src/ which could be mapped by nimble to an import / requirement. They are already namespaced. Just because how you wrote nimble doesn't translate that namespacing doesn't alter that. Just being part of a git clone has them namespaced. Full stop.

You have a nimble file. It can specify practically anything, such as that namespace translation. In light of that alone any mandated project directory structure at all is logically unnecessary. Full stop.

The only question becomes what is easier for users - more complex nimble files or complex package structuring rules. Both have pros and cons and there are many examples in the wild of both. Notably the things that do "whole systems" do make-like things, from BSD ports to Gentoo ebuilds to etc. Prog. lang things avoiding system integration concerns even at the level of site-packages seem to do this lame package structure stuff that I hate, but I haven't surveyed how mandatory all that is outside nimble. I would personally rather learn a new "make-like" thing and have control than deal with this package structuring stuff, but maybe I'm "insane".

So...Sanity is sometimes { all the time? :-) } subjective. Some people might say that you forcing me to have client code import foopkg/whatever instead of import foo/whatever is totally bonkers. "Wait...Why do I need the pkg? It's already a subdirectory? Can nimble not tell the difference between files & dirs???!" As I mentioned earlier, I almost got a subdir of just hldiff/ to work that to work at one point. Maybe you can??? Having to tell people "you must import hldiffpkg/edits because nimble is braindead" seems like bad advertising to me. Maybe you think hldiffpkg and this two level business is the height of sanity?

But I only care mildly about this if I have to tell people "It's this way because nimble sucks. Bug dom96 or genotrance or whoever". I do not mean to turn this into some big debate about package management, but I have seen you routinely act as if you were boxed into exactly one high level design concept, and I think that is just patently false or at the very least argued very poorly when so many examples in the wild going other ways exist. I personally literally never use nimble "as a client" except to test how others might get my code/programs. I doubt I will ever like it -- even a little -- as a client or an author. Sorry. I only set it up for people more patient with its failings to have an easier time. So, thanks very much @dom96 for taking the time to help me do that for @sschwarzer in this case and good evening to all!

@dom96
Copy link

dom96 commented Aug 24, 2020

Oh boy, your response feels very antagonistic to me. I hope my comment didn't come off the same way. If it did, it was not my intention.

No. Just no. Before any of this issue being opened both were in src/ which could be mapped by nimble to an import / requirement. They are already namespaced.

I'm not sure I understand, are you suggesting that Nimble could have moved the files around to make them namespaced automatically? Right now Nimble will install files as-is, and I know a lot of people that swear by those semantics. For your code, before this issue was opened, this means that src/hldiff.nim is importable via import hldiff (perfectly fine) and src/edits.nim is importable via import edits (not fine because it clashes with a potential edits package).

The only way to fix this is to:

  • Ask the user to fix the structure
  • Have Nimble do so automatically at the risk of screwing it up
  • Have the Nim compiler support a new (and currently unimplemented) namespacing mechanism which @Araq was very much against when I was implementing Nimble initially

Am I missing something? My preference would have been for the last solution, but like I say above it wasn't approved. So I compromised on the first one. I sense that you feel that there is a very obvious solution to this that would solve all these problems and be absolutely perfect. So which solution is that?

Based on what you're saying, I guess you mean that you should be able to tell Nimble how it should move files around in order to make them fit the package structure? Something like

onInstall:
  mv("src/edits.nim", "src/hldiff/edits.nim")

Is this your preferred solution? To me it is hardly different to the current structure enforcement rules.

The whole hldiffpkg (i.e. pkg suffix rule) is indeed silly, and I think we actually can and should get rid of it. But forget about that for now. Focus on just the above, imagine your package is just a library, not a hybrid.

but I have seen you routinely act as if you were boxed into exactly one high level design concept, and I think that is just patently false or at the very least argued very poorly when so many examples in the wild going other ways exist.

I have been boxed, as I explained above. The other examples in the wild that I've seen require the language compiler to implement additional features, which is precisely the kind of concessions that I was not allowed

@c-blake
Copy link
Owner

c-blake commented Aug 24, 2020

To me it is hardly different to the current structure enforcement rules.

To me it is very different, better, and only the tip of the iceberg. nimble could be more like make, enabling but not infantilizing package authors to get a build or an install done. Just as another example, nimble could provide a variable "NAMESPACE" (or PKG or NS or whatever) that it controls/regulates amongst all packages based on either the name of the .nimble file or another optional setting in .nimble file and package authors might have to say installInto Namespace or something. Why cannot one package export two names and have both be importable from the global namespace. Could there be collisions? Sure. Should nimble warn/error out on a collision? Sure. Should it outright prevent it? No. Just because 10,000 packages exist doesn't mean they all have to be simultaneously installable. Who ever put that requirement on anything? It certainly does not exist for C packages in Linux distros. Sanity could just be informing would be installers about a conflict. You are already making people install into $HOME anyway meaning there would be no "realized conflict" from a theoretical conflict most of the time. But there could/should also be some kind of system-global site-packages out of user homedirs. In most package ecosystems name conflicts are more rare than many orphaned/dead/fly by night packages and good names are valuable over time. The nim compiler has --path. I think that alone unboxes you..well, there might need to be some minimal mechanism to add to that path, but I doubt that would be (EDIT:) very controversial. Anyway, all package mgmt is too big a conversation for a resolved issue on some almost throwaway package.

@c-blake
Copy link
Owner

c-blake commented Aug 24, 2020

And I don't mean to be antagonistic per se, but I am not a nimble fan, cannot pretend to be, never expect to be, and I mostly just hear excuses rather than valid rebuttals. If you lack other examples, you should maybe look at the BSD ports setup or Portage rather than Rust or Python. C compilers have no special features for any of this stuff other than -I/-L paths. Yet somehow the BSD ports tree exists.

@sschwarzer
Copy link
Author

sschwarzer commented Aug 25, 2020

I'm quite disappointed that people find Nimble's package directory structure errors so confusing.

Yes, Nimble seems to be hard to use in some packaging scenarios.

I spent a lot of time making the errors as informative as possible.

I guess that's a part of the problem. In my opinion, for our case there should be a hint or warning, not an error. An error goes too far, restricting the package maintainer too much.

Personally, I read the error message but it seems I couldn't comprehend that the message was what was actually meant. I didn't expect at all that Nimble would deliberately prevent me from installing two modules when I was able to install one (as seen from the nimble init-generated hybrid package) even though I used the - from my point of view - same package structure. So I thought that I had overlooked something or that the error message meant something else than its actual wording.

In my opinion, it would already be helpful to turn this error into a warning and add background information for the warning. This information should be from the perspective of the package maintainer (who runs nimble check), not from the perspective of the Nimble maintainer. So instead of

Error: Package 'hldiff' has an incorrect structure. The top level of the package source directory should contain at most one module, named 'hldiff.nim', but a file named 'edits.nim' was found.

maybe use

Warning: An installation with the current directory structure and Nimble file would create more than one module that must be imported with import some_module (e. g. import edits), which may unnecessarily pollute the namespace of top-level imports. See https://... for a more detailed description of this message and what you can do about it.

(This message is just a rough idea, it surely can be improved.)

The whole hldiffpkg (i.e. pkg suffix rule) is indeed silly, and I think we actually can and should get rid of it.

Yes, please. If it was only an oddity for the cloned package structure that would be kind of ok, but at the moment this affects also all users of the installed package. As a user, if I do an import some_package / module, I already know I'm importing from a package; I don't want an additional pkg suffix on the package name, which feels like a hack.

onInstall:
  mv("src/edits.nim", "src/hldiff/edits.nim")

I don't know if Nimble should use/allow a free-form package structure and move commands, but it may help. I think at the moment the dependencies between the Nimble file and package structure are relatively complex. (*) @dom96 you may not notice this as a long-time maintainer of Nimble, but it is a problem. I have the impression the current design is more complicated than it has to be. The usability isn't good.

(*) In one forum thread I praised nimble init for creating a working example package structure. On the other hand, without this feature it would be quite hard to get everything right, at least much harder than it should be.

@genotrance
Copy link

If you read this issue in sequence, the following happened:-

  • Binary package didn't install library files
  • On fixing that, nimble complained that you will end up with edits.nim in the global namespace. This was marked as a warning but install was not blocked.
  • When trying to fix the edits.nim location, playing with directory structure, another issue of compiled binary having same name as directory hldiff was seen. The easy workaround is hldiffpkg which no one likes.

Nim doesn't import packages in their own namespace - i.e. ~/.nimble/pkgs/hldiff-0.1.0/hldiff.nim can be imported with import hldiff, you don't need import hldiff/hldiff. This is already known and the foundation that nimble builds on. If Nim required import github.com/c-blake/hldiff/edits or similar variation, life would be different.

The Nimble requirement is simply to not export modules into the global namespace unless the package name matches the module name. If package structure is not followed, a user could use two packages such that edits-0.1.0/edits.nim and hldiff-0.1.0/edits.nim both exist, import edits will always import edits-0.1.0/edits.nim. Good luck after that.

The package needs to consistently work regardless of nimble - Nim is handling the importing here. If two users ship the same package name, it is amply evident you cannot use both at the same time unless you put them in separate namespaces/subdirs. Recommending users only globally export modules that match the package name is reasonable independent of nimble since the errors are more tedious to debug and work around.

It is arguable whether Nimble should enforce package structure but a free for all is just delegating the headache elsewhere. There could be better solutions and we are open to suggestions - it could be documented better, better error messages or what not and while I have been bitten by these issues first hand, I don't think Nimble is wrong in its aspiration. There are other issues around package structure as well that need resolution collectively.

Note also that these are still only warnings, not errors, so it is worth saying that you can very well run into module conflicts today since you have package authors who ignore these warnings.

cc @Araq

@sschwarzer
Copy link
Author

sschwarzer commented Aug 25, 2020

Note also that these are still only warnings, not errors

Just for clarification, for others reading the issue. nimble install gave a warning, nimble check gave an error (that was the error I was talking about in my previous comment).

@c-blake
Copy link
Owner

c-blake commented Aug 25, 2020

The Nimble requirement is simply to not export modules into the global namespace unless the package name matches the module name.

This rigidity & pettiness of nimble feels very un-Nim to me and very weakly related to the compiler. Why anyone thinks name conflicts among installed (not the many more potentially installed) packages are so super likely that they need to protect the global namespace so fiercely baffles me. nimble's json has like 1400 packages while the average nim developer probably has less than 100 installed packages or like me, zero. That 1400/0..100 ratio will only grow. A regular Nim module can export however many procs. The compiler just helps disambiguation along.

Additionally, nimble does not own The Code Universe (and the less flexible it is the more likely it owns less). Someone can always create a conflict with nim --path. A new compiler diagnostic might be nice to warn that two or more packages with the same name are found in the search path. I'm with you there. But that's trivial. No change to import syntax or semantics. All it takes is searching the whole path (only if Hint[warnCollide]=on, say). Then people would have the usual (rare) name collision notification mechanism.

Having the usual name collision resolution mechanism of qualifying (in this case with a package name/directory name) should also be able to be done in a purely nimble-internal way with no compiler changes. import foo/edits or import bar/edits (as per the importer's desires).

Indeed, having this ability to package-qualify imports is independently motivated by people wanting to make dependency on external to the current package explicit in the code not just the .nimble file. Many packages are single top-level one .nim file things. It might be very appreciated to be able to say import maybeUnmaintedPackage/nicemodule. nimble just needs to name the directory "package" and record which version that is elsewhere. That's it! Hardly impossible at all. Only adding one more nim --path element - the top-level pkgs/ directory makes it work. Do that, add the nice new diagnostic and module name collisions are as nice as symbol collisions from modules..virtually identical, really. So, what's the problem?

Ok...If they aren't merely "library modules" but have binary executables whose names collide then there could be trouble, but guess what - there already is. In this package both edits and hldiff show up at the top-level of bin/. nimble could always fall back to packagename-binaryname on that, imo, super rare conflict. There should probably be a way to "rename" binary names differently from modules in nimble anyway to make it less ecosystem-likely. Maybe there is. Beyond that nimble doesn't control the whole executable search PATH and never will. So, it should also be possible for an installer to add a prefix/suffix/munge the name.

Even if you hate the above idea for whatever reasons, there is an important abstract point here neglected now by multiple parties for reasons I don't get at all: There is no fundamental problem with packages exporting multiple modules from their top level because they were always namespaced anyway - by the package name. There might still be a problem with multiple versions of the same package or colliding package names, but A) those are independent concerns and B) I don't know the current nimble does anything smart with that situation anyway. I continue to think that this is not the best venue to discuss every problem with nimble, but I will keep responding if others do.

@c-blake
Copy link
Owner

c-blake commented Aug 25, 2020

delegating the headache elsewhere.[...]
I don't think Nimble is wrong in its aspiration

Delegating something tricky to the person publishing a package feels like the correct division of labor to me. nimble makes exactly two things easy - fetching deps and importing them from a global pkgs/. That's it. The only thing it really needs to decide is the install destination layout. Package authors could be free to achieve that "however". It could just provide variables the package author could use in the .nimble script. There could be "library routines" provided for the few common cases to make that like a couple lines of code.

The lazy package author then gets just as much support. The picky one can ignore those library routines or write their own and has all the power they can/should have. The names of the library routines could even clarify what kind of package it is - library, binaries, hybrid, single multiple. A tiny, eentsy amount of explicitness could probably both simplify the task nimble has to actually do and empower picky package authors. (More concretely there could just be an install.nims file and a .version file in any Nim package and nimble could be reduced to mostly computing transitive closures, fetching and running that program. Heck the transitive closure could even be library-ified with some fetch_deps call. It could all be much more "open architecture" but with shortcuts for the common cases.)

That's just an example. I think there are a lot of possibilities to factor the entire agenda differently and it is not very constrained by the compiler.

@dom96
Copy link

dom96 commented Aug 25, 2020

Having the usual name collision resolution mechanism of qualifying (in this case with a package name/directory name) should also be able to be done in a purely nimble-internal way with no compiler changes. import foo/edits or import bar/edits (as per the importer's desires).

So I'm afraid you've lost me after this paragraph, and even this paragraph I find ambiguous. What sort of "usual name collision resolution mechanism of qualifying" do you have in mind?

The only way I can imagine would be to have Nimble wrap your package inside a pkgname directory, i.e. force you to import every package's module via import pkgname/module. Is this what you mean?

That is a perfectly fine solution btw which we may adopt. The only disadvantage that I find with it is that it will break packages that expose pkgname.nim and want it to be imported via import pkgname and not import pkgname/pkgname but in the grand scheme of things this may very well be a worthwhile compromise.

@genotrance
Copy link

I need to digest the responses as well but one note worth mention is that the package search implementation is not isolated within nimble. No doubt nimble could handle everything within itself and avoid some issues but many Nim programmers use nim c all the time without calling nimble after the dependencies are installed.

To support this, Nim has a bunch of code to search ~/.nimble/pkgs. See nim-lang/nimble#654 for more discussions around that.

They are already namespaced
they were always namespaced anyway - by the package name.

All directories in ~/.nimble/pkgs are package names no doubt but Nim just adds the latest of every package in there into the global namespace when you call it directly. Maybe I'm misunderstanding what you are saying but while there is a directory per package, it is ignored by Nim. It is ignored by nimble as well when invoking Nim since it passes --path:~/.nimble/pkgs/abc-1.0.0. And it goes without saying that even if both these were updated to recognize the package name as the namespace, all import abc will now have to become import abc/abc. Any improvements will need to be resolved between Nim and nimble to be mutually consistent and keep backwards compatibility in mind.

people wanting to make dependency on external to the current package explicit in the code not just the .nimble file

Given how things work today, I don't know how you can disambiguate dependencies with just the .nimble file since Nim does the importing and doesn't look at the .nimble file.

@c-blake
Copy link
Owner

c-blake commented Aug 25, 2020

I am thinking:

.nimble/
  bin/
  pkgs/
    nim.cfg # with --path that adds each installed subdir as well as this top-level
    each/
      mod1
      mod2
    subdir/
      modA
      modB
      mod2

So, ok...maybe the nim compiler would need to have yet one more way to search for/apply nim.cfg but that doesn't seem very onerous. I'm not sure it would. It may already. I just haven't tried it. It's definitely not a change to syntax/semantics, though and fits with other uses of nim.cfg. It would be pretty surprised if @Araq or anyone really hated that kind of new use of nim.cfg if it doesn't already work. It's probably way simpler than whatever the nim compiler does to support searching .nimble/pkgs.

Anyway, with that setup, you can A) be warned by nimble at install time that there is a collision with mod2 and B) could always import each/mod2 with or without the conflict or warning. You might only choose to if mod2 is an especially generic name. That's where the new probably unproblematic diagnostic of the nim compiler (re)warning about the ambiguity helps.

So, the notification of conflict is the same - the compiler barking -- and the disambiguation by parent-namespace qualifying is the same. Just as with modulename.symbol you can also always qualify if you are worried, but you aren't forced to unless there's a conflict. It all seems very Nim to me.

You might need a .version file or one file with all of them or something since the version name is not in the directory anymore. And nimble install would have to maintain that pkgs/nim.cfg. Heck..users could even hack part of it or it could have an include or something for "nimble only tweaks" vs. user tweaks, etc. It's certainly conceivable that some flags like warnings or whatever might make sense only for the nimble installed vs. some other local packages.

Indeed, with a nimble-maintained nim.cfg with --paths you could probably remove any special nimble pkgs/ search in the nim compiler proper. No? It sounds to me like you hacked up the compiler to support this already awkward package structure stuff. With the above idea there could be some competing moreNimble or something with no particularly special compiler support at all. You get better loose coupling properties as well.

With the current nimble set up at install time you might need to put pkgclone/src/*.nim into the top-level of the pkgs/pkgname directory, but that doesn't seem hard. That would fit with the current style of "you tell me what to move" not "you do the moving".

@dom96
Copy link

dom96 commented Aug 25, 2020

Ooh, nice idea. I think we might want to implement this even if to just get rid of the --nimblePath logic in the compiler.

The idea you have about disambiguating by package name also sounds like it could work, but personally I don't think it's the right approach. I personally like knowing which package a certain module comes from, and yes, I realise this is inconsistent with how modules in Nim work (and to be honest, I would love for each function call to be prefixed by the module name by default if it wasn't for how UFCS/operators work in Nim).

@c-blake
Copy link
Owner

c-blake commented Aug 25, 2020

It's just more internally consistent with how name ambiguity and disambiguation works with Nim in general, but with my idea import'ers absolutely have the option either way, whichever they like. For single-*.nim source file packages I suspect import foo/foo would be unpopular, but that is also the case where the system is managing that part of the namespace. So, it's only multi-module packages where it might be nice.

It's possible a new --pathPattern=pkgs/* could help (also not nimble-specific), but it also doesn't seem so necessary. Maintaining a list of --path directives in one nim.cfg with a program is pretty easy.

Also, multi-module packages could just put all their files at the top-level of the git repo. No need for src/. Any *.nim is source code. You already have filename extensions. No need for a new directory level. (Except maybe you might want in .nimble to be able to have an exclude list..but I doubt it would be used much). Then any "forest" of git clones is basically fully functional as-is. I could git clone 10 multimodule (or not) things, put them in mysrc/ and then put mysrc/ in my nim search path and bob's your uncle.

I don't see any real backward compatibility concerns, or at least no more contortions than are present now, but I haven't really analyzed it deeply. I've more thought "what could be from scratch".

@c-blake
Copy link
Owner

c-blake commented Aug 25, 2020

Since in this new model the nim compiler needs to know basically nothing about package management (path and nim.cfg are enough), it also makes it pretty easy to have the installed package hierarchy live anywhere the installer has write permission - /opt/nim/(bin/|pkgs/) or /usr/local/nim/(bin/|pkgs/), etc. That lets the install to be multi-user sharable on Unix.

My own interest has less to do with multi-user sharability and more to do with the triplet: (a network-shared $HOME, the desire to do gcc -march=native and a range of CPUs from 2008 designs to 2018 designs). { Ok..quartet and "a desire to not see illegal instruction traps". ;-) } That could be solved with bin/arch subdirectories and PATH set up in the shell rc files, but I don't think nimble supports bin/arch forks either. System dirs are more traditional anyway. Docker and crew are popular, but those images are still built with system package installers that system installer will always be easier to adapt to more traditional installs. Blocking that flexibility seems like trying to change too much of the world.

Regardless of any of this, it still makes sense for the compiler to just provide "general path search/config search facilities" where anyone could write a nimble , moreNimble, or evenMoreNimbler or whatever. It still makes sense to bundle nimble with nim much like nimgrep, though.

@Araq
Copy link

Araq commented Aug 26, 2020

I feel like having to reply here but I cannot add much to the discussion. I have brought up similar ideas as @c-blake in the past so I mostly agree with him. I also wrote a couple of Nimble RFCs about how to improve the situation.

One minor point that hasn't been mentioned, the problem starts with the fact that "simple" package import statements were supposed to look like import jester and not import jester / jester and most of this mess is an outcome of this aesthetic desire. (Which I personally don't care about.) This is why the compiler "throws" away the existing package directory.

@c-blake
Copy link
Owner

c-blake commented Aug 26, 2020

I think just having the "parent level or pkgs/" before any of the pkgs/name follow ons in the search path fixes that. So, the only thing "lost" in this idea that I see is the optionality to import jester/jester if a module author wants. As mentioned, that is probably only desirable or necessary in an export multi-module setting, but giving people options and tooling seems The Nim Way, and it's basically almost already there.

@c-blake
Copy link
Owner

c-blake commented Aug 26, 2020

Maybe I was unclear, but the only time jester/jester would happen is if the package author creates that unnecessary-in-this-simper-scheme directory layer. (Maybe they had some reason!) Otherwise, jester.nim (if any) would just live side-by-side with any other top-level modules. If there is a private/ directory then for sure that will collide and import jester/private might be needed, but that seems appropriate. There could probably be a <top-level>/bin/ directory to put executables which would avoid the jester.nim builds -o:jester problem.

This mess got started by nimble translating package structures into install structures. For old multi-module packages unwilling to restructure there may be a need to translate to the installed structure. But for a new package or a package willing to re-structure, it could be just like a git clone. So, if you relax the idea of "keeping src/" at all in nim-lang/nimble#654 to just having nimble do the same klunky structure translation then nim c does not need to know anything about this going forward. The rule "git clone just works" will need to be qualified to "unless it's set up to appease old nimble" and the need to make that qualification would gradually vanish. To the extent the translation is reliable in the first place there could be a nimble reorg to do it automatically. And you could retire that someday or some ambitious person could re-org all packages and submit PRs to owners. ;-)

Delegating such translation from whatever a package author wants to canonical for-import install could still be done "however". They are kind of separate questions. Nimble could just look for an install.nims and run it -- if present. If not, it could do nothing. Done. If install.nims wants to call cmake then fine. A flatter installed structure would make it more rarely necessary. install.nims could even send a FAX to the Ministry of Approve My Install and wait for an OCR-read reply. Hah. Just kidding. ;) Or instead of install.nims it could be the .nimble file itself.

Maybe the initial mistake was putting/leaving a -version suffix in the directory name or something and it mushroomed from there? Or copying Cargo too much? Or maybe the mistake was not making new users create a one-line nim.cfg: path = "/usr/lib/nim/pkgs" or $home/nim-pkgs or whatever? (not bad, IMO, and only needed once you start depending on nimble packages). A search-path for other nim.cfg's that could easily include such "usual suspect" locations in the nim-installer-hackable system nim.cfg and might be more friendly for the truly lazy.

Anyway, it would be fabulous if we could recover from this mistake someday, somehow. Thanks for listening and best wishes to all!

@dom96
Copy link

dom96 commented Aug 26, 2020

Maybe I was unclear, but the only time jester/jester would happen is if the package author creates that unnecessary-in-this-simper-scheme directory layer. (Maybe they had some reason!) Otherwise, jester.nim (if any) would just live side-by-side with any other top-level modules.

How would this work when you have multiple versions of the same package installed?

Or copying Cargo too much?

Pretty sure Cargo didn't exist in 2012

@c-blake
Copy link
Owner

c-blake commented Aug 26, 2020

How does it work now? I mean, how can I select which one to import on my import line? With some awkward double-quoted monstrosity? Hard coding the version in the import line is also pretty stiff..so, this situation is already fundamentally awkward for importers and therefore should and will be avoided unless absolutely necessary.

Anyway, because of that avoidance, it will be very rare/only impact very few "importees". So, for those rare cases you could always install second & later versions under -version names and symlink/copy/rename whatever the open that "first won" the unversioned slot to a new versioned slot. I doubt I would ever see one, but then I don't really use nimble. Does this really work now? Does anyone use it?

@Araq
Copy link

Araq commented Aug 26, 2020

Once again, here is what to do:

  • Dependencies are installed local to the current project, no sharing of deps. Hence there cannot be version conflicts between dependencies and there is no need to "select the latest" version in Nim.
  • Dependencies are installed by git clone, do not mess with the directory structures, do not leave out example code and documentation.
  • Nimble maintains a section inside the project's nim.cfg file containing --path commands so that the Nim compiler can build a project and all the other tools like nimsuggest, "nim check" understand the dependencies.
  • Conflicts and directory layouts are hardly Nimble's business. In practice the Nim compiler offers superior error messages for conflicts as it tells you why something is incompatible rather than "uh oh version X of Y doesn't work with version Z of A".

This setup naturally allows for lock files, is far less complex to implement than today's Nimble, is more flexible and less annoying.

@c-blake
Copy link
Owner

c-blake commented Aug 26, 2020

I'd bet @disruptek approves of @Araq's ideas as they sounds closer to (a subset of) nimph.

@dom96 - I just did a git clone of every package not on bitbucket (where I don't have an account set up) - 1,333 packages in total from packages_official.json. I cannot find even one single instance matching the regex 'import .*-[0-9]' in any public project. Yeah, it could be on another line in an import block, but I doubt it. I started to do the git log -p history of everything, but there are several packages that take many minutes to compute that. So, I cannot quite say "no one ever wanted it even temporarily in a way that was publicly visible", but I'd bet that's also true. I could probably get back on that tomorrow when all my log -p's complete if anyone cares.

Presumably people wanting one specific version of a dependency just do an == in .nimble and then deal with or complain about whatever version conflict annoyances. I did just test that import "cligen-1.1.0/cligen/procpool.nim" works as expected when both 1.1.0 and 1.2.0 are installed in nimble's pkgs/ (well, fails with 1.1 and works with 1.2). So, with today's nimble doing that == and awkward import could work. Yet, empirically, (almost?) no one seems to have ever wanted this (or maybe known they could do it).

So, this notion of multiple installed versions seems theoretical at best { even without @Araq's better idea that bypasses it completely and also makes for a far more controlled build environment and makes all this due diligence besides the point :-) }.

@dom96
Copy link

dom96 commented Aug 26, 2020

@c-blake you're misunderstanding. You let either Nimble or Nim choose the package version, you don't put that version in the import statement ever.

Nim by default chooses the latest version that's installed in ~/.nimble/pkgs/. Nimble chooses the most up-to-date version that matches whatever requirement is specified in the .nimble file, requires "jester >= 0.1" will choose the latest jester that is installed as long as its version is >= 0.1.

The imports are always import jester, they do not change.


Now Araq's proposal above goes many steps further and outside what we are discussing. I have always and will continue to champion for the ability to quickly write scripts, forcing local deps on people will prevent that use case since it will require a nimble init, followed by nimble install --local jester or whatever. I (and I'm sure many others) often just want to write a quick script that uses jester. Whether that's for a test or another reason, it's handy being able to make use of a global cache of packages.

But offering it as an option is perfectly fine by me, and we can see how popular it becomes. In fact, @genotrance already implemented a basic version of it in Nimble.

@genotrance
Copy link

Noting multiple points here to help drive towards a solution:

  1. Namespace conflicts are not typically discovered by package writers or nim/nimble core devs - they are discovered by users. Delegating to them might be practical but a simple warning to the package writer that globally exported modules should match the package name is still prudent. Regardless of package counts, most developers think in common terms like cli/utils/diff/version/options/walker, etc.
  2. The nim compiler can get into actual module conflicts and resolution, that will not be nimble's responsibility.
  3. The goal is to remove nimble awareness in the compiler so adding any workarounds (e.g. srcDir mapping) are not preferred. The solution should just rely on --path since it is simple and backwards compatible.
  4. The package structure should look the same with git clone and nimble install. There should be no moving/deleting of files by nimble or nim.
  5. All said and done, users do test with multiple nim versions. The solution should not break just because they checked out an older nim version. Your $nimbleDir doesn't change just because you switched to an older nim/nimble combo. Also, changing package structure from legacy nimble and introducing ~/.nimble/pkgs/nim.cfg will break older nims.
  6. As already noted, nimble installs multiple versions of packages since ~/.nimble/pkgs is a shared space. Considering nim.cfg is the interface between nim and nimble and dependency versions are package specific, each package needs its own nim.cfg. A global ~/.nimble/pkgs/nim.cfg will still have to resort to a latest version selection algorithm which is not what a package asks for in its .nimble file and we should end that disconnect with this solution.
  7. Solution should work for both local deps and user deps modes so as not to break existing workflows.
  8. Note that introducing package namespaces either as actual directories or injected by nim/nimble is at odds with 4 and 5 above and is going to break both library authors and users of libraries. The solution requires that nim does not change that behavior.
  9. The pkg suffix issue is not included in this solution since it can be solved independently.

I'm writing down a solution that extends on Araq's suggestion that solves all of these requirements. I'll note that down next but in the interim, please review that the above covers all the concerns.

@c-blake
Copy link
Owner

c-blake commented Aug 26, 2020

In terms of quick scripts, I just nimble installed jester in 3 seconds. It only has 2 deps. Not running nimble init at all is even faster.

@genotrance - that's a pretty good list. I'll personally probably never use nimble except to test compatibility for client codes as long as it installs binary executables into $HOME for reasons already stated. It seems to me that packages are mostly libraries or at least "dependency" is "dependency on the library parts". Don't know if that helps. There could be deps on a code generator binary, I guess.

What I find most objectionable is that (even as a non-user/total avoider of learning anything related to this complex ball of nimbleness because it seems like a morass of poorly justified complexity) it pushes package authors to put things into src/ or pkgname/ and ignores the nice namespace already provided by being in a named package. In this sense, the way nimble works is, to me, literally ecosystem-worse than having no package manager at all! At least for non-nimble users. That pushing of structure makes it hard to just git clone 5 things, point my --path there and be off to the races. Having to rely on nimble to have any deps at all is a pain. Chasing transitive deps is nice, but at what cost if you impose inconvenient structure? Honestly, if it were a for-profit company I'd say this is a diabolical strategy for vendor lock-in. ;-)

Anyway, I eagerly await @genotrance's ideas. (Which he can post over at nimble if he wants with some link back to here.)

@disruptek
Copy link

I would just add that useful changes have grown out of experiences like this. Nimble has some legacy to deal with, but it's definitely improving:

@genotrance has really hit the nail on its head with his idea of using the package manager as the source of unidirectional logic; the pm sets up the environment for subsequent tooling, and what comes next does not feed back into the pm. Coupled with the idea that nim.cfg is fully the domain of package managers and project.nim.cfg is for inclusion in the repository, it frees us to innovate against the compiler, which as you point out, hardly needs help here.

My bias is obviously towards having as little package management as possible, but as Nimble has grown to support local dependencies and (soon) lockfiles, it has liberated me to pursue other development in Nimph, which was really only designed to validate compiler configuration.

So there's less lock-in than there was, and people's taste seems to be trending in the right direction. Have heart!

@c-blake
Copy link
Owner

c-blake commented Aug 27, 2020

So, in terms of how theoretical vs. practical the "multiple installed versions problem" is, I did a different analysis and one of the packages involved is even @disruptek & @genotrance. I realize all of this metadata is just "the first 8 years of nimble" or whatnot, but it's at least something. I think it often really helps to study how people actually use your tool to prioritize.

Correct me if I am wrong, but unless there is a requires something < version or something#head then the latest released version will satisfy a dependency. One example I found is golden.nimble:requires "nimterop >= 0.6.2 & < 1.0.0" (and only one of the 1333 had a requires ...,\n multi-line invocation with a <).

So, almost nobody uses this < or <= version ceiling feature in the 1333 clones of the public packages I did yesterday..only 25 instances in 1333 packages and of those 13 are for the nim compiler version being "< 2.0.0" which seems..well, weird. So, those who do use version ceilings seem to either misuse them. About the only reason I can see to do it is to force using a tagged release instead of #head. So, correct me if I'm wrong, but you still only get multiple versions if some other actually installed package depends on #head. Those deps are even more rare than < deps - (only 9 out of 1333 do that and 3 of those are mine which I'd be happy to switch leaving 6/1333 one of which I know is a copy-paste from my 3..so really 5 only 4 of which are active since 2016).

So, 4/1333 is a pretty small fraction and the people like @disruptek are probably doing those version ceiling fence posts are probably doing the "abundance of caution" thing to cite how USA lawyers always talk..but it may be overabundant. ;-) There is no nimterop#head dependency, for example, though obviously that could change tomorrow. I think a culture of just doing more 1.x.y releases would probably fix that. I know it was only laziness about doing that which motivated me to do a #head dependency. Such release cultures are often tied to behaviors of the most commonly used package manager.

Some other data that might shed light on how "real" the problems being prioritized are - top level module name collisions are also very rare in the 749/1333 src/ layout nimble repos. Only 3.8% collision rate. The collisions are what you might expect..types.nim, utils.nim, constants.nim. But the rarity means it's unlikely that qualification would be necessary.

The 101 <pkgname>/<pkgname> style pick up a lot of private/ collisions (like 55%), then a similar trickle of collisions core.nim, common.nim, and so on. Many have that format even though they are single-module. The remaining 480/1333 or so are single-module repos which are name-collision-protected by being registered in the official packages list.

Anyway, to me this little investigation suggests that there is probably very little to zero need for multiple versions. The latest tagged release is enough, just dropping/ignoring <. If there were a simple set up that allowed easy pkgname-qualifying resolution to import then such qualification would also be very rare. If compiler-assisted with a simple diagnostic pkgname-qualification is probably even less common than module-name qualification and this is without people even trying to help with better top-level module names..just the organic soup that is packages_official.json.

So, I mean, one can always box oneself into a corner trying to solve an overly hard problem. I think @Araq's idea is great for just separating concerns, boom, done and don't exactly understand the resistance to it. In light of all the above quantitative facts and that resistance, there could also maybe be just a "mode" like nimble install --shared/--shallow for network/disk space-impoverished people who could maybe get more sharing but less than 100% support in terms of build control. They're probably used to that and in fact the most impoverished have to cross compile anyway.

Another relevant analysis would be the distribution of sizes (in package count or in bytes) of the transitive closure of per-package dependency graphs. @Araq's proposal may be far less expensive on average than you might at first "guess". And if you do it his way and someone wants to just fire up a text editor then they can just "cd top-level" and start editing a .nim file only later to be packagified. Nothing is faster than that. And once someone has even gone through their basic deps once, they can put several scripts in one package and have a very fluid workflow, only pulling in new deps as they are newly needed.

@sschwarzer
Copy link
Author

So, almost nobody uses this < or <= version ceiling feature in the 1333 clones of the public packages I did yesterday..only 25 instances in 1333 packages and of those 13 are for the nim compiler version being "< 2.0.0" which seems..well, weird. So, those who do use version ceilings seem to either misuse them.

I think what's intended here is "use a compiler that is compatible with 1.0.x". If Nim followed semantic versioning, a ceiling of "< 2.0.0" would actually make sense. However, since Nim does not use semantic versioning and the "LTS" version is 1.0.x, the ceiling should be "< 1.1". So while the actual ceiling 2.0.0 doesn't make sense for the current Nim versioning approach that doesn't mean specifying a ceiling for the compiler is generally useless, IMHO on the contrary.

Just wanted to mention this. In particular, I don't want to start a discussion on Nim versioning here. :-)

@c-blake
Copy link
Owner

c-blake commented Aug 27, 2020

A fair point :-) but the only version ceilings actually used for the Nim compiler are "2.0.0" (in sss simpleot monocypher mnemonic github) and "1.3.0" in nimpc and 0.20.0 in fugitive (very likely stale). Anticipating what is backward incompatible before it's even happened seems pretty weird to me. ;-) So, there may be only 1 correct non-stale usage in over 1300 packages. Or it might be zero. I haven't looked into nimpc in detail.

Either way, nimble doesn't install nim, but just check you have one that should work. So, this is a side issue at best. </<= could be non-ignored for the nim version, but if the nim compiler changes incompatibly with your code then it seems far more likely for it to just not compile than to compile wrongly making you add when clauses to make it work across all Nim versions >/>= x.y.z. Relatedly, package authors can say when (NimMajor,NimMinor,NimPatch) > (0,20,0): do-something that compile-time fails in these very rare cases. The only recourse to having "too recent a Nim" is to somehow get an older version anyway like via choosenim/etc. So, even for the Nim compiler </<= seems unnecessary.

I think disallowing #head (relying on last point releases) and </<=, and not worrying about multiple versions is probably fine, but @Araq's way also solves that; All this would only matter for the nimble install -s/--shared/--shallow use case which might also be noted is a use case where people are likely to use minimal package sets and run into less trouble just via small numbers. So, you could also say multiple versions or </<= features are only allowed in the deep case (which should probably be the default). Once you need special versions of things, then private copies of your deps are more well motivated anyway. In short, this shared case can be thought of as an optimization and maybe people don't often want it, and maybe some one or two extra things needs to be true for the optimization to apply.

Anyway, I'm just trying to scope what/how bad these actual problems are based on what people do/did with facts that are less subjective/theoretical. It's obviously easy to say everyone has misused tools/languages for years to discredit that, but misuse of features is also often an argument against them.

@c-blake
Copy link
Owner

c-blake commented Aug 27, 2020

Oh, and I guess I forgot to even check PKG == VERSION. Only one package uses that -- nsu for flippy==0.4.0 and that is very arguably a mistake. (I.e. some other package could use flippy==0.4.6 yielding multiple versions.)

And forcing people to do that when (NimMajor,...) might even be considered a "feature" to make them may at least consider some when declared or elsewise more precise thing that could make the code work for both before and after the change causing a problem.

@dom96
Copy link

dom96 commented Aug 27, 2020

However, since Nim does not use semantic versioning and the "LTS" version is 1.0.x, the ceiling should be "< 1.1".

Huh? Nim 1.2 does not break compatibility as far as I'm aware. At least as long as adding new features doesn't count. So in essence, Nim does follow semantic versioning.

Anyway, to me this little investigation suggests that there is probably very little to zero need for multiple versions. The latest tagged release is enough, just dropping/ignoring <.

The fact of the matter is that Nim is so immature and the depth of packages is so low that this investigation does not yield a representative sample of the sorts of issues that will become prevalent into the future. I appreciate your efforts but I don't really think they prove much.

I actually think that we should start having a conversation about supporting the ability to import two different versions of the same package inside a single compiled program. The deeper the dependency tree and the more packages are used the more common this scenario will become, and then you'll really run into painful situations when Nimble tells you "Sorry, but Jester needs httpbeast 2.0, but prologue needs Jester 1.0.0".

@disruptek
Copy link

This is already a problem; Nimph breaks with cligen >= 1.0, so it requires an earlier version in conflict with nimterop.

I think what is ultimately needed long-term is scoped imports, but I haven't exhausted all my ideas for working around this.

@c-blake
Copy link
Owner

c-blake commented Aug 27, 2020

@dom96 - what about the idea of saying you lose the "shallow optimization" that seems important to you once that gets "too hard" and just do deep dependencies as per @Araq? Most of my investigations could be framed as trying to suss out how often that shallow optimization would work.

@disruptek - is there something I can do to make cligen >= 1.3 work for nimph? Happy to help. Feel free to raise an issue.

@genotrance
Copy link

I haven't caught up again with the many posts but please read this link for info on minimum version selection. Today nimble picks the latest but it should really move to this algorithm in the future.

@disruptek
Copy link

Do we have a ticket for this? I remember ultimately convincing you but I think we just decided to go with latest because of legacy ecosystem reasons. How would we transition everyone?

@dom96
Copy link

dom96 commented Aug 27, 2020

We may want to discuss this urgently, as Minimal Version Selection can be a replacement for lock files. Perhaps we should adopt it and say screw it to lock files altogether?

@genotrance
Copy link

Some comments:

  • Using #head is a bad idea since it is just a snapshot in time. Today, if you checkout #head of a package, nimble no longer installs newer versions of the package when processing dependencies since #head = latest and is already installed. It also doesn't check if local #head is still latest #head every time since that would be excessive. Any package requesting #head is polluting $nimbleDir for all other packages. Just don't use #head, use a specific commit hash if needed. Nimble should not support #head in my opinion.
  • The low collision rate of global modules could also be because most authors saw the nimble warning messages and acted on them.
  • As linked, the minimal version selection algorithm greatly improves the status quo as far as reliable builds and lock files are concerned. But it also means that you will end up with multiple versions of a dependency in a shared space across projects.

I remember ultimately convincing you but I think we just decided to go with latest because of legacy ecosystem reasons.

At the time of our discussion, I hadn't read this and related articles. It makes total sense for our ecosystem. The question is how to deploy this without breaking users. It might warrant building all packages and opening upstream PRs to update the required versions. People have been lazy since nimble installs the latest so this will require some education, support and new discipline.

MVS warrants simplifying the addition and updating of deps and versions in the .nimble file so that would have to be done as well.

Perhaps we should adopt it and say screw it to lock files altogether?

The writing is on the wall. Go has already implemented this it seems. It warrants more investigation and discussion for sure.

@disruptek
Copy link

Well, you can use Nimph to test packages. nimph downgrade will roll to the MVS.

@c-blake
Copy link
Owner

c-blake commented Aug 27, 2020

Well, ok. I just got rid of all my #heads, pushed releases and am opening issues on the other 5 repos that have #head dependency. Those repos are pretty stale, but so few it's easy to do. So, maybe by next week it will be gone from the nimble world and you can deprecate @#head { EDIT: and no one will notice! :-) }

Re: module name collisions these are collisions after squashing src/ or the pkgname/ levels. Now, an author could have seen the warning and been more careful about naming in addition to moving things into subdirs -- a "belt & suspenders" kind of reaction to the warning, but that does not strike me as likely. In any event, what I did was still a valid way to estimate how much package-qualification would be needed if you just did a nimble migrate on all of them, and it wasn't bad at all. Further, it could become easy for the nim compiler to warn nicely on a collision and in many ways this question is orthogonal to the broader shallow/deep discussion. My concrete proposal only needs path but doesn't support MVs, but if something special is done now to select version directories on import and suppress version numbers then that same thing could probably work with a package prefix, too.

Maybe I was trying to simplify too much at once for nimble maintainer tastes, but it sounds like @Araq would prefer even more radical simplification in exchange for run time expense.

@disruptek
Copy link

I'm in favor of issuing a warning, at least, for #head. Nimph supports it and it's a relatively expensive feature because head can never be pinned, compared to other tags, etc. It's just a super painful corner-case that only really exists to trip people up.

That said, there are plenty of packages that don't specify version requirements or, if they do, the required package may not be tagged. Are these not #head? And if we move to MVS, do these point to the first commit?

@alaviss
Copy link

alaviss commented Aug 27, 2020

My only problem with MVS is this statement:

Modules are assumed to follow the import compatibility rule—packages in any newer version should work as well as older ones—so a dependency requirement gives only a minimum version, never a maximum version or a list of incompatible later versions.

This is definitely not true for Nim. For example, norm recently has version 2 released, and it completely breaks compatibility with the older 1.x series. The post addresses this with:

different major versions are just different modules: for the purposes of these algorithms, B 1 is no more related to B 2 than to C 1.

This assumes that modules follow semantic versioning, but AFAICT, we don't enforce this at all, and in nimble version is a string that can be whatever instead of following a concrete format.

And even if we were to enforce it, here's a scenario where it gets hairy:

  • package A (1.0.0):

    type Obj* = object
       id: int
    
    proc print*(o: Obj) =
       echo "id: ", A.id
  • package A (2.0.0):

    type Obj* = object
      name: string
    
    proc print*(o: Obj) =
      echo "name: ", a.name
    
  • package B (1.1.0):

    requires "A >= 1.0.0"
    requires "C >= 1.2.0"
    import A, C
    
    C.foo(default(Obj))
  • package C (1.2.0):

    requires "A >= 2.0.0"
    import A
    
    proc foo*(o: Obj) = A.print(Obj)

Under the MVS, to compile B, these packages will be installed: A-1.0.0, A-2.0.0 (major bump is considered to be a different module), C-1.2.0. Now here's the problem:

On the Nim side, there's no distinction between A (2.0.0) in C and A (1.0.0) in B, so what would happen here when we pass A (1.0.0)'s Obj to C, which expects A (2.0.0)? Which Obj will be used? And either Obj will cause a runtime error due to the incompatible memory layout.
If the compiler is aware of versions and error out, wouldn't it just annoys the user that the same import A and the same Obj is referring to different versions?

Go completely dodged this problem by making the major version is a part of the import identifier, but I don't think we can realistically apply that to current Nim.

@disruptek
Copy link

Cue @Araq and "relative import paths". 😉

@dom96
Copy link

dom96 commented Aug 27, 2020

That is precisely the problem. Like I said above, Nim does not support two different versions of the same package in a single compiled program.

I only see two solutions to this:

  • You do what Go does and only make it work for major version changes by changing the name of the package (which is pretty much cheating lol)
  • You implement support for this in the compiler by enabling a set of flags/options that can be passed which specify the path to search for each set of modules (my guess of what Cargo/Rust does)

@sschwarzer
Copy link
Author

However, since Nim does not use semantic versioning and the "LTS" version is 1.0.x, the ceiling should be "< 1.1".

Huh? Nim 1.2 does not break compatibility as far as I'm aware. At least as long as adding new features doesn't count. So in essence, Nim does follow semantic versioning.

I was told that 1.4 would (probably) make orc the default memory management. If that's the case, there's a problem with deepCopy (nim-lang/Nim#13997) and using the marshal module on devel gives:

Error: marshal module is not supported in new runtime.
Please use alternative packages for serialization. 
It is possible to reimplement this module using generics and type traits. 
Please contribute new implementation.

The message about a possible reimplementation is relatively new I think; I can't remember having seen it, only that it wasn't supported. So there's hope. :-)

I'm not sure what will happen regarding deepCopy though. But I hope that'll be sorted out by the time Nim 1.4 is released.

So maybe I was too hasty here.

@genotrance
Copy link

genotrance commented Aug 27, 2020

I've posted a detailed problem statement and solution for this discussion on the nimble wiki.

https://github.com/nim-lang/nimble/wiki/Package-structure-and-Interop

Please review and share your thoughts on the forum and thanks in advance.

@Araq
Copy link

Araq commented Aug 28, 2020

We may want to discuss this urgently, as Minimal Version Selection can be a replacement for lock files. Perhaps we should adopt it and say screw it to lock files altogether?

Over my dead body. Lock files are superior.

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

7 participants