Re: [PATCH v2 2/3] dt-bindings: kbuild: Split targets out to separate rules

[Masahiro Yamada]

On Tue, Apr 23, 2024 at 2:46 AM Rob Herring <robh@xxxxxxxxxx> wrote:
>
> On Sat, Apr 20, 2024 at 1:50 AM Masahiro Yamada <masahiroy@kernelorg> wrote:
> >
> > On Sat, Apr 6, 2024 at 7:56 AM Rob Herring <robh@xxxxxxxxxx> wrote:
> > >
> > > Masahiro pointed out the use of if_changed_rule is incorrect and command
> > > line changes are not correctly accounted for.
> > >
> > > To fix this, split up the DT binding validation target,
> > > dt_binding_check, into multiple rules for each step: yamllint, schema
> > > validtion with meta-schema, and building the processed schema.
> > >
> > > One change in behavior is the yamllint or schema validation will be
> > > re-run again when there are warnings present.
> >
> >
> > Is this intentional?
>
> Yes.
>
> > I think it is annoying to re-run the same check
> > when none of the schemas is updated.
>
> But the *only* reason to run dt_binding_check is to check the
> bindings. When a schema is updated and we re-run the checks, *all* the
> schemas are checked so you will get unrelated warnings as well. That's
> because doing validation a file at a time is too slow. We could fix
> that if there was a way to pass only the out of date dependencies, but
> I didn't see a way to do that with make.
>
> > 'make dt_binding_check' is already warning-free.
>
> Right, so normally it won't re-run. If a developer introduces
> warnings, then they are the only ones annoyed by this behavior and
> that's what we want.
>
> > So, I think it is OK to make it fail if any warning occurs.
>
> Well, it has certainly gotten a lot better and we don't seem to end up
> with last minute things breaking rc1, but I'm not sure I want to go
> back to that yet. We started not erroring out because in the past I've
> gotten broken schemas with the submitter saying they couldn't run the
> checks because of errors. I must be in the minority that runs make
> with --keep-going...
>
> It is also not warning free sometimes with new versions of dtschema. I
> usually fix those in parallel, but if anyone runs newer dtschema on
> older kernels that doesn't help.
>
> I suppose it would be better to keep the current behavior in this
> series and make any changes on erroring out or re-running with
> warnings a separate change.
>
> > Besides, yamllint exists with 0 even if it finds a format error.
> > Hence  "|| true" is not sensible.
>
> yamllint has errors and warnings. errors exit with non-zero. There is
> an option for warnings to return error too. Since we currently don't
> distinguish, I'm not sure if our config is exactly the mix we'd want
> to error out or not. I'll have to look and see before we change that.


OK, then.


I applied all of this series to linux-kbuild.
Thanks.










-- 
Best Regards
Masahiro Yamada