verify-upstreams: use more granular errors and clean up CLI args

src/antsibull/cli/antsibull_build.py

@@ -42,6 +42,7 @@
 from ..constants import MINIMUM_ANSIBLE_VERSION, SANITY_TESTS_DEFAULT  # noqa: E402
 from ..dep_closure import validate_dependencies_command  # noqa: E402
 from ..from_source import verify_upstream_command  # noqa: E402
+from ..from_source.verify import LENIENT_FILE_ERROR_IGNORES, FileError  # noqa: E402
 from ..new_ansible import new_ansible_command  # noqa: E402
 from ..sanity_tests import sanity_tests_command  # noqa: E402
 from ..tagging import validate_tags_command, validate_tags_file_command  # noqa: E402
@@ -67,6 +68,7 @@
     "verify-upstreams": verify_upstream_command,
     "sanity-tests": sanity_tests_command,
 }
+DISABLE_VERIFY_UPSTREAMS_IGNORES_SENTINEL = "NONE"
 
 
 def _normalize_commands(
@@ -258,6 +260,18 @@ def _normalize_verify_upstream_options(args: argparse.Namespace) -> None:
             raise InvalidArgumentError(
                 "--tree-dir and --checkouts-dir must be unique values"
             )
+    for arg, value in {
+        "--tree-dir": args.tree_dir,
+        "--checkouts-dir": args.checkouts_dir,
+    }.items():
+        if not value:
+            continue
+        if (directory := value / "ansible_collections").exists():
+            raise InvalidArgumentError(f"{arg}: {directory} must not exist")
+    if DISABLE_VERIFY_UPSTREAMS_IGNORES_SENTINEL in (args.ignores or set()):
+        args.ignore = []
+    if args.ignores is None:
+        args.ignores = LENIENT_FILE_ERROR_IGNORES
 
 
 def parse_args(program_name: str, args: list[str]) -> argparse.Namespace:
@@ -592,25 +606,22 @@ def parse_args(program_name: str, args: list[str]) -> argparse.Namespace:
         action="append",
         help="Only check collections that match the glob(s)",
     )
+    _choices = [v.name for v in FileError] + [DISABLE_VERIFY_UPSTREAMS_IGNORES_SENTINEL]
     verify_upstream_parser.add_argument(
-        "--allow-missing",
-        default=True,
-        action=argparse.BooleanOptionalAction,
-        help="Whether to allow files to be present in the collection artifact"
-        " but not in the git repository."
-        " Default: %(default)s",
-    )
-    verify_upstream_parser.add_argument(
-        "--print-errors",
-        action=BooleanOptionalAction,
-        default=True,
-        help="Whether to print errors to stdout",
+        "-I",
+        "--ignore",
+        action="append",
+        dest="ignores",
+        type=FileError,
+        help=f"List of upstream verification errors to ignore. Choices: {_choices}."
+        f" Default: {[v.name for v in LENIENT_FILE_ERROR_IGNORES]}.",
     )
     verify_upstream_parser.add_argument(
         "-O",
         "--error-output",
         type=Path,
         help="Path to a file to output errors",
+        required=True,
     )
     verify_upstream_parser.add_argument(
         "--tree-dir",

src/antsibull/from_source/clone.py

@@ -25,11 +25,12 @@
 from antsibull.utils.paths import copytree_and_symlinks
 
 from .exceptions import CloneError
+from .verify import FileError, FileErrorOutput
 
 if TYPE_CHECKING:
     from _typeshed import StrPath
 
-mlog = log.fields(mlog=__name__)
+mlog = log.fields(mod=__name__)
 
 
 async def clone_collection(
@@ -95,7 +96,7 @@ class NormalizedCheckout:
     normalized_data: dict[str, Any]
     data_changed: bool
     collection_directory: Path | None
-    errors: list[str] = dataclasses.field(default_factory=list)
+    errors: list[FileErrorOutput] = dataclasses.field(default_factory=list)
 
     def __post_init__(self) -> None:
         self.collection_directory = self._guess_collection_directory()
@@ -158,6 +159,7 @@ async def normalize_clone(
             A tag dictionary. See `antsibull.tagging.get_collections_tags()`.
     """
     flog = mlog.fields(func="normalize_clone", collection=collection)
+    errors: list[FileErrorOutput] = []
 
     collection_directory = checkout_dir / tag_data.get("collection_directory", "./")
     galaxy_path = collection_directory / "galaxy.yml"
@@ -175,6 +177,13 @@ async def normalize_clone(
             f" and galaxy metadata says {gotten_collection!r}"
         )
         flog.warning(msg)
+        errors.append(
+            {
+                "file": "galaxy.yml",
+                "error": FileError.WRONG_GALAXY_YML_NAME,
+                "message": f"`{gotten_collection}` != `{collection}`",
+            }
+        )
 
     # Some collections set version to nil and change it dynamically...
     # Others don't set version at all, hence the .get()
@@ -192,7 +201,7 @@ async def normalize_clone(
     if changed := galaxy_data != new_data:
         store_yaml_file(galaxy_path, new_data)
     return NormalizedCheckout(
-        collection, tag_data, checkout_dir, new_data, changed, None
+        collection, tag_data, checkout_dir, new_data, changed, None, errors
     )
 
 

src/antsibull/from_source/commands.py

@@ -11,19 +11,14 @@
 
 import asyncio
 import json
-from collections.abc import AsyncIterator, Iterable
+from collections.abc import Collection, Iterable
 from pathlib import Path
 from typing import TYPE_CHECKING, Any
 
-import aiofiles
-import aiofiles.os
-import aiofiles.ospath
 import asyncio_pool  # type: ignore[import]
 from antsibull_core import app_context
 from antsibull_core.subprocess_util import async_log_run
-from antsibull_core.utils.hashing import verify_hash
-from antsibull_core.yaml import _SafeDumper, load_yaml_file
-from yaml import dump as dump_yaml_str
+from antsibull_core.yaml import load_yaml_file, store_yaml_file
 
 from antsibull.build_ansible_commands import download_collections
 from antsibull.tagging import CollectionTagData
@@ -32,50 +27,12 @@
 
 from ._utils import filter_tag_data, tag_data_as_version_mapping
 from .clone import NormalizedCheckout, clone_collection, normalize_clone
+from .verify import FileError, verify_files
 
 if TYPE_CHECKING:
     from _typeshed import StrPath
 
 
-async def verify_files(
-    collection: str,
-    collection_dir: Path,
-    files_data: list[dict[str, Any]],
-    allow_missing=True,
-) -> AsyncIterator[str]:
-    """
-    Ensure that files in a collection git repository checkout match the
-    artifact's FILES.json metadata
-
-    Args:
-        checkout_dir:
-            Directory containing a collection checkout
-        files_data:
-            The `files` list from a collection artifact's FILES.json metadata
-        allow_missing:
-            Whether to allow files in the collection artifact that are missing
-            from FILES.json.
-            We'd prefer that they didn't, but some collections may include
-            generated files in collection artifacts, so this is allowed for now.
-
-    Yields:
-        Files whose checksums diverge between the checkout_dir and FILES.json metadata
-    """
-    del collection  # Not used for now. This shuts up the linter.
-    for file in files_data:
-        path = collection_dir / file["name"]
-        is_dir = file["ftype"] == "dir"
-        okay: bool = True
-        if not await aiofiles.ospath.exists(path):
-            okay = allow_missing
-        elif is_dir:
-            okay = await aiofiles.ospath.isdir(path)
-        else:
-            okay = await verify_hash(path, file["chksum_sha256"], "sha256")
-        if not okay:
-            yield file["name"]
-
-
 async def _extract_files_data(collection: StrPath) -> list[dict[str, Any]]:
     data = (await async_log_run(["tar", "-Oxf", collection, "FILES.json"])).stdout
     return json.loads(data)["files"]
@@ -104,27 +61,28 @@ async def _handle_collection(
     tag_data: CollectionTagData,
     artifact: Path,
     checkout: Path,
-    allow_missing: bool,
+    ignore_errors: Collection[FileError] | None,
 ) -> NormalizedCheckout:
     norm = await normalize_clone(collection, checkout, tag_data)
-    missing_files = [
-        f
-        async for f in verify_files(
-            collection,
-            checkout / norm.collection_subdir,
-            (await _extract_files_data(artifact)),
-            allow_missing=allow_missing,
-        )
-    ]
-    norm.errors.extend([f"{f} does not match" for f in missing_files])
+    norm.errors.extend(
+        [
+            output
+            async for output in verify_files(
+                collection,
+                checkout / norm.collection_subdir,
+                (await _extract_files_data(artifact)),
+                ignore_errors,
+            )
+        ]
+    )
     return norm
 
 
 async def _handle_collections(
     tag_data: dict[CollectionName, CollectionTagData],
     artifacts: dict[CollectionName, str],
     checkouts: dict[CollectionName, Path],
-    allow_missing: bool = True,
+    ignore_errors: Collection[FileError] | None,
 ) -> list[NormalizedCheckout]:
     lib_ctx = app_context.lib_ctx.get()
     async with asyncio_pool.AioPool(size=lib_ctx.thread_max) as pool:
@@ -135,7 +93,7 @@ async def _handle_collections(
                     tag_data[collection],
                     Path(artifacts[collection]),
                     checkouts[collection],
-                    allow_missing,
+                    ignore_errors,
                 )
             )
             for collection in tag_data
@@ -172,20 +130,27 @@ def verify_upstream_command() -> int:
 
 async def _verify_upstream_command() -> int:
     app_ctx = app_context.app_ctx.get()
+    tags_file: Path = app_ctx.extra["tags_file"]
+    globs: list[str] | None = app_ctx.extra["globs"]
+    download_dir: Path | None = app_ctx.extra["download_dir"]
+    checkouts_dir_: Path | None = app_ctx.extra["checkouts_dir"]
+    tree_dir: Path | None = app_ctx.extra["tree_dir"]
+    ignores: Collection[FileError] = app_ctx.extra["ignores"]
+    error_output: Path = app_ctx.extra["error_output"]
+
     tags_data: dict[CollectionName, CollectionTagData] = make_collection_mapping(
-        load_yaml_file(app_ctx.extra["tags_file"])
+        load_yaml_file(tags_file)
     )
-    tags_data = filter_tag_data(tags_data, app_ctx.extra["globs"])
+    tags_data = filter_tag_data(tags_data, globs)
     versions = tag_data_as_version_mapping(tags_data)
-    async with atemp_or_dir(app_ctx.extra["download_dir"]) as download_dir:
+    async with atemp_or_dir(download_dir) as download_dir:
         artifacts_dir = download_dir / "collection_artifacts"
         artifacts_dir.mkdir(parents=True)
         checkouts_dir = Path(
-            (app_ctx.extra["checkouts_dir"] or (download_dir / "checkouts")),
+            (checkouts_dir_ or (download_dir / "checkouts")),
             "ansible_collections",
         )
         checkouts_dir.mkdir(parents=True)
-        tree_dir: Path | None = app_ctx.extra["tree_dir"]
         if tree_dir:
             tree_dir /= "ansible_collections"
             tree_dir.mkdir(parents=True)
@@ -201,7 +166,7 @@ async def _verify_upstream_command() -> int:
         checkouts = await _clone_collections(tags_data, checkouts_dir)
 
         normed_collections = await _handle_collections(
-            tags_data, artifacts, checkouts, app_ctx.extra["allow_missing"]
+            tags_data, artifacts, checkouts, ignores
         )
         error_blob = {
             "collections": {
@@ -211,18 +176,13 @@ async def _verify_upstream_command() -> int:
             }
         }
 
-        error_yaml = dump_yaml_str(error_blob, Dumper=_SafeDumper)
-        if app_ctx.extra["print_errors"]:
-            print(error_yaml)
-        error_output: Path | None = app_ctx.extra["error_output"]
-        if error_output:
-            error_output.write_text(error_yaml, "utf-8")
+        store_yaml_file(error_output, error_blob)
 
         if tree_dir:
             # Create symlinks to collections in checkouts_dir instead of
             # copying if checkouts_dir is explicitly passed by the user as
             # opposed to being a temporary download directory.
-            allow_symlink = bool(app_ctx.extra["checkouts_dir"])
+            allow_symlink = bool(checkouts_dir_)
             #
             await _symlink_tree(normed_collections, tree_dir, allow_symlink)
     return bool(error_blob)