Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: shard.yml crystal semantic #365

Closed
bcardiff opened this issue Apr 27, 2020 · 24 comments
Closed

RFC: shard.yml crystal semantic #365

bcardiff opened this issue Apr 27, 2020 · 24 comments

Comments

@bcardiff
Copy link
Member

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.

  • I've seen shards with no crystal version key
  • I've seen shards that do not change the crystal version in the shard.yml
  • I use it in crystal-db as the minimum required version

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:

  • shards resolver could directly discard old releases that will not work with the current crystal version used in the project
  • will push the community to keep an eye and maintain the shard more thoroughly once a crystal > upper_bound is released.
    • Some of the updates might by lifting the upper_bound if everthing is fine.
    • might delay the whole ecosystem to come up to date and be available to the latest crystal version.

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

  • shards will be able to avoid upgrading the shard beyond what is supported in the current crystal version used in that project. (this is important if the current crystal version of the project is not the latest crystal release).

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

waj commented Apr 28, 2020

@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
Copy link
Member

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
Copy link

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

Example:

  • Crystal has deprecated some syntax or an API.
  • exampleshard has versions 1.5x. (lts), 2.1.x (current version), 2.2.x (beta) available.
  • Any version could break.
  • The shard.yaml file contains no useful information helping tools or developers (and can't).

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
Copy link
Contributor

RX14 commented Apr 29, 2020

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

All of them, post 1.0.

@didactic-drunk
Copy link

Crystal 2.0 won't break anything?

@RX14
Copy link
Contributor

RX14 commented Apr 29, 2020

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

@didactic-drunk
Copy link

Let me try a different example:

  • Crystal 1.1 adds feature/class/syntax foo.
  • Shard Bar uses foo at x.y.1 but forgets to update the version String in shard.yml.
  • Many users attempt to download/update their shards some with indirect dependencies on Bar.
  • Shard Bar does what?
    1. x.y.1 can't be retagged because of git.
    1. It could release x.y.2 with an updated shard.yml file but users on crystal 1.0 will skip x.y.2 and I assume fall back to x.y.1 which fails. They should fall back to x.y.0 or earlier but because of the botched release locked to a git tag there's no way to indicate that.

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

@straight-shoota
Copy link
Member

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
Copy link

didactic-drunk commented May 1, 2020

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
Copy link
Member

The solution is testing against multiple versions on CI.

@waj
Copy link
Member

waj commented May 1, 2020

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

@straight-shoota
Copy link
Member

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
Copy link
Member

waj commented May 1, 2020

@didactic-drunk
Copy link

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
Copy link
Contributor

Sija commented May 12, 2020

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
Copy link

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
Copy link

  • Foo 1.x relies on Bar 1.x
  • Bar upgrades to 2.x.
  • Is Foo compatible with Bar 2.x?

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
Copy link
Contributor

Sija commented May 12, 2020

@didactic-drunk

  • that issue is about the semantics of the crystal version field, not an imaginary solution to a general problems in programming ecosystem (valid in many other languages too, and I haven't seen anything close to what you've been suggesting)
  • I haven't seen yet any proposal by yourself to be honest. From what I've read till now, my understanding is that you don't like the status quo, SemVer and reliance on conventions but I don't think I've seen anything like a formalised, technical description of a solution to that problem - and believe me I'd love to :) In any case I believe it's not a proper place to discuss it, so if you have some or want to brainstorm the subject I'd suggest to open a new issue and/or discussion on Crystal Forum instead.

@didactic-drunk
Copy link

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? 🤷. 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
Copy link
Member

@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
Copy link

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:

  • A new user starts a project at crystal v1.5.
  • Does he put 1.5 as the minimum version?
  • 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.

The actual value remains unknown.

Or:

  • A shard uses features available in 1.7-dev and dutifully bumps the minimum version.
  • Several users refactor the code over time unknowingly removing the 1.7-dev dependency.

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
Copy link
Member

waj commented May 18, 2020

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants