-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Nimble improvements #398
Comments
I'll have to read it again, but when trying to leverage nimble with the VSCode extension I ran into a few issues and learned some lessons:
You've covered a number of necessary improvements, awesome! I think a problem that remains with nimble even after the ones above which is that the format and data structure of the declarative manifest do not focus on the end product (artifacts really) and have declarations focused on those. Rather than trying to do that now, one very simple thing could be done, require a nimble file format version for any nimble file. Files without one will have one inserted automatically and eventually fail in subsequent releases if one isn't present. Then down the road some amount of automatic format upgrades can be made such that it becomes much more workable. |
This was shared as a draft to me so I already read most of this and gave my feedback, most (if not all) of which was incorporated into this RFC (thanks @haxscramper!) I would really love to see all this implemented, some of it is already a part of nim-lang/nimble#913 which we'll hopefully be able to merge soon. I hope we can move fast with these proposals, most seem quite nicely isolated so maybe we could create issues to outline the sub-tasks to implement them, that way we can also keep track of who's working on what and what the progress is :) As we discussed, for the dependency resolution changes the prerequisite is to create some nice test infra to reproduce the issues that our users faced. I think this is one of the most important things we can work on so we can get insight into the best way to fix them. |
Completely static toplevelA little more aggressive extension of the ideas proposed in the manifest evaluation section. The idea is simple - make all package metadata static. Based on the analysis of existing package configurations, approximately To be more specific, 'canonical' value format for different metadata fields is
Note that Aside from package metadata might contain:
Right now it possible to put arbitrary nimscript code at the toplevel, and it would be executed each time I want to access some package metadata. For example, if I want to know package version, and it contains code like Instead, I suggest that template task*(name: untyped; description: string; body: untyped): untyped =
proc `name Task`*() = body
nimbleTasks.add (astToStr(name), description) # < This part is placed at nimscript toplevel
if actionName.len == 0 or actionName == "help":
success = true
elif actionName == astToStr(name).normalize:
success = true
`name Task`()
Make binary build optionalThis was first discussed in context of IDEA: Allow requiring only library or binary part of a package as EXAMPLE:
NOTE: That is all modeled as package features (already mentioned in context of optional dependencies). We can only do this for hybrid packages and os-specific dependencies. Latter one is automatically set by package manager - user can't set Provide better control over build configurationIntroduce build backend:
requires "jester"
backend = "c"
# optionally you can override how this gets built
# by writing exec("nim c blah"), otherwise you get this
# by default
build frontend:
requires "karax"
backend = "js" |
In anticipation of potential "scope creep" comments about this RFC - I personally would prefer to have a clear plan that is thought through to and includes description of most of the required changes, instead of going thought multiple smaller RFCs that have to be coordinated with each other ("RFC for When this RFC is discussed and finalized, I want to write a recap includes a clear list of actionable TODO steps, or specification similar to the final draft I did for #245 (comment) |
I am happy to contribute a .nimble file parser that can process a .nimble file without executing it, I have the code lying around somewhere. The Nimble tool itself should use this code so that the restrictions are enforced properly. |
The code is here now, nim-lang/Nim#18497 (parse_requires.nim module) |
I've found Statistics for standard library usage thread and decided to generate updated stats. It is probably related more to #310 than nimble itself, but considering good package almost always comes up in discussions related to stdlib evolution I decided it is fine if I add stats here (and #310 is closed so). Stat script - haxscramper/hack@90e7670 note: I update stats from time to time, so you might also be interested in comparing file/package counts, or popularity growth rate.
|
It seems like I've missed this in my original comment, but |
And again, because HUGE portion of the PR for "lock files" (but-actually-a-lot-more-than-just-lock-files) were discussed elsewhere ("have to implement two additional features for which zah specifically insisted."), some things might need to be re-evaluated to account for the changes in workflows, general expectations and so on. |
Related - nim-lang/nimble#921, for OS-level dependencies. Supporting other build systems somehow might also be nice. Providing full dependency resolution is an overkill IMO, but since we already have Also partially related - #414, though package manager should generally be concerned with packages and libraries, but some functionality is shared (for include files etc). Building various artefacts via nimble would benefit from being able to at least explicitly search for dependency availability instead of failing somewhere mid-way when running cmake for the dependency. |
I'm no longer interested in positioning myself as a main driving force behind the effort to steer nimble design. If nim core development team considers my original goals worth pursuing, they can reopen the RFC. I want to reiterate that I just no longer willing to aim for a role in the nim package management discussion, but at the same time I was told the RFC itself has many good ideas, so I'm not opposed in any way to someone keeping it open. Only that I no longer have the incentive to push for the ideas. I'm moving to https://github.com/disruptek/nimph instead since it already supports all of my necessary workflows, including issues outlined above (such as simple sequential order of |
There is a lot of great ideas in this so I will reopen this. CC @Araq, we discussed the need to have an explicit dependency graph, see the "Use explicit dependency graph in Nimble" section above for examples where this is necessary and what bugs it will resolve. |
- CHANGED :: - Implementation improvements for the code pretty printer - more input nodes supported. - Clean up the implementation of the tree-sitter wrapper generator. - More predictable `treeRepr` output - nodes with comments are no longer split apart at random places. - Make a ton of `func` nodes into `proc`, because accessing comment field is no longer a side-effect-free operations, most likely due to the nim-lang/Nim#18760 - ADDED :: - Tree-sitter wrapper generator now can produce library wrappers that do not depend on hmisc for operation. - `addPragma` for enum declarations - REMOVED :: - `nimble_aux.nim` and dependency on the nimble - I no longer work on the nim-lang/RFCs#398 and I see no reason to try and revese-engineer the dependency management solutions, `nimph` provides much better approach in this case (edit `nim.cfg`, then user can simply dump it as needed).
I think that using MVS is not that good idea. If I set some restrictions on a version of a library I almost always want to see the maximum available version inside the diapasone. This might be illustrated by a python pip versioning. |
The idea is that you use as the minimal version the version that you actually tested your package with. I don't see how you could not "know it at all". Most Nimble packages are not OS specific either and if you don't support an OS, document it as such. |
The idea is that e.x. I've tested my code with 3.0 but later author released 3.0.1 with bugfixes and with <3.1 constraint it will work, but with >3.0 <3.1 I always will get the original 3.0 and need to manually tweak requirements every time when the lower boundary needs to be pushed up. |
Well so you don't like the "minimal version" scheme, that's fine, but if we decide to follow it, you then would have to adapt your workflows. I'm not saying that we should adopt "minimal version", but it does make some sense. If you prefer to use version 3.0.1 because of the bugfixes it contains, version 3.0.1 is your minimal version. Either you depend on these bugfixes or you don't -- if you don't know if you do it seems safer to use the higher version number but that's nothing new, minimal version simply forces you to be honest about it: You tested it against 3.0.1 happily, so that's what your requirements really are, you don't want to go back to 3.0.0 then, it's risky and untested. The problem with "pick the highest available version automatically (except for the 'breaking' versions)" is that nobody knows what it means as it keeps changing. |
Well, the "minimal version" will be a very weird decision and it has no sense in case of a real working flows, because:
No any other package manager which is used now in production follows the "minimal version" procedure. That might be the higher than constraint, but it always gets the latest but not the minimal version. I think that those who has written them and massively using them are not dumb and understand what they're doing. |
The "chain reaction" effect might not be as bad as you think it would be for the reason that "pick minimal version" is an incomplete way to describe the situation: The algorithm actually picks the maximum of the provided minima: If package A depends on B version 2.1 and package C depends on B version 2.2 and your package depends on A and C the version 2.2 will be picked up. |
Hey, I am new to nim and anything nimble related. Now here are a few things I would like you guys to think of:
|
I believe centralized package registry should be optional, especially if it requires non-free software like GitHub.
It's actually useful when you download archive tarballs instead of cloning the repository.
I like Meson's
Bundler, CocoaPods and Shards use Molinillo algorithm, it's doing backtracking too. I think it could be ported for Nim. Although I'd prefer to focus on building and delegate dependency resolution to general-purpose package managers. |
Yes please. Speaking from experience with Python, requiring a central repository makes using package management in the private space a headache, as you end up having to host your own central repository. |
Discussion on Discord on package resolution regarding the new Atlas: https://discord.com/channels/371759389889003530/768367394547957761/1113215118709366835
See OpenSuse presentation at Fosdem2008 - https://en.opensuse.org/images/b/b9/Fosdem2008-solver.pdf |
The RFC is extremely conservative on breakages and new features - it only strives to formalize existing behavior and improve general experience of using nimble. I'm not going to propose any drastic measures as dropping versions and using git hash commits instead, or integrating package management with module system. As a result, majority of existing packages won't be affected in any way and common workflow of using nimble would stay largely the same.
Package registry with dependency metadata
Most package management solutions include centralized package index that keeps track of all package version, thus solving problem of finding requirements for a particular version of the package. For example,
cargo
is centralized and full information, including each package version is stored in git repo in the form of simple json files. When new version of the package is published simple edit to package file is made. Right now nim employs similar solution via nim-lang/packages, but it requires someone to manually merge PR with a new package. Not really scalable solution - sometimes you might need to wait several days before package is added to the registry, but I believe this can be automated. New version is recorded as simple one-line diff that adds information about new package version, it's requirements etc.{"name":"package","vers":"0.4.2","deps":[{"name":"depname","req":"^1.4"}],"url": "https://gitlab.com/XXX/YYY.git"} + {"name":"package","vers":"0.4.3","deps":[{"name":"depname","req":"^1.4"}],"url": "https://gitlab.com/XXX/YYY.git"}
Having package registry which records all
package+version -> dependencies
mapping is important for full dependency resolution. It also allows for projects like nim package directory to exist, which help increase discoverability of different nimble projects, and analyse whole ecosystem at once (which was crucial when writing this RFC - without access to comprehensive list of packages I would not be able to provide any concrete numbers).IDEA: make
nimble publish
put current package metadata in thenim-lang/packages
index - by either creating new package (like it does now), or pushing new version.Changes to package publishing workflow
git commit -m "[VERSION] v1.2.3" # Commit your changes git tag v1.2.3 # tag changes + nimble publish # push new package version to registry git push --tags # upload tags to github git push origin master # upload code to github
NOTE: Convenience command like
nimble newversion
could be introduced to tag, commit and push package all at once. This would also allow to writebefore
hook for running full tests, docgen etc.before newversion: exec "nimble", "test"
Use explicit dependency graph in Nimble
Current implementation of dependency resolution does not construct explicit dependency graph, and instead just loops though requirements, almost immediately installing them which I believe to be the source of such bugs as "nimble loops infinitely trying to install codependent requirements" and "Dependency resolution depends on the order in requires" (could be prevented with explicit dependency graph construction).
Related links:
requires
IDEA: Improve dependency resolution algorithm. Use full knowledge about requirements about each version of each package from previous section.
NOTE: Right now
nimble test
and similar tasks are always interleaved with dependency resolution and potentially package downloads. With full access to existing package metadata these steps can be simplified as only unregistered packages would have to be downloaded.Do not require full manifest evaluation
Right now it is necessary to fully evaluate nimscript configuration in order to determine list of dependencies, as it might contain code with complicated logic. It cannot be statically reasoned about and introduces a lot of complexity in tooling --
nimble
has to generate a.nims
file, evaluate it separately. It processes nim code and modifies global variables likevesion
, then prints result in the end. Output of script execution is parsed bynimble
and only then get list of dependencies.Arbitrary nimscript might be considered a good solution for custom
task
targets, but ultimately this leads to issues where evenauthor = <author name here>
might require full compiler to evaluate.Instead, small subset of the package manifest must be written in a declarative manner (still using nimscript syntax, but with more strict rules). Specifically this concerns
requires
and couple more metadata fields.version = "version"
andpackageName = "name"
must have string literal and be located at the toplevel.requires "dependency1", "depenendency2"
and are located at the toplevel file as well.when defined(windows)
section.when
section must contain static list ofrequires
, identical to the toplevel ones. This part is particularly important as it allows to determine feature-based dependencies to avoid installing unnecessary packages, especially in case if they haveafter install
that might fail whole installation.Existing
nimble
packages almost universally comply with these requirements, and most of ones that don't have simple repeating pattern violating the rule import in nimble. Optional dependencies are already handled this way for most of the packages - treeform/hottie, minefuto/qwertycd and several others.IDEA: Small subset of important metadata like
requires
were written in much stricter declarative manner.NOTE:
version
andpackageName
information is redundant - first one is already stored in git tags (andnimble
actually uses them to fetch required package versions), and second one is optional and"must match name specified inside <packagename>.nimble"
. The only advantage of havingversion
in the.nimble
configuration is that you don't need to shell out togit
in order to find out package version.NOTE: Additional restrictions might be placed of other metadata fields like
foreignDep
,author
,description
etc. Foreign dependencies might also be checked upon installation, but that can be implemented later. Almost all packages that we have today comply with the requirements:toplevel
means top level of the manifest - not inside of any task, when etc.when
shows number of times particular metadata field was encountered inwhen
sectiontask
shows number of encounters inside oftask
Canonical
seq
values "canonical" way of writing isvalue = @["string", "string"]
value = "string"
requires
andforeignDep
isrequires "string", "string"
ident -- metadata was set from identifier (most likely with
import common
, followed byvalue = importedConst
)spec -- any other way of writing value
namedbin
is almost never used.foreignDep
happen inside ofwhen
sectionStreamline nimble-tooling interaction.
Package manager should be a separate tool that does not create two-way information flow. Instead we should adopt simple model
[pm] -> [compiler]
or[pm] -> [build tool] -> [compiler]
. Package manager either runs compiler, or configures environment where compiler can run. Intermediate build tools might include something liketestament
or other tooling. We already have a pretty nice configuration format in form ofnim.cfg
that would allownimble
to inform compiler of all the necessary configuration values.Having volatile configuration file would make it easier to inspect how
nimble
called the compiler, and even though n linked issue suggest that "If the experience becomes seamless then the user really won't need to care about whatNimble
does.", in practice it is quite hard to runnim
compiler the same waynimble
does it - the only option is to wait for compilation to fail and then copy error message that contains the command itself, and I don't believe we would be able to make this seamless enough so nobody would ever need to run compiler manually or check hownimble
communicates with compiler.Another very important advantage -
nim.cfg
has is support for external tooling thatnimble
can't interface with right now. For exampletestament
- if someone wishes to use it for testing their projects simpleexec("testament all")
usually does the right thing in CI, but under the hood it knows nothing about actual project requirements and simply relies on--nimblePaths
. I can't makenimble test
use my own compiler build, nor is it possible to fully integrate external tooling in the project. Forhaxdoc
I basically had to copy-paste dependency resolution part ofnimble
, remove package installation parts and then work based on that.At the same having to run
nimble setup
each time after changing.nimble
file could become annoying pretty fast, so old commands likenimble build
andnimble test
should not be deprecated. The change is mostly make following workflows synced with each other, and make it as easy as sometimes running single command.Counter-argument to this approach is
We can leave
nimblePaths
as they are now, and you would be able to use nim compiler as it is now - out of sync with actual package requirements, but if you don't care thats fine. The fix is onenimble setup
away, so once you need it, you can configure everything very quickly. Any other tool can emulate this behavior as well (liketestament
).This approach enables range of workflows
[nimble build] -> [nim c]
--nimble
updates environment in which nim compiler would operate and then executes it. Simply shorthand fornimble sync
followed bynim c src/main.nim
[nim c]
-- nim compiler is launched as a standalone tool - it can read already existing environment configuration and work the same way as if it was launched directly by nimble.<custom tool>
-- custom tools have full access tonim.cfg
and could easily work the same way as if they were launched bynimble
without having to provide explicit support for that feature.[other PM] -> [nim c]
-- if someone wants to manage their environment using different tools, or even manually (for example using git submodules), it should be possible to writenim.cfg
by hand (or using some helper tool). Submodule-based workflow is not really different from package-based one.nim.cfg
correctly sets environment for all subdirectories it is located in, which means paths are correctly set up for subdirectories, tests, other projects that you might want to develop in the same repository, dependencies and so on.Package manager is allowed to edit
nim.cfg
to modify volalile configuration elements like--path
related to project dependencies. It is possible to provide additional paths in thenim.cfg
, for example when one of the dependencies is located in the git submodule. In that case configuration file might take following form:Dependency resolution
Dependency resolution for nimble have been discussed multiple times in different contexts, specifically in "modern techniques for dependency resolution". Possible options for solving this problem that were mentioned:
All of these options are used in certain package managers, and with some effort they might be reimplemented for nim as well. But, while looking for existing solutions that could be easily adopted I've found one that seems to be suited especially well for the problem at hand -- PubGrub: Next-Generation Version Solving by Natalie Weizenbaum. The article introduces new dependency resolution algorithm called pubgrub. It has already been adopted by
dart
andswift
package managers, and have several reimplementations in other languages. They were both mentioned in the linked article suggested in#890
, which was written in 2016 and mentions both of these package managers, with comments on their currend dependency solver implementation. "Dart's pub includes a backtracking solver that often takes a long time.", "Swift's package manager uses a basic backtracking solver."Basics of the algorithm are explained in the introduction article to the algorithm and much more detailed specification. Overview talk from Dart Conf by the algorithm author and introductory talk for Swift package manager. Implementations in different programming languages:
pub
package managerI've examined dart implementation in close detail and it seems there is no need for any specialized knowledge (compared to
libsolv
and especiallyz3
approach) to adopt the implementation for nim needs. The source code is extremely well documented, and paired with comprehensive documentation, for both algorithm and user-side behavior. The algorithm is designed to provide extremely concise and clear error messages about failure reasons.Compared to alternative approaches discussed in #890, and specifically libsolv pubgrub has several important differences that make it especially well-suite for the task at hand:
when defined(windows)
) and special attention to error messages.nimble
installation.IDEA: adopt pubgrub algorithm for solving nimble dependency graph.
NOTE: With support for full package dependency graph it might be possible to improve current implementation a little more, and introducing such major change in the implementation should be carefully considered. If full dependency graph is introduced as proposed in the previous sections it might become less of an issue.
Quality-of-life features
In addition to changes directly related to package management, some quality-of-life improvements can be made.
End user
nimble install -n foo
automatically declines update, but downloads file and builds all binaries anyway. If current package version is not newest it installs things since no prompt was created..nimble
file - look up in the directory tree?warning()
,error()
orhint()
command for the build tasks, simihar to the cmakemessage
. This is much better (can be configured, filtered out) than semi-randomecho
calls that are placed all over build tasks in some cases.build
/test
without dependency resolution and installation interfering. This becomes a non-issue ifnimble
is used to manage volatile configuration file -nim c
simply does the right thing. But for cases when custombuild
task exists it should be possible to donimble build --skip-dep-scan
.--help
(especially--json
)LICENSE
file in project when choosing license - otherwise github fails to show correct license number. We already know author's name, creation time, and copy pasting strformatted license texts would require almost zero maintenance.git
binary is used - if it fails for some reason (even unrelated to the original query),nimble
falls back tohg
and prints quite unhelpful error message'hg' not in PATH
.Developers
Nimble already provides some api in form of
nimblepkg
package, but it does not provide full features of the nimble itself. For example I had to copy dependency resolution code and remove download/install parts for it in order to be able to correctly compile documentation for the whole project in haxdoc. Ideally most of the internal API for package handling should be available as a library. Also, this might help with testing internal implementation details such as package resolution.This would also allow to freely experiment with alternative package managers that are fully interoperable with nimble - largely because they share the same core implementation and only differ in small details, like dependency resolution algorithm (we can postpone any changes in the nimble dependency resolution core, and if someone wants they can try out pubgrub in proof-of-concept package manager to see if this is really worth it).
Adopting changes
Most existing packages won't have any breakages. For few ones that used certain patterns listed below much easier (and cleaner) solution is provided.
Note: before we get comments like this that start mentioning all possible ways of writing
requires
I want to say that, as it turns out, people right now do indeed writerequires "<string literal>"
almost all the time, so it is not an issue that we are facing right now. Out of2781
requires processed I've found that2738
can already be consireded 'canonical', and ones that don't are written asrequires: "str lit"
In some packages in order to retrieve
version
data followingimport
must be resolvedand then vesion is set as
version = appVersion
. This approach is used very few packages, specifically:version = vPath.staticRead.splitLines[0]
is added on top of module paths checks.There are a couple more packages that have non-standard property configurations, like
installFiles = @[TzDbVersion & ".json"
in timezones andinstallExt = @[when defined(windows): "dll" elif
in pvim. In nwsync packagebin
field uses following code snippetSince
0.11.0
nimble definesNimblePkgVersion
flag - we can simply put it in thenim.cfg
so other tools could pick it up.New features
Task-level dependencies
First requested in the github issue by mratsim. Separate implementation.
task
section might containrequies
orwhen .. requires
sections. When task is executed nimble creates new requirement lists, write resolved dependencies innim.cfg
and executes body. All calls to external tools viaexec("nim doc")
operate as expected.before
section is treated as part of the body itself. Special tasks liketest
are not different in any way.Using minimal version for package resolution
Provide an option to consider minimal allowed dependency option for
requires
range rather than maximal one. This would allow developer to keep package requirements in check by enforcing correct minimal version ranges. It mostly solves "just works" problem keeps the devs honest about their requirements and the user happy. This would benefit package ecosystem as a whole.Example of the problem this would allow to solve:
dep >= 0.1.0
as a version constraint.dep 0.1.3
, but didn't update requirement since0.1.3 >= 0.1.0
dep
to0.1.1
, couldn't use this deps because it won't compile,and there is no way package manager could've prevented this.0.1.0
) the problem wouldn't exist as they just fixed correct requirement range.This idea was suggested in the go article, and later considered by rust, zig, conan.
As of now almost all package
requires
are potentially subject to this issue (to some degree).We already run CI for important packages to make sure there is no compiler regressions. Check like this could be added to futher improve ecosystem health in the long run.
Since this RFC proposes to change manifest format, it would be appropriate to also include format version as well. This can also be used to gradually adopt MVS -
v1
uses current simplified resolution algorithm, andv2
uses MVS selection. For now, this is just an idea that does not address howv1
vsv2
are going to interact with each other.Third-party libraries and foreign dependencies
Due to lack of standard way of building external libraries (compile C++ code for example) and
exec()
,doCmd()
-based workarounds things like Nim - [...] nimble shell command injection (specifically Remove usage of command string-based exec interfaces) are more likely to happen.It is not possible to engineer a way to properly interface with every existing build system for C++ there is, but it providing better and more secure (at least easier to audit) convenience API for calling external program in form of
runCmd(cmd: string{lit}, args: varargs[string])
and deprecate (remove?) usages ofexec
. I would that would help avoid different shell quoting issues as well.I'm pretty sure there is a lot of people who are not entirely thrilled by the idea of executing code with OS sideeffects just to get version number or find out package requirements list. Declarative subset of the manifest would allow to skip package evaluation and just simply read configuration. Right now it is not possible to disable
exec
,staticExec
orexecCmd
execution innimscript
.List of most commonly used
exec
commands - approximately 16% contain some form of string concatenation, and quite a few others build string externally. Overall it seems like on average almost every package calls toexec
in one way or another.Recap
name
,version
andrequires
metainformation.nim.cfg
instead of calling it callnim
directly. Configuration file contains all path for dependencies and additional information.import
to store version in separate file do this only as a a workaround.nimble setup
command that would update package configuration file.nimble newversion [--major] [--minor]
to automatically tag and commit changes.Extra
Random facts
Total number of commits per day in all packages at the time of analysis.
The text was updated successfully, but these errors were encountered: