Open waj opened 4 years ago
The only real alternative I see is keeping an yanked_versions
in shard.yml
and always reading that from that newest version.
I think we should try deleting tags, make sure that it works well, then see if we get bug reports in the real world with that approach.
I'm not sure, I find still being able to look up what the bogus version was exactly to be quite useful, and let's be honest, people will rarely retag it with broken-v1.3.4
on their own.
The other issue with deleting tags I see is that it's actually quite easy to repush it from a repo where you forgot to git fetch --prune
since.
Yeah, deleting tags can work, but unfortunately it's not failsafe.
It could be achieved with a new tag, this time with a +yank
or +revoked
metadata, Shards would then filter out the version marked as yanked. For example v2.0.0+yank
.
@ysbaddaden you're right, we shouldn't use the prerelease section of the version to mark this. Metadata is more appropriate. But now I wonder if it should just be something not matching a semver at all.
It might be a bit problematic that simply deleting a tag might seem to work good enough for shard authors to not consider using an explicit yank tag. This is a bit difficult to solve, because when the tag doesn't exist on the remote anymore shards simply wont see it and thus can't complain about it. There's no easy way to communicate feedback on deleted tags not being the right tool for yanking.
I suppose there's no way around that, so we need to make sure to cover this topic with explicit documenation and a simple to use tool like shards yank <version>
.
Shardbox.org can help by only marking releases as yanked when there's a proper yank tag. Currently it yanks every release that doesn't show up in the versions list that shards sees.
There was an idea of having a special branch for shards metadata in the past. A place where the requirements of all published versions can be available at once without iterating all the repo history.
If we continue that idea it could be a way to yank versions.
Right now the release management of a shard does not require any special tooling, just plain git
. I think it might be worth to try to preserve that property for a while longer, hence I'm not a big fan of the special branch idea. Also it doesn't seem like a very portable approach to different resolvers, already, how would it work with path
dependencies?
My initial comment on this was not meant as an argument against the tag suffix idea, just exploring ideas. I actually think the tag suffix is a fair idea. Do people see any downsides to it?
@bcardiff note that the special branch could be also force pushed, so it's basically the same problem.
@jhass the metadata branch could be optional. That would avoid having to clone the entire repo for shards that implement it and have all the specs from all the versions at once, making the resolution process faster. path
dependencies doesn't provide any versioning either, so the current source is all that is available. But so far it's working great with the current approach and this was just an idea in case things didn't work so well.
@straight-shoota I think we should encourage authors to use the mechanism we decide for yanking instead of removing tags. But at the end the author will decide what works better for them. The effect for Shards is the same. In the future, GitHub could send alerts to owners when a tag was removed in a Crystal repository 😄
Side note: I find a little bit rude when someone thumbs down without giving a proper explanation of the disagreement. Some communities even have that explicitly banned in their code of conduct. Please be nice with those taking the time to expose their ideas.
In the future, GitHub could send alerts to owners when a tag was removed in a Crystal repository 😄
Shardbox could already do that. Currently it just marks a version that was seen before and is not present anymore as yanked. Once a proper yanking mechanism is established, it can trigger warnings to authors. Not sure how that should be automated, but a data source is available.
path
dependencies doesn't provide any versioning either, so the current source is all that is available
Fair, I was just picking it out as an example we already had. The argument goes beyond that to future possible resolvers.
Side note: I find a little bit rude when someone thumbs down without giving a proper explanation of the disagreement. Some communities even have that explicitly banned in their code of conduct. Please be nice with those taking the time to expose their ideas.
@waj Fair point, I was just being lazy, since I felt that others already expressed my doubts.
There was an idea of having a special branch for shards metadata in the past. A place where the requirements of all published versions can be available at once without iterating all the repo history.
If we continue that idea it could be a way to yank versions.
@bcardiff That sounds pretty git-specific, and as @waj mentioned, suffers from the same set of problems as the tags themselves.
A possible option is to use git notes, which adds metadata to commits. A tag is a pointer to a given commit, using a note in our case is like adding metadata to it. It is specific to git, so of course not portable. However, a different way to yank can be defined depending of the resolver's features and limitations.
TIL: git-notes is a thing 😄 I don't see a significant benefit over adding a specific yank tag, though. And it seems notes can't be signed.
Another idea: Deleting tags is not a solution because it doesn't propagate correctly. But does the same apply to updating a tag? If this would work then yanked tags could just be updated to point to an empty orphaned commit (or any commit that doesn't have a shard.yml
).
Yes, you need to explicitly fetch them. Something like git fetch origin "+refs/tags/*:refs/tags/*"
. While we could incorporate that into shards, I'd still heavily argue against it because it's just so easy to cause mayhem for people collaborating on repos. Everybody would need to constantly do this to not have out of sync state with the others. It's just something we shouldn't encourage.
git notes
looks perfect.
Something I realize from #365 is that we don’t have a way to yank versions once tagged. Is it deleting the tag an option for this? We should ensure
--prune
is used when fetching (currently not). But deleting tags is sometimes problematic and also the lost evidence that there was a tagged version is weird. Maybe adding another tag on the same commit with a “yanked” prefix/postfix? So, if I have av0.1.0
tag, adding anotherv0.1.0-yanked
tag to the same commit would make Shards ignore that version. Does that make sense?