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

Stop yanking packages #62795

Closed
ranocha opened this issue Jun 21, 2022 · 14 comments
Closed

Stop yanking packages #62795

ranocha opened this issue Jun 21, 2022 · 14 comments

Comments

@ranocha
Copy link

ranocha commented Jun 21, 2022

I would really like the General registry to stop yanking releases of ordinary Julia packages. For example, #62675 re-introduced a bug in a package with Julia v1.8 we rely on in our packages (details of the packages do not necessarily matter here right now). Since this bug was fixed by the maintainers of our dependency a few days ago, I was quite puzzled. In the end, it cost me quite some valuable time to debug this and find out that the release that worked for us was yanked from the general registry.

In my opinion, yanking packages is completely against all rules of Julia for reproducible science we advocate. Making it impossible to install a version of a package I used for my research project yesterday in the future is just a no go for me. The proper fix for broken releases of packages is to release a new patch version of this broken package, either just reverting to the old working version or including a proper bug fix.

This may sound a bit strong, but please forgive me. I would just like to start a discussion and strongly encourage people to stop yanking releases.

@fredrikekre
Copy link
Member

Making it impossible to install a version of a package I used for my research project yesterday in the future is just a no go for me.

You can still install yanked packages if you have them in the manifest (which you need to reproduce anyway).

@ranocha
Copy link
Author

ranocha commented Jun 21, 2022

Okay, so reproducibility is still fine - I just didn't know how exactly yanked package releases are treated, thanks for the info 👍 But what about the other aspect - why yank a package instead of releasing a new version? In our case, I updated the compat bounds of our package to restrict our dependency to a working version (with the upcoming release of Julia v1.8). Since the dependency release is yanked, this makes this version of our package with updated compat bounds impossible to install. From my point of view, this is highly irritating behavior. Shouldn't we encourage people to just make a new patch release when they discover they introduced a bug in an earlier release? For me, it's confusing that the set of installable versions of a package is not monotonically increasing but sometimes decreasing due to yanking.

@fredrikekre
Copy link
Member

I guess you have to ask the package maintainer about that. But sometimes yanking is the quickfix since you might have time to fix the broken release right away. In the vast majority of cases I think people don't yank and just release a patch version instead. (Otherwise, all package versions that is not the latest patch version should be yanked.) Note also that there is nothing stopping a yanked release from being un-yanked after a new patch version is released.

@fredrikekre
Copy link
Member

I suppose also that it is slightly unusual (?) to have compat on a patch release, but since all Julia packages seem to happily use 0Ver instead of utilizing all 3 available numbers you kind of have no choice...

@ranocha
Copy link
Author

ranocha commented Jun 21, 2022

Doesn't it take longer to yank a package since you need manual intervention of the maintainers? Just running some git revert and making a new patch release via JuliaRegistrator is automated and fast.

@ranocha
Copy link
Author

ranocha commented Jun 21, 2022

I guess what I would like to express here is: Everybody with write access for this repo, please tell people to make a patch release instead of yanking releases? Basically for consistency and because of the reason you wrote above:

Otherwise, all package versions that is not the latest patch version should be yanked.

@sloede
Copy link
Contributor

sloede commented Jun 22, 2022

I have to agree with @ranocha: In my opinion, the whole point of using a consistent versioning scheme is that a single version always refers to the same code (or content, if you want to be more general). Thus if there is no hard technical reason for yanking a release (as I believe has sometimes been the case for JLL packages, since they do not need a compat entry), I think the correct course of action should always be to create a new release that - if nothing else - reverts the previous one. Maybe I am not seeing something here, but anything else is imho just developer vanity...

@ericphanson
Copy link
Member

Just running some git revert and making a new patch release via JuliaRegistrator is automated and fast.

This sounds like a reasonable strategy to me in general, but in the case from the first post, wouldn't this also break your package? Since you said you relied on the patch release fixing a bug, and updated compat to reflect that you needed that patch, but then if they pushed another patch reverting the previous one, your package would now be installable but broken, right?

@sloede
Copy link
Contributor

sloede commented Aug 3, 2022

This sounds like a reasonable strategy to me in general, but in the case from the first post, wouldn't this also break your package?

Right, but at least in this case, there is a clear, documented path of what has happened (one release, then another), and as package maintainers we can then decide how to react best by setting the compat entries appropriately. Yanking a release, on the other hand, makes information disappear, which in turn makes it much harder to reason about what happened and how to fix it.

However, one can also reverse the perspective and ask: Are there any downsides (besides vanity) to always creating a new patch release? If not, this should be the only way a broken release is handled, since yanking a release has already been demonstrated to clearly have negative side effects.

@ericphanson
Copy link
Member

You keep mentioning "vanity", but I don't really get how that's a factor. Package authors don't want buggy releases, but is that vain? And I don't think it looks any better to yank instead of push a new patch; if you yank now there are uninstallable releases of your package which wouldn't really appeal to my vanity at least.

Yanking a release, on the other hand, makes information disappear

What do you mean?

Are there any downsides (besides vanity) to always creating a new patch release?

Well, since you asked, one would be the exact thing I just mentioned: an author declared (by compat) that version X of their package requires version Y of some other package, and now that has been subverted by version Y+1 undoing the change they needed.

But I'm not actually trying to argue in favor of yanking over patch releases...

Yanking was originally developed (AFAIK) so if there was a big security issue (e.g. a bug so bad it could compromise your machine) there was a way to make that version uninstallable (unless you had it in your manifest already, in which case it would continue to work for reproducibility).

I can totally buy that, like any other registry-only change, it is confusing bc the git lineage in the package itself doesn't show what happened and you need to go to the registry to see what happened, and therefore should be used very sparingly, and that patch releases that revert previous changes are more favorable for that reason.

But a patch release that just reverts a previous patch release also sounds pretty confusing to a user ("OK so v0.5.1 has the bugfix we need, and we're on v0.5.3, so we totally have the bugfix, right? So why is it still broken?"), and subverts compat as I mentioned above. So it's not totally clear to me that patch releases are always better either.

@sloede
Copy link
Contributor

sloede commented Aug 12, 2022

You keep mentioning "vanity", but I don't really get how that's a factor. Package authors don't want buggy releases, but is that vain? And I don't think it looks any better to yank instead of push a new patch; if you yank now there are uninstallable releases of your package which wouldn't really appeal to my vanity at least.

Well, to me the yanking of a release implies a notion of "oops, this was premature and should not have happened, let's try to undo this before (hopefully) anyone is affected or even notices". What else would be the reason (please enlighten me if I am missing something)? Only yanking a software release because it is buggy imho completely defies the idea of patch releases and violates the basic rules of semantic versioning.

As far as I know, following semver is not formally required for packages in the General registry (maybe it should be?), but it is the de facto standard in Julia, reinforced by being the mechanism through which Pkg.jl works, e.g., see here or here. Yanking, on the other hand, violates the semver spec at least for item 3:

Once a versioned package has been released, the contents of that version MUST NOT be modified. Any modifications MUST be released as a new version.

Regarding your other question:

Yanking a release, on the other hand, makes information disappear

What do you mean?

Maybe this was an unfortunate choice of words. It's not so much that existing information is removed, it is more that relevant information is withheld.

As far as I can tell, if a package is yanked, the only place where this information is stored is in the deep internals of the General registry. You can still see the package tag in the GH repo, and if they use TagBot then the GH repo likely has an accompanying release. If you ask me, a user should never ever have to descend into the registry repository to find out why a released package version is not installable anymore.

You later mentioned it being confusing if a previous patch release being reverted:

But a patch release that just reverts a previous patch release also sounds pretty confusing to a user ("OK so v0.5.1 has the bugfix we need, and we're on v0.5.3, so we totally have the bugfix, right? So why is it still broken?"), and subverts compat as I mentioned above.

I absolutely disagree in your assessment that this procedure "subverts" the compat process. If anything, this is exactly what should happen: A release is broken. A new patch release is created, fixing the previously broken release. Done.
Instead of spending time trying to get a release yanked, I think the maintainers of such a package should use the time instead to put it in the release notes of the second patch version that they had to undo the previous fix since it broke something else. In this case, a user would not have to descend into registry madness, but could just look up the release notes and immediately see the reason why the previously release code is not available anymore.

Please do not get me wrong: There are circumstances where yanking a release might be called for, e.g., if there is a severe security issue that would cause the dissemination of private information or cause a loss of data. But I think with >100 packages with yanked releases, people are just going overboard with yanking broken releases.

@DilumAluthge
Copy link
Member

Yanking doesn't violate semver. Yanking doesn't delete or modify a version (or the contents thereof); it simply asks Pkg not to install the yanked version.

Yanking is a feature built intentionally into Pkg, and it has a variety of use cases. For example, if you look at this Discourse post, you'll see that yanking is listed in several of the "Mitigation" sections.

It is very unlikely that we are going to implement a ban on yanking in the General registry.

@ericphanson
Copy link
Member

Yanking doesn't violate semver. Yanking doesn't delete or modify a version (or the contents thereof); it simply asks Pkg not to install the yanked version.

Totally agreed.

For example, if you look at this Discourse post, you'll see that yanking is listed in several of the "Mitigation" sections.

But those are all security problems. I don't think anyone here is arguing that there should be a blanked ban on yanking or that yanking should not be done when there are security issues.

The discussion is about whether one should yank or push a revert bugfix as a patch, when there is a non-security related bug.

And I think it's totally within General's remit to say when the registry wants to accept yanks. Current rule is basically:

  1. General accepts PRs to yank a version whenever a committer to the package makes such a PR, or if there is a security issue.

(The latter has never happened, but I at least would merge a PR to General yanking a version with a security hole even if the PR author is not a package committer). My understanding is that their proposed rule is:

  1. General accepts PRs to yank versions with security issues only. PRs to yank versions for other reasons are rejected, and the package committers are told to instead push a revert and tag a new patch release.

I think that's a legit rule, as would be something weaker:

  1. General recommends yanking is reserved for security issues, although will accept PRs from package committers to yank if they are aware of the alternatives/downsides and still wish to yank for some reason.

Anyway, do you really think there's no merit in discussing switching to (2) or (3) @DilumAluthge? To me I don't think there's been a totally convicing reason why any of those rules is really the best, although I'm leaning to (3) personally.

In the past we have been reluctant to make registry-only changes exactly because of the confusion it creates and the fact that the Project.toml / git history can get out of sync with the registry, so I don't really see why we shouldn't be similarly reluctant to yank.

@DilumAluthge
Copy link
Member

For what it's worth, as pointed out here, the yank functionality in Pkg was inspired by the yank functionality in Cargo. And the docs for cargo yank say:

Occasions may arise where you publish a version of a crate that actually ends up being broken for one reason or another (syntax error, forgot to include a file, etc.). For situations such as this, Cargo supports a “yank” of a version of a crate.

So Cargo says it's fine to use yanking for general bugs. So I think it's reasonable for us to consider yanking as an acceptable strategy for general bugs.

I'm not terribly keen on hardcoding a rule that restricts registry maintainers. Our stance up until now is that all decisions are ultimately at the discretion of the registry maintainers. So we don't really document any hard restrictions on what decisions maintainers can make.

I would be fine with something along of the lines of:

Before merging a yank PR, we encourage registry maintainers to counsel package authors on the pros and cons of yanking, as well as the alternatives to yanking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants