Patchwork [v2] ccache: allow dynamic selection of cache directory

login
register
mail settings
Submitter Thomas De Schampheleire
Date April 6, 2012, 6:47 p.m.
Message ID <962414f85797a188de79.1333738037@beantl019720>
Download mbox | patch
Permalink /patch/151245/
State Superseded
Headers show

Comments

Thomas De Schampheleire - April 6, 2012, 6:47 p.m.
The existing ccache infrastructure sets 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.

Additionally it migrates the COMPILERCHECK setting to the wrapper script
as well.

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

---
v2: Update based on Arnout's comments: move compilercheck to wrapper, move
    wrapper to version-controlled file instead of generating it, use exec,
    rename cache_dir variable, fix whitespace.

 Config.in                     |   7 +++++++
 Makefile                      |   5 +++--
 package/ccache/ccache-wrapper |  18 ++++++++++++++++++
 package/ccache/ccache.mk      |  19 +++++--------------
 4 files changed, 33 insertions(+), 16 deletions(-)
Thomas De Schampheleire - May 4, 2012, 7:17 p.m.
On Fri, Apr 6, 2012 at 8:47 PM, Thomas De Schampheleire
<patrickdepinguin+buildroot@gmail.com> wrote:
> The existing ccache infrastructure sets 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.
>
> Additionally it migrates the COMPILERCHECK setting to the wrapper script
> as well.
>
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>
> ---
> v2: Update based on Arnout's comments: move compilercheck to wrapper, move
>    wrapper to version-controlled file instead of generating it, use exec,
>    rename cache_dir variable, fix whitespace.
>
>  Config.in                     |   7 +++++++
>  Makefile                      |   5 +++--
>  package/ccache/ccache-wrapper |  18 ++++++++++++++++++
>  package/ccache/ccache.mk      |  19 +++++--------------
>  4 files changed, 33 insertions(+), 16 deletions(-)
>
> 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,9 @@ 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
> +BUILDROOT_CACHE_DIR = $(call qstrip,$(BR2_CCACHE_DIR))
> +export BUILDROOT_CACHE_DIR
>  HOSTCC  := $(CCACHE) $(HOSTCC)
>  HOSTCXX := $(CCACHE) $(HOSTCXX)
>  endif
> diff --git a/package/ccache/ccache-wrapper b/package/ccache/ccache-wrapper
> new file mode 100644
> --- /dev/null
> +++ b/package/ccache/ccache-wrapper
> @@ -0,0 +1,18 @@
> +#!/bin/sh
> +# ccache wrapper tweaking the ccache environment
> +
> +# Set cache dir based on .config
> +CCACHE_DIR="$BUILDROOT_CACHE_DIR"
> +export CCACHE_DIR
> +
> +# 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.
> +CCACHE_COMPILERCHECK=none
> +export CCACHE_COMPILERCHECK
> +
> +
> +CCACHE_BIN="`dirname $0`/ccache"
> +exec "$CCACHE_BIN" "$@"
> diff --git a/package/ccache/ccache.mk b/package/ccache/ccache.mk
> --- a/package/ccache/ccache.mk
> +++ b/package/ccache/ccache.mk
> @@ -25,22 +25,13 @@ 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
> -# 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.
> -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
> +define HOST_CCACHE_INSTALL_WRAPPER
> +       $(INSTALL) -D -m 0755 package/ccache/ccache-wrapper \
> +               $(HOST_DIR)/usr/bin/ccache-wrapper
>  endef
>
> -HOST_CCACHE_POST_CONFIGURE_HOOKS += \
> -       HOST_CCACHE_FIX_CCACHE_DIR
> +HOST_CCACHE_POST_INSTALL_HOOKS += \
> +       HOST_CCACHE_INSTALL_WRAPPER
>
>  $(eval $(call AUTOTARGETS))
>  $(eval $(call AUTOTARGETS,host))

bump
Arnout Vandecappelle - May 4, 2012, 8:46 p.m.
On 04/06/12 20:47, Thomas De Schampheleire wrote:
> -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


  On second review, a better implementation would be to replace the first
sed by

sed -i 's,getenv("CCACHE_DIR"),getenv("BUILDROOT_CACHE_DIR"),' $(@D)/ccache.c


  Then you no longer need the wrapper script at all.

  Regards,
  Arnout
Thomas De Schampheleire - May 5, 2012, 7:17 a.m.
Op 4 mei 2012 22:46 schreef "Arnout Vandecappelle" <arnout@mind.be> het
volgende:
>
> On 04/06/12 20:47, Thomas De Schampheleire wrote:
>>
>> -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
>
>
>
>  On second review, a better implementation would be to replace the first
> sed by
>
> sed -i 's,getenv("CCACHE_DIR"),getenv("BUILDROOT_CACHE_DIR"),'
$(@D)/ccache.c
>
>
>  Then you no longer need the wrapper script at all.

Well, it boils down to the choice between patching the source or using a
wrapper. I can see benefits for either way.
Additional input from other developers is welcome here...

Best regards,
Thomas
Thomas De Schampheleire - May 15, 2012, 6:13 p.m.
On Sat, May 5, 2012 at 9:17 AM, Thomas De Schampheleire
<patrickdepinguin+buildroot@gmail.com> wrote:
>
> Op 4 mei 2012 22:46 schreef "Arnout Vandecappelle" <arnout@mind.be> het
> volgende:
>
>
>>
>> On 04/06/12 20:47, Thomas De Schampheleire wrote:
>>>
>>> -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
>>
>>
>>
>>  On second review, a better implementation would be to replace the first
>> sed by
>>
>> sed -i 's,getenv("CCACHE_DIR"),getenv("BUILDROOT_CACHE_DIR"),'
>> $(@D)/ccache.c
>>
>>
>>  Then you no longer need the wrapper script at all.
>
> Well, it boils down to the choice between patching the source or using a
> wrapper. I can see benefits for either way.
> Additional input from other developers is welcome here...

Any feedback on this from other developers?

Thanks,
Thomas
Thomas Petazzoni - May 15, 2012, 6:28 p.m.
Le Fri, 04 May 2012 22:46:10 +0200,
Arnout Vandecappelle <arnout@mind.be> a écrit :

>   On second review, a better implementation would be to replace the
> first sed by
> 
> sed -i 's,getenv("CCACHE_DIR"),getenv("BUILDROOT_CACHE_DIR"),'
> $(@D)/ccache.c

Euh? Why wouldn't we use CCACHE_DIR directly? What's the need of
renaming this environment variable here?

For the record, when I did the rework of ccache, I did hardcode the
cache directory into the ccache binary because I had issues passing
CCACHE_DIR everywhere. But I was trying to pass it to every gcc
invocation, and not globally through an export in the Makefile.

In any case, I would definitely prefer a solution that doesn't use a
wrapper. With a wrapper we would have:

 -> ccache wrapper
  -> ccache
   -> external toolchain wrapper
    -> gcc

If we can avoid the additional level of wrapping, it'd be nice.

Best regards,

Thomas
Thomas De Schampheleire - May 15, 2012, 6:49 p.m.
On Tue, May 15, 2012 at 8:28 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Le Fri, 04 May 2012 22:46:10 +0200,
> Arnout Vandecappelle <arnout@mind.be> a écrit :
>
>>   On second review, a better implementation would be to replace the
>> first sed by
>>
>> sed -i 's,getenv("CCACHE_DIR"),getenv("BUILDROOT_CACHE_DIR"),'
>> $(@D)/ccache.c
>
> Euh? Why wouldn't we use CCACHE_DIR directly? What's the need of
> renaming this environment variable here?

Because CCACHE_DIR is already used by gentargets/autotargets...

>
> For the record, when I did the rework of ccache, I did hardcode the
> cache directory into the ccache binary because I had issues passing
> CCACHE_DIR everywhere. But I was trying to pass it to every gcc
> invocation, and not globally through an export in the Makefile.
>
> In any case, I would definitely prefer a solution that doesn't use a
> wrapper. With a wrapper we would have:
>
>  -> ccache wrapper
>  -> ccache
>   -> external toolchain wrapper
>    -> gcc
>
> If we can avoid the additional level of wrapping, it'd be nice.

Thanks for your input, Thomas.
Thomas Petazzoni - May 15, 2012, 6:57 p.m.
Le Tue, 15 May 2012 20:49:35 +0200,
Thomas De Schampheleire <patrickdepinguin+buildroot@gmail.com> a écrit :

> > Euh? Why wouldn't we use CCACHE_DIR directly? What's the need of
> > renaming this environment variable here?
> 
> Because CCACHE_DIR is already used by gentargets/autotargets...

Where?

$ git grep CCACHE_DIR | cat
package/ccache/ccache.mk:define HOST_CCACHE_FIX_CCACHE_DIR
package/ccache/ccache.mk:	sed -i 's,getenv("CCACHE_DIR"),"$(CCACHE_CACHE_DIR)",' $(@D)/ccache.c
package/ccache/ccache.mk:	HOST_CCACHE_FIX_CCACHE_DIR

Regards,

Thomas
Thomas De Schampheleire - May 15, 2012, 7:23 p.m.
On Tue, May 15, 2012 at 8:57 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Le Tue, 15 May 2012 20:49:35 +0200,
> Thomas De Schampheleire <patrickdepinguin+buildroot@gmail.com> a écrit :
>
>> > Euh? Why wouldn't we use CCACHE_DIR directly? What's the need of
>> > renaming this environment variable here?
>>
>> Because CCACHE_DIR is already used by gentargets/autotargets...
>
> Where?
>
> $ git grep CCACHE_DIR | cat
> package/ccache/ccache.mk:define HOST_CCACHE_FIX_CCACHE_DIR
> package/ccache/ccache.mk:       sed -i 's,getenv("CCACHE_DIR"),"$(CCACHE_CACHE_DIR)",' $(@D)/ccache.c
> package/ccache/ccache.mk:       HOST_CCACHE_FIX_CCACHE_DIR

It's not defined directly like that.
ccache is a package in its own right, using autotools.
(package/ccache/ccache.mk).

The variable CCACHE_DIR originates from the following line in
package/pkg-gentargets.mk:
$(2)_DIR        =  $$(BUILD_DIR)/$$($(2)_BASE_NAME)

Best regards,
Thomas
Thomas Petazzoni - May 15, 2012, 7:50 p.m.
Le Tue, 15 May 2012 21:23:27 +0200,
Thomas De Schampheleire <patrickdepinguin+buildroot@gmail.com> a écrit :

> It's not defined directly like that.
> ccache is a package in its own right, using autotools.
> (package/ccache/ccache.mk).
> 
> The variable CCACHE_DIR originates from the following line in
> package/pkg-gentargets.mk:
> $(2)_DIR        =  $$(BUILD_DIR)/$$($(2)_BASE_NAME)

Aaah, yes. Then yes, we need to adjust ccache source code, as suggested
by Arnout.

Thomas
Peter Korsgaard - May 15, 2012, 8:42 p.m.
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 Thomas> Le Tue, 15 May 2012 21:23:27 +0200,
 Thomas> Thomas De Schampheleire <patrickdepinguin+buildroot@gmail.com> a écrit :

 >> It's not defined directly like that.
 >> ccache is a package in its own right, using autotools.
 >> (package/ccache/ccache.mk).
 >> 
 >> The variable CCACHE_DIR originates from the following line in
 >> package/pkg-gentargets.mk:
 >> $(2)_DIR        =  $$(BUILD_DIR)/$$($(2)_BASE_NAME)

Ahh yes, I now remember that problem as well.

 Thomas> Aaah, yes. Then yes, we need to adjust ccache source code, as
 Thomas> suggested by Arnout.

I agree, without the wrapper would be nicest.

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,9 @@  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
+BUILDROOT_CACHE_DIR = $(call qstrip,$(BR2_CCACHE_DIR))
+export BUILDROOT_CACHE_DIR
 HOSTCC  := $(CCACHE) $(HOSTCC)
 HOSTCXX := $(CCACHE) $(HOSTCXX)
 endif
diff --git a/package/ccache/ccache-wrapper b/package/ccache/ccache-wrapper
new file mode 100644
--- /dev/null
+++ b/package/ccache/ccache-wrapper
@@ -0,0 +1,18 @@ 
+#!/bin/sh
+# ccache wrapper tweaking the ccache environment
+
+# Set cache dir based on .config
+CCACHE_DIR="$BUILDROOT_CACHE_DIR"
+export CCACHE_DIR
+
+# 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.
+CCACHE_COMPILERCHECK=none
+export CCACHE_COMPILERCHECK
+
+
+CCACHE_BIN="`dirname $0`/ccache"
+exec "$CCACHE_BIN" "$@"
diff --git a/package/ccache/ccache.mk b/package/ccache/ccache.mk
--- a/package/ccache/ccache.mk
+++ b/package/ccache/ccache.mk
@@ -25,22 +25,13 @@  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
-# 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.
-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
+define HOST_CCACHE_INSTALL_WRAPPER
+	$(INSTALL) -D -m 0755 package/ccache/ccache-wrapper \
+		$(HOST_DIR)/usr/bin/ccache-wrapper
 endef
 
-HOST_CCACHE_POST_CONFIGURE_HOOKS += \
-	HOST_CCACHE_FIX_CCACHE_DIR
+HOST_CCACHE_POST_INSTALL_HOOKS += \
+	HOST_CCACHE_INSTALL_WRAPPER
 
 $(eval $(call AUTOTARGETS))
 $(eval $(call AUTOTARGETS,host))