rhaiscript / rhai

Rhai - An embedded scripting language for Rust.
https://crates.io/crates/rhai
Apache License 2.0
3.77k stars 178 forks source link

Can't use older Rhai versions anymore #830

Closed therealprof closed 8 months ago

therealprof commented 8 months ago

Related to #828 (the proposed solution to which I wanted to benchmark), there's another issue with the breaking change (which is automatically used unless pinned due to semantic versioning of minor versions):

Even if I pin the rhai crate version, Rust will automatically pick the latest rhai_codegen version which is not compatible with older versions:

rhai v1.16.3
├── ahash v0.8.7
│   ├── cfg-if v1.0.0
│   ├── const-random v0.1.17
│   │   └── const-random-macro v0.1.16 (proc-macro)
│   │       ├── getrandom v0.2.12
│   │       │   ├── cfg-if v1.0.0
│   │       │   └── libc v0.2.153
│   │       ├── once_cell v1.19.0
│   │       └── tiny-keccak v2.0.2
│   │           └── crunchy v0.2.2
│   ├── getrandom v0.2.12
│   │   ├── cfg-if v1.0.0
│   │   └── libc v0.2.153
│   ├── once_cell v1.19.0
│   └── zerocopy v0.7.32
│   [build-dependencies]
│   └── version_check v0.9.4
├── bitflags v2.4.2
├── num-traits v0.2.17
│   [build-dependencies]
│   └── autocfg v1.1.0
├── once_cell v1.19.0
├── rhai_codegen v1.17.0 (proc-macro)
│   ├── proc-macro2 v1.0.78
│   │   └── unicode-ident v1.0.12
│   ├── quote v1.0.35
│   │   └── proc-macro2 v1.0.78 (*)
│   └── syn v2.0.48
│       ├── proc-macro2 v1.0.78 (*)
│       ├── quote v1.0.35 (*)
│       └── unicode-ident v1.0.12
├── serde v1.0.196
│   └── serde_derive v1.0.196 (proc-macro)
│       ├── proc-macro2 v1.0.78 (*)
│       ├── quote v1.0.35 (*)
│       └── syn v2.0.48 (*)
├── smallvec v1.13.1
│   └── serde v1.0.196 (*)
└── smartstring v1.0.1
    ├── serde v1.0.196 (*)
    └── static_assertions v1.1.0
    [build-dependencies]
    ├── autocfg v1.1.0
    └── version_check v0.9.4

Pinning an older version of rhai_codegen doesn't work either:

# cargo tree -v -p rhai
    Updating crates.io index
error: failed to select a version for the requirement `rhai_codegen = "~1.16.0"`
candidate versions found which didn't match: 1.17.0, 1.6.0, 1.5.0, ...
location searched: crates.io index
required by package `foo v1.0.0 (/Users/egger/Desktop/Source/foo)`
perhaps a crate was updated and forgotten to be re-vendored?
schungx commented 8 months ago

Just pin rhai_codegen also. Those two crates go in pairs.

therealprof commented 8 months ago

Okay, pinning is possible, if one knows that up to rhai 1.16.0, rhai_codegen 1.6.0 needs to be pinned.

schungx commented 8 months ago

Version 1.16 uses rhai_codegen version 1.6.0

schungx commented 8 months ago

Sorry, I bumped rhai_codegen version numbers to be in sync with rhai starting this version.

You can look at https://crates.io/crates/rhai/1.16.3/dependencies to see that 1.16.3 requires at least 1.5.0 of rhai_codegen.

therealprof commented 8 months ago

Just pin rhai_codegen also. Those two crates go in pairs.

Yeah, one needs to know that rhai_codegen 1.6.0 needs to be used, which is not obvious or great. Ideally rhai would depend on a fixed version of rhai_codegen and not run into the same semver foul as rhai itself.

schungx commented 8 months ago

Yeah, one needs to know that rhai_codegen 1.6.0 needs to be used, which is not obvious or great. Ideally rhai would depend on a fixed version of rhai_codegen and not run into the same semver foul as rhai itself.

True... but that would remove the capability of putting out compatible patch versions of rhai_codegen.

