Detect version mismatches between shard.yml and git tags

[straight-shoota]

Can you illustrate the motivation for this? Is there a specific reason or just because it feels right? 😄

I had considered such a strict matching and it sure is correct that a tagged release should report the tagged version in the spec file. But, this is actually not trivial to simply error. This would break installing lots of published shards because shard authors seem to be very lax about that. It probably contributed that shards did never check this, so there have been no errors so far. We don't even provide a tool to automaticaly verify correctness before releasing (see #29).

To put some numbers on this: The database of shardbox.org currently lists 5311 releases, 768 of them report a different version in the spec file than the git tag. That's 14%.

These mismatches are hard to fix retroactively because you would have to revisit each tagged commit, append a fix commit and update the tag. And then the main repo ends up with different tags than its clones.

All in all it would probably be better to not be hypercritical about this. The relevant part for resolving versions is the actual tagged commit. The version property in the spec can be understood as primarily informational to convey this information to the checked out working dir. Ideally, it should be accurate. But it doesn't affect the function of shards when it doesn't match.

[RX14]

These mismatches are hard to fix retroactively because you would have to revisit each tagged commit, append a fix commit and update the tag

This is not true - I made sure to only error if the installed version was wrong. Shard authors would simply need to tag a single new release and have everyone upgrade.

How many of that 14% are the latest release?

The version should either be enforced or removed. It's clearly not actually used in the resolver. I'm fine with warning on this for a release, then making it an error next shards release.

[straight-shoota]

Currently 71 shards report a different version in the spec file of their latest release. Out of 822 total shards, that's about 9%.

[asterite]

I like this, but I would try to enforce it in some other way. Maybe we can have a shards tag command. But doing it on install can be a bit disruptive.

[RX14]

I think we should warn, at least, then we can discuss switching to an error later.

[asterite]

Warning is good. Then authors can be notified of this mismatch.

[waj]

What's the function of the version field in shards.yml? I know other dependency managers also have this attribute, but I always wonder what's the real reason behind it. The value is always "wrong" besides on the tag, unless you carefully update it to contain a prerelease value before tagging and don't forget to remove it on the commit finally tagged. In other words, too error prone and not the mandatory field that represents which is the current version.

I'm not strictly saying that we should remove it... I want to understand the motivation to have it.

[ysbaddaden]

@waj A version tag freezes a release, but it doesn't mean that it's always "wrong" otherwise. For example:

1. when mixing refs (tag, commit, branch) and version constraints: we need to know whether the selected refs (branch: some-fix) still matches a version constraint (~> 4.0);
2. the path resolver needs to find a version;
3. we can also imagine other resolvers without tags, for example downloading tarballs.
4. the shards version command also uses it from shard.yml (to void repeating the version everywhere).

TBH I think the version tag came after the version field. It's merely a mean to easily find and extract the release from the Git history.

[straight-shoota]

Issuing a warning sounds fine. Could also consider adding such a warning on shardbox.org as well. Maybe even alert the maintainer.

And for the future it would help to implement #29 and maybe even a tool that completely manages the release process.

[waj]

@ysbaddaden yes, I agree, specially with point 1. With "wrong" I mean that if I'm planning to release a version and I update the shard.yml, until I tag the release somehow all the commits have the same version. We found this problematic at Manas for example, when the QA team has to report issues about a deployed version of an app. If the team just look at the version within the code, the reported issues wouldn't be precise about where the bug was found. That's why in our (automated) process we always add some prerelease attribute and the git commit to the version, (1.0.1-dev-abcdef0) unless the deploy was made from an exact tag (1.0.1).

So yes, I agree with this PR but only with a warning. Outdated version in a shard.yml wont affect shard resolution when using tags, and that's when the mismatch will be detected. But it will produce some issues when git refs are used for example.

[RX14]

Well, mismatched versions with tags does break shards check as reported in https://github.com/vladfaust/migrate.cr/issues/15.

I think it'd be better to start disallowing them at a certain point.

[waj]

@RX14 but that's a bug we could fix :)

[RX14]

I guess, though I also consider that mismatched versions are accepted a "bug".

[RX14]

Changed the error into a warning.

[RX14]

Two of the same obvious feedback at the same time :)

My bad, the shard name is definitely required.

[RX14]

Should be ready to go now? Shame this didn't make the release though.

[bcardiff]

It is expected that there will be another release of shards before 1.0. Let's release what we have today. I'm sending the release PR soon.

[RX14]

or you could press merge :)

[bcardiff]

No without reviewing it first (which I hadn't :-) ). There are more changes to come in a next release.