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

retract v1.31.0 and v1.32.1 #2139

Closed
wants to merge 1 commit into from
Closed

retract v1.31.0 and v1.32.1 #2139

wants to merge 1 commit into from

Conversation

eafzali
Copy link

@eafzali eafzali commented Feb 10, 2022

This should be released as v1.32.2 to avoid users to upgrade to these faulty versions until #2133 is merged.

refs #2129

https://go.dev/ref/mod#go-mod-file-retract

This should be released as v1.32.2 to avoid users to upgrade to these faulty versions until IBM#2133 is merged.
@eafzali eafzali requested a review from bai as a code owner February 10, 2022 13:14
@ghost ghost added the cla-needed label Feb 10, 2022
@@ -2,6 +2,8 @@ module github.com/Shopify/sarama

go 1.16

retract [v1.31.0, v1.31.2] // https://github.com/Shopify/sarama/issues/2129
Copy link
Contributor

@slaunay slaunay Feb 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the fix is applied in v1.31.2, we probably want:

Suggested change
retract [v1.31.0, v1.31.2] // https://github.com/Shopify/sarama/issues/2129
retract [v1.31.0, v1.31.1] // https://github.com/Shopify/sarama/issues/2129

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea but the idea is to release only the retraction in v1.31.2, quickly after you verified that a potential harmful bug exists in the release, until you release the actual fix. To prevent more people to upgrade to this version until the fix is released. Maybe it's already late for this bug but good to adopt the process for future.

As an example, consider a case where the author of module example.com/m publishes version v1.0.0 accidentally. To prevent users from upgrading to v1.0.0, the author can add two retract directives to go.mod, then tag v1.0.1 with the retractions.

retract (
v1.0.0 // Published accidentally.
v1.0.1 // Contains retractions only.
)

from https://go.dev/ref/mod#go-mod-file-retract

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the retract should be added to the next release (likely v1.31.2) and I believe only then can the Go tooling knows about those.
But we want to prevent or warn a user from upgrading to v1.31.0 or v1.31.1 (where the producer deadlock can happen).
If we have retract v1.31.2 then that version (with a fix) would not be considered when installing @latest and because v1.31.1 is not retracted then it would be picked instead.
I never use that feature but that is my understanding.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a range so it includes "1.31.1" as well. The idea is that if you merge this and release it as "1.31.2" then when a user uses go tooling to "upgrade" or get the latest version, because 1.31.0, 1.31.1 and 1.31.2 are retracted, 1.30.1 will be considered as the latest.

So for example if a user is currently using 1.31.1 they will be "upgraded" to 1.30.1.

After you release the fix in 1.31.3 everything will be back to normal.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got it now 🤦, I was expecting v1.31.2 being the next release to contain the bugfix and I was not interpreting the retract directive to be a range.

Depending on how long it takes to verify/approve the fix, it might makes sense to release v1.31.2 with this change to prevent users (using a recent enough version of Go) from pulling those versions.
We just want to make sure we update the range if the fix makes it to v1.31.2.

@dnwe
Copy link
Collaborator

dnwe commented Feb 24, 2022

Thanks for the pointers. Covered by #2160

@dnwe dnwe closed this Feb 24, 2022
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