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

Missing "v1.0.0:shard.yml" #346

Closed
mamantoha opened this issue Apr 6, 2020 · 9 comments · Fixed by #362
Closed

Missing "v1.0.0:shard.yml" #346

mamantoha opened this issue Apr 6, 2020 · 9 comments · Fixed by #362
Assignees
Labels

Comments

@mamantoha
Copy link

mamantoha commented Apr 6, 2020

Hi there.

There is an issue with installing some shards after updating to Crystal 0.34.0.

Steps to reproduce:

  • Create new lib (e.g. crystal init lib test_redis)
  • Add dependency to shard.yml
    dependencies:
      redis:
        github: stefanwille/crystal-redis
  • Run shards

Output:

Missing "v1.0.0:shard.yml" for "redis"

Previously it took the latest version from the master branch.

shards --version
Shards 0.10.0 [4091ac5] (2020-04-06)
crystal --version
Crystal 0.34.0 [4401e90f0] (2020-04-06)

LLVM: 8.0.0
Default target: x86_64-unknown-linux-gnu
@straight-shoota
Copy link
Member

I noticed this error, too.

The reason is that redis@1.0.0 doesn't provide a shard.yml but shards tries to read all older releases to figure out which dependencies match. A version that doesn't provide a shard.yml should just be ignored instead of failing. I think that's how this worked before 0.10.0.

A quick fix is to pin the shard to a specific version. For example

dependencies:
  redis:
    github: stefanwille/crystal-redis
    version: 2.5.3

This avoids shards trying to read shard.yml of oder releases.

@waj waj self-assigned this Apr 17, 2020
@waj
Copy link
Member

waj commented Apr 17, 2020

Shards could also assume no dependencies for a tag without shard.yml.
@straight-shoota do you foresee any conflict by adopting that behavior?

@waj waj closed this as completed Apr 17, 2020
@waj waj reopened this Apr 17, 2020
@straight-shoota
Copy link
Member

I'm not sure whether this would necessarily create a conflict. But treating a tag as a shards version when the fundamental shard.yml file is missing completely, doesn't seem like a good idea.
Especially if other versions of the same repo include a shard.yml.

@waj
Copy link
Member

waj commented Apr 20, 2020

I find quite strange to just ignore the tags. We should at least provide a warning or error if that version is selected. These versions exist probably from pre-shards era and not used so often anyway.

@straight-shoota
Copy link
Member

Sure, making it an error should be fine. I just meant it shouldn't be a version you can actually use.

@waj
Copy link
Member

waj commented Apr 20, 2020

But why not just a warning? What's the critical issue we're trying to avoid?

@straight-shoota
Copy link
Member

Okay let's be clear:

  • In the example case form the OP, there should be no mention at all about missing shard.yml in v1.0.0 because this is not relevant to the user. This is simply a legacy issue and needs neither a warning nor an error. Either the version is simply not considered for dependency resolution, or it is silently accepted. Verbose logging could inform about that.
  • If you explicitly ask for a version that doesn't have a shard.yml (for example add version: 1.0.0 to redis dependency), I'm not sure what should happen. Maybe it should show a warning.
Btw. shardbox knows about 84 releases with missing `shard.yml`
         shard         |                               versions
-----------------------+----------------------------------------------------------------------
 prax-cr               | 0.4.1, 0.4.0, 0.2.0, 0.6.1, 0.5.0, 0.3.0, 0.5.1, 0.7.0, 0.4.2, 0.6.0
 crul                  | 0.0.1b, 0.3.1, 0.0.1a, 0.1.0, 0.2.0, 0.2.1, 0.3.0
 html-pipeline         | 0.2.1, 0.2.0, 0.3.1, 0.2.2, 0.0.1, 0.3.0, 0.1.0
 crustache~gitlab      | 0.1.1, 0.1.0, 0.3.2, 0.3.0, 0.2.1, 0.2.0, 0.3.1
 crustache~jwaldrip    | 0.2.0, 0.3.2, 0.1.0, 0.1.1, 0.2.1, 0.3.0, 0.3.1
 crustache             | 0.1.1, 0.1.0, 0.3.2, 0.3.1, 0.2.0, 0.2.1, 0.3.0
 amethyst              | 0.1, 0.1.7, 0.0.2, 0.0.7, 0.1.3
 power_assert          | 0.2.2, 0.2.3, 0.2.0, 0.2.1
 mysql~waterlink       | 0.0.2, 0.1.0, 0.0.1
 redis~maiha           | 1.0.1, 1.0.0, 1.1.0
 redis                 | 1.0.1, 1.1.0, 1.0.0
 mpp                   | 0.1.0, 0.2.1, 0.2.0
 memcached             | 0.0.1, 0.1.0
 shards                | 0.2.0, 0.1.0
 emoji                 | 0.0.1, 0.1.0
 cryload               | 0.0.2
 minitest~ragmaanir    | 0.1.0
 timecop~taylorfinnell | 0.0.2
 spellout              | 1.0.0
 timecop~waterlink     | 0.0.2
 cltk                  | 0.0.2
 minitest              | 0.1.0
 mocks                 | 0.1.0
 host_meta             | 0.1.0
 web_finger            | 0.1.0
 amatista              | 0.3.1
 icr~TechMagister      | HEAD
(27 rows)

@waj
Copy link
Member

waj commented Apr 20, 2020

Agree with the first point. For the second, I'm writing a fix to show a warning when the version gets selected, explicitly or with a version expression. I think Shards should be as annoying as possible until the problem gets fixed, but not introduce blocking issues unnecessary.

Shardbox is becoming an essential tool for this project. I think I'm going to ask you access to that database or install an instance myself 😉

@straight-shoota
Copy link
Member

I should probably make database exports available. 👍

@waj waj closed this as completed in #362 Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants