2023-12-20 14:56:01

by André Draszik

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

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 <[email protected]>
Signed-off-by: André Draszik <[email protected]>
---
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



2024-01-03 23:58:38

by Rob Herring

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


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 <[email protected]>
> Signed-off-by: Andr? Draszik <[email protected]>
> ---
> Documentation/devicetree/bindings/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>

Applied, thanks!


2024-01-15 09:21:01

by Michal Simek

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



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 <[email protected]>
>> Signed-off-by: André Draszik <[email protected]>
>> ---
>> 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


2024-01-15 09:40:47

by André Draszik

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

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'


2024-01-15 16:29:37

by Conor Dooley

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

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".


Attachments:
(No filename) (886.00 B)
signature.asc (235.00 B)
Download all attachments

2024-01-15 16:38:13

by Michal Simek

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



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

2024-01-15 16:57:40

by André Draszik

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

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.


2024-01-15 17:43:54

by Rob Herring

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

On Mon, Jan 15, 2024 at 10:57 AM André Draszik <[email protected]> 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