[PATCH] dt-bindings: ignore paths outside kernel for DT_SCHEMA_FILES

[PATCH] dt-bindings: ignore paths outside kernel for DT_SCHEMA_FILES

[André Draszik]

If the location of the kernel sources contains the string that we're
filtering for using DT_SCHEMA_FILES, then all schemas will currently be
matched, returned and checked, not just the ones we actually expected.
As an example, if the kernel sources happen to be below a directory
'google', and DT_SCHEMA_FILES=google, everything is checked. More
common examples might be having the sources below people's home
directories that contain the string st or arm and then searching for
those. The list is endless.

Fix this by only matching for schemas below the kernel source's
bindings directory.

Note that I opted for the implementation here so as to not having to
deal with escaping DT_SCHEMA_FILES, which would have been the
alternative if the grep match itself had been updated.

Cc: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 Documentation/devicetree/bindings/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
index 3e886194b043..2323fd5b7cda 100644
--- a/Documentation/devicetree/bindings/Makefile
+++ b/Documentation/devicetree/bindings/Makefile
@@ -28,7 +28,7 @@ $(obj)/%.example.dts: $(src)/%.yaml check_dtschema_version FORCE
 find_all_cmd = find $(srctree)/$(src) \( -name '*.yaml' ! \
 		-name 'processed-schema*' \)
 
-find_cmd = $(find_all_cmd) | grep -F -e "$(subst :," -e ",$(DT_SCHEMA_FILES))"
+find_cmd = $(find_all_cmd) | sed 's|^$(srctree)/$(src)/||' | grep -F -e "$(subst :," -e ",$(DT_SCHEMA_FILES))" | sed 's|^|$(srctree)/$(src)/|'
 CHK_DT_DOCS := $(shell $(find_cmd))
 
 quiet_cmd_yamllint = LINT    $(src)
-- 
2.43.0.472.g3155946c3a-goog

Re: [PATCH] dt-bindings: ignore paths outside kernel for DT_SCHEMA_FILES

[Rob Herring]

On Wed, 20 Dec 2023 14:55:37 +0000, André Draszik wrote:
> If the location of the kernel sources contains the string that we're
> filtering for using DT_SCHEMA_FILES, then all schemas will currently be
> matched, returned and checked, not just the ones we actually expected.
> As an example, if the kernel sources happen to be below a directory
> 'google', and DT_SCHEMA_FILES=google, everything is checked. More
> common examples might be having the sources below people's home
> directories that contain the string st or arm and then searching for
> those. The list is endless.
> 
> Fix this by only matching for schemas below the kernel source's
> bindings directory.
> 
> Note that I opted for the implementation here so as to not having to
> deal with escaping DT_SCHEMA_FILES, which would have been the
> alternative if the grep match itself had been updated.
> 
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---
>  Documentation/devicetree/bindings/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Applied, thanks!

Re: [PATCH] dt-bindings: ignore paths outside kernel for DT_SCHEMA_FILES

[Michal Simek]

On 1/4/24 00:58, Rob Herring wrote:
> 
> On Wed, 20 Dec 2023 14:55:37 +0000, André Draszik wrote:
>> If the location of the kernel sources contains the string that we're
>> filtering for using DT_SCHEMA_FILES, then all schemas will currently be
>> matched, returned and checked, not just the ones we actually expected.
>> As an example, if the kernel sources happen to be below a directory
>> 'google', and DT_SCHEMA_FILES=google, everything is checked. More
>> common examples might be having the sources below people's home
>> directories that contain the string st or arm and then searching for
>> those. The list is endless.
>>
>> Fix this by only matching for schemas below the kernel source's
>> bindings directory.
>>
>> Note that I opted for the implementation here so as to not having to
>> deal with escaping DT_SCHEMA_FILES, which would have been the
>> alternative if the grep match itself had been updated.
>>
>> Cc: Masahiro Yamada <masahiroy@kernel.org>
>> Signed-off-by: André Draszik <andre.draszik@linaro.org>
>> ---
>>   Documentation/devicetree/bindings/Makefile | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
> 
> Applied, thanks!

This patch is causing issue for me. Look at log below.
I am running it directly on the latest linux-next/master.

Thanks,
Michal

$ make DT_SCHEMA_FILES="Documentation/devicetree/bindings/arm/arm,cci-400.yaml" 
dt_binding_check
   HOSTCC  scripts/basic/fixdep
   HOSTCC  scripts/dtc/dtc.o
   HOSTCC  scripts/dtc/flattree.o
   HOSTCC  scripts/dtc/fstree.o
   HOSTCC  scripts/dtc/data.o
   HOSTCC  scripts/dtc/livetree.o
   HOSTCC  scripts/dtc/treesource.o
   HOSTCC  scripts/dtc/srcpos.o
   HOSTCC  scripts/dtc/checks.o
   HOSTCC  scripts/dtc/util.o
   LEX     scripts/dtc/dtc-lexer.lex.c
   YACC    scripts/dtc/dtc-parser.tab.[ch]
   HOSTCC  scripts/dtc/dtc-lexer.lex.o
   HOSTCC  scripts/dtc/dtc-parser.tab.o
   HOSTLD  scripts/dtc/dtc
   HOSTCC  scripts/dtc/libfdt/fdt.o
   HOSTCC  scripts/dtc/libfdt/fdt_ro.o
   HOSTCC  scripts/dtc/libfdt/fdt_wip.o
   HOSTCC  scripts/dtc/libfdt/fdt_sw.o
   HOSTCC  scripts/dtc/libfdt/fdt_rw.o
   HOSTCC  scripts/dtc/libfdt/fdt_strerror.o
   HOSTCC  scripts/dtc/libfdt/fdt_empty_tree.o
   HOSTCC  scripts/dtc/libfdt/fdt_addresses.o
   HOSTCC  scripts/dtc/libfdt/fdt_overlay.o
   HOSTCC  scripts/dtc/fdtoverlay.o
   HOSTLD  scripts/dtc/fdtoverlay
   LINT    Documentation/devicetree/bindings
usage: yamllint [-h] [-] [-c CONFIG_FILE | -d CONFIG_DATA] [--list-files] [-f 
{parsable,standard,colored,github,auto}] [-s] [--no-warnings] [-v] [FILE_OR_DIR ...]
yamllint: error: one of the arguments FILE_OR_DIR - is required
   CHKDT   Documentation/devicetree/bindings/processed-schema.json
   SCHEMA  Documentation/devicetree/bindings/processed-schema.json

Re: [PATCH] dt-bindings: ignore paths outside kernel for DT_SCHEMA_FILES

[André Draszik]

Re: [PATCH] dt-bindings: ignore paths outside kernel for DT_SCHEMA_FILES

[André Draszik]

Hi,

On Mon, 2024-01-15 at 10:20 +0100, Michal Simek wrote:
> This patch is causing issue for me. Look at log below.
> I am running it directly on the latest linux-next/master.
> 
> Thanks,
> Michal
> 
> $ make DT_SCHEMA_FILES="Documentation/devicetree/bindings/arm/arm,cci-400.yaml" 
> dt_binding_check

It'll work if you drop the 'Documentation/devicetree/bindings' part from the path, as
it is implied since bindings can only be in that directory anyway:

    make DT_SCHEMA_FILES="arm/arm,cci-400.yaml" dt_binding_check
    make DT_SCHEMA_FILES="arm,cci-400.yaml" dt_binding_check

both work.

Cheers,
Andre'

Re: [PATCH] dt-bindings: ignore paths outside kernel for DT_SCHEMA_FILES

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

On Mon, Jan 15, 2024 at 09:40:37AM +0000, André Draszik wrote:
> Hi,
> 
> On Mon, 2024-01-15 at 10:20 +0100, Michal Simek wrote:
> > This patch is causing issue for me. Look at log below.
> > I am running it directly on the latest linux-next/master.
> > 
> > Thanks,
> > Michal
> > 
> > $ make DT_SCHEMA_FILES="Documentation/devicetree/bindings/arm/arm,cci-400.yaml" 
> > dt_binding_check
> 
> It'll work if you drop the 'Documentation/devicetree/bindings' part from the path, as
> it is implied since bindings can only be in that directory anyway:
> 
>     make DT_SCHEMA_FILES="arm/arm,cci-400.yaml" dt_binding_check
>     make DT_SCHEMA_FILES="arm,cci-400.yaml" dt_binding_check
> 
> both work.

Requiring that is pretty user unfriendly though I think, passing the
full path from the root directory of the kernel tree would be my
assumption of the "default".

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: ignore paths outside kernel for DT_SCHEMA_FILES

[Michal Simek]

On 1/15/24 17:29, Conor Dooley wrote:
> On Mon, Jan 15, 2024 at 09:40:37AM +0000, André Draszik wrote:
>> Hi,
>>
>> On Mon, 2024-01-15 at 10:20 +0100, Michal Simek wrote:
>>> This patch is causing issue for me. Look at log below.
>>> I am running it directly on the latest linux-next/master.
>>>
>>> Thanks,
>>> Michal
>>>
>>> $ make DT_SCHEMA_FILES="Documentation/devicetree/bindings/arm/arm,cci-400.yaml"
>>> dt_binding_check
>>
>> It'll work if you drop the 'Documentation/devicetree/bindings' part from the path, as
>> it is implied since bindings can only be in that directory anyway:
>>
>>      make DT_SCHEMA_FILES="arm/arm,cci-400.yaml" dt_binding_check
>>      make DT_SCHEMA_FILES="arm,cci-400.yaml" dt_binding_check
>>
>> both work.
> 
> Requiring that is pretty user unfriendly though I think, passing the
> full path from the root directory of the kernel tree would be my
> assumption of the "default".

I am using full path like this for years.
I can fix my scripts but would be good to consider correct path inside the 
kernel is something what this patch should also allow.
Because path above is correct and it is not outside of the kernel that's why at 
least commit message should be massage a little bit.
I will let Rob to decide.

Thanks,
Michal

Re: [PATCH] dt-bindings: ignore paths outside kernel for DT_SCHEMA_FILES

[André Draszik]

On Mon, 2024-01-15 at 17:37 +0100, Michal Simek wrote:
> 
> 
> On 1/15/24 17:29, Conor Dooley wrote:
> > On Mon, Jan 15, 2024 at 09:40:37AM +0000, André Draszik wrote:
> > > Hi,
> > > 
> > > On Mon, 2024-01-15 at 10:20 +0100, Michal Simek wrote:
> > > > This patch is causing issue for me. Look at log below.
> > > > I am running it directly on the latest linux-next/master.
> > > > 
> > > > Thanks,
> > > > Michal
> > > > 
> > > > $ make DT_SCHEMA_FILES="Documentation/devicetree/bindings/arm/arm,cci-400.yaml"
> > > > dt_binding_check
> > > 
> > > It'll work if you drop the 'Documentation/devicetree/bindings' part from the path, as
> > > it is implied since bindings can only be in that directory anyway:
> > > 
> > >      make DT_SCHEMA_FILES="arm/arm,cci-400.yaml" dt_binding_check
> > >      make DT_SCHEMA_FILES="arm,cci-400.yaml" dt_binding_check
> > > 
> > > both work.
> > 
> > Requiring that is pretty user unfriendly though I think, passing the
> > full path from the root directory of the kernel tree would be my
> > assumption of the "default".
> 
> I am using full path like this for years.

I just just went by Documentation/devicetree/bindings/writing-schema.rst
which doesn't suggest adding Documentation/devicetree/bindings/. In an
attempt to make it more robust for anybody following this doc, I opted
for the current implementation.

> I can fix my scripts but would be good to consider correct path inside the 
> kernel is something what this patch should also allow.
> Because path above is correct and it is not outside of the kernel that's why at 
> least commit message should be massage a little bit.

I hear you, and I'll make a v2 to not imply the bindings directory.



Cheers,
A.

Re: [PATCH] dt-bindings: ignore paths outside kernel for DT_SCHEMA_FILES

[Rob Herring]

On Mon, Jan 15, 2024 at 10:57 AM André Draszik <andre.draszik@linaro.org> wrote:
>
> On Mon, 2024-01-15 at 17:37 +0100, Michal Simek wrote:
> >
> >
> > On 1/15/24 17:29, Conor Dooley wrote:
> > > On Mon, Jan 15, 2024 at 09:40:37AM +0000, André Draszik wrote:
> > > > Hi,
> > > >
> > > > On Mon, 2024-01-15 at 10:20 +0100, Michal Simek wrote:
> > > > > This patch is causing issue for me. Look at log below.
> > > > > I am running it directly on the latest linux-next/master.
> > > > >
> > > > > Thanks,
> > > > > Michal
> > > > >
> > > > > $ make DT_SCHEMA_FILES="Documentation/devicetree/bindings/arm/arm,cci-400.yaml"
> > > > > dt_binding_check
> > > >
> > > > It'll work if you drop the 'Documentation/devicetree/bindings' part from the path, as
> > > > it is implied since bindings can only be in that directory anyway:
> > > >
> > > >      make DT_SCHEMA_FILES="arm/arm,cci-400.yaml" dt_binding_check
> > > >      make DT_SCHEMA_FILES="arm,cci-400.yaml" dt_binding_check
> > > >
> > > > both work.
> > >
> > > Requiring that is pretty user unfriendly though I think, passing the
> > > full path from the root directory of the kernel tree would be my
> > > assumption of the "default".
> >
> > I am using full path like this for years.
>
> I just just went by Documentation/devicetree/bindings/writing-schema.rst
> which doesn't suggest adding Documentation/devicetree/bindings/. In an
> attempt to make it more robust for anybody following this doc, I opted
> for the current implementation.

It originally worked only with the full tree path. It's now enhanced
to take any substring for a match. As that is preferred (and shorter)
that's what the documentation has.

> > I can fix my scripts but would be good to consider correct path inside the
> > kernel is something what this patch should also allow.
> > Because path above is correct and it is not outside of the kernel that's why at
> > least commit message should be massage a little bit.
>
> I hear you, and I'll make a v2 to not imply the bindings directory.

A follow-up, not a v2 because v1 is already applied.

Rob