Add validate-tags command #456

gotmax23 · 2022-11-11T03:24:11Z

This adds a new validate-tags command to retrieve the tags in each collection's git repository and ensure that the current version is tagged.

Currently, this is a separate command, but I'd like to have antsibull-build include the data from get_collections_tags() in ansible-build-data and the ansible sdist after this has gotten more testing/feedback.

I also split CollectionsMetadata into a separate file and added the new keys. Previously, this class was part of changelog.py and only used for retrieving changelog URLs.

Relates: ansible-community/community-topics#148
Depends-on: ansible-community/ansible-build-data#176

gotmax23 · 2022-11-11T03:27:53Z

$ antsibull-build validate-tags -i tags.yaml --data-dir ./build/ansible-build-data/7 7.0.0b1 
ERROR:antsibull:func=validate_tags:mod=antsibull.tagging|cisco.nso 1.0.3 is not tagged in https://github.com/CiscoDevNet/ansible-nso
ERROR:antsibull:func=validate_tags:mod=antsibull.tagging|cyberark.pas 1.0.14 is not tagged in https://github.com/cyberark/ansible-security-automation-collection
ERROR:antsibull:func=validate_tags:mod=antsibull.tagging|f5networks.f5_modules 1.20.0 is not tagged in https://github.com/F5Networks/f5-ansible
ERROR:antsibull:func=validate_tags:mod=antsibull.tagging|fortinet.fortimanager 2.1.6 is not tagged in https://github.com/fortinet-ansible-dev/ansible-galaxy-fortimanager-collection
ERROR:antsibull:func=validate_tags:mod=antsibull.tagging|hpe.nimble 1.1.4 is not tagged in https://github.com/hpe-storage/nimble-ansible-modules
ERROR:antsibull:func=validate_tags:mod=antsibull.tagging|inspur.sm 2.3.0 is not tagged in https://github.com/ISIB-Group/inspur.sm
ERROR:antsibull:func=validate_tags:mod=antsibull.tagging|mellanox.onyx 1.0.0 is not tagged in https://github.com/ansible-collections/mellanox.onyx
ERROR:antsibull:func=validate_tags:mod=antsibull.tagging|ovirt.ovirt 2.3.1 is not tagged in https://github.com/ovirt/ovirt-ansible-collection

src/antsibull/tagging.py

gotmax23

I've marked this as ready for review. I'm happy with the tag retrieval code, but I'd like your opinion on my other question :).

src/antsibull/cli/antsibull_build.py

src/antsibull/collection_meta.py

changelogs/fragments/456-validate-tags.yaml

src/antsibull/tagging.py

This adds a new validate-tags command to retrieve the tags in each collection's git repository and ensure that the current version is tagged. Currently, this is a separate command, but I'd like to have `antsibull-build` include the data from `get_collections_tags()` in ansible-build-data and the ansible sdist after this has gotten more testing/feedback. I also split CollectionsMetadata into a separate file and added the new keys. Previously, this class was part of changelog.py and only used for retrieving changelog URLs. Relates: ansible-community/community-topics#148 Depends-on: ansible-community/ansible-build-data#176

We should ensure that the input file exists (if that option is passed) and that the deps file exists to avoid exceptions down the line. Also, this fixes a typo in _normalize_validate_tags_options.

gotmax23 · 2022-11-13T22:55:19Z

antsibull/src/antsibull/cli/antsibull_build.py

Lines 342 to 352 in e8c91b6

    
           collection_parser = subparsers.add_parser('collection', 
        
                                                     parents=[build_parser], 
        
                                                     description='Build a collection which will' 
        
                                                     ' install Ansible') 
        
           collection_parser.add_argument('--deps-file', default=None, 
        
                                          help='File which contains the list of collections and' 
        
                                          ' versions which were included in this version of Ansible.' 
        
                                          '  This is considered to be relative to --data-dir.' 
        
                                          f'  The default is {DEFAULT_FILE_BASE}-X.Y.Z.deps') 
        
           build_parser.add_argument('--collection-dir', default='.', 
        
                                     help='Directory to write collection to')

Is --collection-dir supposed to be added to build_parser here or should the last line above be collection_parser.add_argument(...)? build_parser is defined 120 lines up. --collection-dir is being added as an argument to validate-tags which I don't want.

gotmax23 · 2022-11-13T22:56:14Z

Anyways, I've added a separate validate-tags-file command and applied the rest of your feedback.

gotmax23 · 2022-11-13T23:08:25Z

Also...

ovirt.ovirt 2.3.1 is not tagged in https://github.com/ovirt/ovirt-ansible-collection

ovirt.ovirt 2.3.1 is tagged as 2.3.1-1. All of that collection's tag names are suffixed with -1. Do we want to add some sort of tag_format field or ask them to change the way they tag releases? The requirements say "the tag name MUST exactly match the Galaxy version number," but they also say that "tag names MUST have a consistent format from release to release." There's a reason I wrote the exactly match part, but telling them to change their already established format is unreasonable, IMO.

Pass needed variables as function arguments instead of using app_ctx.

felixfontein · 2022-11-14T17:36:03Z

Also...

ovirt.ovirt 2.3.1 is not tagged in https://github.com/ovirt/ovirt-ansible-collection

ovirt.ovirt 2.3.1 is tagged as 2.3.1-1. All of that collection's tag names are suffixed with -1. Do we want to add some sort of tag_format field or ask them to change the way they tag releases? The requirements say "the tag name MUST exactly match the Galaxy version number," but they also say that "tag names MUST have a consistent format from release to release." There's a reason I wrote the exactly match part, but telling them to change their already established format is unreasonable, IMO.

How about allowing to configure a regex per collection that allows to extract the version? The default would be ^v?(.*)$, and for that collection we could set ^(.*)-1$.

gotmax23 · 2022-11-16T21:53:21Z

ovirt.ovirt's tags are fetched correctly with the latest commit and

diff --git a/7/collection-meta.yaml b/7/collection-meta.yaml
index ff380d8..d9e9c42 100644
--- a/7/collection-meta.yaml
+++ b/7/collection-meta.yaml
@@ -275,6 +275,7 @@ collections:
     repository: https://github.com/ansible-collections/openvswitch.openvswitch
   ovirt.ovirt:
     repository: https://github.com/ovirt/ovirt-ansible-collection
+    tag_version_regex: "^(.*)-1$"
   purestorage.flasharray:
     repository: https://github.com/Pure-Storage-Ansible/FlashArray-Collection
   purestorage.flashblade:

gotmax23 · 2022-11-16T21:56:04Z

#456 (comment) is still a problem :).

src/antsibull/cli/antsibull_build.py

felixfontein · 2022-11-16T22:08:46Z

src/antsibull/tagging.py

+        repository,
+    )
+    flog.debug(f'Running {args}')
+    proc = await asyncio.create_subprocess_exec(


Why not use sh for running commands as all other parts of antsibull do?

Yes, but isn't it blocking (in the asyncio sense)?

It's blocking (sh does not support asyncio yet, see amoffat/sh#552).

src/antsibull/tagging.py

Remove --input from the validate-tags subcommand in favor of a separate validate-tags-file command.

We don't want git to ask for a password when a repository is inaccessible. The program should consider it a failure and move on.

Some collections don't tag versions as v{version} or {version}. While collections should use a standard tag name format per the Collection Requirements, we won't impose this on existing collections.

felixfontein · 2022-11-23T06:42:18Z

@gotmax23 thanks for your contribution!

gotmax23 mentioned this pull request Nov 11, 2022

[Associated vote ended on 2022-10-26] Collections that don't tag releases ansible-community/community-topics#148

Closed

7 tasks

gotmax23 force-pushed the validate-tags branch from b7ed559 to 6ec7e23 Compare November 11, 2022 03:29

gotmax23 commented Nov 11, 2022

View reviewed changes

src/antsibull/tagging.py Outdated Show resolved Hide resolved

gotmax23 force-pushed the validate-tags branch from 6ec7e23 to 8a04307 Compare November 11, 2022 06:04

gotmax23 changed the title ~~[Draft] Add validate-tags command~~ Add validate-tags command Nov 12, 2022

gotmax23 marked this pull request as ready for review November 12, 2022 19:01

gotmax23 commented Nov 12, 2022

View reviewed changes

src/antsibull/cli/antsibull_build.py Outdated Show resolved Hide resolved

felixfontein mentioned this pull request Nov 13, 2022

Fix asyncio.run compatibility #459

Merged

felixfontein reviewed Nov 13, 2022

View reviewed changes

gotmax23 added 3 commits November 13, 2022 15:13

Fix validation of validate-tags options

73c2f46

We should ensure that the input file exists (if that option is passed) and that the deps file exists to avoid exceptions down the line. Also, this fixes a typo in _normalize_validate_tags_options.

Add 456-validate-tags.yaml changelog fragment

ac403a4

gotmax23 force-pushed the validate-tags branch from da0077f to 80580c5 Compare November 13, 2022 21:13

gotmax23 added 4 commits November 13, 2022 17:13

Remove duplicate attribute type defs

25c385f

Define tag regex as a file-level constant

d291269

validate_tags: Return list of errors instead of int

310a290

Make get_collections_tags() a library function

972cc60

Pass needed variables as function arguments instead of using app_ctx.

gotmax23 force-pushed the validate-tags branch from e8c91b6 to 38e42aa Compare November 13, 2022 23:13

gotmax23 force-pushed the validate-tags branch from 38e42aa to 006f606 Compare November 16, 2022 21:50

gotmax23 force-pushed the validate-tags branch from 006f606 to 3386c32 Compare November 16, 2022 21:54

felixfontein mentioned this pull request Nov 16, 2022

Make --collection-dir apply only to 'collection' and 'build-collection' subcommands #461

Merged

felixfontein reviewed Nov 16, 2022

View reviewed changes

Add validate-tags-file subcommand and docstrings

edfa6bf

Remove --input from the validate-tags subcommand in favor of a separate validate-tags-file command.

gotmax23 added 3 commits November 16, 2022 16:17

get_tags: Set GIT_TERMINAL_PROMPT=0

6c24051

We don't want git to ask for a password when a repository is inaccessible. The program should consider it a failure and move on.

validate-tags: Support a custom tag_version_regex

360a92b

Some collections don't tag versions as v{version} or {version}. While collections should use a standard tag name format per the Collection Requirements, we won't impose this on existing collections.

Fix type annotation and log collection name

e5c194a

gotmax23 force-pushed the validate-tags branch from 3386c32 to e5c194a Compare November 16, 2022 22:33

felixfontein approved these changes Nov 17, 2022

View reviewed changes

felixfontein merged commit b38fd8b into ansible-community:main Nov 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add validate-tags command #456

Add validate-tags command #456

gotmax23 commented Nov 11, 2022

gotmax23 commented Nov 11, 2022

gotmax23 left a comment

gotmax23 commented Nov 13, 2022

gotmax23 commented Nov 13, 2022

gotmax23 commented Nov 13, 2022 •

edited

Loading

felixfontein commented Nov 14, 2022

gotmax23 commented Nov 16, 2022

gotmax23 commented Nov 16, 2022

felixfontein Nov 16, 2022

gotmax23 Nov 16, 2022

felixfontein Nov 17, 2022

felixfontein commented Nov 23, 2022

Add validate-tags command #456

Add validate-tags command #456

Conversation

gotmax23 commented Nov 11, 2022

gotmax23 commented Nov 11, 2022

gotmax23 left a comment

Choose a reason for hiding this comment

gotmax23 commented Nov 13, 2022

gotmax23 commented Nov 13, 2022

gotmax23 commented Nov 13, 2022 • edited Loading

felixfontein commented Nov 14, 2022

gotmax23 commented Nov 16, 2022

gotmax23 commented Nov 16, 2022

felixfontein Nov 16, 2022

Choose a reason for hiding this comment

gotmax23 Nov 16, 2022

Choose a reason for hiding this comment

felixfontein Nov 17, 2022

Choose a reason for hiding this comment

felixfontein commented Nov 23, 2022

gotmax23 commented Nov 13, 2022 •

edited

Loading