Skip to content

Instantly share code, notes, and snippets.

@genotrance
Last active November 12, 2019 18:34
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save genotrance/d07bfabd9ef8f66450c7e9732ffaf6f4 to your computer and use it in GitHub Desktop.
Save genotrance/d07bfabd9ef8f66450c7e9732ffaf6f4 to your computer and use it in GitHub Desktop.
Nimble RFC for lock file support

The following RFC aims to implement lock file support for Nimble. This is being tracked in issue 127 but this proposal is a simpler variation.

Intro:

  • lock mode will be enabled by a flag in nimble.ini
    • Since it is a new feature, this will reduce impact on the existing user base
    • It can be turned on by default in the future when stable enough
  • Lock information will be stored in $prj/nim.cfg
    • Nimble will append autogenerated section if existing
    • Nimble will update when any dep changes occur
    • Nimble will consume when having to install deps
    • Nim will consume per usual

Lock file format:

  • Separate section in $prj/nim.cfg maintained by Nimble
  • Nimble related metadata will be stored as nim comments
  • E.g.
# Nimble autogenerated start
--noNimblePath
# url = https://...
# checksum = xyzxyz
--path:$nimbleDir/pkgs/package-1.0.0
# Nimble autogenerated end
  • Format currently includes package name, version, url and checksum
  • Nimble does not store source control hashes so that's not being considered for now

Nimble changes:

  • Add code to write and read back from $prj/nim.cfg
  • After any package install
    • Calculate and store checksum in either nimblemeta.json or a new file in pkg root
  • processDeps()
    • Get packages from .nimble file
    • If $prj/nim.cfg exists
      • Get package versions from cfg file
    • For each package in .nimble requires
      • If package locked version exists
        • Install specific version of package if not already in ~/.nimble
      • Else
        • Install package using standard workflow today
        • Remember package version installed
        • Set create flag
    • If create flag set, create/update $prj/nim.cfg
  • nimble install - use updated processDeps() workflow
  • nimble install url|pkg - run same workflow as today
  • nimble uninstall pkg - remove package per usual
  • nimble build | docs | c | run
    • Use updated processDeps() workflow
    • Run Nim without --path: flags
  • nimble develop - use updated processDeps() workflow
    • Impacted by develop --all RFC - TBD

Nim required changes:

  • Replace instances of $nimbleDir in --path:$nimbleDir/xyz with path to OS specific Nimble directory
  • Once lock files are always on and the only way to use Nimble, as part of issue 654
    • Deprecate and eventually remove --NimblePath and --cleanNimblePath
    • Remove code that searches packages in NimblePath

No changes:

  • Dependencies will need to be added to the .nimble file in the requires section per usual to get installed and included in $prj/nim.cfg
    • Once nimble can update the .nimble file requires section automatically in the future during install/uninstall, the lock file can get auto-updated as well

Other issues addressed:

  • Project isolation - issue 131
    • Since lock files point to specific versions, changes due to other projects will not matter
    • If someone does remove a dependency, it can be reinstalled with one of the nimble commands that runs processDeps()
  • Reducing Nim's knowledge of Nimble - issue 654

Other issues expecting fix via lock files - not solved yet:

  • Nimble develop of http package - issue 543
    • Nimble does not currently save sufficient info about packages to know for sure
  • Skip dep checks - issue 589
    • Nimble will still run processDeps() - unclear if lock file will be faster

Notes:

  • Does not include strictMode in original issue proposal
  • URL is simply base URL to project and no commit hash - Nimble does not store this info today
@dom96
Copy link

dom96 commented Nov 6, 2019

You keep saying that these things are well known. Well, I've just went through #654 which is where we last discussed this and you've never responded to my points. So I don't think that is the case, the disadvantages in my eyes are still not at all well communicated by you.

@Araq
Copy link

Araq commented Nov 6, 2019

I did address them in shashlick's other gist.

@dom96
Copy link

dom96 commented Nov 6, 2019

It would really help if you could link to these comments. Even in that gist the only thing I can find is this: https://gist.github.com/genotrance/ee2ce321a56c95df2d4bb7ce4bd6b5ab#gistcomment-3074504, and you cop out by saying "but it's been discussed to death already". My goodness, you can't expect to never repeat yourself, or at the very least link to what you've written already.

@genotrance
Copy link
Author

I don't like having to store extra metadata that Nimble needs inside nim.cfg comments. - @dom96

