| Message ID | 20210323090217.1131301-1-dominique.martinet@atmark-techno.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | build system: do not rebuild if nothing changed | expand |
And last bump for today :) As said in comment, I don't mind abandonning this patch, but I'd like it to be a concious decision and not just a "oh that look complicated will look at later" which became never. Please just say if you're not interested, or if anything would help (as there are four different changes there I could split into four patches if that makes review easier...) I think at least packagers would be interested, it's a bit weird to have swupdate build in the build phase just to see it being built again in the install phase!! hah. Well it's all automated so most people probably wouldn't notice. Thanks!
On 23.03.21 10:02, Dominique Martinet wrote: > Try to avoid rebuilding things needlessly by: > - removing FORCE from direct output files, trust make dependencies > to do the right thing. > > - make cfg-sanity-check a real file that depends on .config, > so the check can be reexecuted on config change but not otherwise. > > - make tools create tools/built-in.o: that file is expected to exist > and used as timestamp comparison, but nothing creates it currently. > That one is quite ugly as trying to create the file directly would > update it needlessly so an extra "pseudo-phony" target was added. > > - remove KBUILD_STR: the # character was incorrectly truncating > commands in $(cmd_$@), making Kbuild think the command line changed > (can be tested with KBUILD_VERBOSE=2) ; in our case filling simple > quotes should always work for version number, so skip the macro > altogether. > > Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> > --- > > This one is mostly me being over-zealous and nothing strictly required > as swupdate is pretty quick to build, but we might as well spare > ourselves precious seconds everytime we change a single file. > > I checked updated headers, c file, EXTRA_CFLAGS etc all are properly > taken into account so this should not break anything but it's probably > best to test again in different environments just in case... > > Also tested swupdate --version still works. > > > (Actually now I'm looking at it again there might be a problem with the > first point (removing FORCE for direct target), in that we create them > and strip in the same command so if that is interrupted in the middle > of the strip command make would have no way of knowing and would not > attempt to strip again. Worst case a binary is left unstripped so it's > not too bad in my opinion and I'm leaving it as such, a proper solution > would be to do like swupdate with swupdate_unstripped for all tools) > > Makefile | 18 +++++++++++------- > Makefile.flags | 2 +- > scripts/Makefile.lib | 1 - > 3 files changed, 12 insertions(+), 9 deletions(-) > > diff --git a/Makefile b/Makefile > index de15b8f6454e..f005d0286399 100644 > --- a/Makefile > +++ b/Makefile > @@ -388,8 +388,7 @@ bindings-dirs := $(bindings-y) > bindings-libs := $(patsubst %,%/built-in.o, $(bindings-y)) > bindings-all := $(bindings-libs) > > -PHONY += cfg-sanity-check > -cfg-sanity-check: > +.cfg-sanity-check: .config > @if [ "x$(CONFIG_SETSWDESCRIPTION)" = "xy" -a -z "$(patsubst "%",%,$(strip $(CONFIG_SWDESCRIPTION)))" ]; then \ > echo "ERROR: CONFIG_SETSWDESCRIPTION set but not CONFIG_SWDESCRIPTION"; \ > exit 1; \ > @@ -398,6 +397,7 @@ cfg-sanity-check: > echo "ERROR: CONFIG_SETEXTPARSERNAME set but not CONFIG_EXTPARSERNAME"; \ > exit 1; \ > fi > + @touch .cfg-sanity-check > > all: swupdate ${tools-bins} ${lua_swupdate} > > @@ -413,7 +413,7 @@ quiet_cmd_swupdate = LD $@ > "$(swupdate-libs)" \ > "$(LDLIBS)" > > -swupdate_unstripped: ${swupdate-ipc-lib} $(swupdate-all) FORCE > +swupdate_unstripped: ${swupdate-ipc-lib} $(swupdate-all) > $(call if_changed,swupdate) > > quiet_cmd_addon = LD $@ > @@ -437,10 +437,10 @@ quiet_cmd_shared = LD $@ > "" \ > "$(LDLIBS)" > > -lua_swupdate.so.0.1: $(bindings-libs) ${swupdate-ipc-lib} FORCE > +lua_swupdate.so.0.1: $(bindings-libs) ${swupdate-ipc-lib} > $(call if_changed,shared,$(bindings-libs) $(ipc-lib)) > > -${swupdate-ipc-lib}: $(ipc-lib) FORCE > +${swupdate-ipc-lib}: $(ipc-lib) > $(call if_changed,shared,$(ipc-lib)) > > ifeq ($(SKIP_STRIP),y) > @@ -452,10 +452,14 @@ cmd_strip = $(STRIP) -s --remove-section=.note --remove-section=.comment \ > $@_unstripped -o $@; chmod a+x $@ > endif > > -swupdate: cfg-sanity-check swupdate_unstripped > +swupdate: .cfg-sanity-check swupdate_unstripped > $(call cmd,strip) > > -${tools-bins}: ${swupdate-ipc-lib} ${tools-objs} ${swupdate-libs} FORCE > +.tools-built-in: tools/built-in.o > + @touch tools/built-in.o > + @touch .tools-built-in > + > +${tools-bins}: ${swupdate-ipc-lib} ${tools-objs} ${swupdate-libs} .tools-built-in > $(call if_changed,addon,$@.o) > @mv $@ $@_unstripped > $(call cmd,strip) > diff --git a/Makefile.flags b/Makefile.flags > index 7003af14d767..91d959475fe5 100644 > --- a/Makefile.flags > +++ b/Makefile.flags > @@ -16,7 +16,7 @@ endif > KBUILD_CPPFLAGS += $(call cc-option,-std=gnu99,) > > KBUILD_CPPFLAGS += -D_GNU_SOURCE -DNDEBUG \ > - -D"SWU_VER=KBUILD_STR($(SWU_VER))" > + -D"SWU_VER=\"$(SWU_VER)\"" > > KBUILD_CFLAGS += $(call cc-option,-Wall,) > KBUILD_CFLAGS += $(call cc-option,-Wshadow,) > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 19561e0322fb..2f87bfe13cf3 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -82,7 +82,6 @@ endif > > c_flags = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \ > $(__c_flags) $(modkern_cflags) \ > - -D"KBUILD_STR(s)=\#s" > > a_flags = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \ > $(__a_flags) $(modkern_aflags) > Applied to -master, thanks ! Best regards, Stefano Babic
diff --git a/Makefile b/Makefile index de15b8f6454e..f005d0286399 100644 --- a/Makefile +++ b/Makefile @@ -388,8 +388,7 @@ bindings-dirs := $(bindings-y) bindings-libs := $(patsubst %,%/built-in.o, $(bindings-y)) bindings-all := $(bindings-libs) -PHONY += cfg-sanity-check -cfg-sanity-check: +.cfg-sanity-check: .config @if [ "x$(CONFIG_SETSWDESCRIPTION)" = "xy" -a -z "$(patsubst "%",%,$(strip $(CONFIG_SWDESCRIPTION)))" ]; then \ echo "ERROR: CONFIG_SETSWDESCRIPTION set but not CONFIG_SWDESCRIPTION"; \ exit 1; \ @@ -398,6 +397,7 @@ cfg-sanity-check: echo "ERROR: CONFIG_SETEXTPARSERNAME set but not CONFIG_EXTPARSERNAME"; \ exit 1; \ fi + @touch .cfg-sanity-check all: swupdate ${tools-bins} ${lua_swupdate} @@ -413,7 +413,7 @@ quiet_cmd_swupdate = LD $@ "$(swupdate-libs)" \ "$(LDLIBS)" -swupdate_unstripped: ${swupdate-ipc-lib} $(swupdate-all) FORCE +swupdate_unstripped: ${swupdate-ipc-lib} $(swupdate-all) $(call if_changed,swupdate) quiet_cmd_addon = LD $@ @@ -437,10 +437,10 @@ quiet_cmd_shared = LD $@ "" \ "$(LDLIBS)" -lua_swupdate.so.0.1: $(bindings-libs) ${swupdate-ipc-lib} FORCE +lua_swupdate.so.0.1: $(bindings-libs) ${swupdate-ipc-lib} $(call if_changed,shared,$(bindings-libs) $(ipc-lib)) -${swupdate-ipc-lib}: $(ipc-lib) FORCE +${swupdate-ipc-lib}: $(ipc-lib) $(call if_changed,shared,$(ipc-lib)) ifeq ($(SKIP_STRIP),y) @@ -452,10 +452,14 @@ cmd_strip = $(STRIP) -s --remove-section=.note --remove-section=.comment \ $@_unstripped -o $@; chmod a+x $@ endif -swupdate: cfg-sanity-check swupdate_unstripped +swupdate: .cfg-sanity-check swupdate_unstripped $(call cmd,strip) -${tools-bins}: ${swupdate-ipc-lib} ${tools-objs} ${swupdate-libs} FORCE +.tools-built-in: tools/built-in.o + @touch tools/built-in.o + @touch .tools-built-in + +${tools-bins}: ${swupdate-ipc-lib} ${tools-objs} ${swupdate-libs} .tools-built-in $(call if_changed,addon,$@.o) @mv $@ $@_unstripped $(call cmd,strip) diff --git a/Makefile.flags b/Makefile.flags index 7003af14d767..91d959475fe5 100644 --- a/Makefile.flags +++ b/Makefile.flags @@ -16,7 +16,7 @@ endif KBUILD_CPPFLAGS += $(call cc-option,-std=gnu99,) KBUILD_CPPFLAGS += -D_GNU_SOURCE -DNDEBUG \ - -D"SWU_VER=KBUILD_STR($(SWU_VER))" + -D"SWU_VER=\"$(SWU_VER)\"" KBUILD_CFLAGS += $(call cc-option,-Wall,) KBUILD_CFLAGS += $(call cc-option,-Wshadow,) diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 19561e0322fb..2f87bfe13cf3 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -82,7 +82,6 @@ endif c_flags = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \ $(__c_flags) $(modkern_cflags) \ - -D"KBUILD_STR(s)=\#s" a_flags = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \ $(__a_flags) $(modkern_aflags)
Try to avoid rebuilding things needlessly by: - removing FORCE from direct output files, trust make dependencies to do the right thing. - make cfg-sanity-check a real file that depends on .config, so the check can be reexecuted on config change but not otherwise. - make tools create tools/built-in.o: that file is expected to exist and used as timestamp comparison, but nothing creates it currently. That one is quite ugly as trying to create the file directly would update it needlessly so an extra "pseudo-phony" target was added. - remove KBUILD_STR: the # character was incorrectly truncating commands in $(cmd_$@), making Kbuild think the command line changed (can be tested with KBUILD_VERBOSE=2) ; in our case filling simple quotes should always work for version number, so skip the macro altogether. Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> --- This one is mostly me being over-zealous and nothing strictly required as swupdate is pretty quick to build, but we might as well spare ourselves precious seconds everytime we change a single file. I checked updated headers, c file, EXTRA_CFLAGS etc all are properly taken into account so this should not break anything but it's probably best to test again in different environments just in case... Also tested swupdate --version still works. (Actually now I'm looking at it again there might be a problem with the first point (removing FORCE for direct target), in that we create them and strip in the same command so if that is interrupted in the middle of the strip command make would have no way of knowing and would not attempt to strip again. Worst case a binary is left unstripped so it's not too bad in my opinion and I'm leaving it as such, a proper solution would be to do like swupdate with swupdate_unstripped for all tools) Makefile | 18 +++++++++++------- Makefile.flags | 2 +- scripts/Makefile.lib | 1 - 3 files changed, 12 insertions(+), 9 deletions(-)