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

New upstream release tool #80179

Merged
merged 13 commits into from Mar 27, 2023
Merged

New upstream release tool #80179

merged 13 commits into from Mar 27, 2023

Conversation

mattclay
Copy link
Member

@mattclay mattclay commented Mar 8, 2023

SUMMARY

New upstream release tool.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

packaging/release.py

ADDITIONAL INFORMATION

I looked at quite a few Python CLI frameworks, but finally settled on implementing something on top of argparse. Not having an extra requirement was nice, but the main reason was that none of the frameworks offered quite what I was looking for.

I wanted a simple, low boilerplate framework that allowed easily chaining commands together, like the notify method in nox. I also wanted support for global activation of argcomplete. Often this wasn’t supported, and some frameworks had no argcomplete support or even lacked support for tab completion entirely.

I tried to keep the tool flexible, while minimizing the risk of making mistakes during a release. After discussing the workflow briefly with @nitzmahone I made sure to include support for working with a detached HEAD.

The requirements to run the tool are:

  • jinja2
  • packaging

All other requirements are installed using virtual environments managed by the tool.

Aside from the twine upload, all other steps go through human review in the user’s browser.

Publishing releases requires the following access:

  • Permission to merge PRs and create GitHub releases for ansible/ansible.
  • Permission to upload to PyPI.
  • Permission to restart AZP jobs (in case of transient CI failures).
  • Email (it is recommended that your mailto: handler works).

I’ve done some end-to-end testing of the release process using this tool. I’ve provided links to some of the results:

2.14.4

ansible/test-ansible-release#1
ansible/test-ansible-release#2
https://test.pypi.org/project/ansible-core/2.14.4/
https://github.com/ansible/test-ansible-release/releases/tag/v2.14.4

2.14.5rc1

ansible/test-ansible-release#3
ansible/test-ansible-release#4
https://test.pypi.org/project/ansible-core/2.14.5rc1/
https://github.com/ansible/test-ansible-release/releases/tag/v2.14.5rc1

@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. affects_2.15 feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. labels Mar 8, 2023
@mattclay mattclay removed the needs_triage Needs a first human triage before being processed. label Mar 8, 2023
@mattclay
Copy link
Member Author

mattclay commented Mar 8, 2023

Here's the current status of support for the stable branches:

Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

Here's an incomplete first impression.

packaging/release.py Show resolved Hide resolved
packaging/release.py Show resolved Hide resolved
wheel_file = get_wheel_path(version)
env = ensure_venv()

if prompt:
Copy link
Member

Choose a reason for hiding this comment

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

I think this would be unusable from GHA since it'd require using the action with OIDC support instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

The --no-prompt option for this command will allow it to be used non-interactively, if required.

Copy link
Member

Choose a reason for hiding this comment

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

@mattclay that's not enough. It should also NOT run twine upload at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

If the tool is being used in a workflow where a command, such as publish, isn't desired, then the command should not be used.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, that'll do, I guess. But then, my initial suggestion stands — if there's no need for two modes, why implement both?

Copy link
Member Author

Choose a reason for hiding this comment

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

The option was added in case someone wanted to bypass the prompt while using twine.

packaging/release.py Show resolved Hide resolved
packaging/release.py Show resolved Hide resolved
packaging/release.py Show resolved Hide resolved
packaging/release.py Outdated Show resolved Hide resolved
packaging/release.py Outdated Show resolved Hide resolved
packaging/release.py Outdated Show resolved Hide resolved
packaging/release.py Outdated Show resolved Hide resolved
packaging/release.py Outdated Show resolved Hide resolved
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 20, 2023
packaging/release.py Outdated Show resolved Hide resolved
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 22, 2023
@mattclay mattclay marked this pull request as ready for review March 22, 2023 22:24
@ansibot ansibot removed the WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. label Mar 22, 2023
@mattclay mattclay merged commit a6bfa82 into ansible:devel Mar 27, 2023
86 checks passed
@mattclay mattclay deleted the release-tool branch March 27, 2023 19:40
mattclay added a commit to mattclay/ansible that referenced this pull request Mar 27, 2023
(cherry picked from commit a6bfa82)

Co-authored-by: Matt Clay <matt@mystile.com>
mattclay added a commit to mattclay/ansible that referenced this pull request Mar 27, 2023
(cherry picked from commit a6bfa82)

Co-authored-by: Matt Clay <matt@mystile.com>
mattclay added a commit to mattclay/ansible that referenced this pull request Mar 27, 2023
(cherry picked from commit a6bfa82)

Co-authored-by: Matt Clay <matt@mystile.com>
Comment on lines +564 to +565
# assume the devel branch has its upstream remote pointing to the user's fork
fork_remote_name = git("branch", "--list", "devel", "--format=%(upstream:remotename)", capture_output=True).stdout.strip()
Copy link
Member

Choose a reason for hiding this comment

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

This assumption is broken for me:

$ git branch --list devel '--format=%(upstream:remotename)'
origin

$ git branch --list packaging/pep517-in-tree-backend-mutation-tests '--format=%(upstream:remotename)'
fork

Copy link
Member Author

Choose a reason for hiding this comment

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

@webknjaz Your devel branch remote points at ansible/ansible instead of your fork?

Copy link
Member

Choose a reason for hiding this comment

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

Yep. This is because my userspace automation clones a lot of repos in a loop from their canonical locations, so they all have origin pointing upstream: https://github.com/webknjaz/ansible-gentoo-laptop/blob/e71ff8f/roles/gentoo-userspace/defaults/main.yml#L252-L573. It then adds a fork remote to select repos. It's worked like this for the last 5 years and is duplicated across different systems I have...
So I always push to fork while pulling from origin upstream, unless origin is where I work directly, that is.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe, we could implement having a setting in .git/config? I've implemented some helpers for this @ https://github.com/python/cherry-picker/blob/2c1f6b9/cherry_picker/cherry_picker.py#L995-L1022.

Copy link
Member Author

Choose a reason for hiding this comment

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

@webknjaz There's no issue with the origin remote pointing upstream to the ansible/ansible repo. Based on your description, it sounds like you never push to your devel branch, since that wouldn't go to your fork. Is that correct?

Copy link
Member

Choose a reason for hiding this comment

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

Sometimes I sync the fork via something like git fetch --all && git push fork origin/devel:devel.

mattclay added a commit that referenced this pull request Mar 27, 2023
mattclay added a commit that referenced this pull request Mar 27, 2023
mattclay added a commit that referenced this pull request Mar 27, 2023
@ansible ansible locked and limited conversation to collaborators Apr 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.15 feature This issue/PR relates to a feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants