Closed sesam closed 6 years ago
A few things:
the ^ operator isn't meaningless, it gives cargo's dependency builder a range of options when trying to resolve dependencies. There's a good explanation of how this works here: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html
It's considered bad practice to specify hard dependencies, and a particular set of dependencies can be commit to for a particular release by checking in cargo.lock for a release. I was confused about this initially, but Cargo actually acts more as a 'dependency compiler', and for the most part it works fairly well. There a good doc here explaining the philosophy: https://blog.rust-lang.org/2016/05/05/cargo-pillars.html
We need to have a discussion at some point about how we handle versioning, so we can leave updates to the versioning until we have a strategy.
With respect to pulling out bits of functionality such as tables, etc as advocated here, https://github.com/mimblewimble/grin/issues/510#issuecomment-373588315, these things took some time and effort to put in get working properly, and ripping them out to make the dependency graph look nicer doesn't make sense to me.
Other than that, I don't see a particular need to obsess over the dependency graph or the cargo build files at the moment. There is the odd issue with a moving dependency here and there but that's expected and intentional at this stage of development to ensure we're keeping up with the latest developments on dependencies (which is why we're not checking in cargo.lock at the moment). There might be one or two things that could be cleaned up or a lib here and there that can be standardised, but it can be done on a case-by-case basis (or if there's a particular need), and it would take far less time to just PR them with a stated reason than write up a dissertation about them.
Hi. Sorry, didn't intend a long read.
The page you linked has this:
The string "0.1.12" is a semver version requirement. Since this string does not have any operators in it, it is interpreted the same way as if we had specified "^0.1.12", which is called a caret requirement.
It seems we're not on the same page :/.
I lifted this topic with good intentions of course, and it's long because I don't know if it's all obvious.
Would you prefer I close this issue and take it piece by piece as PRs?
Apologies, I misinterpreted the ^ issue. I'd still rather have them in as it's more explicit what the intent was.
I would vastly prefer concrete PRs in general.
Seems like as good time as any to point out the existence of Cargo.lock
. 😀
The way I'd personally like to think about this, coming mainly from experience using bundler in ruby which is -
a) a huge reason for why Cargo.lock
exists in the first place, and
b) arguably one of, if not the the best solution out there
Cargo.toml
should be as flexible as possible, specifying the minimum restrictions around dependencies we can get away with.
Cargo.lock
is where the actual version numbers get locked down. And we let cargo do this - it is significantly better at resolving transitive dependencies and conflict resolution than any of us are.
To say it another way - no hard dependencies in Cargo.toml
(unless absolutely necessary), these live in Cargo.lock
and are automatically managed by the tool.
Once we get Cargo.lock
checked into the repo (and I want to advocate again for doing this) - then every new dev checking the project out will build with _exactly_the same set of dependencies that we used in the last successful build.
And the workflow goes from "everyone arbitrarily running cargo update
and hoping for the best" to "explicitly running cargo update
to keep Cargo.lock
up to date".
Yes, definitely with @antiochp on this after coming to appreciate the approach cargo takes to dependency resolution (and having struggled with many other half-baked dependency management tools). It is one of the things that cargo/Rust is getting very right
What do you guys think about checking in cargo.lock
when we have a testnet2 "release"?
Also, I think we should change the authors to "Grin Developers".
:+1: on both suggestions. Feels a little weird right now wondering if/when to add my name to things...
Yes, agreed on both. I think I’ve mostly used Grin Developers, but there’s probably the odd Yeastplume here and there
Together with https://github.com/mimblewimble/grin/pull/865 all points are covered, so closing this now.
We have some variation :) in the Cargo.toml files:
I'd like to
A grin for your thoughs :-D