Bad release? Sled 0.15.3 [has evolved into a discussion on versioning]

[passcod]

It doesn't build, and looks like 0.15.3 is only a version in the tyler_failpoint branch. Possibly revert?

[spacejam]

@passcod works for me. are you on a mac?

[spacejam]

please post specific breakage or I can't debug this.

[rubdos]

Works for me too:

test result: ok. 18 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

[passcod]

Yeah, on a Mac.

error[E0616]: field `read_only` of struct `pagecache::ConfigBuilder` is private
   --> /Users/passcod/.cargo/registry/src/github.com-1ecc6299db9ec823/sled-0.15.3/src/tree/mod.rs:259:12
    |
259 |         if self.config.read_only {
    |            ^^^^^^^^^^^^^^^^^^^^^
    |
    = note: a method `read_only` also exists, perhaps you wish to call it

error[E0616]: field `blink_fanout` of struct `pagecache::ConfigBuilder` is private
   --> /Users/passcod/.cargo/registry/src/github.com-1ecc6299db9ec823/sled-0.15.3/src/tree/mod.rs:392:34
    |
392 |             if node.should_split(self.config.blink_fanout) {
    |                                  ^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: a method `blink_fanout` also exists, perhaps you wish to call it

error: aborting due to 6 previous errors

error: Could not compile `sled`.

To learn more, run the command again with --verbose.

(other 4 errors are the same messages at other places)

[spacejam]

@passcod what version of rust are you using?

[spacejam]

as you can see, 0.15.3 contains public read_only field: https://docs.rs/pagecache/0.2.1/src/pagecache/config.rs.html#69

it builds fine and passes tests on osx on travis

I'm not sure how you're building this, but the signs point to user error. Try cargo update or building with a clean checkout.

[passcod]

cargo 0.24.0 (45043115c 2017-12-05)
rustc 1.23.0 (766bd11c8 2018-01-01)
macos High Sierra 10.13.3 on MacBookAir6,2

Ah. sled 0.15.3 specifies pagecache 0.2 but actually depends on 0.2.1?

[passcod]

Yeah, if I specify

sled = "=0.15.3"
pagecache = "=0.2.0"

it matches the version constraints (of sled) but fails to build.

[passcod]

Not terribly broken and easily fixable.

[spacejam]

not broken at all. specifying 0.2 will fetch the latest patch version, unless you're rebuilding the project with an existing Cargo.lock file without first issuing cargo update.

[passcod]

We're... interpreting version specifiers differently? I suppose. 0.2 says "I'm compatible with all 0.2.x, but fetch the latest on update/new".

[spacejam]

I think that's the contract post-1.0 with semver, but I can see how it might be beneficial in some cases to be more strict. Could you tell me how you were building sled? If you were pulling it in as a dependency and it failed to build because of this, it's something worth tightening up the process around for sure.

…

On Feb 11, 2018 04:39, "Félix Saparelli" ***@***.***> wrote: We're... interpreting version specifiers differently? I suppose. 0.2 says "I'm compatible with all 0.2.x, but fetch the latest on update/new". — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#236 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABb40A9YkBaJkYga-RaETdE8UH29_9Evks5tTmDigaJpZM4SAkGB> .

[passcod]

Raw semver, yeah, but not how cargo does it:

This compatibility convention is different from SemVer in the way it treats versions before 1.0.0. While SemVer says there is no compatibility before 1.0.0, Cargo considers 0.x.y to be compatible with 0.x.z, where y ≥ z and x > 0.

And from what I can reconstruct, I used cargo upgrade. I pulled sled 0.15.2 first a few days ago, then cargo-upgrade yesterday saw there was an update and put it up to 0.15.3, and then I just did a normal cargo build instead of running a cargo update.

[passcod]

I mean, the same thing would have happened if I'd manually updated the version in the Cargo.toml without doing a cargo update.

[jeehoonkang]

I also prefer specifying pagecache = "0.2.1" to specifying pagecache = "0.2.0", but Cargo developers said that specifying page = "0.2.0" will fetch the most recent patch-level update. Releated issue: crossbeam-rs/crossbeam-epoch#58 rust-lang/cargo#4910

[passcod]

That's because requirements default to caret. You can specify other modes, e.g. pagecache = "=0.2.0" would fetch only 0.2.0. Carets were defined first by npm, I think, they're not part of semver per se, more an extension.

[daboross]

The behavior of = "0.2.0" or = "0.2" meaning "largest 0.2.*" is something I rely on a lot in my programs too, though I don't know of a truly good alternative. Always updating the minor versions via something like cargo-upgrade is... viable, but prevents people from using an older version of a dependency if you're actually still compatible.

What would be ideal is including some command like cargo downgrade to test a Cargo.lock with all minimum versions Cargo.toml in CI. I don't think repositories should advance the minor versions of dependencies if there isn't a requirement in case the updated dependency breaks something unrelated. However, a systematic way to ensure the minor version stays strictly minimally accurate would be good.

[passcod]

There's a cargo issue for it! rust-lang/cargo#4100

[jeehoonkang]

@passcod Strictly speaking, caret as currently defined in Cargo document doesn't guarantee that you'll get the most recent version. It just allows to fetch more recent ones.

[passcod]

Yes, that is what I'm saying. Requirements, not guarantees. Hence this issue.

Reading back, my answer to you might have been based on a misreading of what you were saying, sorry if it didn't make sense because of that.

[daboross]

There's a cargo issue for it! rust-lang/cargo#4100

Until we do get this, I think = "0.2" is the most reasonable form. It allows for some failed compilations, but it otherwise has the greatest flexibility with the least required maintenance.

[passcod]

= "0.2.1" would be the exact same behaviour, but without this particular compile fail. Otherwise, maybe add a caveat in the readme saying e.g. "as this is pre-alpha, we don't guarantee semver, so always cargo update when updating any component"?

[daboross]

@passcod Err, yes. I mean, incrementing the minor versions to keep exactly what is required would be ideal.

In my opinion, though, it's a large maintenance burden to keep every dependency at the "right" version, and the advantage is not worth it. pagecache is one dependency, but sled also depends on crossbeam-epoch, bincode, serde and serde_derive. With each dependency there's a possibility that we're depending on features introduced in newer versions: my argument is that it's unreasonable to ensure we require the exact right version of every dependency without automated tooling.

[passcod]

Ah, I see what you mean. Maybe in the meantime do the inverse? Always cargo upgrade before publishing? You'd cut out people who require bound-lower versions of your deps, and it's one more step, but it's not manually checking everything.

However, for crates developed in the same repo, like sled and pagecache are here, is it too much to update the version reqs? Just for those crates, not other/external deps.

[daboross]

Personally I would prefer a slightly incorrect version specification over forcing updated versions of dependency-dependencies. I've had a dependency of a dependency of a dependency break on my system in the past, and being able to downgrade it was useful.

I would definitely support using cargo update --minimal when that exists, but before that why not just keep = "0.2"?

It isn't like I'm making any decisions here anyways, but I only see downsides to using cargo upgrade. It would let sled support using cargo upgrade in consumer crates, but without a clear advantage to that, it also disallows downgrading dependencies, and creates more janitorial work.

Is there a particular reason you've used cargo upgrade over cargo update?

If there is, then I'd be all for this! I've just not seen it yet, and having sled use cargo upgrade would mean less potential support for reverting dependencies-of-dependencies.

---

As for keeping sled/pagecache in sync: I don't have any strong opinions one way or the other. If @spacejam finds this necessary, it seems reasonable.

[passcod]

They're do different things that have orthogonal uses. cargo update changes the lock file within what Cargo.toml requires. cargo upgrade automates the process of checking for updated crates and bumping the version in the toml. I would have obtained the same result from a manual bump without cargo-upgrade. For minor versions it's not terribly useful, but I did it for the recent tokio release and sled got caught through that. It's not using one over the other.

[spacejam]

I'm reopening this because it seems like something that is generating some useful discussion, and I'm learning things from all of your comments! Thanks for going deeper on this issue!

It is not very much work for me to specify patch versions in sled for pagecache, and I think it's the correct thing to do when relying on new functionality, as it will prevent confusion like in the original issue here. And I can still be lazy when adding a patch to address an internal pagecache issue, because in the majority of use cases of building this as a dependency for something else, it will usually get picked up.

I'm going to keep this open for a few days in case there are more people who want to share their opinions, but for the time being I'm going to start specifying minimum patch numbers in sled for pagecache when relying on a new feature.

[spacejam]

Marking this as resolved, but if the strategy of always bumping patch versions when relying on new functionality (and maybe sometimes being lazy about it otherwise) is insufficient to handle a particular case for anyone, please feel free to open another issue!