Re: [PATCH] kbuild: Split up DT binding build targets

[Masahiro Yamada]

On Thu, Aug 25, 2022 at 5:39 AM Rob Herring <robh@xxxxxxxxxx> wrote:
>
> The DT binding validation target, dt_binding_check, is composed of
> multiple steps which can't be run individually. This resulted in
> the passing of make variables to control which steps were run for
> 'dtbs_check'. Some steps are also doing multiple things in a single rule
> which is error prone[1].
>
> Rework the build to split each of the steps into its own make target.
> This allows users more fine grained control over what's run and makes
> for easier make dependencies.


I do not think it makes the code easier.


A tricky case is that multiple targets run in parallel.


"make  -j$(nproc)  dtbs_check  dt_binding_examples"


Two different threads dive into Documentation/devicetree/bindings/Makefile,
and try to build the same file simultaneously.

If you run the command above, you will see two lines of

  SCHEMA  Documentation/devicetree/bindings/processed-schema.json

processed-schema.json may result in a corrupted file.





> The new targets are:
>
> dt_binding_lint - Runs yamllint on the bindings
> dt_binding_schemas - Validates the binding schemas
> dt_binding_examples - Builds and validates the binding examples


I still do not understand why so many phony targets are necessary.

Why isn't the change as simple as the attached file?


-- 
Best Regards
Masahiro Yamada
diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
index 1eaccf135b30..428eb01d2fc2 100644
--- a/Documentation/devicetree/bindings/Makefile
+++ b/Documentation/devicetree/bindings/Makefile
@@ -3,9 +3,6 @@ DT_DOC_CHECKER ?= dt-doc-validate
 DT_EXTRACT_EX ?= dt-extract-example
 DT_MK_SCHEMA ?= dt-mk-schema
 
-DT_SCHEMA_LINT = $(shell which yamllint || \
-  echo "warning: python package 'yamllint' not installed, skipping" >&2)
-
 DT_SCHEMA_MIN_VERSION = 2022.3
 
 PHONY += check_dtschema_version
@@ -32,13 +29,25 @@ find_cmd = $(find_all_cmd) | grep -F "$(DT_SCHEMA_FILES)"
 CHK_DT_DOCS := $(shell $(find_cmd))
 
 quiet_cmd_yamllint = LINT    $(src)
-      cmd_yamllint = ($(find_cmd) | \
+      cmd_yamllint = if ! command -v yamllint >/dev/null; then \
+                         echo "warning: python package 'yamllint' not installed, skipping" >&2; \
+                         exit 0; \
+                     fi; \
+                     ($(find_cmd) | \
                      xargs -n200 -P$$(nproc) \
-		     $(DT_SCHEMA_LINT) -f parsable -c $(srctree)/$(src)/.yamllint >&2) || true
+                     yamllint -f parsable -c $(srctree)/$(src)/.yamllint >&2) || true; \
+                     touch $@
+
+$(obj)/dt_binding_lint.checked: $(CHK_DT_DOCS) $(src)/.yamllint FORCE
+	$(call if_changed,yamllint)
 
-quiet_cmd_chk_bindings = CHKDT   $@
+quiet_cmd_chk_bindings = CHKDT   $(src)
       cmd_chk_bindings = ($(find_cmd) | \
-                         xargs -n200 -P$$(nproc) $(DT_DOC_CHECKER) -u $(srctree)/$(src)) || true
+                         xargs -n200 -P$$(nproc) $(DT_DOC_CHECKER) -u $(srctree)/$(src)) || true; \
+                         touch $@
+
+$(obj)/dt_binding_schemas.checked: $(CHK_DT_DOCS) check_dtschema_version FORCE
+	$(call if_changed,chk_bindings)
 
 quiet_cmd_mk_schema = SCHEMA  $@
       cmd_mk_schema = f=$$(mktemp) ; \
@@ -46,14 +55,11 @@ quiet_cmd_mk_schema = SCHEMA  $@
                       $(DT_MK_SCHEMA) -j $(DT_MK_SCHEMA_FLAGS) @$$f > $@ ; \
 		      rm -f $$f
 
-define rule_chkdt
-	$(if $(DT_SCHEMA_LINT),$(call cmd,yamllint),)
-	$(call cmd,chk_bindings)
-	$(call cmd,mk_schema)
-endef
-
 DT_DOCS = $(patsubst $(srctree)/%,%,$(shell $(find_all_cmd)))
 
+$(obj)/processed-schema.json: $(DT_DOCS) check_dtschema_version FORCE
+	$(call if_changed,mk_schema)
+
 override DTC_FLAGS := \
 	-Wno-avoid_unnecessary_addr_size \
 	-Wno-graph_child_address \
@@ -64,10 +70,8 @@ override DTC_FLAGS := \
 # Disable undocumented compatible checks until warning free
 override DT_CHECKER_FLAGS ?=
 
-$(obj)/processed-schema.json: $(DT_DOCS) $(src)/.yamllint check_dtschema_version FORCE
-	$(call if_changed_rule,chkdt)
-
 always-y += processed-schema.json
+always-$(CHECK_DT_BINDING) += dt_binding_lint.checked dt_binding_schemas.checked
 always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dts, $(CHK_DT_DOCS))
 always-$(CHECK_DT_BINDING) += $(patsubst $(srctree)/$(src)/%.yaml,%.example.dtb, $(CHK_DT_DOCS))