Re: [RFC PATCH] Don't reset timestamps in include/generated if not needed

[Michal Marek <mmarek@suse.cz>]

On Sun, Mar 08, 2015 at 04:08:06PM -0700, Linus Torvalds wrote:
> On Tue, Mar 3, 2015 at 5:07 PM, Valdis Kletnieks
> <Valdis.Kletnieks@vt.edu> wrote:
> >
> > Kbuild regenerates bounds.h and asm-offsets.h, resetting the timestamps
> > and forcing rebuilds even if the contents haven't changed.  Add a bit of
> > shell magic to only replace the file if the contents have in fact changed,
> > which should speed up git bisects and similar.
> > ...
> > RFC because I can't wrap my head around why this wasn't done ages ago.
> > If I'm missing something something obvious, please apply a cluestick. :)

We do not regenerate the files *always*, but only if bounds.c /
asm-offsets.c or one of the headers they include change. So the rule is
not any worse than the rule for compiling a regular .c file into an .o
file. You can use ccache to avoid repeated rebuilds.

However, I can see that especially bounds.h ends up being included in
many places, so cutting its dependencies helps in some cases.


> > Lightly tested - if I rm one of those two files, it gets rebuilt. If I
> > insert some whitespace, it gets replaced. If I don't touch it, the datestamp
> > doesn't change.
> 
> I don't think this is wrong, but I'd really prefer to do the whole
> move-if-changed thing as a separate rule, and a separate command.
> 
> IOW, could we please have something that generates the "build.h" file
> by separately creating a new temporary file, and then having the
> traditional kind of move-if-changed definition
> 
>    define move-if-changed
>         if ! cmp -s $(1) $(2); then mv -f $(1) $(2); else rm -f $(1); fi
>    endef

We already have it and it is called "filechk." Valdis, can you check if
the below patch works equally well for you?

diff --git a/Kbuild b/Kbuild
index ab8ded9..47d0630 100644
--- a/Kbuild
+++ b/Kbuild
@@ -13,8 +13,9 @@ define sed-y
 	s:->::; p;}"
 endef
 
-quiet_cmd_offsets = GEN     $@
-define cmd_offsets
+# Use filechk to avoid rebuilds when a header changes, but the resulting file
+# does not
+define filechk_offsets
 	(set -e; \
 	 echo "#ifndef $2"; \
 	 echo "#define $2"; \
@@ -24,9 +25,9 @@ define cmd_offsets
 	 echo " * This file was generated by Kbuild"; \
 	 echo " */"; \
 	 echo ""; \
-	 sed -ne $(sed-y) $<; \
+	 sed -ne $(sed-y); \
 	 echo ""; \
-	 echo "#endif" ) > $@
+	 echo "#endif" )
 endef
 
 #####
@@ -44,7 +45,7 @@ kernel/bounds.s: kernel/bounds.c FORCE
 
 $(obj)/$(bounds-file): kernel/bounds.s Kbuild
 	$(Q)mkdir -p $(dir $@)
-	$(call cmd,offsets,__LINUX_BOUNDS_H__)
+	$(call filechk,offsets,__LINUX_BOUNDS_H__)
 
 #####
 # 2) Generate asm-offsets.h
@@ -63,7 +64,7 @@ arch/$(SRCARCH)/kernel/asm-offsets.s: arch/$(SRCARCH)/kernel/asm-offsets.c \
 	$(call if_changed_dep,cc_s_c)
 
 $(obj)/$(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s Kbuild
-	$(call cmd,offsets,__ASM_OFFSETS_H__)
+	$(call filechk,offsets,__ASM_OFFSETS_H__)
 
 #####
 # 3) Check for missing system calls