Patchwork ccache: allow dynamic selection of cache directory

login
register
mail settings
Submitter Thomas De Schampheleire
Date March 23, 2012, 2:52 p.m.
Message ID <3be175a4efe814978ed4.1332514323@beantl019720>
Download mbox | patch
Permalink /patch/148449/
State Superseded
Headers show

Comments

Thomas De Schampheleire - March 23, 2012, 2:52 p.m.
The existing ccache infrastructure set the cache directory hardcoded in the
ccache binary. As this directory was set to ~/.buildroot-ccache, the cache
is not necessarily local (e.g. in corporate environments the home directories
may be mounted over NFS.)
Previous versions of buildroot did allow to set the cache directory, but this
was also hardcoded (so you had to rebuild ccache to change it), plus that
support was removed.
See http://lists.busybox.net/pipermail/buildroot/2011-July/044511.html for
a discussion on this.

This patch introduces a ccache wrapper script that uses a shell variable
(exported from the Makefile and coming from .config) to set the right
CCACHE_DIR.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

---
Note: I'm not sure what to do with the recently added COMPILERCHECK. Should
it remain hardcoded, or also use the wrapper script for uniformity?


 Config.in                |   7 +++++++
 Makefile                 |   6 ++++--
 package/ccache/ccache.mk |  21 ++++++++++++++-------
 3 files changed, 25 insertions(+), 9 deletions(-)
Arnout Vandecappelle - March 25, 2012, 8:55 p.m.
On Friday 23 March 2012 15:52:03 Thomas De Schampheleire wrote:
[snip]
> ---
> Note: I'm not sure what to do with the recently added COMPILERCHECK. Should
> it remain hardcoded, or also use the wrapper script for uniformity?

 I would say: use the wrapper script.


>  ifeq ($(BR2_CCACHE),y)
> -CCACHE:=$(HOST_DIR)/usr/bin/ccache
> -CCACHE_CACHE_DIR=$(HOME)/.buildroot-ccache
> +CCACHE:=$(HOST_DIR)/usr/bin/ccache-wrapper
> +CCACHE_BIN:=$(HOST_DIR)/usr/bin/ccache
> +CCACHE_CACHE_DIR:=$(call qstrip,$(BR2_CCACHE_DIR))
> +export CCACHE_CACHE_DIR

 You could take the opportunity to fix up the whitespace here.  Also,
there is no need for the := (except maybe for the _DIR itself, I'm a
bit confused as how assignment interacts with export).

 I would also rename the variable to BUILDROOT_CCACHE_DIR to clearly
distinguish it from and relate it to the normal CCACHE_DIR.

> -# We directly hardcode configuration into the binary, as it is much
> +# We directly hardcode some configuration into the binary, as it is much
>  # easier to handle than passing an environment variable. Our
 This statement has become rather ridiculous, since after this patch we
do both: setting environment variables and fixing up the executable.
And it becomes completely wrong if COMPILERCHECK moves to the wrapper
as well.

>  # configuration is:
> -#  - the cache location
>  #  - the fact that ccache shouldn't use the compiler binary mtime to
> -#  - detect a change in the compiler, because in the context of
> -#  - Buildroot, that completely defeats the purpose of ccache. Of
> -#  - course, that leaves the user responsible for purging its cache
> -#  - when the compiler changes.
> +#    detect a change in the compiler, because in the context of
> +#    Buildroot, that completely defeats the purpose of ccache. Of
> +#    course, that leaves the user responsible for purging its cache
> +#    when the compiler changes.
>  define HOST_CCACHE_FIX_CCACHE_DIR
> -	sed -i 's,getenv("CCACHE_DIR"),"$(CCACHE_CACHE_DIR)",' $(@D)/ccache.c

 An alternative implementation would be to do the following here:
sed -i 's,getenv("CCACHE_DIR"),getenv("BUILDROOT_CCACHE_DIR"),' $(@D)/ccache.c

 I actually prefer moving everything to a wrapper script, though.
Patching source files is inherently more fragile.

>  	sed -i 's,getenv("CCACHE_COMPILERCHECK"),"none",' $(@D)/ccache.c
>  endef
>  
>  HOST_CCACHE_POST_CONFIGURE_HOOKS += \
>  	HOST_CCACHE_FIX_CCACHE_DIR
>  
> +define HOST_CCACHE_CREATE_WRAPPER
> +	echo "#!/bin/sh" > $(CCACHE)
> +	echo 'CCACHE_DIR=$$CCACHE_CACHE_DIR $(CCACHE_BIN) "$$@"' >> $(CCACHE)

 Small optimization: use exec, i.e.
echo 'CCACHE_DIR=$$CCACHE_CACHE_DIR exec $(CCACHE_BIN) "$$@"' >> $(CCACHE)

 I think it's cleaner to create the wrapper as a file in package/ccache
and install it to the host directory.  It can autodiscover the location
of the ccache bin like so:

CCACHE_BIN="$(dirname $0)/ccache"


> +	chmod +x $(CCACHE)
> +endef
> +
> +HOST_CCACHE_POST_INSTALL_HOOKS += \
> +	HOST_CCACHE_CREATE_WRAPPER
> +
>  $(eval $(call AUTOTARGETS))
>  $(eval $(call AUTOTARGETS,host))


 Regards,
 Arnout

Patch

diff --git a/Config.in b/Config.in
--- a/Config.in
+++ b/Config.in
@@ -198,6 +198,13 @@  config BR2_CCACHE
 	  ccache cache by removing the $HOME/.buildroot-ccache
 	  directory.
 
+config BR2_CCACHE_DIR
+	string "Compiler cache location"
+	depends on BR2_CCACHE
+	default "$(HOME)/.buildroot-ccache"
+	help
+	  Where ccache should store cached files.
+
 config BR2_DEPRECATED
 	bool "Show packages that are deprecated or obsolete"
 	help
diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -285,8 +285,10 @@  TOOLCHAIN_DIR=$(BASE_DIR)/toolchain
 TARGET_SKELETON=$(TOPDIR)/fs/skeleton
 
 ifeq ($(BR2_CCACHE),y)
-CCACHE:=$(HOST_DIR)/usr/bin/ccache
-CCACHE_CACHE_DIR=$(HOME)/.buildroot-ccache
+CCACHE:=$(HOST_DIR)/usr/bin/ccache-wrapper
+CCACHE_BIN:=$(HOST_DIR)/usr/bin/ccache
+CCACHE_CACHE_DIR:=$(call qstrip,$(BR2_CCACHE_DIR))
+export CCACHE_CACHE_DIR
 HOSTCC  := $(CCACHE) $(HOSTCC)
 HOSTCXX := $(CCACHE) $(HOSTCXX)
 endif
diff --git a/package/ccache/ccache.mk b/package/ccache/ccache.mk
--- a/package/ccache/ccache.mk
+++ b/package/ccache/ccache.mk
@@ -25,23 +25,30 @@  HOST_CCACHE_CONF_ENV = \
 # has zero dependency besides the C library.
 HOST_CCACHE_CONF_OPT += ccache_cv_zlib_1_2_3=no
 
-# We directly hardcode configuration into the binary, as it is much
+# We directly hardcode some configuration into the binary, as it is much
 # easier to handle than passing an environment variable. Our
 # configuration is:
-#  - the cache location
 #  - the fact that ccache shouldn't use the compiler binary mtime to
-#  - detect a change in the compiler, because in the context of
-#  - Buildroot, that completely defeats the purpose of ccache. Of
-#  - course, that leaves the user responsible for purging its cache
-#  - when the compiler changes.
+#    detect a change in the compiler, because in the context of
+#    Buildroot, that completely defeats the purpose of ccache. Of
+#    course, that leaves the user responsible for purging its cache
+#    when the compiler changes.
 define HOST_CCACHE_FIX_CCACHE_DIR
-	sed -i 's,getenv("CCACHE_DIR"),"$(CCACHE_CACHE_DIR)",' $(@D)/ccache.c
 	sed -i 's,getenv("CCACHE_COMPILERCHECK"),"none",' $(@D)/ccache.c
 endef
 
 HOST_CCACHE_POST_CONFIGURE_HOOKS += \
 	HOST_CCACHE_FIX_CCACHE_DIR
 
+define HOST_CCACHE_CREATE_WRAPPER
+	echo "#!/bin/sh" > $(CCACHE)
+	echo 'CCACHE_DIR=$$CCACHE_CACHE_DIR $(CCACHE_BIN) "$$@"' >> $(CCACHE)
+	chmod +x $(CCACHE)
+endef
+
+HOST_CCACHE_POST_INSTALL_HOOKS += \
+	HOST_CCACHE_CREATE_WRAPPER
+
 $(eval $(call AUTOTARGETS))
 $(eval $(call AUTOTARGETS,host))