crystal-lang / shards

Dependency manager for the Crystal language
Other
765 stars 102 forks source link

RFC: shard.yml crystal semantic #365

Closed bcardiff closed 4 years ago

bcardiff commented 4 years ago

Motivation

I think it is important to allow shards to work and deal with dependencies that will require some range of language/std-lib version to work with. Up until now we frameworks usually force to run on Crystal latest only for convenience as the ecosystem mature and gets bigger.

Current status

Currently SPEC says:

The last known Crystal version that is capable to compile the Shard (String). Purely informational.

Thoughts

I think we should enforce a different semantic on the crystal version mentioned in shard.yml.

For a given shard release, if we restrict that it will work with crystal <= upper_bound this means that:

For a given shard release, if we restrict that it will work with crystal >= lower_bound this means that:

Depending on the upper_bound chosen it might become a burden for the ecosystem to upgrade things on every minor crystal release.

Picking an upper_bound should balance between a guess on what could happen and committing to a maintenance for lifting.

Although shards packages are not required to adhere to semver, the compiler and std-lib could / should.

Proposal

Even if we don't implement the whole logic now in shards, it is good to state what is the semantic of the crystal key.

If we assume Crystal & std-lib will adhere to semver then we can push the burden of forcing the ecosystem to republish shards on mayor releases only.

So, I would like to propose that crystal: x.y.z be treated as ~> x.y, >= x.y.z (ie: >= x.y.z, < (x+1).0.0). When x >= 1 this is what semver should guarantee, right?

Implementation detail: bundle/molinillo seems to use the ruby version as another dependency. We can do the same and use some magic string like crystal or crystal-lang/crystal as a dependency name in the algorithm. Even if multiple expressions like ~> x.y, >= x.y.z are currently not implemented in shards, we can fix some semantics in crystal: x.y.z, different from the dependency section, for convenience.

straight-shoota commented 4 years ago

Again, some statistics collected at shardbox.org: 504 (59.86%) of registered shards have a crystal property in shard.yml of the latest release. Distribution of values is available at https://shardbox.org/stats or below

version | count ------------|------- | (none) | 336 0.34.0 | 60 0.34 | 2 0.33.0 | 30 0.32.1 | 22 0.32.0 | 13 0.32 | 1 0.31.1 | 38 0.31.0 | 17 0.30.1 | 27 0.30.0 | 16 0.29.0-dev | 1 0.29.0 | 18 0.28.0 | 12 0.27.2 | 12 0.27.0 | 24 0.27 | 4 0.26.1 | 18 0.26.0 | 18 0.25.1 | 7 0.25.0 | 24 0.24.2 | 16 0.24.1 | 22 0.24.0+58 | 1 0.23.1 | 38 0.23.0+1 | 1 0.23.0 | 9 0.22.0 | 13 0.21.1 | 11 0.21.0 | 2 0.20.5+70 | 1 0.20.5 | 7 0.20.4 | 4 0.20.1 | 8 0.20.0 | 4 0.19.4 | 3
straight-shoota commented 4 years ago

I agree that the crystal property is currently not very useful and we need to define proper semantics.

When your proposal suggests to treat the crystal value as a version restriction, it would probably be reasonable to allow specifying not only a specific version, but a restriction expression. The implicit ~> x.y, >= x.y.z restriction is probably good for most cases, but can't express everything. For example it is entirely possible that a specific shard release is compatible with multiple major versions of crystal.

Also, if we're going to treat it like a dependency, we could consider to remove the top-level property and instead have crystal be a special item in dependencies list. Maybe this would better express how it works.

name: foo
version: 0.1.0

dependencies:
  crystal:
    version: "~> 1.0"

Composer uses this mechanism to specify the PHP version.

waj commented 4 years ago

@straight-shoota yes, I think the idea is use the implicit expression, unless there is one more specific. This is just to make it compatible with existing shards.

I wouldn't make it part of the dependencies list though. I prefer to see the dependencies list empty for shards without dependencies. We could make it work both ways actually, but the semantic difference when a specific version is specified, I think could make it confusing.

straight-shoota commented 4 years ago

The semantic difference of a specific version might be confusing nonetheless. So maybe it would be better to use actual version restrictions instead?

didactic-drunk commented 4 years ago

When Crystal releases a new version how do i find out which current shards are compatible?

Example:

How does a developer find out each crystal version compatible for a new shard release? How does he determine the upper and lower bounds for the version string proposed above?

Failure examples:

  1. A new shard release on Crystal 0.34 using the Log API. The developer forgets to test on 0.33 and lower or forgets to update the version string. Result: The version string is incorrect.
  2. Same as above. This time he remembers and updates the version string. n Releases later on crystal 0.4x he removes the Log calls (for unrelated reasons) making the shard compatible with crystal <= 0.33. Since he never tested older versions the crystal compatibility string is wrong again.

This PR's approach involves too much manual labor/testing and is not future proof. Why is the information collected and updated manually in an error prone way?

Why isn't this automated?

RX14 commented 4 years ago

When Crystal releases a new version how do i find out which current shards are compatible?

All of them, post 1.0.

didactic-drunk commented 4 years ago

Crystal 2.0 won't break anything?

RX14 commented 4 years ago

Well you assume that all shards are imcompatible with 2.0 unless explicitly stated. This is rare, so it's fine.

didactic-drunk commented 4 years ago

Let me try a different example:

My point is this information would benefit from updates outside of a shard version release and should go somewhere else (somewhere modifyable).

straight-shoota commented 4 years ago

My point is this information would benefit from updates outside of a shard version release and should go somewhere else (somewhere modifyable).

