Skip to content

Conversation

KAWAHARA-souta
Copy link
Contributor

@KAWAHARA-souta KAWAHARA-souta commented Jul 2, 2025

part of #760

First off, the BomRefHelper and LicenseRepositoryHelper classes, which you can find in cyclonedx/serialization/init.py, are already deprecated. We can fix this by applying the @deprecated annotation to them. On the other hand, the License ExpressionAcknowledgement symbol in cyclonedx/model/license.py is also deprecated, but since it's not a class, function, or method, we can't use the @deprecated annotation there. I haven't made any changes to it in this patch.

Currently, most of our DeprecationWarning outputs happen when a class attribute is deprecated. Take the BomMetaData class, for instance:

@serializable.serializable_class
class BomMetaData:
    def __init__(
        self, *,
        ... many other args ... 
        # Deprecated as of v1.6
        manufacture: Optional[OrganizationalEntity] = None,
    ) -> None:
        ... many other codes ...
        self.manufacture = manufacture
        if manufacture:
            warn(
                '`bom.metadata.manufacture` is deprecated from CycloneDX v1.6 onwards. '
                'Please use `bom.metadata.component.manufacturer` instead.',
                DeprecationWarning)

Because of this, we can't simply replace these direct warn() calls with @deprecated annotations.

We also looked into other parts of the source code that are deprecated but aren't currently throwing DeprecationWarnings. I've found two candidates: dataClassification and the Tool class, both of which might have been deprecated in CycloneDX 1.5.

For the Tool class, it's pretty clear that tool-related data should now be stored in other classes, like Component. This means the Tool class itself should be deprecated, and since it's a class, we can use the @deprecated annotation on it.

Conversely, the dataClassification class has a different story. Here, it's more about where the data is placed rather than the class itself being deprecated.
So, similar to existing implementations, I've added a warn() call to the Service class, which holds a dataClassification instance as an attribute, to emit the warning there.

@KAWAHARA-souta KAWAHARA-souta requested a review from a team as a code owner July 2, 2025 23:14
Souta Kawahara added 2 commits July 3, 2025 08:16
Signed-off-by: Souta Kawahara <souta.kawahara@almalinux.org>
Signed-off-by: Souta Kawahara <souta.kawahara@almalinux.org>
@KAWAHARA-souta KAWAHARA-souta force-pushed the update-deprecation-warning branch from dd002bf to 61c5449 Compare July 2, 2025 23:18
…ocations

Signed-off-by: Souta Kawahara <souta.kawahara@almalinux.org>
@KAWAHARA-souta KAWAHARA-souta force-pushed the update-deprecation-warning branch from 61c5449 to 223ebf7 Compare July 3, 2025 04:26
@KAWAHARA-souta
Copy link
Contributor Author

KAWAHARA-souta commented Jul 3, 2025

mypy tests seem to be failing.
The error message is as follows:

mypy-lowest: commands[0]> poetry run mypy --python-version=3.9
cyclonedx/serialization/__init__.py:31: error: Module "warnings" has no
attribute "deprecated"  [attr-defined]
        from warnings import deprecated
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cyclonedx/model/tool.py:30: error: Module "warnings" has no attribute
"deprecated"  [attr-defined]
        from warnings import deprecated
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Found 2 errors in 2 files (checked 46 source files)

The corresponding source code is as follows:

try:
    from warnings import deprecated
except ImportError:
    from typing_extensions import deprecated

I've implemented this try-except block because the deprecated attribute became part of the standard warnings module in Python 3.13. For earlier Python versions, it falls back to typing_extensions. I think this approach should work correctly at runtime.
Given this, could you please advise on how I should address this mypy error? Do you have any specific policy or recommendation for handling such conditional imports with mypy?

@jkowalleck
Copy link
Member

jkowalleck commented Jul 3, 2025

I've found two candidates: dataClassification and the Tool class, both of which might have been deprecated in CycloneDX 1.5.

[...]

For the Tool class, it's pretty clear that tool-related data should now be stored in other classes, like Component. This means the Tool class itself should be deprecated, and since it's a class

the data models are deprecated from a specification's perspective,
not from a library perspective.
therefore, the respective symbols are not intended to be removed in the future.

I might revert these changes...

as out code clearly states:

[FOO] might be deprecated since CycloneDX 1.[Y], but it is not deprecated in this library.
In fact, this library will try to provide a compatibility layer if needed.

@@ -259,6 +261,10 @@ def data(self) -> 'SortedSet[DataClassification]':

@data.setter
def data(self, data: Iterable[DataClassification]) -> None:
if data:
warn('`@.data` is deprecated from CycloneDX v1.5 onwards. '
Copy link
Member

Choose a reason for hiding this comment

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

revertred. adding new warnings is not in the scope of the ticket, nor shall it be part of this very PR.

please create a new ticket/PR for this topic.

@jkowalleck jkowalleck force-pushed the update-deprecation-warning branch from f8802e1 to 7708f47 Compare July 3, 2025 10:43
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck jkowalleck force-pushed the update-deprecation-warning branch from 7708f47 to 39de6d1 Compare July 3, 2025 10:44
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@@ -37,6 +42,12 @@
from py_serializable import ObjectMetadataLibrary, ViewType


@deprecated(
Copy link
Member

Choose a reason for hiding this comment

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

reverted, as the added statement is not true. the data model itself was never deprecated. it is just the usage in another model, that is deprecated in certain versions of CycloneDX.

@jkowalleck jkowalleck changed the title feat: improve deprecation warnings - PEP-387 feat: decorate deprecated symbols Jul 3, 2025
Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
@jkowalleck
Copy link
Member

thanks for all the effort.

some feedback:

  • the imports with try ... except ... was completely fine. this works in all versions of python on runtime. but this might lead to outdated code easily - so we use comparison against the sys.version_info - so code migration might pick it up and remove dead code.
  • currently, we dont want to deprecate properties - unless we have proper transformers that guarantee backwards compatibility.
  • please dont mix scopes in PRs. i've removed the deprecation warning on Service.data - so this PR can focus on deprecating symbols on code level.
    Feel free to create a different ticket/pullrequst for this.

in general, this library/implementation intents to support many versions of CycloneDX on a semantics level.
we dont plan on removing data model features that a supported version of CycloneDX supports. therefore, we dont want to flag such features(properties) as deprecated on a code level -- this library does not intent to replace the CycloneDX specification/documentation.
but we might warn downstream users on runtime, that they are using CycloneDX features that are deprecated per spec, so they might adapt or just be aware about possible incompatibilities or data loss when serializing the data.

@jkowalleck jkowalleck merged commit 33daaf1 into CycloneDX:main Jul 3, 2025
42 checks passed
@jkowalleck
Copy link
Member

this was released via https://github.com/CycloneDX/cyclonedx-python-lib/releases/tag/v10.4.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants