[3/3] toolchain/toolchain-wrapper: don't hardcode CCACHE_BASEDIR

The CCACHE_BASEDIR variable was hardcoded to $(BASE_DIR), and
therefore when moving around the toolchain-wrapper as part of the SDK,
the CCACHE_BASEDIR would still point to the original location of the
Buildroot build.

Instead of doing this, leverage the logic we already have in the
toolchain wrapper, which determines based on the toolchain wrapper
location itself what is the base directory.

So now, toolchain-wrapper.mk is only passing a boolean to
toolchain-wrapper.c, that tells whether the BR2_USE_CCACHE_BASEDIR
option is enabled or not.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 toolchain/toolchain-wrapper.c  | 6 +++---
 toolchain/toolchain-wrapper.mk | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

Thomas, All,

On 2022-07-27 20:48 +0200, Thomas Petazzoni via buildroot spake thusly:
> The CCACHE_BASEDIR variable was hardcoded to $(BASE_DIR), and
> therefore when moving around the toolchain-wrapper as part of the SDK,
> the CCACHE_BASEDIR would still point to the original location of the
> Buildroot build.
> 
> Instead of doing this, leverage the logic we already have in the
> toolchain wrapper, which determines based on the toolchain wrapper
> location itself what is the base directory.
> 
> So now, toolchain-wrapper.mk is only passing a boolean to
> toolchain-wrapper.c, that tells whether the BR2_USE_CCACHE_BASEDIR
> option is enabled or not.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  toolchain/toolchain-wrapper.c  | 6 +++---
>  toolchain/toolchain-wrapper.mk | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/toolchain/toolchain-wrapper.c b/toolchain/toolchain-wrapper.c
> index 37b24dd24a..bed90d6292 100644
> --- a/toolchain/toolchain-wrapper.c
> +++ b/toolchain/toolchain-wrapper.c
> @@ -538,9 +538,9 @@ int main(int argc, char **argv)
>  		return 3;
>  	}
>  #endif
> -#ifdef BR_CCACHE_BASEDIR
> -	/* Allow compilercheck to be overridden through the environment */
> -	if (setenv("CCACHE_BASEDIR", BR_CCACHE_BASEDIR, 0)) {
> +#ifdef BR_USE_CCACHE_BASEDIR
> +	/* Allow basedir to be overridden through the environment */
> +	if (setenv("CCACHE_BASEDIR", absbasedir, 0)) {

You forgot the one instance in the debug code, lines 524 and 525.

And once this is fixed, you'll notice that the old and the new
CCACHE_BASEDIR will be different:

    (before)$ BR2_DEBUG_WRAPPER=2 ./host/bin/arm-linux-gcc --help
    ...
    Toolchain wrapper executing:
        CCACHE_BASEDIR='/home/ymorin/dev/buildroot/O/master'
        '/home/ymorin/dev/buildroot/O/master/host/opt/ext-toolchain/bin/arm-linux-gcc.br_real'
        ...

    (after)$ BR2_DEBUG_WRAPPER=2 ./host/bin/arm-linux-gcc --help
    ...
    Toolchain wrapper executing:
        CCACHE_BASEDIR='/home/ymorin/dev/buildroot/O/master/host'
        '/home/ymorin/dev/buildroot/O/master/host/opt/ext-toolchain/bin/arm-linux-gcc.br_real'
        ...

So as you can see, before, CCACHE_BASEDIR was pointing to BASE_DIR, but
now it points to HOST_DIR. This is one-level too deep, as it will not
match the path where packages are actually built, which is
BASE_DIR/build, and hapenned to be CCACHE_BASEDIR/build.

Even though I had the debug code path fixed, the second issue was not
trivial enough for me to fix when applying, so I amrked the patch as
changes requested.

Regards,
Yann E. MORIN.

>  		perror(__FILE__ ": Failed to set CCACHE_BASEDIR");
>  		return 3;
>  	}
> diff --git a/toolchain/toolchain-wrapper.mk b/toolchain/toolchain-wrapper.mk
> index cbf46f15fa..c87120f0f7 100644
> --- a/toolchain/toolchain-wrapper.mk
> +++ b/toolchain/toolchain-wrapper.mk
> @@ -70,7 +70,7 @@ endif
>  endif
>  
>  ifeq ($(BR2_CCACHE_USE_BASEDIR),y)
> -TOOLCHAIN_WRAPPER_ARGS += -DBR_CCACHE_BASEDIR='"$(BASE_DIR)"'
> +TOOLCHAIN_WRAPPER_ARGS += -DBR_USE_CCACHE_BASEDIR
>  endif
>  
>  ifeq ($(BR2_PIC_PIE),y)
> -- 
> 2.37.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

Howdy,

On Sat, 30 Jul 2022 14:11:03 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> And once this is fixed, you'll notice that the old and the new
> CCACHE_BASEDIR will be different:
> 
>     (before)$ BR2_DEBUG_WRAPPER=2 ./host/bin/arm-linux-gcc --help
>     ...
>     Toolchain wrapper executing:
>         CCACHE_BASEDIR='/home/ymorin/dev/buildroot/O/master'
>         '/home/ymorin/dev/buildroot/O/master/host/opt/ext-toolchain/bin/arm-linux-gcc.br_real'
>         ...
> 
>     (after)$ BR2_DEBUG_WRAPPER=2 ./host/bin/arm-linux-gcc --help
>     ...
>     Toolchain wrapper executing:
>         CCACHE_BASEDIR='/home/ymorin/dev/buildroot/O/master/host'
>         '/home/ymorin/dev/buildroot/O/master/host/opt/ext-toolchain/bin/arm-linux-gcc.br_real'
>         ...
> 
> So as you can see, before, CCACHE_BASEDIR was pointing to BASE_DIR, but
> now it points to HOST_DIR. This is one-level too deep, as it will not
> match the path where packages are actually built, which is
> BASE_DIR/build, and hapenned to be CCACHE_BASEDIR/build.
> 
> Even though I had the debug code path fixed, the second issue was not
> trivial enough for me to fix when applying, so I amrked the patch as
> changes requested.

You're totally right. And in fact, it makes the change useless. Indeed,
CCACHE_BASEDIR only makes sense when ccache is used during the
Buildroot build, because it points to $(BASE_DIR)... which once we are
outside of Buildroot, in the SDK, would no longer point to anything
sensible.

So instead, I've sent a different patch that sets the
CCACHE_COMPILERCHECK and CCACHE_BASEDIR environment variables, only if
ccache support is used (BR2_USE_CCACHE=1). Let me know what you think.

Thanks again for the good review!

Best regards,

Thomas