jkelleyrtp commented 8 months ago

Hi, we use cargo-generate in our dependency tree for the dioxus-cli crate and rhai being a dependency means our CLI no longer compiles.

https://github.com/DioxusLabs/dioxus/issues/1884

I hate to be that guy... but maybe put up a 1.17.1 with the compatibility fixes and yank 1.17.0?

therealprof commented 8 months ago

Yeah, one needs to know that rhai_codegen 1.6.0 needs to be used, which is not obvious or great. Ideally rhai would depend on a fixed version of rhai_codegen and not run into the same semver foul as rhai itself.

True... but that would remove the capability of putting out compatible patch versions of rhai_codegen.

I'm reading https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#tilde-requirements differently.

If you specify a dependy version of "~1.17.0" for rhai_codegen, then you could still release a rhai_codegen 1.17.1, 1.17.2, ... and it would be used.

schungx commented 8 months ago

I'm reading https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#tilde-requirements differently.

If you specify a dependy version of "~1.17.0" for rhai_codegen, then you could still release a rhai_codegen 1.17.1, 1.17.2, ... and it would be used.

True, but that prevents me from putting out a new backwards-compatible version with new features (that are just not used in the older version). Those versions will necessarily bump the minor number.

Say I have 1.5.0. I put out 1.6.0 with new features (that older Rhai versions do not use) and improvements. Technically speaking older Rhai versions are free to use this 1.6.0 as it is backwards compatible.

Backwards compatible versions no not always need to be on the patch number...

Patch versions say that I haven't added anything new, even if those new features are backwards compatible.

Gisleburt commented 8 months ago

Observationally (and sorry if this isn't helpful to add to the conversation), from a semver point of view:

Aside, I wish instead of Major.Minor.Patch they'd used Breaking.Feature.Fix, partly because its easier to understand but mostly cos then its BFF 😊 ❤️

schungx commented 8 months ago

True... semver is a simple concept but quite difficult to stick to completely...

Usually there are times when you forget (or make a breaking change without knowing).

In this case, it is unfortunate that I cannot specify a minimum rhai version to work with that rhai_codegen -- that would create a dependency cycle. Otherwise I can simply lock that version to a minimum rhai version and be done with it.

schungx commented 8 months ago

But, come to think of it more clearly, you probably would be right. rhai_codegen should be bumped to the next major version if it is non-backwards compatible. That way, things will "just work" for people, but that also means that it is difficult to modify a certain part of it in non-compatible ways and still stay within a minor release.

therealprof commented 8 months ago

Since rhai_codegen is an internal package really, I wouldn't at all be concerned with minor complications in the choice of a different versions. The important bit here is not to cause surprises for people using rhai. I would suspect is rather unusual for people to even declare a specific rhai_codegen dependency, but it is very typical to select a rhai version and somewhat common for that version to be pinned at least via Cargo.lock, so that is that case that should work by default.

schungx commented 8 months ago

True. I'll have to give it some thoughts to come to the best arrangement.

jkelleyrtp commented 8 months ago

I just want to chime in that breaking semver breaks all downstream consumers of rhai.

https://github.com/cargo-generate/cargo-generate/issues/1118 https://github.com/DioxusLabs/dioxus/issues/1884

Nether cargo-generate nor dioxus should have to yank released versions to set seemingly unrelated associated dependencies. Being on 1.x.x of rhai means that cargo is free to pick whatever version it wants provided that the version it's picking is newer than what's specified in cargo.

Since rhai is on 1.x.x all new features must be additive. If you want to make breaking changes while on a 1.x.x those breaking changes need to be made on a 2.x.x. That's just the tough reality of being on 1.x.x and having consumers of the published crate.

schungx commented 8 months ago

I plan to yank rhai_codegen 1.17.0 and put out rhai_codegen 2.0.0.

Rhai 1.17.0 should be bumped to 1.17.1 which locks to rhai_codegen 2.0.0.

schungx commented 8 months ago

rhai_codegen is bumped to 2.0.0 while rhai is updated to 1.17.1.

Thanks everybody for flushing out this issue.