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

Renaming a shard leads to failure #272

Closed
vladfaust opened this issue May 9, 2019 · 6 comments

Comments

@vladfaust
Copy link

commented May 9, 2019

I have an a shard depending on time_format.cr:

dependencies:
  time_format:
    github: vladfaust/time_format.cr
    version: ~> 0.1.0

I've recently renamed the dependency name from time_format.cr to time-span-humanize, bumping the version and tag from 0.1.1 to 0.2.0. Unexpectedly, it led to build failure on CI with the following message:

Fetching https://github.com/vladfaust/time_format.cr.git
Error shard name (time-span-humanize) doesn't match dependency name (time_format)

I think it should work, because the shard.yml file at 0.1.1 version has proper shard name. IIRC if specifying certain commit or tag instead of version, it would install without any problems.

@vladfaust

This comment has been minimized.

Copy link
Author

commented May 9, 2019

Looks like it has been fixed in 0.9.0 🎉

vladfaust added a commit to onyxframework/sql that referenced this issue May 9, 2019
vladfaust added a commit to onyxframework/http that referenced this issue May 9, 2019
vladfaust added a commit to vladfaust/migrate.cr that referenced this issue May 9, 2019
vladfaust added a commit to vladfaust/tarantool.cr that referenced this issue May 9, 2019
vladfaust added a commit to onyxframework/sql that referenced this issue May 9, 2019
vladfaust added a commit to onyxframework/http that referenced this issue May 9, 2019
@vladfaust

This comment has been minimized.

Copy link
Author

commented May 9, 2019

Well, it's super-broken. I've updated time_format.cr dependency to tag: v0.1.1 in onyx-sql, onyx-http and migrate.cr. They build themselves nicely, but.

I have Crystalworld, an application which depends on those three. Running shards update and then shards install both on shards version 0.8.1 and 0.9.0 leads to failures.

You can experiment with it yourself — clone https://github.com/vladfaust/crystalworld and run shards update and then shards install.

@straight-shoota

This comment has been minimized.

Copy link
Member

commented May 9, 2019

I don't think the rename is the direct issue here. The shard actually doesn't care how it's named and if the name is different in different releases.

The error is in you dependency definition: The name of the dependency needs to match the name of the shard. So when the name is different between 0.1.1 and 0.2.0, you need different dependency definitions depending on whether you're using 0.1.1 or 0.2.0

I would strongly advise against renaming a shard. This is only going to cause confusion.
IMO it's a better solution to simply discontinue the old one and create a completely different shard with a new name. That's a clean solution. It could build on the version history of the old one but should have no releases with the old name. So either remove the previous tags from the new repo or change the name in each of them.

@vladfaust

This comment has been minimized.

Copy link
Author

commented May 9, 2019

I've fixed the issue rolling back original https://github.com/vladfaust/time_format.cr repository and creating new https://github.com/vladfaust/time-span-humanize, because I have a number of users depending on my projects. I'll try to follow @straight-shoota's advice from now, but IMO it's a dark unexplored area full of bugs...

@ysbaddaden

This comment has been minimized.

Copy link
Member

commented May 9, 2019

Renaming a shard will only cause issues, because suddenly a shard named foo is now called bar. This is unexpected.

The problem with crystal shards is that the source code repository is also the registry. When both are distinct, we can keep the same repository for the source code but will have to create a new library in the registry, under the new name. When both are mixed, then a new repository must be created, in order to create a new registry for the library along it.

That being said, with carefully laid out version constraints that match either the old or new namings, it may be possible to trick 0.9.0, which may only consider the shard.yml of matching version numbers (to speed up resolution). Yet, that will break quickly, as demonstrated, whenever an indirect dependency has (or had) a loose version constraint.

IMO this can't be fixed.

@ysbaddaden ysbaddaden closed this May 9, 2019
@RX14

This comment has been minimized.

Copy link
Member

commented May 15, 2019

This appears fixable with some complexity: identify shards by their set of root commit hashes (git rev-list --max-parents=0 HEAD) internally, then validate naming is consistent and correct throughout the dependency graph once the dependencies are fully resolved.

This is rather complex though, and unlikely to be worth the effort in the near-term.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.