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

Release 24.1.0 to unbreak users who use the latest PyPy #12097

Closed
itamarst opened this issue Jan 29, 2024 · 23 comments · Fixed by #12106
Closed

Release 24.1.0 to unbreak users who use the latest PyPy #12097

itamarst opened this issue Jan 29, 2024 · 23 comments · Fixed by #12106
Assignees

Comments

@itamarst
Copy link
Contributor

#12084 is breaking CI for Tahoe-LAFS, so probably is breaking real-world usage for someone somewhere too. So it'd be good to have a release sooner rather than later.

@adiroiban
Copy link
Member

adiroiban commented Jan 29, 2024

Happy to do a release. I am planing to use this issue as the release issue. This is why I have renamed the title.

We have one Twisted Web issue, that hasn't received much love and I think is a security issue.

I have marked it as a release blocker, hoping that it can receive more attention.

release-blocker


I guess we can do a release with the current trunk, and then have another release once that PR is merged

@adiroiban adiroiban changed the title New release to unbreak users who use the latest PyPy Release 24.1.0 to unbreak users who use the latest PyPy Jan 29, 2024
@adiroiban
Copy link
Member

adiroiban commented Jan 29, 2024

@itamarst do you want to wait for #12090 to be merged before this release?

I think that the PR can be merged.

@adiroiban
Copy link
Member

adiroiban commented Jan 29, 2024

One thing that I forgot.

If you want to do the release PR, documentation is here https://docs.twisted.org/en/latest/development/release-process.html

Let me know if you need help with the release.

If you don't have time for the release, I can look into this and do the release PR.


With @glyph we discuss to have a release every month ... but for now my goal is a release every 2 months.

It's just that to do a release, you need a reviewer.

And seeing my PRs not being reviewed for more than 1 month... and I not in the mood of creating another PR that will stay in the queue for months :(

@itamarst
Copy link
Contributor Author

itamarst commented Jan 29, 2024

I can probably do a release, or review a release PR.

@adiroiban
Copy link
Member

If you have time to do a release, that would be great.
We can use this oportunity to validate that the current documentation is good enough.

In theory, anyone from the current commiters can push a release.
but for the last few releases, I was the only one doing the releases... so some things might not be well documented.

If you don't have time, I will try to create the release and will ask you for a review.

Cheers

@itamarst
Copy link
Contributor Author

This still requires the blocker to be merged? Or should I just proceed without it?

@itamarst
Copy link
Contributor Author

(If it was filed in 2020 I am not sure a fix should be a release blocker, even if it's a security issue...)

@adiroiban
Copy link
Member

I have removed the blocker marker. it was more a nice to have thing.

@itamarst
Copy link
Contributor Author

itamarst commented Feb 2, 2024

@adiroiban OK, started looking at this. First thing I noticed in https://docs.twisted.org/en/latest/development/release-process.html was a bunch of references to Trac:

  1. The regression report (I assume the equivalent is a GitHub label? Or is this no longer a thing?)
  2. More importantly, it talks about a branch naming convention tied to Trac issue numbers, which are no longer a thing.

Second, I don't understand how pre-releases work. It seems like the docs suggest using a tag that matches the final version for the pre-release tag.

Third, there's no mention of tagging a final release, or what it should look like.

Is the intention that "pre-release" only mean the release notes aren't final, and it just gets unchecked in the UI once release notes are tweaked?

Fourth, I don't understand bullet 22 in https://docs.twisted.org/en/latest/development/release-process.html#prepare-the-branch.

@itamarst
Copy link
Contributor Author

itamarst commented Feb 2, 2024

Hm, except later docs talk about testing the pre-release, which suggests multiple uploads to PyPI so I don't understand how this works.

@adiroiban
Copy link
Member

Thanks for the feeback.
There is this ticket to update the release process after the migration. - #11621

It wasn't a priority as I felt there is no much interest into that.

I tried to push some dev doc changes in the past... but the changes were reviewed after 2 months of waiting in the queue...so it wasn't fun to work on this.


The regression report (I assume the equivalent is a GitHub label? Or is this no longer a thing?)

Yes. Is the regression label filter in GitHub Issues

More importantly, it talks about a branch naming convention tied to Trac issue numbers, which are no longer a thing.

The convenion is now tied to GitHub Issues

so the release branch for this should be release-22.2.01-12097

It's important for the branch name to be prefixed with release- as there is some automation triggered for these branches.


Second, I don't understand how pre-releases work. It seems like the docs suggest using a tag that matches the final version for the pre-release tag.

Third, there's no mention of tagging a final release, or what it should look like.

The pypi publish is triggered by creating a tag

see

- name: Publish to PyPI - on tag
if: startsWith(github.ref, 'refs/tags/twisted-')
uses: pypa/gh-action-pypi-publish@v1.5.1
with:
password: ${{ secrets.PYPI_UPLOAD_TOKEN }}

And the tag is created using the GitHub Releases UI - https://github.com/twisted/twisted/releases/new

In the UI there is: Choose an existing tag, or create a new tag when you publish this release.

In the UI there is also the "Mark as pre-release" option

Does it make sense?

@itamarst
Copy link
Contributor Author

itamarst commented Feb 2, 2024

I'll get back to this next week, thank you!

@glyph
Copy link
Member

glyph commented Feb 3, 2024

Love to see this knowledge transfer happening, I hope we can get this reflected in the docs soon :)

@itamarst
Copy link
Contributor Author

itamarst commented Feb 9, 2024

So getting back to this... it's still not clear to me how pre-releases actually work. If I tag release-24.2.01-12097, it doesn't matter if I mark it as pre-release in GitHub, it's going to be released to PyPI so it's not really a pre-release at all, it's a final release that will get used by users who do normal pip install twisted. So I'm just doing a final release and pretending it's a pre-release?

@adiroiban
Copy link
Member

I think that pip install twisted will skip the release candidate by default.

pip/pypi are using semantic versioning to detect a release candidate.
See https://pypi.org/project/Twisted/#history ... the RC are highligted.


Now, checking "Mark as pre-release" in GitHub does not modifies anything on PyPi.

It's just that GitHub releaes don't have automatic release candidate detection, so you need to manualy check it.

Makes sense?

Regards

@glyph
Copy link
Member

glyph commented Feb 9, 2024

It's just that GitHub releaes don't have automatic release candidate detection, so you need to manualy check it.

But, as Itamar says, manually checking it will do nothing, and then a final release will be pushed to PyPI.

It seems like the disconnect is that https://docs.twisted.org/en/latest/development/release-process.html#prepare-the-branch leaves "VERSION" variable somewhat ambiguous. My understanding here is that the "VERSION" in the prerelease tag includes the rcN version suffix that python -m incremental.update Twisted --rc will stamp Twisted with. But reading Itamar's comments here it seems like he was thinking it would be the final release version, i.e. $RELEASE.

This appears to be validated by the presence of tags like twisted-23.8.0rc1 in the repo today.

@itamarst have I missed something here?

It would be good to actually have a shell command that just picks up the relevant version so that we don't need to leave this implied.

@glyph
Copy link
Member

glyph commented Feb 9, 2024

It is somewhat frustrating to read this document and see instructions like this:

Incremental automatically picks the correct version number for you. Please retrieve it after you run it.

Retrieve… how? Where do you retrieve it from? :)

@glyph
Copy link
Member

glyph commented Feb 9, 2024

(It looks like twist --version is a pretty good proxy for the "correct" version number, assuming you're in the right virtual environment)

@itamarst
Copy link
Contributor Author

OK, so it sounds like there's two tags and one of them is an rc tag. Somehow. But I can't tell if I do that manually or not.

@adiroiban
Copy link
Member

The release candidate tag should be named: twisted-24.02.0rc1
This will trigger a normal release in PyPI. PyPi is smart enought and wil automatically tag this as pre-release.

GitHub Releases tool is not that smart, and you need to manually tell the tool that you want to do a pre-release.

The final tag will be named twisted-24.02.0.


Retrieve… how? Where do you retrieve it from? :)

Regarding the incremental documentation.
I think that we are too hard to ourselves :)
After you run the incremental magic, you can just check the git diff and you can see what version was auto-magically set by incremental

Before pushing the code, you need to review the changes anyway, so you can see the changes.

It might be not obvious if you just read the docs, without doing the release, but I think that if you try to do the release, it's not that hard :p


btw, after the 21.7.0 release, I have created a PR to document the process
#1616

The PR was ready by the end of July 2021, but it was picked up for review in January 2022

With that into consideration, my feeling was that current Twisted developers don't care that much about the release process, so I haven't considered a priority to have top notch release documentation.


I hope that with the help of Itamar we can update the documentation for the release process.

I hope that Itamar can pick up the release, then suggest documentation changes and then I can help with the review.


btw... if you are in a hurry for a review, let me know and I can do the release.

But I would like to have this knowledge transfer for the release process, so that we have more than 1 person that can do a release.

Cheers

@adiroiban
Copy link
Member

@itamarst if it's easier, we can schedule a meeting tomorrow over IRC or Matrix and look at the release process.

Let me know what works best for you.

I know that the current documentation is not good enough, and that the process can be frustrating. So I am happy to help to make this less of a frustration.

@itamarst
Copy link
Contributor Author

Having two tags seems like the piece I'm missing. So I'll go try and do it and see what happens.

@itamarst itamarst self-assigned this Feb 21, 2024
@adiroiban
Copy link
Member

The main reason for checking the "Pre-release" option in GitHub Release is so that the old stable will continue to be the "recommended" version in GitHub UI

image

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 a pull request may close this issue.

3 participants