edtlib: indirect "include:"s break property filters

[dottspina]

Filters set by an including binding to select the properties it chooses to inherit are not applied as expected when specifications are inherited indirectly (as in A includes B includes C):

• filters set by A are ignored when B includes C
• if both A and B set property filters, these filters are merged then applied to C, where we instead expect that A will filter the outcome of "B includes C"

Passing the simplest (former) test case is easy, but misleading, as fixing the general (later) case might be more involved. [Edit: Reverting #65221 seems to fix all use cases described in this issue.]

Basics

The simplest test case is:

• simple.yaml: a base binding file, specifies some properties at the binding, child-binding and grandchild-binding levels
• simple_inherit.yaml: just include:s the base binding file above (without modification nor filter)
• simple_filter.yaml: top-level binding of the test case, which intends to simply filter the properties it inherits like bellow

include:
  - name: simple_inherit.yaml
    property-allowlist: [prop-1]
    child-binding:
      property-allowlist: [child-prop-1]
      child-binding:
        property-allowlist: [grandchild-prop-1]   

Here we'd expect to finally get only child-prop-1 at the child-binding level and grandchild-prop-1 at the grandchild-binding level: we instead inherit all properties specified in simple.yaml at these levels.

Although this test case could also be addressed on top of #65221, I'm going to fix it by doing a revert (I'll put the rationale for this in the PR). [Edit: see #80030]

Generalization

But there might be a more general issue (independently of #65221).

In my understanding, when B includes I includes X, the filters set by B to select the properties it chooses to inherit should be deferred until they can be applied to the appropriate input:

• first, I includes X, with or without filters
• then B includes I, applying its own filters on top of the properties I inherited from X

Examples that seem natural:

• if B only allows properties which are not in the allowlist of the intermediary binding file (here I), we should end with no property at all
• if B blocks all properties which are in the allowlist of the intermediary binding file, we should end with no property at all

More generally, still IIUC, child-binding filters should compose, not be merged:

• if B allows [x1, x3] and I allows [x1, x2], B should inherit only x1, not [x1, x2, x3]
• if I allows [x1, x2] and B blocks x2, B should inherit x1, not cause an error by merging the allowlist and blocklist from B and I ("should not specify both 'property-allowlist:' and 'property-blocklist:'")
• if B allows [x1, x2] and Y blocks x2, B should inherit x1, not cause an error by merging the allowlist and blocklist from B and I
• if B blocks x2 and I blocks x3, B should inherit x1, not [x1, x3]

This is what I would intuitively expect, assuming it's permitted to set filters in an including binding and in an intermediary included file.

AFAICT, the Zephyr documentation doesn't specifically address the rules that apply to this scenario, but it seems that only filtering simultaneously with an allowlist and a blocklist is explicitly prohibited (Devicetree bindings syntax, Include):

You cannot have a single map element with both property-allowlist and property-blocklist keys.

If this reasoning is correct, implementing it properly seems not obvious to me, it's just not how bindings are initialized: without some design changes we can't compose the filters as we include bindings on top of each other, like B=f1(f2(f3(X))), they're instead merged before use, like in B=(f1xf2xf3)(X).

[Edit: all expectations above are actually honored when #65221 is reverted]

I will also open a PR for this general case, with the corresponding unit tests (doomed to fail), but for now without a fix: I don't see how we could get to B=f1(f2(f3(X))) without rethinking the whole Binding initialization.

[Edit: these unit tests, which are no longer supposed to fail, will be added to #79900 instead.]

Thanks.

[mmahadevan108]

@dottspina , I have assigned this issue to you since you are working on a fix.

[dottspina]

@mmahadevan108, I'm fine with handling this:

• fix under review: edtlib: Revert a regression affecting property filters at the child-binding level and bellow #80030
• comprehensive unit tests available in edtlib: tests: refine coverage of Binding objects initialization #79900 (currently disabled)

[dottspina]

Fixed by #80030.
I'm closing this, feel free to reopen if similar issues appear.
Thanks.