scripts: Don't fail CI if using deprecated DT prop

[decsny]

Add parameter to EDT class init for enabling a warning if a property on a node in DT is deprecated the default is True since this is the current behavior.

Add argument to gen_defines.py to suppres warning if a DT property appearing on a node is deprecated. If this argument is not set, the current behavior (no suppression) will be used.

Set the gen_defines argument in the twister runner.py so that CI doesn't fail when using a deprecated DT property.

The purpose of this is to allow deprecating DT properties and still using them in-tree if they have not yet been removed from a binding.

[nordicjm]

The purpose of this is to allow deprecating DT properties and still using them in-tree if they have not yet been removed from a binding.

Bit confused with this, if we deprecate something, it should not be used as per https://docs.zephyrproject.org/latest/build/dts/bindings-syntax.html#deprecated

(This warning is upgraded to an error in the Test Runner (Twister) for upstream pull requests.)

[henrikbrixandersen]

The purpose of this is to allow deprecating DT properties and still using them in-tree if they have not yet been removed from a binding.

This goes against our policy. If something is deprecated, it should not be used in-tree. This ensures that a migration strategy is in place whenever we deprecate something, be it code, devicetree bindings, properties, Kconfig options, ...

[decsny]

The purpose of this is to allow deprecating DT properties and still using them in-tree if they have not yet been removed from a binding.

This goes against our policy. If something is deprecated, it should not be used in-tree. This ensures that a migration strategy is in place whenever we deprecate something, be it code, devicetree bindings, properties, Kconfig options, ...

We already have things deprecated in tree and we don't fail CI for it. How is a DT binding any different. The issue is that marking a property as deprecated will cause CI to fail unless you remove all of that thing from tree, when the rule as far as I know is that it should be around for 2 more releases.

[decsny]

The purpose of this is to allow deprecating DT properties and still using them in-tree if they have not yet been removed from a binding.

Bit confused with this, if we deprecate something, it should not be used as per https://docs.zephyrproject.org/latest/build/dts/bindings-syntax.html#deprecated

(This warning is upgraded to an error in the Test Runner (Twister) for upstream pull requests.)

Thanks, I need to update that documentation in this PR too. I don't think using a deprecated property should fail twister.

[decsny]

Also to be clear, the warnings are still there from DTC all the time and from EDT when building without twister.

[dleach02]

The purpose of this is to allow deprecating DT properties and still using them in-tree if they have not yet been removed from a binding.

This goes against our policy. If something is deprecated, it should not be used in-tree. This ensures that a migration strategy is in place whenever we deprecate something, be it code, devicetree bindings, properties, Kconfig options, ...

@henrikbrixandersen, we have a policy to mark something for deprecation to alert the downstream user and then after a period to actually remove the item. What we discovered is that you can't mark a DT prop for deprecation because CI treats the warning generated as an error thus failing CI.

So I'll turn the question around a bit. How would you propose we mark a DT prop for deprecation? Should we just outright delete it and let the downstream just discover the deletion on their own or do we want to provide for a mechanism that is similar to how we deprecate APIs where there is the warning period before it goes away?

[henrikbrixandersen]

@henrikbrixandersen, we have a policy to mark something for deprecation to alert the downstream user and then after a period to actually remove the item. What we discovered is that you can't mark a DT prop for deprecation because CI treats the warning generated as an error thus failing CI.

So I'll turn the question around a bit. How would you propose we mark a DT prop for deprecation? Should we just outright delete it and let the downstream just discover the deletion on their own or do we want to provide for a mechanism that is similar to how we deprecate APIs where there is the warning period before it goes away?

From https://docs.zephyrproject.org/latest/develop/api/api_lifecycle.html#deprecated:

Code using the deprecated API needs to be modified to remove usage of said API

So, deprecation requires removal of all in-tree uses of the deprecated API. Devicetree properties are APIs in this context.

[henrikbrixandersen]

We already have things deprecated in tree and we don't fail CI for it.

@decsny Can you please provide examples of this?

[dleach02]

Code using the deprecated API needs to be modified to remove usage of said API

So, deprecation requires removal of all in-tree uses of the deprecated API. Devicetree properties are APIs in this context.

Okay. Fair enough. We discussed this more in today's standup. This is related to a PR @mmahadevan108 has. He did not remove the references to the property so this is our disconnect.

[gmarull]

deprecated items must not be used in-tree

[nordicjm]

The purpose of this is to allow deprecating DT properties and still using them in-tree if they have not yet been removed from a binding.

This goes against our policy. If something is deprecated, it should not be used in-tree. This ensures that a migration strategy is in place whenever we deprecate something, be it code, devicetree bindings, properties, Kconfig options, ...

We already have things deprecated in tree and we don't fail CI for it. How is a DT binding any different. The issue is that marking a property as deprecated will cause CI to fail unless you remove all of that thing from tree, when the rule as far as I know is that it should be around for 2 more releases.

This is not true, use of deprecated e.g. macros in code will emit warnings and a warning is an instant build failure on CI

[decsny]

The purpose of this is to allow deprecating DT properties and still using them in-tree if they have not yet been removed from a binding.

This goes against our policy. If something is deprecated, it should not be used in-tree. This ensures that a migration strategy is in place whenever we deprecate something, be it code, devicetree bindings, properties, Kconfig options, ...

We already have things deprecated in tree and we don't fail CI for it. How is a DT binding any different. The issue is that marking a property as deprecated will cause CI to fail unless you remove all of that thing from tree, when the rule as far as I know is that it should be around for 2 more releases.

This is not true, use of deprecated e.g. macros in code will emit warnings and a warning is an instant build failure on CI

Deprecated Kconfigs fail CI?

[henrikbrixandersen]

Deprecated Kconfigs fail CI?

As requested earlier - do you have an example of in-tree usage of deprecated APIs/features being used, but not triggering a failure in CI? If yes, that is a bug and needs to be fixed.

[dleach02]

Folks, the disconnect on our side is that when you mark something for deprecation you need to also remove all the intree usage of the deprecated item. We didn't do that on the PR that initiated this particular PR. We concede that point and agree..