add --check-tag option#4051
Conversation
this option can be used to reject version bumps that conflict with existing remote tags. When --check-tag is passed, versionutils now queries remote tags via git ls-remote before proceeding, and aborts if the new version already exists as a tag. This prevents accidentally re-releasing an existing version.
| if args.increment: | ||
| new_version = update_version(args.filename, args.update_type) | ||
|
|
||
| if args.check_tag: |
There was a problem hiding this comment.
if we think this is ok, we can invoke versionutils.py with the new --check-tag parameter in .github/workflows/release.yml.
There was a problem hiding this comment.
Yeah, that sounds fine. I left a comment about the actual mechanics, but if that's addressed, modifying the release.yml to do this seems fine. It seems like the worst that happens is that we block some releases that we didn't need to, at which point we just remove the check and retry
| # limit search to tags >= 4.x | ||
| remote_tags = subprocess.check_output(['git', 'ls-remote', '--tags', 'upstream', 'refs/tags/4.*'], text=True) |
There was a problem hiding this comment.
Why limit the search to 4.x tags? And won't this cause problems when we update the major version?
Could this instead be:
| # limit search to tags >= 4.x | |
| remote_tags = subprocess.check_output(['git', 'ls-remote', '--tags', 'upstream', 'refs/tags/4.*'], text=True) | |
| remote_tags = subprocess.check_output(['git', 'ls-remote', '--tags', 'upstream', 'refs/tags/{tag}*'], text=True) |
Then I think it would look more precisely for tags with the right contents. I think you could then just base the response on whether the list was empty or not.
But also, I don't think we actually have access to a remote repository called upstream in CI. I think what we may actually want is:
| # limit search to tags >= 4.x | |
| remote_tags = subprocess.check_output(['git', 'ls-remote', '--tags', 'upstream', 'refs/tags/4.*'], text=True) | |
| existing_tags = subprocess.check_output(['git', 'tag', '-l', '{tag}*'], text=True) |
We need to then double check in the places where we invoke the checkout action that we specify fetch-tags: true so that this check works. See: https://github.com/actions/checkout?tab=readme-ov-file#usage
| if args.increment: | ||
| new_version = update_version(args.filename, args.update_type) | ||
|
|
||
| if args.check_tag: |
There was a problem hiding this comment.
Yeah, that sounds fine. I left a comment about the actual mechanics, but if that's addressed, modifying the release.yml to do this seems fine. It seems like the worst that happens is that we block some releases that we didn't need to, at which point we just remove the check and retry
this option can be used to reject version bumps that conflict with existing remote tags. When
--check-tagis passed,versionutilsnow queries remote tags viagit ls-remotebefore proceeding, and aborts if the new version already exists as a tag.This prevents accidentally re-releasing an existing version.