| Message ID | 20191113210544.1894-1-robh@kernel.org |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | dt-bindings: Improve validation build error handling | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On 11/13/2019 2:05 PM, Rob Herring wrote: > Schema errors can cause make to exit before useful information is > printed. This leaves developers wondering what's wrong. It can be > overcome passing '-k' to make, but that's not an obvious solution. > There's 2 scenarios where this happens. > > When using DT_SCHEMA_FILES to validate with a single schema, any error > in the schema results in processed-schema.yaml being empty causing a > make error. The result is the specific errors in the schema are never > shown because processed-schema.yaml is the first target built. Simply > making processed-schema.yaml last in extra-y ensures the full schema > validation with detailed error messages happen first. > > The 2nd problem is while schema errors are ignored for > processed-schema.yaml, full validation of the schema still runs in > parallel and any schema validation errors will still stop the build when > running validation of dts files. The fix is to not add the schema > examples to extra-y in this case. This means 'dtbs_check' is no longer a > superset of 'dt_binding_check'. Update the documentation to make this > clear. > > Cc: Jeffrey Hugo <jhugo@codeaurora.org> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > Signed-off-by: Rob Herring <robh@kernel.org> I injected a syntax error into a random binding file, and compared the output with and without this patch. This patch makes a massive improvement in giving the user the necessary information to identify and fix issues. Thanks! Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> > --- > Documentation/devicetree/bindings/Makefile | 5 ++++- > Documentation/devicetree/writing-schema.rst | 6 ++++-- > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile > index 5138a2f6232a..646cb3525373 100644 > --- a/Documentation/devicetree/bindings/Makefile > +++ b/Documentation/devicetree/bindings/Makefile > @@ -12,7 +12,6 @@ $(obj)/%.example.dts: $(src)/%.yaml FORCE > $(call if_changed,chk_binding) > > DT_TMP_SCHEMA := processed-schema.yaml > -extra-y += $(DT_TMP_SCHEMA) > > quiet_cmd_mk_schema = SCHEMA $@ > cmd_mk_schema = $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $(real-prereqs) > @@ -26,8 +25,12 @@ DT_DOCS = $(shell \ > > DT_SCHEMA_FILES ?= $(addprefix $(src)/,$(DT_DOCS)) > > +ifeq ($(CHECK_DTBS),) > extra-y += $(patsubst $(src)/%.yaml,%.example.dts, $(DT_SCHEMA_FILES)) > extra-y += $(patsubst $(src)/%.yaml,%.example.dt.yaml, $(DT_SCHEMA_FILES)) > +endif > > $(obj)/$(DT_TMP_SCHEMA): $(DT_SCHEMA_FILES) FORCE > $(call if_changed,mk_schema) > + > +extra-y += $(DT_TMP_SCHEMA) > diff --git a/Documentation/devicetree/writing-schema.rst b/Documentation/devicetree/writing-schema.rst > index 3fce61cfd649..efcd5d21dc2b 100644 > --- a/Documentation/devicetree/writing-schema.rst > +++ b/Documentation/devicetree/writing-schema.rst > @@ -133,11 +133,13 @@ binding schema. All of the DT binding documents can be validated using the > > make dt_binding_check > > -In order to perform validation of DT source files, use the `dtbs_check` target:: > +In order to perform validation of DT source files, use the ``dtbs_check`` target:: > > make dtbs_check > > -This will first run the `dt_binding_check` which generates the processed schema. > +Note that ``dtbs_check`` will skip any binding schema files with errors. It is > +necessary to use ``dt_binding_check`` to get all the validation errors in the > +binding schema files. > > It is also possible to run checks with a single schema file by setting the > ``DT_SCHEMA_FILES`` variable to a specific schema file. >
On Thu, Nov 14, 2019 at 9:21 AM Jeffrey Hugo <jhugo@codeaurora.org> wrote: > > On 11/13/2019 2:05 PM, Rob Herring wrote: > > Schema errors can cause make to exit before useful information is > > printed. This leaves developers wondering what's wrong. It can be > > overcome passing '-k' to make, but that's not an obvious solution. > > There's 2 scenarios where this happens. > > > > When using DT_SCHEMA_FILES to validate with a single schema, any error > > in the schema results in processed-schema.yaml being empty causing a > > make error. The result is the specific errors in the schema are never > > shown because processed-schema.yaml is the first target built. Simply > > making processed-schema.yaml last in extra-y ensures the full schema > > validation with detailed error messages happen first. > > > > The 2nd problem is while schema errors are ignored for > > processed-schema.yaml, full validation of the schema still runs in > > parallel and any schema validation errors will still stop the build when > > running validation of dts files. The fix is to not add the schema > > examples to extra-y in this case. This means 'dtbs_check' is no longer a > > superset of 'dt_binding_check'. Update the documentation to make this > > clear. > > > > Cc: Jeffrey Hugo <jhugo@codeaurora.org> > > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > > Signed-off-by: Rob Herring <robh@kernel.org> > > I injected a syntax error into a random binding file, and compared the > output with and without this patch. This patch makes a massive > improvement in giving the user the necessary information to identify and > fix issues. Thanks! BTW, update dtschema and you'll get better (or more at least) messages when 'is not valid under any of the given schemas' errors occur. > Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> Thanks. Rob
On Fri, Nov 15, 2019 at 1:34 AM Rob Herring <robh@kernel.org> wrote: > > On Thu, Nov 14, 2019 at 9:21 AM Jeffrey Hugo <jhugo@codeaurora.org> wrote: > > > > On 11/13/2019 2:05 PM, Rob Herring wrote: > > > Schema errors can cause make to exit before useful information is > > > printed. This leaves developers wondering what's wrong. It can be > > > overcome passing '-k' to make, but that's not an obvious solution. > > > There's 2 scenarios where this happens. > > > > > > When using DT_SCHEMA_FILES to validate with a single schema, any error > > > in the schema results in processed-schema.yaml being empty causing a > > > make error. The result is the specific errors in the schema are never > > > shown because processed-schema.yaml is the first target built. Simply > > > making processed-schema.yaml last in extra-y ensures the full schema > > > validation with detailed error messages happen first. > > > > > > The 2nd problem is while schema errors are ignored for > > > processed-schema.yaml, full validation of the schema still runs in > > > parallel and any schema validation errors will still stop the build when > > > running validation of dts files. The fix is to not add the schema > > > examples to extra-y in this case. This means 'dtbs_check' is no longer a > > > superset of 'dt_binding_check'. Update the documentation to make this > > > clear. > > > > > > Cc: Jeffrey Hugo <jhugo@codeaurora.org> > > > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > > > Signed-off-by: Rob Herring <robh@kernel.org> > > > > I injected a syntax error into a random binding file, and compared the > > output with and without this patch. This patch makes a massive > > improvement in giving the user the necessary information to identify and > > fix issues. Thanks! Could you show me an example of the injected syntax error, and how this commit will improve the diagnostic? For example, I changed as follows: diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml index 47bc1ac36426..358cb1fa4bb6 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml @@ -89,7 +89,7 @@ required: - clocks allOf: - - if: + - if2: properties: compatible: contains: The tool clearly explains the cause of the error. I am struggling to understand what the current problem is. masahiro@pug:~/ref/linux$ make -j8 ARCH=arm dt_binding_check CHKDT Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml DTC Documentation/devicetree/bindings/arm/altera.example.dt.yaml DTC Documentation/devicetree/bindings/arm/nxp/lpc32xx.example.dt.yaml DTC Documentation/devicetree/bindings/arm/pmu.example.dt.yaml DTC Documentation/devicetree/bindings/arm/rockchip.example.dt.yaml DTC Documentation/devicetree/bindings/arm/altera/socfpga-clk-manager.example.dt.yaml DTC Documentation/devicetree/bindings/arm/primecell.example.dt.yaml DTC Documentation/devicetree/bindings/arm/renesas.example.dt.yaml CHECK Documentation/devicetree/bindings/arm/nxp/lpc32xx.example.dt.yaml CHECK Documentation/devicetree/bindings/arm/pmu.example.dt.yaml CHECK Documentation/devicetree/bindings/arm/altera.example.dt.yaml CHECK Documentation/devicetree/bindings/arm/rockchip.example.dt.yaml CHECK Documentation/devicetree/bindings/arm/altera/socfpga-clk-manager.example.dt.yaml CHECK Documentation/devicetree/bindings/arm/primecell.example.dt.yaml CHECK Documentation/devicetree/bindings/arm/renesas.example.dt.yaml /home/masahiro/ref/linux/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml: allOf:0: 'if2' is not one of ['$ref', 'if', 'then', 'else'] make[1]: *** [Documentation/devicetree/bindings/Makefile;12: Documentation/devicetree/bindings/gpu/arm,mali-midgard.example.dts] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile;1266: dt_binding_check] Error 2 > BTW, update dtschema and you'll get better (or more at least) messages > when 'is not valid under any of the given schemas' errors occur. > > > Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> > > Thanks. > > Rob
On Thu, Nov 14, 2019 at 11:59 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > On Fri, Nov 15, 2019 at 1:34 AM Rob Herring <robh@kernel.org> wrote: > > > > On Thu, Nov 14, 2019 at 9:21 AM Jeffrey Hugo <jhugo@codeaurora.org> wrote: > > > > > > On 11/13/2019 2:05 PM, Rob Herring wrote: > > > > Schema errors can cause make to exit before useful information is > > > > printed. This leaves developers wondering what's wrong. It can be > > > > overcome passing '-k' to make, but that's not an obvious solution. > > > > There's 2 scenarios where this happens. > > > > > > > > When using DT_SCHEMA_FILES to validate with a single schema, any error > > > > in the schema results in processed-schema.yaml being empty causing a > > > > make error. The result is the specific errors in the schema are never > > > > shown because processed-schema.yaml is the first target built. Simply > > > > making processed-schema.yaml last in extra-y ensures the full schema > > > > validation with detailed error messages happen first. > > > > > > > > The 2nd problem is while schema errors are ignored for > > > > processed-schema.yaml, full validation of the schema still runs in > > > > parallel and any schema validation errors will still stop the build when > > > > running validation of dts files. The fix is to not add the schema > > > > examples to extra-y in this case. This means 'dtbs_check' is no longer a > > > > superset of 'dt_binding_check'. Update the documentation to make this > > > > clear. > > > > > > > > Cc: Jeffrey Hugo <jhugo@codeaurora.org> > > > > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > > > > Signed-off-by: Rob Herring <robh@kernel.org> > > > > > > I injected a syntax error into a random binding file, and compared the > > > output with and without this patch. This patch makes a massive > > > improvement in giving the user the necessary information to identify and > > > fix issues. Thanks! > > > Could you show me an example of the injected syntax error, > and how this commit will improve the diagnostic? > > > For example, I changed as follows: > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml > b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml > index 47bc1ac36426..358cb1fa4bb6 100644 > --- a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml > @@ -89,7 +89,7 @@ required: > - clocks > > allOf: > - - if: > + - if2: > properties: > compatible: > contains: > > > > The tool clearly explains the cause of the error. > I am struggling to understand what the current problem is. > > > > masahiro@pug:~/ref/linux$ make -j8 ARCH=arm dt_binding_check Before this patch, you should see processed-schema.yaml built first. Clean your tree and do: make -j8 ARCH=arm DT_SCHEMA_FILES=Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml dt_binding_check Building processed-schema.yaml will fail because it will be empty as the 1 file included had an error. > CHKDT Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml > DTC Documentation/devicetree/bindings/arm/altera.example.dt.yaml > DTC Documentation/devicetree/bindings/arm/nxp/lpc32xx.example.dt.yaml > DTC Documentation/devicetree/bindings/arm/pmu.example.dt.yaml > DTC Documentation/devicetree/bindings/arm/rockchip.example.dt.yaml > DTC Documentation/devicetree/bindings/arm/altera/socfpga-clk-manager.example.dt.yaml > DTC Documentation/devicetree/bindings/arm/primecell.example.dt.yaml > DTC Documentation/devicetree/bindings/arm/renesas.example.dt.yaml > CHECK Documentation/devicetree/bindings/arm/nxp/lpc32xx.example.dt.yaml > CHECK Documentation/devicetree/bindings/arm/pmu.example.dt.yaml > CHECK Documentation/devicetree/bindings/arm/altera.example.dt.yaml > CHECK Documentation/devicetree/bindings/arm/rockchip.example.dt.yaml > CHECK Documentation/devicetree/bindings/arm/altera/socfpga-clk-manager.example.dt.yaml > CHECK Documentation/devicetree/bindings/arm/primecell.example.dt.yaml > CHECK Documentation/devicetree/bindings/arm/renesas.example.dt.yaml > /home/masahiro/ref/linux/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml: > allOf:0: 'if2' is not one of ['$ref', 'if', 'then', 'else'] > make[1]: *** [Documentation/devicetree/bindings/Makefile;12: > Documentation/devicetree/bindings/gpu/arm,mali-midgard.example.dts] > Error 1 > make[1]: *** Waiting for unfinished jobs.... > make: *** [Makefile;1266: dt_binding_check] Error 2 > > > > > > > > BTW, update dtschema and you'll get better (or more at least) messages > > when 'is not valid under any of the given schemas' errors occur. > > > > > Tested-by: Jeffrey Hugo <jhugo@codeaurora.org> > > > > Thanks. > > > > Rob > > > > -- > Best Regards > Masahiro Yamada
diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile index 5138a2f6232a..646cb3525373 100644 --- a/Documentation/devicetree/bindings/Makefile +++ b/Documentation/devicetree/bindings/Makefile @@ -12,7 +12,6 @@ $(obj)/%.example.dts: $(src)/%.yaml FORCE $(call if_changed,chk_binding) DT_TMP_SCHEMA := processed-schema.yaml -extra-y += $(DT_TMP_SCHEMA) quiet_cmd_mk_schema = SCHEMA $@ cmd_mk_schema = $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $(real-prereqs) @@ -26,8 +25,12 @@ DT_DOCS = $(shell \ DT_SCHEMA_FILES ?= $(addprefix $(src)/,$(DT_DOCS)) +ifeq ($(CHECK_DTBS),) extra-y += $(patsubst $(src)/%.yaml,%.example.dts, $(DT_SCHEMA_FILES)) extra-y += $(patsubst $(src)/%.yaml,%.example.dt.yaml, $(DT_SCHEMA_FILES)) +endif $(obj)/$(DT_TMP_SCHEMA): $(DT_SCHEMA_FILES) FORCE $(call if_changed,mk_schema) + +extra-y += $(DT_TMP_SCHEMA) diff --git a/Documentation/devicetree/writing-schema.rst b/Documentation/devicetree/writing-schema.rst index 3fce61cfd649..efcd5d21dc2b 100644 --- a/Documentation/devicetree/writing-schema.rst +++ b/Documentation/devicetree/writing-schema.rst @@ -133,11 +133,13 @@ binding schema. All of the DT binding documents can be validated using the make dt_binding_check -In order to perform validation of DT source files, use the `dtbs_check` target:: +In order to perform validation of DT source files, use the ``dtbs_check`` target:: make dtbs_check -This will first run the `dt_binding_check` which generates the processed schema. +Note that ``dtbs_check`` will skip any binding schema files with errors. It is +necessary to use ``dt_binding_check`` to get all the validation errors in the +binding schema files. It is also possible to run checks with a single schema file by setting the ``DT_SCHEMA_FILES`` variable to a specific schema file.
Schema errors can cause make to exit before useful information is printed. This leaves developers wondering what's wrong. It can be overcome passing '-k' to make, but that's not an obvious solution. There's 2 scenarios where this happens. When using DT_SCHEMA_FILES to validate with a single schema, any error in the schema results in processed-schema.yaml being empty causing a make error. The result is the specific errors in the schema are never shown because processed-schema.yaml is the first target built. Simply making processed-schema.yaml last in extra-y ensures the full schema validation with detailed error messages happen first. The 2nd problem is while schema errors are ignored for processed-schema.yaml, full validation of the schema still runs in parallel and any schema validation errors will still stop the build when running validation of dts files. The fix is to not add the schema examples to extra-y in this case. This means 'dtbs_check' is no longer a superset of 'dt_binding_check'. Update the documentation to make this clear. Cc: Jeffrey Hugo <jhugo@codeaurora.org> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> Signed-off-by: Rob Herring <robh@kernel.org> --- Documentation/devicetree/bindings/Makefile | 5 ++++- Documentation/devicetree/writing-schema.rst | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-)