| Message ID | 1519776926-30459-6-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | kconfig: some clean-ups and rename silentoldconfig | expand |
On Wed, Feb 28, 2018 at 09:15:25AM +0900, Masahiro Yamada wrote: > The purpose of local{yes,mod}config is to arrange the .config file > based on actually loaded modules. It is unnecessary to update > include/generated/autoconf.h and include/config/* stuff here. > > They will be automatically updated during the build. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- > > Changes in v2: > - newly added > > scripts/kconfig/Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile > index c5d1d1a..bf9289a 100644 > --- a/scripts/kconfig/Makefile > +++ b/scripts/kconfig/Makefile > @@ -49,11 +49,11 @@ localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf (Unrelated: $(obj)/streamline_config.pl is a checked-in file, so I wonder if there's any point to having it as a prerequisite of the phony targets local{yes,mod}config.) > cmp -s .tmp.config .config || \ > (mv -f .config .config.old.1; \ > mv -f .tmp.config .config; \ > - $(obj)/conf $(silent) --silentoldconfig $(Kconfig); \ > + $(obj)/conf $(silent) --oldconfig $(Kconfig); \ Maybe add extra space to keep \ aligned. > mv -f .config.old.1 .config.old) \ > else \ > mv -f .tmp.config .config; \ > - $(obj)/conf $(silent) --silentoldconfig $(Kconfig); \ > + $(obj)/conf $(silent) --oldconfig $(Kconfig); \ Ditto here. > fi > $(Q)rm -f .tmp.config > > -- > 2.7.4 > I'm not an expert on the Makefiles, but seems reasonable to me. Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com> Cheers, Ulf
2018-02-28 14:15 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>: > On Wed, Feb 28, 2018 at 09:15:25AM +0900, Masahiro Yamada wrote: >> The purpose of local{yes,mod}config is to arrange the .config file >> based on actually loaded modules. It is unnecessary to update >> include/generated/autoconf.h and include/config/* stuff here. >> >> They will be automatically updated during the build. >> >> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> >> --- >> >> Changes in v2: >> - newly added >> >> scripts/kconfig/Makefile | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile >> index c5d1d1a..bf9289a 100644 >> --- a/scripts/kconfig/Makefile >> +++ b/scripts/kconfig/Makefile >> @@ -49,11 +49,11 @@ localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf > > (Unrelated: $(obj)/streamline_config.pl is a checked-in file, so I > wonder if there's any point to having it as a prerequisite of the phony > targets local{yes,mod}config.) > Indeed. It it unrelated, but I think it is worth fixing. Care to send a patch, please? -- Best Regards Masahiro Yamada
2018-03-01 20:18 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>: > The local{yes,mod}config targets currently have streamline_config.pl as > a prerequisite. This is redundant, because streamline_config.pl is a > checked-in file with no prerequisites. > > Remove the prerequisite and reference streamline_config.pl directly in > the recipe of the rule instead. > > Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com> > --- > scripts/kconfig/Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile > index 1f74336d4e23..58be52cb464d 100644 Thanks! Almost good. Just small nits. > --- a/scripts/kconfig/Makefile > +++ b/scripts/kconfig/Makefile > @@ -77,9 +77,9 @@ silentoldconfig: $(obj)/conf > touch include/generated/autoksyms.h > $< $(silent) --$@ $(Kconfig) > > -localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf > +localyesconfig localmodconfig: $(obj)/conf > $(Q)mkdir -p include/config include/generated > - $(Q)perl $< --$@ $(srctree) $(Kconfig) > .tmp.config > + $(Q)perl $(obj)/streamline_config.pl --$@ $(srctree) $(Kconfig) > .tmp.config '$(src)/streamline_config.pl' is better than '$(obj)/streamline_config.pl' since it is a checked-in file. '$(src)' and '$(obj)' are always the same. (https://github.com/torvalds/linux/blob/master/scripts/Makefile.build#L6) So, there is no effective difference. It is just a coding convention to use $(obj)/ for generated files, and $(src)/ for source files. The original code already used $(obj)/, so this is not your fault but I want to fix it while we are here. One more nit. $(obj)/conf $(silent) --silentoldconfig $(Kconfig); can be $< $(silent) --silentoldconfig $(Kconfig); I guess you did not touch this line to avoid conflict with my patch. If you agree those two, shall I fix it up when I apply it? > $(Q)if [ -f .config ]; then \ > cmp -s .tmp.config .config || \ > (mv -f .config .config.old.1; \ > -- > 2.14.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards Masahiro Yamada
2018-03-01 23:39 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>: > 2018-03-01 20:18 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>: >> The local{yes,mod}config targets currently have streamline_config.pl as >> a prerequisite. This is redundant, because streamline_config.pl is a >> checked-in file with no prerequisites. >> >> Remove the prerequisite and reference streamline_config.pl directly in >> the recipe of the rule instead. >> >> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com> >> --- >> scripts/kconfig/Makefile | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile >> index 1f74336d4e23..58be52cb464d 100644 > > > Thanks! Almost good. > > Just small nits. > > >> --- a/scripts/kconfig/Makefile >> +++ b/scripts/kconfig/Makefile >> @@ -77,9 +77,9 @@ silentoldconfig: $(obj)/conf >> touch include/generated/autoksyms.h >> $< $(silent) --$@ $(Kconfig) >> >> -localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf >> +localyesconfig localmodconfig: $(obj)/conf >> $(Q)mkdir -p include/config include/generated >> - $(Q)perl $< --$@ $(srctree) $(Kconfig) > .tmp.config >> + $(Q)perl $(obj)/streamline_config.pl --$@ $(srctree) $(Kconfig) > .tmp.config > > > > '$(src)/streamline_config.pl' is better than '$(obj)/streamline_config.pl' > since it is a checked-in file. > > '$(src)' and '$(obj)' are always the same. > (https://github.com/torvalds/linux/blob/master/scripts/Makefile.build#L6) > > > So, there is no effective difference. > It is just a coding convention to use $(obj)/ for generated files, > and $(src)/ for source files. > > The original code already used $(obj)/, so this is not your fault > but I want to fix it while we are here. No. This is not a matter of taste, but a bug that must be fixed. This patch breaks out-of-tree build. If O=... is given, it is error. $ make O=foo localyesconfig make[1]: Leaving directory '/home/masahiro/workspace/linux-yamada/foo' masahiro@grover:~/workspace/linux-yamada$ make O=foo localyesconfig make[1]: Entering directory '/home/masahiro/workspace/linux-yamada/foo' GEN ./Makefile Can't open perl script "scripts/kconfig/streamline_config.pl": No such file or directory ../scripts/kconfig/Makefile:46: recipe for target 'localyesconfig' failed make[2]: *** [localyesconfig] Error 2 /home/masahiro/workspace/linux-yamada/Makefile:521: recipe for target 'localyesconfig' failed make[1]: *** [localyesconfig] Error 2 make[1]: Leaving directory '/home/masahiro/workspace/linux-yamada/foo' Makefile:146: recipe for target 'sub-make' failed make: *** [sub-make] Error 2 The right fix is $(srctree)/$(src)/streamline_config.pl -- Best Regards Masahiro Yamada
2018-03-01 23:44 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>: > 2018-03-01 23:39 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>: >> 2018-03-01 20:18 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>: >>> The local{yes,mod}config targets currently have streamline_config.pl as >>> a prerequisite. This is redundant, because streamline_config.pl is a >>> checked-in file with no prerequisites. >>> >>> Remove the prerequisite and reference streamline_config.pl directly in >>> the recipe of the rule instead. >>> >>> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com> >>> --- >>> scripts/kconfig/Makefile | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile >>> index 1f74336d4e23..58be52cb464d 100644 >> >> >> Thanks! Almost good. >> >> Just small nits. >> >> >>> --- a/scripts/kconfig/Makefile >>> +++ b/scripts/kconfig/Makefile >>> @@ -77,9 +77,9 @@ silentoldconfig: $(obj)/conf >>> touch include/generated/autoksyms.h >>> $< $(silent) --$@ $(Kconfig) >>> >>> -localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf >>> +localyesconfig localmodconfig: $(obj)/conf >>> $(Q)mkdir -p include/config include/generated >>> - $(Q)perl $< --$@ $(srctree) $(Kconfig) > .tmp.config >>> + $(Q)perl $(obj)/streamline_config.pl --$@ $(srctree) $(Kconfig) > .tmp.config >> >> >> >> '$(src)/streamline_config.pl' is better than '$(obj)/streamline_config.pl' >> since it is a checked-in file. >> >> '$(src)' and '$(obj)' are always the same. >> (https://github.com/torvalds/linux/blob/master/scripts/Makefile.build#L6) >> >> >> So, there is no effective difference. >> It is just a coding convention to use $(obj)/ for generated files, >> and $(src)/ for source files. >> >> The original code already used $(obj)/, so this is not your fault >> but I want to fix it while we are here. > > > No. > > This is not a matter of taste, > but a bug that must be fixed. > > > This patch breaks out-of-tree build. > > If O=... is given, it is error. > > > $ make O=foo localyesconfig > make[1]: Leaving directory '/home/masahiro/workspace/linux-yamada/foo' > masahiro@grover:~/workspace/linux-yamada$ make O=foo localyesconfig > make[1]: Entering directory '/home/masahiro/workspace/linux-yamada/foo' > GEN ./Makefile > Can't open perl script "scripts/kconfig/streamline_config.pl": No such > file or directory > ../scripts/kconfig/Makefile:46: recipe for target 'localyesconfig' failed > make[2]: *** [localyesconfig] Error 2 > /home/masahiro/workspace/linux-yamada/Makefile:521: recipe for target > 'localyesconfig' failed > make[1]: *** [localyesconfig] Error 2 > make[1]: Leaving directory '/home/masahiro/workspace/linux-yamada/foo' > Makefile:146: recipe for target 'sub-make' failed > make: *** [sub-make] Error 2 > > > > The right fix is $(srctree)/$(src)/streamline_config.pl > > For somebody wondering why the current code works for out-of-tree building. Kbuild sets VPATH (https://github.com/torvalds/linux/blob/master/Makefile#L208) So, Make searches for pre-requisites in both objtree and srctree when building out-of-tree. VPATH does not work for recipe lines. So, we need to specify the exact path to streamline_config.pl -- Best Regards Masahiro Yamada
On Thu, Mar 1, 2018 at 3:39 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > 2018-03-01 20:18 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>: >> The local{yes,mod}config targets currently have streamline_config.pl as >> a prerequisite. This is redundant, because streamline_config.pl is a >> checked-in file with no prerequisites. >> >> Remove the prerequisite and reference streamline_config.pl directly in >> the recipe of the rule instead. >> >> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com> >> --- >> scripts/kconfig/Makefile | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile >> index 1f74336d4e23..58be52cb464d 100644 > > > Thanks! Almost good. > > Just small nits. > > >> --- a/scripts/kconfig/Makefile >> +++ b/scripts/kconfig/Makefile >> @@ -77,9 +77,9 @@ silentoldconfig: $(obj)/conf >> touch include/generated/autoksyms.h >> $< $(silent) --$@ $(Kconfig) >> >> -localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf >> +localyesconfig localmodconfig: $(obj)/conf >> $(Q)mkdir -p include/config include/generated >> - $(Q)perl $< --$@ $(srctree) $(Kconfig) > .tmp.config >> + $(Q)perl $(obj)/streamline_config.pl --$@ $(srctree) $(Kconfig) > .tmp.config > > > > '$(src)/streamline_config.pl' is better than '$(obj)/streamline_config.pl' > since it is a checked-in file. > > '$(src)' and '$(obj)' are always the same. > (https://github.com/torvalds/linux/blob/master/scripts/Makefile.build#L6) > > > So, there is no effective difference. > It is just a coding convention to use $(obj)/ for generated files, > and $(src)/ for source files. > > The original code already used $(obj)/, so this is not your fault > but I want to fix it while we are here. > > > > One more nit. > > $(obj)/conf $(silent) --silentoldconfig $(Kconfig); > > can be > > $< $(silent) --silentoldconfig $(Kconfig); > > > I guess you did not touch this line to avoid > conflict with my patch. Yep, plus I wanted to keep the patch focused. > > > > If you agree those two, shall I fix it up > when I apply it? That's fine by me. I'll assume less sanity from the existing code from now on. :) > > > > > >> $(Q)if [ -f .config ]; then \ >> cmp -s .tmp.config .config || \ >> (mv -f .config .config.old.1; \ >> -- >> 2.14.1 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > Best Regards > Masahiro Yamada Thanks, Ulf
2018-03-02 0:23 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>: > On Thu, Mar 1, 2018 at 3:39 PM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> 2018-03-01 20:18 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>: >>> The local{yes,mod}config targets currently have streamline_config.pl as >>> a prerequisite. This is redundant, because streamline_config.pl is a >>> checked-in file with no prerequisites. >>> >>> Remove the prerequisite and reference streamline_config.pl directly in >>> the recipe of the rule instead. >>> >>> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com> >>> --- >>> scripts/kconfig/Makefile | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile >>> index 1f74336d4e23..58be52cb464d 100644 >> >> >> Thanks! Almost good. >> >> Just small nits. >> >> >>> --- a/scripts/kconfig/Makefile >>> +++ b/scripts/kconfig/Makefile >>> @@ -77,9 +77,9 @@ silentoldconfig: $(obj)/conf >>> touch include/generated/autoksyms.h >>> $< $(silent) --$@ $(Kconfig) >>> >>> -localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf >>> +localyesconfig localmodconfig: $(obj)/conf >>> $(Q)mkdir -p include/config include/generated >>> - $(Q)perl $< --$@ $(srctree) $(Kconfig) > .tmp.config >>> + $(Q)perl $(obj)/streamline_config.pl --$@ $(srctree) $(Kconfig) > .tmp.config >> >> >> >> '$(src)/streamline_config.pl' is better than '$(obj)/streamline_config.pl' >> since it is a checked-in file. >> >> '$(src)' and '$(obj)' are always the same. >> (https://github.com/torvalds/linux/blob/master/scripts/Makefile.build#L6) >> >> >> So, there is no effective difference. >> It is just a coding convention to use $(obj)/ for generated files, >> and $(src)/ for source files. >> >> The original code already used $(obj)/, so this is not your fault >> but I want to fix it while we are here. >> >> >> >> One more nit. >> >> $(obj)/conf $(silent) --silentoldconfig $(Kconfig); >> >> can be >> >> $< $(silent) --silentoldconfig $(Kconfig); >> >> >> I guess you did not touch this line to avoid >> conflict with my patch. > > Yep, plus I wanted to keep the patch focused. Applied to linux-kbuild/kconfig. Thanks! I changed [1] $(obj)/streamline_config.pl -> $(srctree)/$(src)/streamline_config.pl Mandatory change to avoid out-of-tree build error [2] $(obj)/conf -> $< Clean-up -- Best Regards Masahiro Yamada
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile index c5d1d1a..bf9289a 100644 --- a/scripts/kconfig/Makefile +++ b/scripts/kconfig/Makefile @@ -49,11 +49,11 @@ localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf cmp -s .tmp.config .config || \ (mv -f .config .config.old.1; \ mv -f .tmp.config .config; \ - $(obj)/conf $(silent) --silentoldconfig $(Kconfig); \ + $(obj)/conf $(silent) --oldconfig $(Kconfig); \ mv -f .config.old.1 .config.old) \ else \ mv -f .tmp.config .config; \ - $(obj)/conf $(silent) --silentoldconfig $(Kconfig); \ + $(obj)/conf $(silent) --oldconfig $(Kconfig); \ fi $(Q)rm -f .tmp.config
The purpose of local{yes,mod}config is to arrange the .config file based on actually loaded modules. It is unnecessary to update include/generated/autoconf.h and include/config/* stuff here. They will be automatically updated during the build. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- Changes in v2: - newly added scripts/kconfig/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.7.4