I agree that it isn't ideal but we need nim.cfg anyway to communicate with Nim so we can just reuse the file. It is just a flat file after all. We could use JSON or YML or whatever in comments if its any better.

Or we could add -d:NimbleChecksum:abc, etc. and Nim doesn't do anything with it. This way, Nim doesn't need to learn any new lock file related stuff and Nimble can store all the metadata it needs.

if I add a dependency to my .nimble file I'll need to remember to run some Nimble command to update the nim.cfg - @dom96

I don't see why we think users should be able to use packages installed by a package manager but otherwise completely avoid the package manager. And once we add functionality to edit the .nimble file when (un)installing dependencies, no one needs to do that manually either. If they did manually edit, yes, you can do nimble install -d and no new command is needed.

Encourage end users to use nimble c, nimble check, etc. to compile programs - @dom96

Why do we need to completely change the user experience when running the same nimble command once will update the nim.cfg and the user can go on their merry way?

@dom96
Copy link

dom96 commented Nov 6, 2019

I don't see why we think users should be able to use packages installed by a package manager but otherwise completely avoid the package manager.

So you say this and yet you're against encouraging our users to use nimble c, nimble check, etc? Putting these paths into nim.cfg is all about avoiding the package manager... and is precisely what I argue against. So I don't understand why you're asking me this and arguing against encouraging people to avoid the package manager.

@genotrance
Copy link
Author

Putting these paths into nim.cfg is all about avoiding the package manager - @dom96

It is really about simplifying interop with Nim. This method informs Nim exactly where to find packages and the package manager is fully responsible for that experience so we are not avoiding the package manager. Neither are we forcing it to be the mandatory all the time. Nim also doesn't need any special code related to finding packages cause that can be facilitated by the package manager.

I'll summarize with the following points:

  • The nim.cfg proposal is simple - it has minimal impact on Nim, and even opens the possibility of reducing existing code around finding packages. Nimble already knows how to generate --path lists, all we need to do is put it in a file and we get a basic lock file for free. More rigor can be added over time.

  • We don't have the luxury of time or resources to get this done - the community needs a solution and a simpler solution is practical and achievable. Otherwise, we leave the community to thrash around the issues and limitations.

  • Our user base wants both workflows and we cannot force them into one way. There are strong opinions on both sides and concerns are legitimate but both sides have to compromise and proceed and help iron out corner cases and issues.

  • Nim is already 1.0 and we have to build on what we have and carefully deprecate legacy. We cannot simply toss out stuff nor make ambitious changes. There has to be a roadmap and we have to keep evolving and improving incrementally.

@Araq
Copy link

Araq commented Nov 7, 2019

In addition to that nimble c already doesn't work too well either:

  • Some options like -d:release are understood by nimble, others are now possible via --passNim. More complex to remember than a nimble prepare && nim c project.nim would be.
  • Nimble likes to hide the compiler's output and wraps it in its own style (three dots after indentation etc).

@dom96
Copy link

dom96 commented Nov 7, 2019

Some options like -d:release are understood by nimble, others are now possible via --passNim.

That's simply not true. Every flag you pass when running nimble c, nimble doc and nimble build is passed directly to Nimble, this is even described in the docs (!): https://github.com/nim-lang/nimble#nimble-c and here is the code to prove it as well: https://github.com/nim-lang/nimble/blob/master/src/nimblepkg/options.nim#L371

If you're going to make a claim like this can you please check that it's actually true? Otherwise arguing about it is at best a waste of time and at worst a malicious attempt at FUD.

Nimble likes to hide the compiler's output and wraps it in its own style (three dots after indentation etc).

Yes, it does. But why is this a problem? To be honest I don't care that much about it, if you can give me a good reason to change it I will be happy to.

@Araq
Copy link

Araq commented Nov 7, 2019

That's simply not true.

I'm sorry, I misremembered, it affects passing options to Nimble tasks.

If you're going to make a claim like this can you please check that it's actually true? Otherwise arguing about it is at best a waste of time and at worst a malicious attempt at FUD.

I usually do check, sorry I got confused.

Yes, it does. But why is this a problem?

There was a bug report about its buffering behavior but I'm not sure if it got fixed or not.

@dom96
Copy link

dom96 commented Nov 7, 2019

There was a bug report about its buffering behavior but I'm not sure if it got fixed or not.

I believe it did.

@SolitudeSF
Copy link

SolitudeSF commented Nov 8, 2019

But why is this a problem?

it silences all warnings. it silences all compiletime output like expandMacros or from static blocks.

and yes, the fact that it doesnt pass compiler output transparently, but buffers it and wraps it into own strange formatting is very annoying, since it also strips all colors.

why does it even do that? i would undestand if only nimble install silenced compiler, but nimble build is supposed to be just nim c with nimble's supervision.

@dom96
Copy link

dom96 commented Nov 8, 2019

It does this because that is how all of Nimble's output is formatted. It's something that never bothered me so I didn't improve it, but since it does bother others I'm happy to improve it (It should already be better AFAIK)

This being said, nimble build is meant for end-users so all the nim output shouldn't be shown by default for it.

@SolitudeSF
Copy link

nimble build is meant for end-users so all the nim output shouldn't be shown by default for it

i dont really understand what that means.

@Araq
Copy link

Araq commented Nov 9, 2019

Let's see how things would work with these proposals:

You add a dependency to the .nimble file. You run nim c which does not use --nimblePath anymore. The build fails, you remember to use nimble setup (or whatever command we decide on), nim.cfg is updated, nim c starts to work. Simple, effective, works for all Nim related tools. Both Nimble and Nim become simpler as a result.

@dom96
Copy link

dom96 commented Nov 9, 2019

That sounds like a far more limited version of these proposals. For example this proposal wants to change the behaviour of install.

I can agree to make nimble setup work, but I couldn't bring myself to recommend this workflow to anyone. Ignoring the fact that our IDE plugins run nim check and nimsuggest, why would I run nimble setup every time I add a dependency when I can just run nimble c from the get go?

@Araq
Copy link

Araq commented Nov 9, 2019

Why do you continue to assume that package dependency changes are common in anybody's workflow? You simply claim "It'll be a PITA" without any data to back up your claims...

@dom96
Copy link

dom96 commented Nov 10, 2019

I'm speaking entirely from my own experience. In fact, we both are, I don't see you referencing any data.

But I'll give you some hard facts:

  • There is at least one person in the Rust community that has asked for the behaviour that we have right now [1] (i.e. implicit --nimblePath for nim). I doubt this was as a result of seeing them in Nim.

  • Dependency changes happen often enough, in my experience, to make running nimble setup a PITA.

    • There are also questions of what to do with these paths, should they be checked into git? If so then these would just be poor man's lock files. If not, then we'll have lock files too and then you'll need to run nimble setup and nimble lock/nimble update every so often...
  • There isn't a single language that has this roundabout way of passing package paths to a compiler (as far as I know)

  • We already have nimble c and it works, the only problem you mentioned is that we won't support nim check and nimsuggest. But these can be supported easily, and that is a fact.

    • The only problem here is new Nim commands, but my assumption is that it is unlikely that Nim will gain new commands that Nimble will need to replicate. Based on historical changes I believe we can say with pretty good certainty that this is the case.

@Araq
Copy link

Araq commented Nov 10, 2019

My argument is not based on statistics but on overall simplicity and hence I don't need data, but you do. ;-)

Dependency changes happen often enough, in my experience, to make running nimble setup a PITA.

So count the average number of dependencies of a Nimble package. If it's less than N it cannot be "often enough".

There isn't a single language that has this roundabout way of passing package paths to a compiler (as far as I know)

I don't know what that means, every compiler has something resembling of a "search path". Python, clang, gcc all have it.

We already have nimble c and it works, the only problem you mentioned is that we won't support nim check and nimsuggest. But these can be supported easily, and that is a fact.

No it's a fact that for nimsuggest it would be messy.

@dom96
Copy link

dom96 commented Nov 10, 2019

My argument is not based on statistics but on overall simplicity and hence I don't need data, but you do. ;-)

You need to at least prove that your solution is simpler, which it really isn't. Look at this proposal, it's huge.

So count the average number of dependencies of a Nimble package. If it's less than N it cannot be "often enough".

That doesn't capture how often a dependency changed. When you're writing a new program or feature you might experiment with various dependencies, and you often don't want to have extra steps hindering you from doing this.

I don't know what that means, every compiler has something resembling of a "search path". Python, clang, gcc all have it.

It means no package manager asks you to run pkgman setup (which generates a file) so that you can run a compiler. There are build systems that do something similar (./configure followed by make, right?), and they aren't a pleasant experience.

No it's a fact that for nimsuggest it would be messy.

