crystal-lang / shards

Dependency manager for the Crystal language
Other
763 stars 100 forks source link

Crystal property in shard.yml should work like any other version restriction #449

Open straight-shoota opened 3 years ago

straight-shoota commented 3 years ago

The semantics of the crystal property have changed in #395 and the result is largely fine.

But the implicit semantics of crystal: "x.y.z" being equivalent to crystal: "~> x.y, >= x.y.z" is really unexpected. A single value looks like a restriction to a specific version, not a version range. That's how it works for version restriction on all other depdencies (cf spec).

When I specify a concrete version, I don't expect other versions to match that. There should be no suprises about this. I don't see any real benefit for having x.y.z as a short-hand version for ~>x.y (which it is mostly equivalent to). If you need to specify patch-level compatibility, it's a little bit longer. But much less potential for confusion overall. And probably rarely necessary.

/cf https://github.com/crystal-lang/shards/issues/413#issuecomment-648459984

bcardiff commented 3 years ago

Since the crystal version should be a semver it is safe to use that semantic. ~> x.y, >= x.y.z reflects semver guarantees.

Are you proposing to use = x.y.z? That would be very restrictive.

If all shards would obey semver I would make dependencies work that way also actually, so they match.

straight-shoota commented 3 years ago

Are you proposing to use = x.y.z? That would be very restrictive.

Yes. That's how most dependency managers (including shards for git dependencies) usually interpret a single value: as one specific version. It's restrictive and I don't expect that to be used much (it really shouldn't).

When I see a version restriction crystal: x.y.z, I assume it means exactly crystal x.y.z. When that also matches crystal x.y.(z+1) that's unexpected. And I'd wonder what it actually means. The implicit semantics are not evident. Why can't we just make it explict? There's no real loss, you can just use ~> operator, but then it's clear to everyone what it means.

Sija commented 3 years ago

@straight-shoota Good point, yet IIRC this was done in order to keep BC with already existing shards.

straight-shoota commented 3 years ago

That's not the case. Before #395 the crystal property meant "The last known Crystal version that is capable to compile the Shard". And shards will need to update the restriction for 1.0 anyways.

bcardiff commented 3 years ago

The semantic was informal, not enforced. The value was initialized and usually updated when the lower bound needed to change. So in some way was >= x.y.z, but then to prune the search space in dependency resolution and to state more clear expectations the upper limit is also added.

Again, if we understand the version as semver using ~> x.y, >= x.y.z it's safe and make sense.

asterite commented 3 years ago

I think in any context, explicit is better than implicit. If I see "1.2.3" I expect that version. I can also say "= 1.2.3". If I want something more relaxed I'd use "~> 1.2.3".

I just tried specifying version "2.8.2" of "benchmark-ips" in Ruby for a project, it downgraded it from 2.8.3 to 2.8.2. That means it's doing an exact match.

straight-shoota commented 3 years ago

Again, if we understand the version as semver using ~> x.y, >= x.y.z it's safe and make sense.

I'm not debating that. The problem is that nobody expects a plain version restriction to mean this.

bcardiff commented 3 years ago

So, what is the alternative?

If we give crystal: x.y.z the semantic of crystal: =x.y.z then is a breaking-change. If we give = semantic only on non 0.x versions then the rule is not straight forward IMO. Not giving semantics is not an option.

I am fine if the default templates expands to crystal: ~> x.y, >= x.y.z or something more explicit, but that does not solve what would be a proposed semantic for crystal: x.y.z.

straight-shoota commented 3 years ago

If we give crystal: x.y.z the semantic of crystal: =x.y.z then is a breaking-change.

That's not really a problem. Every shard.yml is going to need an update for 1.0 because with 1.0 the currently omnipresent 0.x restriction breaks everything.

We can keep the current semantics for 0.x restriction for backward compatibility and after 1.0 this inconsistency won't stay relevant for long.

straight-shoota commented 3 years ago

On second thought (or rather, x-teenth thought): Maybe the current semantics aren't even that bad after 1.0 when you only specify major (or minor versions) 🤔

I still maintain that the specific details of how this works can be confusing and unexpected. But in practical sense, what's going to happen is probably that all shards eventually get crystal: 1.0 and it will work with any Crystal 1.x version. If a shard depends on a minor release, it gets crystal: 1.a and it will work with any Crytal 1.x version with x >= a.

bcardiff commented 3 years ago

That's not really a problem. Every shard.yml is going to need an update for 1.0

I think new versions of shards should work with previously released shards. That's why I don't think using = is a good breacking change, while the current implementation still play nice with the shards out in the wild.

I still maintain that the specific details of how this works can be confusing and unexpected. But in practical sense, ...

If users don't pay attention the current used crystal will be the minimal version required until the next mayor release.

Do you think that a change in the shard.yml template is enough to settle this discussion or you hold that the semantics of the crystal: 0.x and crystal: 1.x should be different?

straight-shoota commented 3 years ago

Sorry, I'm not sure right now.

I guess a troubling piece is still the story about progressing major Crystal versions. This is relevant right now but also in the future. We hopefully progress so eventually Crystal 2.0 comes around and we have the same trouble as currently with 1.0.0-pre nightlies. What to do now? The current syntax doesn't allow something like crystal: 1.0, 2.0. Not that I think that would be a particularly good solution. To express support for multiple major versions, you need a more complex version requirement like crystal: >= 1.0, < 3.0. So I'm wondering what benefit does a short-hand notation like crystal: 1.0 provide when it is only useful is your shard only supports a single Crystal version. It's unlikely to keep backwards compatibility with lots of older Crystal versions, but the long term support strategy is completely undetermined. Supporting at least two Crystal versions simultaneously is a very likely scenario because it's very useful around Crystal releases to allow for gradual updates. I wouldn't expect shard releases to be strictly tied to Crystal versions.

(--ignore-crystal-version isn't a useful option as detailled in https://github.com/crystal-lang/shards/issues/413#issuecomment-727633776)

m-o-e commented 2 years ago

As an idea for a migration path:

How about retaining the current behaviour when the specified version is < 2.0 but make shards raise a syntax error when it is greater?

My 3 cents: