| Message ID | 20231220145537.2163811-1-andre.draszik@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | dt-bindings: ignore paths outside kernel for DT_SCHEMA_FILES | expand |
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!
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
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'
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".
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
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.
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
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)
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(-)