Due to the default of `crystal: <1.0.0`, can't install shards for Crystal nightlies

oprypin commented 4 years ago

In the wild:

https://github.com/oprypin/crystal-chipmunk/runs/796810944?check_suite_focus=true#step:16:12

My shard became not installable as a dependency ever since crystal-1.0.0-dev.

I think that (despite @jhass hinting that this might be intentional), from the point of view of any user, this looks like a bug in Shards. Some things come to mind:

Is it that >= 0.25.1 and < 1.0.0 are conflicting?
Wait, there's never any < 1.0.0 specified..?
Why did it stop working?

Repro:

$ cat shard.yml

name: main
version: 0.1.0

dependencies:
  sub:
    path: sub

$ cat sub/shard.yml

name: sub
version: 0.1.0

$ PATH=~/repos/crystal/.build:/usr/bin shards install

Resolving dependencies
Unable to satisfy the following requirements:

- `crystal (< 1.0.0)` required by `sub 0.1.0`
Failed to resolve dependencies

$ shards --version

Shards 0.11.1 (2020-06-21)

$ ~/repos/crystal/.build/crystal env

CRYSTAL_CACHE_DIR=/tmp/cache-blaxpirit/crystal
CRYSTAL_PATH=lib
CRYSTAL_VERSION=1.0.0-dev
CRYSTAL_LIBRARY_PATH=''
CRYSTAL_OPTS=''

bcardiff commented 4 years ago

If there is no crystal property the semantic is crystal: < 1.0.0

shards install --ignore-crystal-version can be used as a workaround until a more fine grained alternative of forcing the resolution is added.

Maybe shards should also read a SHARDS_OPTS env variable so this can be widely configured 🤔

oprypin commented 4 years ago

If there is no crystal property the semantic is crystal: < 1.0.0

Yea I get it, but nobody asked for that to be that way??

bcardiff commented 4 years ago

It is a choice that make things work similar to what will happen from 1.0 to 2.0 eventually. Major releases will require some work from the ecosystem to migrate, or for the user to use some workaround.

It also mimic what would happen if the property was mandatory since the beginning + the semantics choosen for crystal: x.y.z

oprypin commented 4 years ago

No I don't understand why you want to break almost all shards; again, who asked for that?

oprypin commented 4 years ago

Oh yeah and the full fallout from most things breaking globally will exactly coincide with 1.0.0 release, just why do it?

straight-shoota commented 4 years ago

This change was introduced in #395 and there's a lengthy discussion about this behaviour: what the default should be when no crystal property is present specifically and more importantly, which effect the crystal property has. The core team was devided on this topic. We still went trough with it and agreed that it can be reverted if it goes south and everyone complains about this (that part was discussed off Github in a private core team meeting).

While it may be a good idea in general (not sure if that's true), I'm still expecting that introducing this hard breaking change on short notice in 1.0 is going to cause a huge mess for the ecosystem.

oprypin commented 4 years ago

I don't know if it's related, but even after specifying crystal attribute I still get a failure. Can't reproduce locally. Looks really weird.

https://github.com/oprypin/crystal-chipmunk/runs/800778520#step:16:10

Unable to satisfy the following requirements:

- `crystal (~> 0.24, >= 0.24.1)` required by `spec_assert 0.1.0+git.commit.29f719936bc600b98b665cf91be7f6583a20be1f`
- `crystal (~> 0.25, >= 0.25.1)` required by `crsfml 2.5.1`
- `crystal (~> 0.30, >= 0.30.1)` required by `chipmunk 0.1.0`

bcardiff commented 4 years ago

Either append --ignore-crystal-version in https://github.com/oprypin/crystal-chipmunk/commit/899e676b5d3f4b8aa17be6cb6a6dd51848d88414#diff-e9f950f17198d3d5e3122a44230a09b9R100

Or update dependencies to a crystal: 1.0.0 like https://github.com/oprypin/crsfml/blob/master/shard.yml#L15 and others.

The dependencies are the ones that are incompatible with crystal 1.0.0-dev

oprypin commented 4 years ago

Excuse me? Should I take this to understand that crystal 1.0.0 is somehow not >= 0.30.1 and that's intended behavior?

bcardiff commented 4 years ago

The issue is that

crystal: 0.30.1 is ~> 0.30, >= 0.30.1.
~> 0.30 is >= 0.30, < 1.0. Same as in bundler. Ref: https://thoughtbot.com/blog/rubys-pessimistic-operator

That is where the < 1.0 is coming from.

When just a version number is used it has a different semantic compared to dependencies: x.y.z will be interpreted as ~> x.y, >= x.y.z (ie: >= x.y.z, < (x+1).0.0) honoring semver. Ref: SPEC.md

oprypin commented 4 years ago

OK, now do you want to explain all of these fine points to literally every user on the day of the 1.0.0 release? When there are like 5 layers of "surely this is a bug"?

oprypin commented 4 years ago

Now I realize that both of the cases, which together surely cover 99% of existing shards:

crystal: 0.*
not specified

imply crystal < 1.0.0.

So is the idea that every single shard author is expected to add crystal: 1.0.0 exactly on the day of its release and drop support for all other versions of Crystal at the same time? How is this supposed to work at all?

Now, I also realize that it's possible for me to replace all my crystal: 0.24.1 etc with crystal: '>= 0.24.1' to get rid of this problem, but hmmmmm

asterite commented 4 years ago

I think the idea is that there will be a pre-release. Then all shards authors will be encouraged to release a shard version compatible with 1.0.0. Then when 1.0.0 releases only maintained shards are usable.

waj commented 4 years ago

@oprypin could you first please consider change your tone and attitude? We're human beings, with feelings, emotions and mistakes. We did what it's done for a reason and with a goal in mind. The fact that you have different viewpoints doesn't necessarily makes ours wrong or less important than yours. I could try to re-explain the rationale but it's really hard and discouraging if you just try to impose with messages full of irony or rage.

straight-shoota commented 4 years ago

IMO a major implementation issue is the implicit meaning of crystal: x.y.z. It's far from obvious that it's equivalent to crystal: ~> x.y, >= x.y.z. It's against expectations. The established semantics of version: x.y.z in dependencies refer to a single version, not a range.

Following that, as soon as you notice that crystal: x.y.z also matches x.y.z+1 the first assumption would be that it means crystal: >= x.y.z.

oprypin commented 4 years ago

@waj

I could try to re-explain the rationale but it's really hard and discouraging if you just try to impose with messages full of irony or rage

Well no, I could also try to explain my numerous thoughts how this is not the right direction to take, but there are so many big and obvious points that I thought it was not necessary. At some point I'll need to do that, though.

I suppose the moment of realizing that this is not a bug but intentional really did trigger rage; not necessarily due to the fact itself, just that it's an instant switch in thinking. What followed directly was disbelief, rather than a rational re-evaluation.

I don't want to let a self-imposed and easily predicted catastrophe to happen and honestly the prospect of having to live through the fallout from that is just scary on some level. I literally imagined how I'd feel participating in chat on that day, trying to contain antagonistic demeanor. So yes, the feelings really are hard for me to contain at this time as well.

bcardiff commented 4 years ago

The discussion on why restricting the compatible language version beforehand can be followed at https://github.com/crystal-lang/shards/issues/365.

I think that the same policy should apply from 0.x to 1.x and from 1.x to 2.x. When shards are built with crystal: 1.2, that version will not be a candidate for crystal 2.x. The fact that the language is not going to change that much between 0.x and 1.x is exceptional. It might do between 1.x and 2.x.

This allows pruning the space search and more importantly allows the language to keep evolving. Without this restriction it will be harder to make decisions of breacking-changes on the language itself.

Nothing prevents you to use crystal: * explicitly to say that that shard version is available for every crystal version, same as with dependencies. But it's not a practice I encourage. There are no guarantees today to support that claim.

As mentioned in https://github.com/crystal-lang/shards/issues/413#issuecomment-647768257 you can use --ignore-crystal-version when installing shards to turn off this check during transition periods. In #417 I propose that that can come from an env var since that might be more comportable in CIs and build scripts.

jhass commented 4 years ago

Can we mention --ignore-crystal-version in the error message, alongside a little text blob explaining that it's worth a shot if you just installed Crystal 1.0, or something along the lines of that?

bcardiff commented 4 years ago

Yes. #418 :-)

straight-shoota commented 4 years ago

Even shard's nightlies are broken, btw.: https://app.circleci.com/pipelines/github/crystal-lang/shards/347/workflows/5c706c91-30e7-473b-8675-6d94428e9618/jobs/1043/steps

straight-shoota commented 4 years ago

It's a common practice to have CI builds against latest and nightly Crystal releases. Currently, lot's of nightlies are failing because shards refuses to install dependencies unless they're explicitly enabled with crystal: 1.0.0.

This problem shows a real dilemma. How's this supposed to work when latest and nightly Crystal releases have different major versions?

A trivial solution would be to use --ignore-crystal-version for the nightly build. But that's not good because it's a different behaviour than in normal use cases. I want nightlies to work the same way as a normal user would and it should help identify problems with installing shards.

Alternatively, all dependencies could be changed to crystal: 1.0.0 (or rather something like crystal: ~> 0.35.0, ~>1.0.0 to work with Crystal latest). But that might be a lie. Or maybe a declaration of intent. Until the next major version is released, there's no way to tell whether a shard is actually compatible with that release.

An modified approach would be using crystal: 1.0.0-dev for dependencies (that version restriction by itself doesn't work btw., only crystal: '~> 1.0.0-dev'). That's more clearly expression than crystal: 1.0.0. But it's still not really accurate because we don't know whether it's actually going to be compatible with 1.0.0. So we could make the version even stricter crystal: '~> 1.0.0-dev, < 1.0.0'.

That might be the proper definition. But it's also rather complex. And it requires two changes to the shard dependencies: When Crystal nightly becomes a development version of a new major version and when that major version is finally released. Considering that even one such change throughout the ecosystem is going to be incredibly hard, I can't imagine such an approach to be feasible.

jwoertink commented 4 years ago

Hey, just wanted to chime in real quick that I'm seeing the same deal when trying to upgrade the Lucky website to our latest version https://github.com/luckyframework/website/pull/341. My build fails due to Crystal version conflicts.

Unable to satisfy the following requirements:
  - `crystal (~> 0.35, >= 0.35.0)` required by `lucky 0.23.0`
  - `crystal (~> 0.35, >= 0.35.0)` required by `authentic 0.6.1`
  - `crystal (~> 0.35, >= 0.35.0)` required by `carbon 0.1.2`
  - `crystal (~> 0.35, >= 0.35.0)` required by `habitat 0.4.4`
  - `crystal (~> 0.35, >= 0.35.0)` required by `dexter 0.3.1`
  - `crystal (~> 0.33, >= 0.33.0)` required by `exception_page 0.1.4`
  - `crystal (~> 0.33, >= 0.33.0)` required by `cry 0.4.2`
  - `crystal (~> 0.22, >= 0.22.0)` required by `shell-table 0.9.2+git.commit.078a04ea58ead5203bb435a3b5fff448ddabaeea`
  - `crystal (~> 0.35, >= 0.35.0)` required by `lucky_router 0.3.0`
  - `crystal (~> 0.35, >= 0.35.0)` required by `avram 0.16.0`
  - `crystal (~> 0.26, >= 0.26.1)` required by `wordsmith 0.2.0`
  - `crystal (~> 0.35, >= 0.35.0)` required by `lucky_cli 0.23.0`
  - `crystal (~> 0.35, >= 0.35.0)` required by `lucky_flow 0.7.0`
  - `crystal (~> 0.34, >= 0.34.0)` required by `cadmium_transliterator 0.1.0+git.commit.6ee68d64869ade8f39d2ec16cae3c7b80d5a4805`
  - `crystal (~> 0.35, >= 0.35.0)` required by `markd 0.2.1+git.commit.620845abb6776d1765d20d86cc0220aa40f89a91`
  - `crystal (< 1.0.0)` required by `dotenv 0.7.0`
  - `crystal (< 1.0.0)` required by `blank 0.1.0`
  - `crystal (< 1.0.0)` required by `pg 0.21.1`
  - `crystal (~> 0.35, >= 0.35.0)` required by `teeplate 0.8.2`
  - `crystal (~> 0.35, >= 0.35.0)` required by `selenium 0.6.0`
  - `crystal (~> 0.35, >= 0.35.0)` required by `webdrivers 0.3.0`
  - `crystal (~> 0.25, >= 0.25.0)` required by `db 0.9.0`
  - `crystal (~> 0.34, >= 0.34.0)` required by `future 0.1.0`
  - `crystal (~> 0.35, >= 0.35.1)` required by `crystar 0.1.7`
  Failed to resolve dependencies

jwoertink commented 4 years ago

For anyone else that might land here, I updated to use that shards install --ignore-crystal-version, and that fixed it for me.

asterite commented 4 years ago

It would be nice if the error message would tell you why crystal (~> 0.35, >= 0.35.0) didn't match (what's the crystal version being used?).

straight-shoota commented 4 years ago

That looks really strange. It seems this action runs in a crystallang/crystal:0.35.0 container, so all the crystal (~> 0.35, >= 0.35.0) and crystal (<1.0.0) requirements should actually match. All other requirements are broken though with the current behaviour of shards. Their crystal property is based on outdated semantics.

@jwoertink Could you add a crystal version somewhere before the shards install runs? Just to be sure.

oprypin commented 4 years ago

FYI for that particular run: It uses Crystal 0.35.0 but one dependency has >= 0.35.1

jwoertink commented 4 years ago

I think there may have been a few other things going on here. I hacked around it so I could close out the issue, but in that action, I had 2 containers. The first container didn't specify a version where the second one did. Then on top of that, I was caching the shards install

I can probably try and push another "test" PR from that version to recreate it with that crystal version if that would help @straight-shoota

paulcsmith commented 4 years ago

At first I thought this would be helpful, but now I'm thinking it may cause some issues with launch and unnecessary burden/time crunch on shard authors.

I have a proposal for how we might get the best of both worlds. In Crystal 1.0 show a warning for this and say it will be an error by default in Crystal 1.1. This gives shard authors time to update and doesn't result in errors on day 1 for shards that may need a bit of time to be updated and released (maybe author is on vacation or doesn't know).

For Crystal 1.0 show roughly what is there now, but as a warning instead of an error. Say it is a warning and that in Crystal 1.1 it will be an error. And they can use --fail-on-crystal-version-mismatch to prep for Crystal 1.1
--ignore-crystal-version is still there and will not show the warning
Add an option for --fail-on-crystal-version-mismatch. People can use this on 1.0 to prepare for 1.1
In Crystal 1.1 show an error instead of warning. Show warning if --fail-on-crystal-version-mismatch is used that tells the user this is now the default behavior.

I think this solves the problem fairly well. It warns users that shards they are using might not work on 1.0, but allows them to give it a shot. It sets the path for 1.1 to use the more strict behavior. And it gives some time for authors to test on 1.0 and update their shard.yml

Thoughts?

Sija commented 4 years ago

@paulcsmith https://github.com/crystal-lang/shards/pull/418#discussion_r446421572, so yeah, warnings would be way easier to manage for every1. Also, I can't imagine 1.0 without being able to override shards.

bcardiff commented 4 years ago

The issue appears while we are on a transition period of major release. While developing 1.0.0-dev, how does the ecosystem prepare for it if it's not a closed release? We don't want packages to be forced to do releases while 1.0.0-dev is not closed. On regular dependencies we've been using branch requirements to stay up to date with upstream changes.

If we want to use the dependency as is, using --ignore-crystal-version is enough. On the next shards release (that will be bundled at some point in 1.0.0-dev), #417 will be included at might be even easier to include them in the CI for nightlies.

As soon as there is a breaking-change in 1.0.0-dev that affect a dependency you will need to update the version restriction to branch.

Having some logic on shards to accept packages with crystal: 0.x on 1.0.0-dev/1.0.0 will offer more confusion IMO. I think using --ignore-crystal-version (or setting CRYSTAL_VERSION) is good enough as a temporal solution while 1.0.0 si being developed.

Being able to override shards as in #412 will also offer a more fine grained alternative that will allow developers to pick branches for shards that are getting updates for next release.

Once 1.0.0 is released, the ecosystem should catch up, at worst pushing new mainteiners for abandoned shards.

straight-shoota commented 3 years ago

In my opinion --ignore-crystal-version is not very useful as a workaround. I run nightly tests to see that my shards are compatible with upcoming Crystal (and shards) releases. It needs to be as close as possible to the stable environment, just based on a more recent Crystal version. I don't expect users to install their shards with --ignore-crystal-version, so to me it makes no sense to use it for nightly builds. I want to know if incompatible versions fail installation of dependencies before it becomes a problem.

It seems that some people currently tend to simply resort to --ignore-crystal-version to fix their dependency resolution. That may be out of annoyance or because it's recommended. However, I don't understand why this is encouraged. What's the point in having crystal version enforcement when everyone disables it?

It may be acceptable as a temporary solution in the approach of 1.0. But I can't see why we wouldn't have the same problem when we approach 2.0 and later major releases? I honestly don't expect we'd want such a massively annoying situation again. And, again, it doesn't really help much because running --ignore-crystal-version for nightlies doesn't tell you if installing without that flag would succeed.

I have come to the conclusion to treat crystal version restriction mainly as a lower boundary of support level. I know my shard works at this version and I intend to support it (and at least its patch releases). So currently, this is what I assign:

crystal: '>= 0.35.0'

I don't place an upper bound because:

I'm currently pretty confident about compatibility with 1.0.
I don't know anything about compatibility with releases even more in the future. So I'll rather leave it open for the benefit of the doubt. Maybe adding < 2.0 would be sensible extension, but I'm not sure about it.

The lower bound intended to express the intended support level. It doesn't necessarily mean it doesn't work below that, but it's just not tested and may fail. That would be a use case for --ignore-crystal-version if I want to get a shard working with an older crystal version (because I'm somehow stuck with that). Then you can justr install it and try. If it works, it's fine. If not, you'll notice.

oprypin commented 3 years ago

The replies before have been focused on "mm maybe it's not so bad, maybe we can find a compromise, maybe we have to just stick through this"

But can someone clarify, what exactly is the advantage to preventing people from installing shards? There's only one possible advantage I can imagine, it's that possible errors will show up during the installation of a shard, rather than during first compilation.

There's no advantage from also asking library authors to do a meaningless bump for "certified working for 1.0.0", those that maintain their libraries will keep doing that anyway, and those that don't won't. So that is already just a disadvantage. Additionally, all non-root libraries and applications will also need to do a meaningless bump of deps just to get a release that includes the meaningless bump.

And libraries can very well keep working just fine without the certification, I have libraries that easily work on the entire range 0.25-0.35-1.0. Certainly agree with @straight-shoota, I had simply gone and updated all my libs to have >=[whatever was there before]. That's the best the community can do with this kind of hand of cards.

Now, giving Shards a reputation that "you just always have to disable that check" maybe even by itself is not worth it.

straight-shoota commented 3 years ago

Even shards' nightly test fails because of crystal version restriction: https://app.circleci.com/pipelines/github/crystal-lang/shards/564/workflows/b9287f8f-50be-4a68-93d9-a8284ca78a9e/jobs/1358

jessedoyle commented 3 years ago

Hey guys! I know I'm late to the game here, but I wanted to chime with my own experience as a shard maintainer that may not follow the Crystal landscape as often as I should.

I recently ran into this exact issue here: https://github.com/jessedoyle/duktape.cr/pull/64.

My first thought was "there's no way that Crystal core would default compatibility of all shards implicitly to < 1.0.0". After reading the discussion here, it turns out that assumption was incorrect.

I know there's arguments for both sides here, but I just wanted to share my experience. I do think the current behaviour is very surprising to developers.

oprypin commented 3 years ago

The fallout of this is still ongoing, people are coming back to Crystal now that it's 1.0.0 and this is the first thing they're hit in the face with. I opened https://github.com/crystal-lang/shards/pull/493 to remove this check by default.

crystal-lang / shards

Due to the default of `crystal: <1.0.0`, can't install shards for Crystal nightlies #413