No, it's not. Can you explain why it would be messy?

@disruptek
Copy link

I'm not from the Rust community, but I don't want tools to have to run a VM just to parse the configuration used to setup the compiler's command-line. This is one of the main complaints I have against Nimble.

The advantage of having a trivial path specification is that I can make many more assumptions about crafting that specification. In fact, I just have my package manager manage it for me in the nim.cfg and it works perfectly for the compiler and all related tools. The interface should be as simple as it can be, and no simpler.

This is an opportunity for Nim and Nimble to streamline their interaction even while enabling more tooling to grow up around them.

@Araq
Copy link

Araq commented Nov 10, 2019

Look at this proposal, it's huge.

The proposal also deals with how to do lock files which we currently do not have. I'm not talking about lock files but shashlick's and my ideas would in fact allow for lock files, yes.

That doesn't capture how often a dependency changed.

Sure, so look at the git history of the average .nimble file.

No, it's not. Can you explain why it would be messy?

nimsuggest (and nim check etc etc) needs to call Nimble which calls into Nim for .nimble file evaluation. 2 points of potential failure, 2 interprocess communications, messy setup.

@genotrance
Copy link
Author

For example this proposal wants to change the behaviour of install. - @dom96
Look at this proposal, it's huge. - @dom96

We are proposing changes in dependency processing - the same change will propagate in all functions that use it. It doesn't mean the proposal is huge, it is just documenting how the tool behavior will change so that we can confirm it is correct and test for it.

I even noted that this is a simpler proposal and easy to implement and improve.

There is at least one person in the Rust community that has asked for the behavior that we have right now - @dom96

That anecdote is actually aligned with this proposal. People don't like using cargo for all things Rust. Nim already does that and we want to leverage it to implement lock files. Pushing nimble to be the only way is like pushing cargo to that guy.

Further the suggestion to remove Nimble specific code is simply based on issue 654 but isn't required at all. It surely can be discussed separately and even way in the future if at all.

We already have nimble c and it works - @dom96

No one is saying we shouldn't use nimble at all. It will continue to work like you propose 100%. In fact, adding lock file support will make nimble even more attractive. Further, you have to use nimble for package management and cfg file generation anyway. Why does implementing this proposal sound contrary to your goals?

Overall, we are not talking about this proposal at all if we are debating whether nimble should be the frontend at all. I'm talking about a simple incremental change that improves the status quo whereas you are talking about changing the entire workflow of a project that just went 1.0.

@dom96
Copy link

dom96 commented Nov 11, 2019

nimsuggest (and nim check etc etc) needs to call Nimble which calls into Nim for .nimble file evaluation. 2 points of potential failure, 2 interprocess communications, messy setup.

This isn't necessary, nimble check would work the same way as nimble c does currently (it would call nim and pass the correct --path flags)

We can do the same thing for nimsuggest.

That anecdote is actually aligned with this proposal. People don't like using cargo for all things Rust. Nim already does that and we want to leverage it to implement lock files. Pushing nimble to be the only way is like pushing cargo to that guy.

So first, I'm only pushing Nimble in this direction because Araq (and others?) have a problem with the current status quo (i.e. implicit --nimblePath in Nim). I'm perfectly happy with how things are.

As far as leveraging this to implement lock files is concerned, there is nothing about this proposal that enables a lock file implementation. All the metadata that will be used by Nimble to retrieve locked dependencies will be specified in the comments. The --path really isn't much use here. So I see absolutely no point in hackishly putting it into the nim.cfg file.

So it might be best to discuss this proposal on its own merits, without the promise that "it makes lock files possible" which just isn't true.

No one is saying we shouldn't use nimble at all. It will continue to work like you propose 100%.

This proposal adds another workflow to Nimble/Nim. I am not a fan of providing two different workflows which have the same use cases. I don't see the use in offering both and want to avoid the situation that we have now with using koch nimble vs. just compiling Nimble manually, two different ways to do the same thing, each with their own flaws. It's not good enough for a package manager that strives for simplicity.

I'm talking about a simple incremental change that improves the status quo whereas you are talking about changing the entire workflow of a project that just went 1.0.

I disagree with you here. This is as big of a workflow change as the proposal to encourage the use of nimble c exclusively. You are expecting people to run nimble setup before any nim execution(!)

@genotrance
Copy link
Author

This is as big of a workflow change - @dom96

It is step 1 in implementing lock files. It is turned off by default in nimble.ini and can be enhanced over time so as not to disrupt the community. Second, it is only relevant when you want lock files. If not, you still have the status quo. There is no workflow change if you don't want lock files. And if you want it, it is a new workflow anyway.

there is nothing about this proposal that enables a lock file implementation - @dom96

It charts a way to get there. Paths lock versions for you and that's all we have today. Nimble does not store enough data in its database to get a 100% solution in step 1.

So I see absolutely no point in hackishly putting it into the nim.cfg file. - @dom96

Agreed 100% it isn't pretty but a lock file does not have to be user facing. Why does the format matter if it functions correctly? It isn't ideal for sure but neither is adding more complexity into the tool for some ideal. The legacy is a .nimble and a nim.cfg so I'm trying to capitalize on that.

You are expecting people to run nimble setup before any nim execution(!) - @dom96

These people are showing willingness to using Nimble to setup their lock files before using Nim. Instead of recognizing that concession, you want them to change their workflows altogether. Why is nimble setup a bad idea when it can encourage more Nimble adoption?

This proposal adds another workflow to Nimble/Nim. I am not a fan of providing two different workflows which have the same use cases. - @dom96

Diversity is a fact of life. You can adapt it into Nimble or leave it for others to deal with. In either case, both will continue to exist.

@Araq
Copy link

Araq commented Nov 12, 2019

We can do the same thing for nimsuggest.

No, we can't. And we won't. Already Nimble doesn't wrap other binaries well. And already nimsugggest is a fragile piece of software.

@dom96
Copy link

dom96 commented Nov 12, 2019

Already Nimble doesn't wrap other binaries well.

@Araq Yes, it does. I've already proven you wrong on this, why do you keep repeating this as fact?

there is nothing about this proposal that enables a lock file implementation - @dom96

Nimble does not store enough data in its database to get a 100% solution in step 1.

@genotrance I don't believe this is true, what information does Nimble not store in its database to make lock files possible?

Agreed 100% it isn't pretty but a lock file does not have to be user facing. Why does the format matter if it functions correctly? It isn't ideal for sure but neither is adding more complexity into the tool for some ideal.

@genotrance because that is in fact messy, I mean come on, you want to put lock-file-specific information into comments? I'm frankly shocked that @Araq is completely fine with this.

But forget about this seriously. You are misunderstanding what I'm saying, my point is that lock files are completely orthogonal to this proposal, so saying that this proposal is good because it "enables lock files" makes no sense. Let's stop talking about how this proposal enables lock files and focus on just the --path aspect.

It is turned off by default in nimble.ini and can be enhanced over time so as not to disrupt the community.

nimble c is already there! and it doesn't need to be used. People can choose to use nim c or nimble c. Once again, I do not want to force people to use nimble c, but that is the solution if using nim c as it is today is such a horrible thing (which I completely disagree that it is)

@Araq
Copy link

Araq commented Nov 12, 2019

why do you keep repeating this as fact?

Because others chimed in and agreed with me. For instance, nimble build swallows the compiler's warnings.

I'm frankly shocked that @Araq is completely fine with this.

Actually I'm not, but I am not concerned with lock files.

@dom96
Copy link

dom96 commented Nov 12, 2019

Because others chimed in and agreed with me. For instance, nimble build swallows the compiler's warnings.

nimble build is a completely different topic. It doesn't replace any Nim commands, it's Nimble's own functionality.

I am still waiting to hear other's opinions about nimble c and why they dislike it. Can you point me to some comments from others about this? better yet, quote the passages here?

I'm frankly shocked that @Araq is completely fine with this.

Actually I'm not, but I am not concerned with lock files.

Whew. Thank goodness for that, this world hasn't turned upside down after all. This again goes to show that we shouldn't be conflating lock files with this proposal.

@genotrance
Copy link
Author

This proposal is off the table.

Despite being titled Nimble RFC for lock file support, the whole conversation here has gone on and on about Nim <=> Nimble interop.

The only real on-topic feedback I got was that you don't like the file format and this is not a full implementation, both of which are documented and explained multiple times. This was a simpler variation from the beginning, avoiding additional toml or json files, and leveraging existing infrastructure with minimal effort.

If this is just unbearable then there's nothing more to discuss.

@dom96
Copy link

dom96 commented Nov 12, 2019

Oh, in that case I must apologise. I didn't realise this proposal was primarily about lock files. :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment