diff --git a/changelogs/fragments/177-removal-updates.yml b/changelogs/fragments/177-removal-updates.yml new file mode 100644 index 0000000..28244e8 --- /dev/null +++ b/changelogs/fragments/177-removal-updates.yml @@ -0,0 +1,4 @@ +minor_changes: + - "Allow information on removed or deprecated collections to be updated. + This is needed to generate a consistent changelog + (https://github.com/ansible-community/antsibull-core/pull/177)." diff --git a/src/antsibull_core/collection_meta.py b/src/antsibull_core/collection_meta.py index a74205d..818410e 100644 --- a/src/antsibull_core/collection_meta.py +++ b/src/antsibull_core/collection_meta.py @@ -17,6 +17,7 @@ import pydantic as p from antsibull_fileutils.yaml import load_yaml_file +from packaging.version import Version as PypiVer from .pydantic import forbid_extras, get_formatted_error_messages from .schemas.collection_meta import ( @@ -24,6 +25,7 @@ CollectionMetadata, CollectionsMetadata, RemovalInformation, + RemovalUpdate, RemovedCollectionMetadata, RemovedRemovalInformation, ) @@ -46,6 +48,95 @@ def __init__(self, *, all_collections: list[str], major_release: int): self.all_collections = all_collections self.major_release = major_release + def _update_state_value( + self, + state: str | None, + accepted_states: list[str | None], + prefix: str, + index: int, + field_name: str, + ) -> str: + if state not in accepted_states: + if state is not None: + self.errors.append( + f"{prefix}[{index}] -> {field_name}: Unexpected update after {state}" + ) + else: + self.errors.append( + f"{prefix}[{index}] -> {field_name}: Unexpected first update" + ) + return field_name + + def _update_state( + self, state: str | None, index: int, update: RemovalUpdate, prefix: str + ) -> tuple[str | None, PypiVer | None, str]: + if update.cancelled_version: + state = self._update_state_value( + state, + [None, "deprecated_version", "redeprecated_version"], + prefix, + index, + "cancelled_version", + ) + return state, update.cancelled_version, "cancelled_version" + if update.deprecated_version: + state = self._update_state_value( + state, [None], prefix, index, "deprecated_version" + ) + return state, update.deprecated_version, "deprecated_version" + if update.redeprecated_version: + state = self._update_state_value( + state, + ["readded_version", "cancelled_version"], + prefix, + index, + "redeprecated_version", + ) + return state, update.redeprecated_version, "redeprecated_version" + if update.removed_version: + state = self._update_state_value( + state, [None], prefix, index, "removed_version" + ) + return state, update.removed_version, "removed_version" + if update.readded_version: + state = self._update_state_value( + state, ["removed_version"], prefix, index, "readded_version" + ) + return state, update.readded_version, "readded_version" + # The following lines should never be reached: + self.errors.append(f"{prefix}[{index}]: Internal error") # pragma: no cover + return state, None, "" # pragma: no cover + + def _validate_removal_updates( + self, + removal: BaseRemovalInformation, + indirect_updates: list[RemovalUpdate], + prefix: str, + ) -> None: + prefix += " -> updates" + state = None + for update in indirect_updates: + state, _, __ = self._update_state(state, -1, update, prefix) + last_version = None + for index, update in enumerate(removal.updates): + state, version, field_name = self._update_state( + state, index, update, prefix + ) + if version is None: + # The following line should never be reached: + pass # pragma: no cover + elif version.major != self.major_release: + self.errors.append( + f"{prefix}[{index}] -> {field_name}: Version's major version {version.major}" + f" must be the current major version {self.major_release}" + ) + elif last_version is not None and version <= last_version: + self.errors.append( + f"{prefix}[{index}] -> {field_name}: Version {version}" + f" must be after the previous update's version {last_version}" + ) + last_version = version + def _validate_removal_base( self, collection: str, removal: BaseRemovalInformation, prefix: str ) -> None: @@ -59,10 +150,14 @@ def _validate_removal( removal.major_version != "TBD" and removal.major_version <= self.major_release # pyre-ignore[58] ): - self.errors.append( - f"{prefix} major_version: Removal major version {removal.major_version} must" - f" be larger than current major version {self.major_release}" + is_ok = removal.major_version == self.major_release and any( + update.removed_version for update in removal.updates ) + if not is_ok: + self.errors.append( + f"{prefix} major_version: Removal major version {removal.major_version} must" + f" be larger than current major version {self.major_release}" + ) if ( removal.announce_version is not None @@ -75,6 +170,17 @@ def _validate_removal( self._validate_removal_base(collection, removal, prefix) + indirect_updates = [] + if removal.announce_version is not None: + indirect_updates.append( + RemovalUpdate( + deprecated_version=removal.announce_version, + reason=removal.reason, + reason_text=removal.reason_text, + ) + ) + self._validate_removal_updates(removal, indirect_updates, prefix) + def _validate_collection( self, collection: str, meta: CollectionMetadata, prefix: str ) -> None: diff --git a/src/antsibull_core/schemas/collection_meta.py b/src/antsibull_core/schemas/collection_meta.py index b5df573..477ac0e 100644 --- a/src/antsibull_core/schemas/collection_meta.py +++ b/src/antsibull_core/schemas/collection_meta.py @@ -44,6 +44,88 @@ def _convert_pypi_version(v: t.Any) -> t.Any: PydanticPypiVersion = Annotated[PypiVer, BeforeValidator(_convert_pypi_version)] +class RemovalUpdate(p.BaseModel): + """ + Stores metadata about removal updates, like when a deprecation has been cancelled, + the collection has been re-deprecated, when a removal has been undone, etc. + """ + + model_config = p.ConfigDict(arbitrary_types_allowed=True) + + # Exactly one of the following must be provided + cancelled_version: t.Optional[PydanticPypiVersion] = None + deprecated_version: t.Optional[PydanticPypiVersion] = None + redeprecated_version: t.Optional[PydanticPypiVersion] = None + removed_version: t.Optional[PydanticPypiVersion] = None + readded_version: t.Optional[PydanticPypiVersion] = None + + # Overwrites the discussion link from BaseRemovalInformation if present + discussion: t.Optional[str] = None + + # If deprecated_version or redeprecated_version: the reason because of which the + # collection will be removed. + reason: t.Optional[ + t.Literal[ + "deprecated", + "considered-unmaintained", + "renamed", + "guidelines-violation", + "other", + ] + ] = None + + # If reason is not provided, or if reason is 'other', an optional extra text appended + # to the message. + reason_text: t.Optional[str] = None + + @p.model_validator(mode="after") # pyre-ignore[56] + def _exactly_one_required(self) -> Self: + count = sum( + 1 if x is not None else 0 + for x in ( + self.cancelled_version, + self.deprecated_version, + self.redeprecated_version, + self.removed_version, + self.readded_version, + ) + ) + if count != 1: + fields = ( + "cancelled_version", + "deprecated_version", + "redeprecated_version", + "removed_version", + "readded_version", + ) + raise ValueError(f"Exactly one of {', '.join(fields)} must be specified") + return self + + @p.model_validator(mode="after") # pyre-ignore[56] + def _check_reason(self) -> Self: + if self.reason and not (self.deprecated_version or self.redeprecated_version): + raise ValueError( + "Reason can only be provided if 'deprecated_version'" + " or 'redeprecated_version' is used" + ) + return self + + @p.model_validator(mode="after") # pyre-ignore[56] + def _check_reason_text(self) -> Self: + reasons_with_text = ("other", "guidelines-violation") + if self.reason in reasons_with_text: + if self.reason_text is None: + raise ValueError( + f"Reason text must be provided if reason is '{self.reason}'" + ) + elif self.reason: + if self.reason_text is not None: + raise ValueError( + f"Reason text must not be provided if reason is '{self.reason}'" + ) + return self + + class BaseRemovalInformation(p.BaseModel): """ Stores metadata on why a collection was/will get removed. @@ -75,6 +157,9 @@ class BaseRemovalInformation(p.BaseModel): # contents have been replaced by deprecated redirects. redirect_replacement_major_version: t.Optional[int] = None + # Updates to the removal + updates: list[RemovalUpdate] = [] + @p.model_validator(mode="after") # pyre-ignore[56] def _check_reason_text(self) -> Self: reasons_with_text = ("other", "guidelines-violation") @@ -135,6 +220,22 @@ def _check_renamed(self) -> Self: ) return self + def get_updates_including_indirect(self) -> list[RemovalUpdate]: + prefix = [] + if self.announce_version: + prefix.append(RemovalUpdate(deprecated_version=self.announce_version)) + return prefix + self.updates + + def is_deprecated(self) -> bool: + result = True + for update in self.get_updates_including_indirect(): + result = bool( + update.deprecated_version + or update.redeprecated_version + or update.removed_version + ) + return result + class RemovedRemovalInformation(BaseRemovalInformation): """ diff --git a/tests/functional/test_collection_meta.py b/tests/functional/test_collection_meta.py index 2914b7b..a864cdb 100644 --- a/tests/functional/test_collection_meta.py +++ b/tests/functional/test_collection_meta.py @@ -11,6 +11,8 @@ from antsibull_core.schemas.collection_meta import ( CollectionMetadata, CollectionsMetadata, + RemovalInformation, + RemovalUpdate, ) LINT_COLLECTION_META_DATA = [ @@ -59,6 +61,13 @@ major_version: 7 reason: deprecated announce_version: 10.1.0 + updates: + - removed_version: 9.2.0 + - cancelled_version: 9.3.0 + - cancelled_version: 9.4.0 + - redeprecated_version: 9.4.0 + - removed_version: 9.3.0 + - readded_version: 10.6.0 bad.bar2: repository: https://github.com/ansible-collections/collection_template removal: @@ -76,6 +85,23 @@ reason: considered-unmaintained discussion: https://forum.ansible.com/... announce_version: 9.3.0 + correct.foo10: + repository: https://github.com/ansible-collections/collection_template + removal: + major_version: 9 + reason: considered-unmaintained + discussion: https://forum.ansible.com/... + announce_version: 9.3.0 + updates: + - removed_version: 9.0.0a1 + - readded_version: 9.1.0 + correct.foo11: + repository: https://github.com/ansible-collections/collection_template + removal: + major_version: 9 + reason: considered-unmaintained + discussion: https://forum.ansible.com/... + announce_version: 9.3.0 correct.foo2: repository: https://github.com/ansible-collections/collection_template removal: @@ -89,7 +115,8 @@ major_version: 10 reason: renamed new_name: namespace.name - announce_version: 9.3.0 + updates: + - readded_version: 10.1.0 correct.foo4: repository: https://github.com/ansible-collections/collection_template removal: @@ -161,6 +188,8 @@ [ "foo.bar", "correct.foo1", + "correct.foo10", + "correct.foo11", "correct.foo2", "correct.foo3", "correct.foo4", @@ -174,6 +203,13 @@ [ "The collection list must be sorted; 'baz.bam' must come before foo.bar", "The removed collection list must be sorted; 'bad.baz1' must come before bad.baz2", + "collections -> bad.bar1 -> removal -> -> updates[0] -> removed_version: Unexpected update after deprecated_version", + "collections -> bad.bar1 -> removal -> -> updates[1] -> cancelled_version: Unexpected update after removed_version", + "collections -> bad.bar1 -> removal -> -> updates[2] -> cancelled_version: Unexpected update after cancelled_version", + "collections -> bad.bar1 -> removal -> -> updates[3] -> redeprecated_version: Version 9.4.0 must be after the previous update's version 9.4.0", + "collections -> bad.bar1 -> removal -> -> updates[4] -> removed_version: Unexpected update after redeprecated_version", + "collections -> bad.bar1 -> removal -> -> updates[4] -> removed_version: Version 9.3.0 must be after the previous update's version 9.4.0", + "collections -> bad.bar1 -> removal -> -> updates[5] -> readded_version: Version's major version 10 must be the current major version 9", "collections -> bad.bar1 -> removal -> announce_version: Major version of 10.1.0 must not be larger than the current major version 9", "collections -> bad.bar1 -> removal -> major_version: Removal major version 7 must be larger than current major version 9", "collections -> bad.bar1: Collection not in ansible.in", @@ -181,6 +217,10 @@ "collections -> bad.bar2: Collection not in ansible.in", "collections -> baz.bam -> repository: Required field not provided", "collections -> baz.bam: Collection not in ansible.in", + "collections -> correct.foo10 -> removal -> -> updates[0] -> removed_version: Unexpected update after deprecated_version", + "collections -> correct.foo11 -> removal -> major_version: Removal major version 9 must be larger than current major version 9", + "collections -> correct.foo3 -> removal -> -> updates[0] -> readded_version: Unexpected first update", + "collections -> correct.foo3 -> removal -> -> updates[0] -> readded_version: Version's major version 10 must be the current major version 9", "collections -> foo.bar -> repository: Required field not provided", "collections: No metadata present for not.there", "removed_collections -> bad.baz1 -> repository: Required field not provided", @@ -296,6 +336,30 @@ removal: major_version: 11 reason: foo + bad.foo15: + repository: https://github.com/ansible-collections/collection_template + removal: + major_version: 11 + reason: considered-unmaintained + updates: + - {} + - foo: bar + - cancelled_version: 1.2.3 + extra: bad + - cancelled_version: [] + - cancelled_version: 1.2.3 + deprecated_version: 1.2.3 + redeprecated_version: 1.2.3 + removed_version: 1.2.3 + readded_version: 1.2.3 + - reason_text: foo + - readded_version: 1.2.3 + reason: deprecated + - deprecated_version: 1.2.3 + reason: deprecated + reason_text: bla + - deprecated_version: 1.2.3 + reason: other removed_collections: bad.foo1: repository: https://github.com/ansible-collections/collection_template @@ -313,6 +377,13 @@ reason: renamed new_name: bad.foo3_new redirect_replacement_major_version: 12 + bad.foo4: + repository: https://github.com/ansible-collections/collection_template + removal: + version: 9.0.0 + reason: renamed + new_name: bad.foo3_new + updates: {} extra_stuff: baz """, [], @@ -324,11 +395,19 @@ "collections -> bad.foo13 -> removal -> major_version -> int: Input should be a valid integer, unable to parse string as an integer", "collections -> bad.foo13 -> removal -> major_version -> literal['TBD']: Input should be 'TBD'", "collections -> bad.foo14 -> removal -> reason: Input should be 'deprecated', 'considered-unmaintained', 'renamed', 'guidelines-violation' or 'other'", + "collections -> bad.foo15 -> removal -> updates -> 0: Value error, Exactly one of cancelled_version, deprecated_version, redeprecated_version, removed_version, readded_version must be specified", + "collections -> bad.foo15 -> removal -> updates -> 1 -> foo: Extra inputs are not permitted", + "collections -> bad.foo15 -> removal -> updates -> 2 -> extra: Extra inputs are not permitted", + "collections -> bad.foo15 -> removal -> updates -> 3 -> cancelled_version: Value error, must be a string or PypiVer object, got []", + "collections -> bad.foo15 -> removal -> updates -> 4: Value error, Exactly one of cancelled_version, deprecated_version, redeprecated_version, removed_version, readded_version must be specified", + "collections -> bad.foo15 -> removal -> updates -> 5: Value error, Exactly one of cancelled_version, deprecated_version, redeprecated_version, removed_version, readded_version must be specified", + "collections -> bad.foo15 -> removal -> updates -> 6: Value error, Reason can only be provided if 'deprecated_version' or 'redeprecated_version' is used", + "collections -> bad.foo15 -> removal -> updates -> 7: Value error, Reason text must not be provided if reason is 'deprecated'", + "collections -> bad.foo15 -> removal -> updates -> 8: Value error, Reason text must be provided if reason is 'other'", "collections -> bad.foo2 -> removal -> announce_version: Value error, must be a version with three release numbers (e.g. 1.2.3, 2.3.4a1), got '9.3'", "collections -> bad.foo3 -> removal -> announce_version: Value error, must be a non-trivial string, got ''", "collections -> bad.foo4 -> removal: Value error, major_version must not be TBD if reason is not 'renamed'", - "collections -> bad.foo5 -> removal: Value error, " - "redirect_replacement_major_version must be smaller than major_version", + "collections -> bad.foo5 -> removal: Value error, redirect_replacement_major_version must be smaller than major_version", "collections -> bad.foo6 -> removal: Value error, reason_text must not be provided if reason is not 'other', 'guidelines-violation'", "collections -> bad.foo7 -> removal: Value error, reason_text must be provided if reason is 'other', 'guidelines-violation'", "collections -> bad.foo8 -> removal: Value error, new_name must not be provided if reason is not 'renamed'", @@ -337,6 +416,7 @@ "removed_collections -> bad.foo1 -> removal -> version: Value error, Invalid version: 'TBD'", "removed_collections -> bad.foo2 -> removal -> version: Field required", "removed_collections -> bad.foo3 -> removal: Value error, redirect_replacement_major_version must be smaller than version's major version", + "removed_collections -> bad.foo4 -> removal -> updates: Input should be a valid list", ], ), ] @@ -418,3 +498,51 @@ def test_collections_metadata_methods(tmp_path: Path): assert "not.there" not in meta.collections assert meta.get_meta("not.there") == CollectionMetadata() assert "not.there" in meta.collections + + +def test_removal_info_methods(): + ri = RemovalInformation(reason="considered-unmaintained", major_version=10) + assert ri.get_updates_including_indirect() == [] + assert ri.is_deprecated() is True + + ri = RemovalInformation( + reason="considered-unmaintained", + major_version=10, + announce_version="1.2.0", + updates=[ + RemovalUpdate(cancelled_version="1.3.0"), + RemovalUpdate(redeprecated_version="1.4.0"), + RemovalUpdate(removed_version="1.5.0"), + ], + ) + assert ri.get_updates_including_indirect() == [ + RemovalUpdate(deprecated_version="1.2.0"), + RemovalUpdate(cancelled_version="1.3.0"), + RemovalUpdate(redeprecated_version="1.4.0"), + RemovalUpdate(removed_version="1.5.0"), + ] + assert ri.is_deprecated() is True + + ri = RemovalInformation( + reason="considered-unmaintained", + major_version=10, + announce_version="1.2.0", + updates=[ + RemovalUpdate(cancelled_version="1.3.0"), + RemovalUpdate(redeprecated_version="1.4.0"), + RemovalUpdate(removed_version="1.5.0"), + RemovalUpdate(readded_version="1.6.0"), + RemovalUpdate(deprecated_version="1.7.0"), + RemovalUpdate(cancelled_version="1.8.0"), + ], + ) + assert ri.get_updates_including_indirect() == [ + RemovalUpdate(deprecated_version="1.2.0"), + RemovalUpdate(cancelled_version="1.3.0"), + RemovalUpdate(redeprecated_version="1.4.0"), + RemovalUpdate(removed_version="1.5.0"), + RemovalUpdate(readded_version="1.6.0"), + RemovalUpdate(deprecated_version="1.7.0"), + RemovalUpdate(cancelled_version="1.8.0"), + ] + assert ri.is_deprecated() is False