From: Masahiro Yamada <masahiroy@kernel.org>
To: Rob Herring <robh@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Conor Dooley <conor+dt@kernel.org>,
Nathan Chancellor <nathan@kernel.org>,
Nicolas Schier <nicolas@fjasle.eu>,
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
Marijn Suijten <marijn.suijten@somainline.org>,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kbuild@vger.kernel.org
Subject: Re: [PATCH v2 2/3] dt-bindings: kbuild: Split targets out to separate rules
Date: Tue, 23 Apr 2024 18:34:24 +0900 [thread overview]
Message-ID: <CAK7LNATCgcJ5hhbyyKsbg+1TgN=tJo9y4hNKrrykhQjef_xttg@mail.gmail.com> (raw)
In-Reply-To: <CAL_JsqKiwtVwD6snXqyEuHTx7CB2aZcLGxo7xVL9pVD0qC+8qA@mail.gmail.com>
On Tue, Apr 23, 2024 at 2:46 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sat, Apr 20, 2024 at 1:50 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Sat, Apr 6, 2024 at 7:56 AM Rob Herring <robh@kernel.org> 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
next prev parent reply other threads:[~2024-04-23 9:35 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-05 22:56 [PATCH v2 0/3] dt-bindings: kbuild: Rework build rules and dependencies Rob Herring
2024-04-05 22:56 ` [PATCH v2 1/3] dt-bindings: kbuild: Simplify examples target patsubst Rob Herring
2024-04-05 22:56 ` [PATCH v2 2/3] dt-bindings: kbuild: Split targets out to separate rules Rob Herring
2024-04-20 6:50 ` Masahiro Yamada
2024-04-22 17:46 ` Rob Herring
2024-04-23 9:34 ` Masahiro Yamada [this message]
2024-04-05 22:56 ` [PATCH v2 3/3] dt-bindings: kbuild: Add separate target/dependency for processed-schema.json Rob Herring
2024-04-07 10:59 ` Conor Dooley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAK7LNATCgcJ5hhbyyKsbg+1TgN=tJo9y4hNKrrykhQjef_xttg@mail.gmail.com' \
--to=masahiroy@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dmitry.baryshkov@linaro.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marijn.suijten@somainline.org \
--cc=nathan@kernel.org \
--cc=nicolas@fjasle.eu \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).