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

yank [email protected] #119607

Closed
wants to merge 1 commit into from
Closed

Conversation

aplavin
Copy link
Contributor

@aplavin aplavin commented Nov 17, 2024

The version was tagged as breaking but actually isn't. To fix that, it has been released as 0.6.20 afterwards (same exact content).
Meanwhile, 0.7.0 should probably be yanked to avoid further confusion – this PR.

@aplavin
Copy link
Contributor Author

aplavin commented Nov 17, 2024

@giordano @DilumAluthge

@goerz
Copy link
Member

goerz commented Nov 17, 2024

Just for the record, I strongly object to the laissez-faire attitude towards yanking that seems to be developing here. It should only be done in the most extreme emergencies. As I said on Slack:

I’d just point out that SemVer does not require a major release to have breaking changes (only the other way around). On the other hand, yanking versions is extremely disruptive. I don’t think we should ever yank versions, except maybe in a malware situation

@aplavin
Copy link
Contributor Author

aplavin commented Nov 17, 2024

Not sure I see how is yanking suitable for malware situations... Then, best would be to just remove the version so that nobody installs it anymore.

While in these scenarios, yanking seems like a clean way to say "please don't resolve to this version in the future, but if you already did that – fine, everything will continue working". Exactly what's needed :)

@DilumAluthge
Copy link
Member

Suppose that someone releases a version Foo.jl version 1.1.0, but they actually made a breaking change (and thus they should have actually released Foo 2.0.0). In that case, if a downstream user (that has Foo = "1" in their [compat] section) installs Foo 1.1.0, there is a real chance that their code might break. Therefore, in the registry we often choose to yank Foo 1.1.0.

This is the opposite situation. Someone released a version Bar.jl version 0.7.0, but they didn't actually have any breaking changes, and thus they could have (and should have) released a Bar 0.6.x. In this case, if a downstream user installs Bar 0.7.0, their code won't actually break (because Bar 0.7.0 didn't have any breaking changes). In this case, because there is no risk of a downstream user having their code break, we do not need to yank Bar 0.7.0 in the registry.

@goerz
Copy link
Member

goerz commented Nov 17, 2024

Not sure I see how is yanking suitable for malware situations... Then, best would be to just remove the version so that nobody installs it anymore.

I'm not sure if I fully appreciated the specific way "yanking" is implemented, as opposed to deleting/reverting a package or release. I do agree that actual hypothetical malware should be purged from the registry in a way that makes it impossible to install.

Suppose that someone releases a version […] 1.1.0, but they actually made a breaking change […], in the registry we often choose to yank

That's not really necessary, and arguably violates SemVer. The SemVer specification explicitly discusses this particular situation:

As soon as you realize that you’ve broken the Semantic Versioning spec, fix the problem and release a new minor version that corrects the problem and restores backward compatibility. Even under this circumstance, it is unacceptable to modify versioned releases. If it’s appropriate, document the offending version and inform your users of the problem so that they are aware of the offending version.

Maybe the "yanking" feature in the registry was intended as a way to implement "inform your users"? If so, that would be a good thing, but I've seen situations recently where things got screwed up because of a yanked package. If not, I still think we should almost never yank anything. There might be some situations related to wrong compat bounds where mistakes can't be repaired within SemVer, but I'm not even sure about that.

@goerz
Copy link
Member

goerz commented Nov 18, 2024

Just to conclude this: the intended behavior of yanking is described in the General Registry README. The way it is described there, it should generally be okay to yank "broken" releases under the conditions described there.

Note that this PR is not one of those conditions.

Generally speaking, an accidental major release that actually wasn't breaking is certainly unfortunate / confusing (in the sense of "unexpected"), but it's not a violation of SemVer. There's nothing that can be done that wouldn't violate SemVer except to just keep going from the major release.

I would also point out that StructArrays is pre-1.0, so a change from v0.6 to v0.7 is actually not breaking (despite Pkg treating it as such). The SemVer spec is extremely clear about this: Before v1.0, "anything can happen". Pre-1.0 packages simply do not have a "stable" API, and thus you can't "break" it. Of course, it's fine to use v0.x releases as "big change" (a.k.a, probably breaks code depending on this unstable API), and v0.x.y as "small change (a.k.a. probably does not break dependent code). But this is a social convention of top of SemVer, and a bit on the fuzzy side.

It might be worth looking at whether StructArray should tag a 1.0 release: If you’re worrying a lot about backward compatibility, you should probably already be 1.0.0.. Just for good measure, I would point out that going from v0.x to v1.0 is not a breaking change! That is, it's fine to tag a v1.0 simply to indicate "the API is now stable according to SemVer". In fact, I strongly advocate for a v1.0 release not to have any major changes compared to the last v0.x release, up to changes in documentation and maybe removing "deprecated" code (even though "deprecation" doesn't make sense pre-1.0, since, again, there is no such thing as a stable API pre-.1.0)

At this time, it's unclear whether Pkg might have a bug where instantiating an environment that has a yanked version might fail. There seems to be some anecdotal evidence, but @DilumAluthge hasn't been able to confirm it with a reproducible example. So let's assume yanking works as intended. In that case, it seems fine to use yanking as "informing your users" about very broken releases that shouldn't be installed.

This might be a moot point, and I also might be misunderstanding something, but the reasoning for when yanking is necessary seems off to me. Specifically,

reverting the code changes would not be valid in a patch bump.

seems just wrong: it is never invalid to fix a bug in a patch bump. The only way to fix the bug in that example is to revert the feature, but that doesn't make it breaking/not-a-bugfix. The correct way to handle the "compat bounds too wide" example is to make two bugfix releases: one that reverts the previous release (but keeps the compat bound), and then one that re-applies the previous release but also fixes the compat bound. So, it's not necessary to yank anything, although I certainly don't object to yanking the broken release assuming yanking behaves as described.

@aplavin
Copy link
Contributor Author

aplavin commented Nov 19, 2024

I would also point out that StructArrays is pre-1.0, so a change from v0.6 to v0.7 is actually not breaking (despite Pkg treating it as such).

Not really sure what you mean here?
In the context of Julia packages, it's only relevant how Pkg treats version numbers, not how the same numbers would've been treated elsewhere. Neither Julia itself nor Pkg+packages take semver as-is, they specifically defined changes from 0.x to 0.x+1 to be breaking – and 0.x.y to 0.x.y+1 as non-breaking.

Before v1.0, "anything can happen". Pre-1.0 packages simply do not have a "stable" API, and thus you can't "break" it.

That's definitely not how the Julia ecosystem works, there are lots of pre-1.0 packages with stable API. There's really no difference between 0.3.x and 3.x.y in that regard, other than the latter providing an extra number.

Just for good measure, I would point out that going from v0.x to v1.0 is not a breaking change!

Same, not how Pkg works. It definitely treats 0.x to 1.0 as a breaking change. Such a bump can make sense when one feels the need for an extra number in the version, but otherwise it doesn't matter if the versions go 0.1, 0.2, 0.3, ..., 0.23 or 1, 2, 3, ..., 23.

@goerz
Copy link
Member

goerz commented Nov 19, 2024

Not really sure what you mean here?

I'd really want to make a distinction (maybe an overly subtle / hairsplitting one) between what Pkg considers "compatible"/"incompatible" vs "non-breaking"/"breaking". I'm on board with Pkg treating "0.x" version as "incompatible", but that doesn't mean they're "breaking", even if we sometimes might use that word, in a handwaving manner.

there are lots of pre-1.0 packages with stable API

There shouldn't be! SemVer specifically says that a package does not have a stable API if it is pre-1.0. This isn't just something where we can just say "Julia uses a minor variation of SemVer". Pre-1.0 versions are a core part of the SemVer philosophy, with a specific meaning. Disregarding that is completely antithetical to SemVer.

Such a bump can make sense when one feels the need for an extra number in the version, but otherwise it doesn't matter if the versions go 0.1, 0.2, 0.3, ..., 0.23 or 1, 2, 3, ..., 23.

I couldn't disagree more! There is a very fundamental difference between the 0.x (unstable) development life cycle and the post-1.0 (stable) development life cycle of a project: In the former, backwards-compatibility is not a concern, in the latter, it is. If there is no difference between 0.x minor version and major versions, then why even have 0.x versions? Indeed, disallowing pre-1.0 registrations is precisely what people have proposed based on this misunderstanding: #111019. It just completely disregards a core part of SemVer, as I elaborate on in my comment there. I feel very strongly that pushing people to tag releases that are not stable as v1.0 (or, conversely, not tagging a package that in fact has a stable API as v1.0) hurts the quality of the ecosystem.

It's fine for Julia to have some minor variations on top of SemVer, like making an additional distinction between v0.x and v0.x.y releases as incompatible/compatible, or allowing some additional ordering or semantics for build numbers (which I'm very much in favor of). But we should really not throw something so fundamental as stable vs non-stable releases overboard. Maybe from the perspective of Pkg, it's splitting hairs. From a larger perspective of how we should push people to tag their packages, or how worried they should be about some mild "SemVer-violation" when their package is still pre-1.0 (the situation in the PR), I think it's pretty fundamental.

@aplavin
Copy link
Contributor Author

aplavin commented Nov 20, 2024

That's definitely a subjective view, not the fundamental truth.

There shouldn't be! SemVer specifically says that a package does not have a stable API if it is pre-1.0. This isn't just something where we can just say "Julia uses a minor variation of SemVer". Pre-1.0 versions are a core part of the SemVer philosophy, with a specific meaning. Disregarding that is completely antithetical to SemVer.

But... Julia Pkg differs from semver in this exact aspect, in handling 0.x versions.

I couldn't disagree more! There is a very fundamental difference between the 0.x (unstable) development life cycle and the post-1.0 (stable) development life cycle of a project: In the former, backwards-compatibility is not a concern, in the latter, it is.

Doesn't really feel like this sentiment is universally shared by the Julia community, judging by a lot of stable packages using 0.x versions. It's fine to have this opinion and follow this version scheme – but that's clearly not the only opinion.

If there is no difference between 0.x minor version and major versions, then why even have 0.x versions?

Why not have them? Julia Pkg version treatment is very consistent, without special casing: changing the first non-zero version means a breaking/incompatible release. That's it!
It's up to the devs of each package to choose if they need 3 numbers in a version, or want to just go with the default 0.x.y scheme.

PS: sorry to the maintainers, I guess this discussion is really not about this specific PR anymore :)

@aplavin
Copy link
Contributor Author

aplavin commented Nov 21, 2024

Back from that extended discussion – I think I have a specific question to @DilumAluthge regarding this PR :)
You said that

In this case, because there is no risk of a downstream user having their code break, we do not need to yank Bar 0.7.0 in the registry.

I assumed we don't need to do anything extra, can just go on with StructArrays development as before. And technically everything does work fine, but only as long as people don't switch from StructArrays = "0.6" compat to StructArrays = "0.6, 0.7".
As soon as they do this switch (encouraged by compathelper), Pkg installs [email protected] – even though this is a version registered by mistake, and the actual development continues in @0.6.x as before.
This was noticed in JuliaArrays/StructArrays.jl#321.

Isn't it exactly the problem supposed to be resolved by yanking? "The version is still there if you already resolved before, but new resolves won't consider it, avoiding confusion".

@goerz
Copy link
Member

goerz commented Nov 21, 2024

My reading of how yanking is supposed to work is that v0.7.0 should be yanked only after a bugfix release v0.7.1 is released. The yanking is only relevant in some unusual circumstances where the problem with v0.7.0 was with compat bounds that are too wide, and would thus prevent the bugfix to be seen by Julia or dependents outside of the compat bounds updated in the fix.

The actual development continues in @0.6.x as before.

I don't think that's a good way to handle this. Specifically, you will never be able to release another version v0.7.0. The v0.7.0 release exists, whether it was yanked or not. You also can't release a breaking change to v0.6.x as v0.7.1, because that would violate SemVer with respect to v0.7.0 (accepting your position that SemVer is relevant <v1.0). So you'd have to go from v0.6.x straight to v0.8.0 with your next breaking release. I think the better way to handle this is to just continue development from v0.7.0, and have v0.7.1 be the next non-breaking release. You just made a v0.7.0 release that happened to not actually break anything with respect to the last v0.6.x release. Yes, that's confusing/unexpected. You should note that in your changelog, and people will have to update their compat bound to 0.6,0.7. But it's not a problem, and it's not even a SemVer violation.

@aplavin
Copy link
Contributor Author

aplavin commented Nov 21, 2024

If the yanking has become as strict as you say, I struggle to see any usecase for it anymore. Malware? Remove the release completely. Wrong compat bounds? Change them in the registry.

Wasn't the motivation for yanking being able to avoid Pkg resolving new envs with an erroneously tagged release? If v0.7.0 was just yanked here, no additional changes would be required, neither from the StructArray side nor from the users. Now, instead, further actions are required from both.

@goerz
Copy link
Member

goerz commented Nov 21, 2024

If the yanking has become as strict as you say

I'm not sure it has "become" strict… I don't think it's changed (but I'm not sure about the history). I'm only reacting to how it is currently described in the README.

Wasn't the motivation for yanking being able to avoid Pkg resolving new envs with an erroneously tagged release?

Not as far as I can tell (how it's currently documented). It's to (optionally) mark a release as "broken" after a patch release to fix the broken release, or to deal with the "special category of bugged releases" described in the README.

Wrong compat bounds? Change them in the registry.

I don't think that's possible. That would break re-instantiating existing environments, and "reproducibility" is basically the core tenet of General. It would be fine in a LocalRegistry that didn't have such strict rules. Releases are basically immutable.

Quite honestly, I don't think yanking is really ever necessary. Even in the "special category" of compat bound issues. I think these can be fixed entirely within SemVer by making two immediate bugfix releases (I might be wrong). But yanking still seems like a good way to attach a "broken" tag to a particular release. So it's probably helpful to have, as a feature.

If v0.7.0 was just yanked here, no additional changes would be required

Does the CompatHelper bot take into account yanking (i.e., would yanking stop CompatHelper prompting peopel to update their compat bounds to v0.7.0)? If so, and if it's not already too late (CompatHelper has already notified everyone), then I suppose yanking would save people from having to update the compat bounds now.

But you'd still have to make v0.8.0 your next breaking release, and then people will just be even more confused at that point. I don't see any way to avoid people needing to deal with the existence of the v0.7.0 release at some point, and it seems to me the least confusing way is do it now: document it in your changelog ("Upgrade guidelines: v0.7 contained no breaking changes, just set your compat spec to 0.6,0.7"), and move on.

The bottom line is that yanking does not "undo" a release. There is absolutely no way to undo a release except to completely delete it from the registry, which would only be done for malware, would break the reproducibility promise of General, and has never happend.

@aplavin
Copy link
Contributor Author

aplavin commented Nov 21, 2024

Wrong compat bounds? Change them in the registry.

I don't think that's possible. That would break re-instantiating existing environments, and "reproducibility" is basically the core tenet of General. It would be fine in a LocalRegistry that didn't have such strict rules. Releases are basically immutable.

This is not just routinely done, but even planned to make more straightforward - #104849.

Does the CompatHelper bot take into account yanking (i.e., would yanking stop CompatHelper prompting peopel to update their compat bounds to v0.7.0)?

Pretty sure it does take it into account.

But you'd still have to make v0.8.0 your next breaking release

Don't see any issues with that whatsoever. It would be a breaking version, that's it – be it 0.7 or 0.8 or 0.9 doesn't really matter.

@goerz
Copy link
Member

goerz commented Nov 21, 2024

This is not just routinely done

You seem to be right, e.g., #115069

but even planned to make more straightforward – #104849

I don't get how that doesn't break reproducibility. But that's a discussion I'll take over there…

But you'd still have to make v0.8.0 your next breaking release

Don't see any issues with that whatsoever.

Then I guess v0.7.0 really should be yanked. You'll have to deal with the fallout of having apparently skipped a version later on. Not what I would do. I'm pretty sure it will cause confusion down the road, but that's between you and the dependents of StructArrays

@goerz goerz reopened this Nov 21, 2024
@goerz
Copy link
Member

goerz commented Nov 21, 2024

Changed my mind on reopening this, based on the discussion in JuliaArrays/StructArrays.jl#321

If people have already updated their compat bounds to v0.7 and made releases with that, yanking v0.7 just doesn't seem right. So your choices are to move on with v0.7 or to maintain v0.6 and v0.7 in parallel via backports, as proposed in JuliaArrays/StructArrays.jl#321 (very reasonable, but more work than I would take on, personally).

Of course, if @DilumAluthge still wants to re-open/merge this after all of this discussion, that's okay with me. 🤷

P.S.: If you're going the backport route, then there's not going to be any confusion about v0.8 either, so that seems totally fine. The only thing that would cause confusion is if the continued development was only on the v0.6 branch, but not the v0.7 branch.

@goerz goerz closed this Nov 21, 2024
@aplavin
Copy link
Contributor Author

aplavin commented Nov 21, 2024

Would still be nice to get a response from General maintainers (@DilumAluthge @giordano @LilithHafner ?) regarding this. The yanking situation is quite confusing...

On one hand, there's a "common sense" understanding I had

Isn't it exactly the problem supposed to be resolved by yanking? "The version is still there if you already resolved before, but new resolves won't consider it, avoiding confusion".

and the fact that yanking versions is routinely done here.
On the other hand, the fact that this PR was immediately rejected.

These sides seem to be in a conflict, so would be happy to hear where my understanding is wrong :)

@DilumAluthge
Copy link
Member

My thinking is that if user can end up with broken code, then it's probably worth yanking in the registry. But in this case nobody's code will be broken if they end up with [email protected], so it doesn't seem like we need to yank it in the registry.

@giordano @fredrikekre Any thoughts here?

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

Successfully merging this pull request may close these issues.

3 participants