| Message ID | 20220716093122.137494-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: add dtbs_prepare target | expand |
On Sat, 16 Jul 2022 18:31:22 +0900 Masahiro Yamada wrote: > Factor out the common prerequisites for DT compilation into the new > target, dtbs_prepare. > > Add comments in case you wonder why include/config/kernel.release is > the prerequisite. Our policy is that installation targets must not > (re)compile any build artifacts in the tree. If we make modules_install > depend on include/config/kernel.release and it is executed under the > root privilege, it may be owned by root. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > Makefile | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/Makefile b/Makefile > index a9bd55edb75e..8aa4dbb8f878 100644 > --- a/Makefile > +++ b/Makefile > @@ -1367,16 +1367,22 @@ endif > > ifneq ($(dtstree),) > > -%.dtb: include/config/kernel.release scripts_dtc > +%.dtb: dtbs_prepare > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > -%.dtbo: include/config/kernel.release scripts_dtc > +%.dtbo: dtbs_prepare > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ Is there a reason, why both rules are not unified? I guess it is, but I can't see it. > > -PHONY += dtbs dtbs_install dtbs_check > -dtbs: include/config/kernel.release scripts_dtc > +PHONY += dtbs dtbs_prepare dtbs_install dtbs_check > +dtbs: dtbs_prepare > $(Q)$(MAKE) $(build)=$(dtstree) > > +# include/config/kernel.release is not actually required for building DTBs, > +# but for installing DTBs because INSTALL_DTBS_PATH contains $(KERNELRELEASE). > +# We do not want to move it to dtbs_install. The policy is installation > +# targets (, which may run as root) must not modify the tree. Is the comma after the opening parenthesis intended? Kind regards, Nicolas > +dtbs_prepare: include/config/kernel.release scripts_dtc > + > ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),) > export CHECK_DTBS=y > dtbs: dt_binding_check > -- > 2.34.1
On Fri, Jul 22, 2022 at 7:14 PM Nicolas Schier <nicolas@fjasle.eu> wrote: > > On Sat, 16 Jul 2022 18:31:22 +0900 Masahiro Yamada wrote: > > Factor out the common prerequisites for DT compilation into the new > > target, dtbs_prepare. > > > > Add comments in case you wonder why include/config/kernel.release is > > the prerequisite. Our policy is that installation targets must not > > (re)compile any build artifacts in the tree. If we make modules_install > > depend on include/config/kernel.release and it is executed under the > > root privilege, it may be owned by root. > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > --- > > > > Makefile | 14 ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/Makefile b/Makefile > > index a9bd55edb75e..8aa4dbb8f878 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1367,16 +1367,22 @@ endif > > > > ifneq ($(dtstree),) > > > > -%.dtb: include/config/kernel.release scripts_dtc > > +%.dtb: dtbs_prepare > > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > > > -%.dtbo: include/config/kernel.release scripts_dtc > > +%.dtbo: dtbs_prepare > > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > Is there a reason, why both rules are not unified? I guess it is, but > I can't see it. See the GNU Make manual: https://www.gnu.org/software/make/manual/html_node/Pattern-Examples.html The last paragraph, "This pattern rule has two targets ..." %.dtb %.dtbo: dtbs_prepare ... means foo.dtb and foo.dtbo are generated at the same time by the rule. This is strange. > > > > > -PHONY += dtbs dtbs_install dtbs_check > > -dtbs: include/config/kernel.release scripts_dtc > > +PHONY += dtbs dtbs_prepare dtbs_install dtbs_check > > +dtbs: dtbs_prepare > > $(Q)$(MAKE) $(build)=$(dtstree) > > > > +# include/config/kernel.release is not actually required for building DTBs, > > +# but for installing DTBs because INSTALL_DTBS_PATH contains $(KERNELRELEASE). > > +# We do not want to move it to dtbs_install. The policy is installation > > +# targets (, which may run as root) must not modify the tree. > > Is the comma after the opening parenthesis intended? I will rephrase the comment in v2. > > Kind regards, > Nicolas > > > +dtbs_prepare: include/config/kernel.release scripts_dtc > > + > > ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),) > > export CHECK_DTBS=y > > dtbs: dt_binding_check > > -- > > 2.34.1
On Sat, Jul 23, 2022 at 09:33:03AM +0900, Masahiro Yamada wrote: > On Fri, Jul 22, 2022 at 7:14 PM Nicolas Schier <nicolas@fjasle.eu> wrote: > > > > On Sat, 16 Jul 2022 18:31:22 +0900 Masahiro Yamada wrote: > > > Factor out the common prerequisites for DT compilation into the new > > > target, dtbs_prepare. > > > > > > Add comments in case you wonder why include/config/kernel.release is > > > the prerequisite. Our policy is that installation targets must not > > > (re)compile any build artifacts in the tree. If we make modules_install > > > depend on include/config/kernel.release and it is executed under the > > > root privilege, it may be owned by root. > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > --- > > > > > > Makefile | 14 ++++++++++---- > > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > > > diff --git a/Makefile b/Makefile > > > index a9bd55edb75e..8aa4dbb8f878 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -1367,16 +1367,22 @@ endif > > > > > > ifneq ($(dtstree),) > > > > > > -%.dtb: include/config/kernel.release scripts_dtc > > > +%.dtb: dtbs_prepare > > > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > > > > > -%.dtbo: include/config/kernel.release scripts_dtc > > > +%.dtbo: dtbs_prepare > > > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > > > Is there a reason, why both rules are not unified? I guess it is, but > > I can't see it. > > > > See the GNU Make manual: > https://www.gnu.org/software/make/manual/html_node/Pattern-Examples.html > > The last paragraph, "This pattern rule has two targets ..." > > > > %.dtb %.dtbo: dtbs_prepare > ... > > means foo.dtb and foo.dtbo are generated at the same > time by the rule. This is strange. uh, I'm working with Makefile since years but I wasn't aware of that. Thanks a lot for the pointer and explanation! Kind regards, Nicolas > > > > > > > > > -PHONY += dtbs dtbs_install dtbs_check > > > -dtbs: include/config/kernel.release scripts_dtc > > > +PHONY += dtbs dtbs_prepare dtbs_install dtbs_check > > > +dtbs: dtbs_prepare > > > $(Q)$(MAKE) $(build)=$(dtstree) > > > > > > +# include/config/kernel.release is not actually required for building DTBs, > > > +# but for installing DTBs because INSTALL_DTBS_PATH contains $(KERNELRELEASE). > > > +# We do not want to move it to dtbs_install. The policy is installation > > > +# targets (, which may run as root) must not modify the tree. > > > > Is the comma after the opening parenthesis intended? > > > I will rephrase the comment in v2. > > > > > > > > Kind regards, > > Nicolas > > > > > +dtbs_prepare: include/config/kernel.release scripts_dtc > > > + > > > ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),) > > > export CHECK_DTBS=y > > > dtbs: dt_binding_check > > > -- > > > 2.34.1 > > > > -- > Best Regards > Masahiro Yamada
On 16/07/2022 12:31, Masahiro Yamada wrote: > Factor out the common prerequisites for DT compilation into the new > target, dtbs_prepare. > > Add comments in case you wonder why include/config/kernel.release is > the prerequisite. Our policy is that installation targets must not > (re)compile any build artifacts in the tree. If we make modules_install > depend on include/config/kernel.release and it is executed under the > root privilege, it may be owned by root. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> With the comment fixed: Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Tested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > > Makefile | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/Makefile b/Makefile > index a9bd55edb75e..8aa4dbb8f878 100644 > --- a/Makefile > +++ b/Makefile > @@ -1367,16 +1367,22 @@ endif > > ifneq ($(dtstree),) > > -%.dtb: include/config/kernel.release scripts_dtc > +%.dtb: dtbs_prepare > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > -%.dtbo: include/config/kernel.release scripts_dtc > +%.dtbo: dtbs_prepare > $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ > > -PHONY += dtbs dtbs_install dtbs_check > -dtbs: include/config/kernel.release scripts_dtc > +PHONY += dtbs dtbs_prepare dtbs_install dtbs_check > +dtbs: dtbs_prepare > $(Q)$(MAKE) $(build)=$(dtstree) > > +# include/config/kernel.release is not actually required for building DTBs, > +# but for installing DTBs because INSTALL_DTBS_PATH contains $(KERNELRELEASE). > +# We do not want to move it to dtbs_install. The policy is installation > +# targets (, which may run as root) must not modify the tree. > +dtbs_prepare: include/config/kernel.release scripts_dtc > + > ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),) > export CHECK_DTBS=y > dtbs: dt_binding_check
diff --git a/Makefile b/Makefile index a9bd55edb75e..8aa4dbb8f878 100644 --- a/Makefile +++ b/Makefile @@ -1367,16 +1367,22 @@ endif ifneq ($(dtstree),) -%.dtb: include/config/kernel.release scripts_dtc +%.dtb: dtbs_prepare $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ -%.dtbo: include/config/kernel.release scripts_dtc +%.dtbo: dtbs_prepare $(Q)$(MAKE) $(build)=$(dtstree) $(dtstree)/$@ -PHONY += dtbs dtbs_install dtbs_check -dtbs: include/config/kernel.release scripts_dtc +PHONY += dtbs dtbs_prepare dtbs_install dtbs_check +dtbs: dtbs_prepare $(Q)$(MAKE) $(build)=$(dtstree) +# include/config/kernel.release is not actually required for building DTBs, +# but for installing DTBs because INSTALL_DTBS_PATH contains $(KERNELRELEASE). +# We do not want to move it to dtbs_install. The policy is installation +# targets (, which may run as root) must not modify the tree. +dtbs_prepare: include/config/kernel.release scripts_dtc + ifneq ($(filter dtbs_check, $(MAKECMDGOALS)),) export CHECK_DTBS=y dtbs: dt_binding_check
Factor out the common prerequisites for DT compilation into the new target, dtbs_prepare. Add comments in case you wonder why include/config/kernel.release is the prerequisite. Our policy is that installation targets must not (re)compile any build artifacts in the tree. If we make modules_install depend on include/config/kernel.release and it is executed under the root privilege, it may be owned by root. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- Makefile | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)