I don't understand what that could possibly be. shard.yml is the only source of information that a shard provides about its dependencies. The same thing you describe already applies to regular dependencies. If you make a release with incorrect dependency restriction, it won't work. There's no way to avoid that. Maybe yanking the faulty release could help. Otherwise it's up to the users of the dependency to restrict the versions and exclude x.y.1

didactic-drunk commented 4 years ago

A crystal developer tends to have a single intrepreter/compiler version installed and not test against others. The version may be ahead/behind the current release depending on the developers environment. I think this is true of most languages.

What I propose is in improvement over the status quo where shards can provide up to date information on which crystal versions they run on and when it stops/starts again possibly automatically.

Put simply, if I start my project on crystal v1.0 and over time upgrade to crystal v1.5, how will I know what the minimum crystal version is unless I test against past versions? (I've never met a developer that does this). So the information provided may start off accurate but will mostly be a guess or outdated.

straight-shoota commented 4 years ago

The solution is testing against multiple versions on CI.

waj commented 4 years ago

Is this a problem in other platforms? Ruby has a similar setting in gemspec. How do they solve it?

straight-shoota commented 4 years ago

A version in the ruby setting in Gemfile is strict. If you specify ruby 2.7.0 it forces to use 2.7.0 and won't even run with 2.7.1. But you can omit the patch level in which case ruby 2.7 would match both 2.7.0 and 2.7.1. And it also accepts version restriction operators, just like gem dependencies. IMO that seems more reasonable than applying an automagical operator on a specific version value.

waj commented 4 years ago

No, I mean in the .gemspec file: https://guides.rubygems.org/specification-reference/#required_ruby_version=

didactic-drunk commented 4 years ago

The solution is testing against multiple versions on CI.

I agree, but shard.yml is inappropriate for storing what versions are compatible.

Otherwisehow do you test against future versions and update an unmodifiable git tag?

Sija commented 4 years ago

Allowing a SemVer expression (with explicit version string resolved as in Gemfile) sounds to me like the most reasonable approach.

Otherwisehow do you test against future versions and update an unmodifiable git tag?

@didactic-drunk you don't. that's why SemVer came to existence. If the developers adhere to the semver semantics, the consumer should be safe to state ~working version, with which he knows his stuff will work with. There's no way to be sure, since it's all convention-based game.

didactic-drunk commented 4 years ago

SemVer is guesswork. Testing is proof.

Am I the only one that runs in to SemVer violations regularly?

Do you check the SemVer of each shard before running shards update? Does anyone? Ok I could have locked the version but now I'm stuck with manual upgrades of each and every shard one at a time. Do average developers do that on most projects for most shards? No.

The solutions given are not what people actually do all the time. I'm arguing for provable process improvements rather than established norms.

The status quo sucks and is easily improved with automated testing updating compatibility forward and back including major version upgrades. You can't get that relying on SemVer.

didactic-drunk commented 4 years ago

SemVer doesn't tell me and there's no way to know without testing or possibly manual review.

My proposal has a way to accurately provide the cut off point for automatic shard dependency matching without locking versions. And provides a way to set version compatibility/incompatibility when SemVer is violated.

Sija commented 4 years ago

@didactic-drunk

didactic-drunk commented 4 years ago

I keep these posts short as I've been criticized for writing too much by a core team member.
I've also written about this before with a long technical proposal that I can't find.

My minimal imagination solution:

Every shard:

on.release do
  test each shard against every direct dependency major version.
  save results
end

on_dependency_release do
  test shard against specific dependency
  save results
end

Testing is incremental when new versions are published. Over time a comprehensive compatibility listing is available for shards to reference removing the need for most version locks in shard.yml.

Results are PASS/FAIL. Whether or where to save the command output can be decided later.

Should minor or patch versions be tested? :shrug:. Seems like an implementation detail that can change later.

Is it 100% complete? No. It's still more accurate and less work than manual SemVer setting or locking.

straight-shoota commented 4 years ago

@didactic-drunk The general idea sounds interesting, but maybe there might be better approaches to that. However, this is definitely a entirely different issue and not necessarily mutually exclusive with crystal: property in shard.yml. Maybe it could eventually lead to some simplifications there, but I imagine that's somewhere in the future. I don't see an immediate effect on this discussion here.

didactic-drunk commented 4 years ago

I see you want to provide something and my proposal is somewhat more work. However, whatever is placed in a shards crystal version string is likely to be incorrect for the vast majority of projects over time.

Consider:

The actual value remains unknown.

Or:

Without testing no one knows actual compatibility (except the core team of course).

With automated testing the string is automatic and tested with all forward + backward versions available until some condition is met. Say 2-3 failures in a row. At that point it stops searching assuming it found the oldest/newest compatible version. Or bisect. The implementation isn't important.

I don't think what I'm proposing is terribly hard. Just use a CI build matrix and aggregate the result to a compatibity data structure or string.

waj commented 4 years ago

@didactic-drunk the same arguments could be applied to any dependency added to the shard. Normally anyone would use the latest version available (if possible) and use ~> operator to match any new release of the same library that should be compatible by semver rules. If that rule is broken, hopefully someone else will open an issue or PR, fix the problem and go on.

Your proposal to automate the detection of compatible versions might sound interesting. But that could be developed independently of this dependency manager and provide values for the shard.yml automatically.

What if it works on 1.4 and 1.3? He/she has no intention of installing older versions of crystal just to find the oldest version.

That's a fair point. Actually, it doesn't truly matters if a library works with a previous version of Crystal. What matters is if the author is willing to support those older versions. Maybe the support is unintentional and he/she doesn't want to refrain using new features just to keep compatibility.

So, again, the automated proposal sounds interesting, but it won't be implemented as part of this dependency manager. I'll proceed with the implementation of the semantics here described and tools like that could be used in the future to provide more accurate description of supported Crystal versions when desired.