diff mbox

ccache dependency issues

Message ID CAAXf6LWo29hYTDoGfmoQRV4krkhy_vCy7-irvB1qas6RuGbJsw@mail.gmail.com
State Not Applicable
Headers show

Commit Message

Thomas De Schampheleire Aug. 1, 2013, 9:26 a.m. UTC
All,

I have tested following patch successfully:
> http://patchwork.ozlabs.org/patch/256744 tar: avoid ccache chicken and egg problem when bootstrapping tar

This patch changes package/tar/tar.mk to set the host compiler to
HOSTCC_NOCCACHE. Thinking further however, I think there could be a
better solution.

host-ccache is built is part of BASE_PKGS, which comes after the
'dependencies' target. This means that when the dependencies are
built, there will never be a ccache yet. Hence, a more global solution
looks to be:


Here we force HOSTCC and HOSTCXX to the no-ccache version, only during
the dependencies step. This also fixes the host-tar/ccache chicken-egg
problem (tested), and will also help with other dependencies (for
example I assume that the host-xzcat will suffer from the same issue
when ccache is enabled).

What do you think of this solution?

Loosely related to this: it seems there also is a HOSTCPP variable,
but it is never changed in the context of ccache settings (see e.g.
Makefile), while it is passed to configure scripts of autotools
packages. Shouldn't there be a HOSTCPP_NOCCACHE as well, to be
correct?

Best regards,
Thomas

Comments

Thomas Petazzoni Aug. 1, 2013, 9:35 a.m. UTC | #1
Dear Thomas De Schampheleire,

On Thu, 1 Aug 2013 11:26:33 +0200, Thomas De Schampheleire wrote:

> I have tested following patch successfully:
> > http://patchwork.ozlabs.org/patch/256744 tar: avoid ccache chicken and egg problem when bootstrapping tar
> 
> This patch changes package/tar/tar.mk to set the host compiler to
> HOSTCC_NOCCACHE. Thinking further however, I think there could be a
> better solution.
> 
> host-ccache is built is part of BASE_PKGS, which comes after the

I guess you meant BASE_TARGETS.

> 'dependencies' target. This means that when the dependencies are
> built, there will never be a ccache yet. Hence, a more global solution
> looks to be:
> 
> diff --git a/support/dependencies/dependencies.mk
> b/support/dependencies/dependencies.mk
> index a2d229c..6fd1df1 100644
> --- a/support/dependencies/dependencies.mk
> +++ b/support/dependencies/dependencies.mk
> @@ -25,6 +25,7 @@ core-dependencies:
>                 DL_TOOLS="$(sort $(DL_TOOLS_DEPENDENCIES))" \
>                 $(TOPDIR)/support/dependencies/dependencies.sh
> 
> +dependencies: HOSTCC=$(HOSTCC_NOCCACHE) HOSTCXX=$(HOSTCXX_NOCCACHE)
>  dependencies: core-dependencies $(DEPENDENCIES_HOST_PREREQ)
> 
>  dependencies-source:
> 
> Here we force HOSTCC and HOSTCXX to the no-ccache version, only during
> the dependencies step. This also fixes the host-tar/ccache chicken-egg
> problem (tested), and will also help with other dependencies (for
> example I assume that the host-xzcat will suffer from the same issue
> when ccache is enabled).
> 
> What do you think of this solution?

This indeed looks like a much better solution. I think it would be even
better if host-ccache was moved from BASE_TARGETS to
DEPENDENCIES_HOST_PREREQ. I.e, the main Makefile chunk

ifeq ($(BR2_CCACHE),y)
BASE_TARGETS += host-ccache
endif

would be moved to support/dependencies/dependencies.mk as:

ifeq ($(BR2_CCACHE),y)
DEPENDENCIES_HOST_PREREQ += host-ccache
endif

and then we can also remove the usage of HOSTCC_NOCCACHE from
package/ccache/ccache.mk.

This way, all the package that should not rely on ccache availability
are built as part of the 'dependencies' target, and as soon as we enter
$(BASE_TARGETS), we now that we have ccache available.

> Loosely related to this: it seems there also is a HOSTCPP variable,
> but it is never changed in the context of ccache settings (see e.g.
> Makefile), while it is passed to configure scripts of autotools
> packages. Shouldn't there be a HOSTCPP_NOCCACHE as well, to be
> correct?

Does executing cpp through ccache makes sense? For now, HOSTCPP is
always 'cpp', regardless of whether ccache is enabled or not, so there
should not be any problem.

Best regards,

Thomas
Thomas De Schampheleire Aug. 1, 2013, 10:08 a.m. UTC | #2
Hi Thomas,

On Thu, Aug 1, 2013 at 11:35 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> On Thu, 1 Aug 2013 11:26:33 +0200, Thomas De Schampheleire wrote:
>>
>> host-ccache is built is part of BASE_PKGS, which comes after the
>
> I guess you meant BASE_TARGETS.

Correct.

>
>> 'dependencies' target. This means that when the dependencies are
>> built, there will never be a ccache yet. Hence, a more global solution
>> looks to be:
>>
>> diff --git a/support/dependencies/dependencies.mk
>> b/support/dependencies/dependencies.mk
>> index a2d229c..6fd1df1 100644
>> --- a/support/dependencies/dependencies.mk
>> +++ b/support/dependencies/dependencies.mk
>> @@ -25,6 +25,7 @@ core-dependencies:
>>                 DL_TOOLS="$(sort $(DL_TOOLS_DEPENDENCIES))" \
>>                 $(TOPDIR)/support/dependencies/dependencies.sh
>>
>> +dependencies: HOSTCC=$(HOSTCC_NOCCACHE) HOSTCXX=$(HOSTCXX_NOCCACHE)
>>  dependencies: core-dependencies $(DEPENDENCIES_HOST_PREREQ)
>>
>>  dependencies-source:
>>
>> Here we force HOSTCC and HOSTCXX to the no-ccache version, only during
>> the dependencies step. This also fixes the host-tar/ccache chicken-egg
>> problem (tested), and will also help with other dependencies (for
>> example I assume that the host-xzcat will suffer from the same issue
>> when ccache is enabled).
>>
>> What do you think of this solution?
>
> This indeed looks like a much better solution. I think it would be even
> better if host-ccache was moved from BASE_TARGETS to
> DEPENDENCIES_HOST_PREREQ. I.e, the main Makefile chunk
>
> ifeq ($(BR2_CCACHE),y)
> BASE_TARGETS += host-ccache
> endif
>
> would be moved to support/dependencies/dependencies.mk as:
>
> ifeq ($(BR2_CCACHE),y)
> DEPENDENCIES_HOST_PREREQ += host-ccache
> endif
>
> and then we can also remove the usage of HOSTCC_NOCCACHE from
> package/ccache/ccache.mk.
>
> This way, all the package that should not rely on ccache availability
> are built as part of the 'dependencies' target, and as soon as we enter
> $(BASE_TARGETS), we now that we have ccache available.

We will have to start expressing some dependencies between
dependencies, I think: in order to build ccache, the package has to be
extracted, but for this one would need host-tar (or host-xzcat for
example).

This could be done by creating an order in which
support/dependencies/check-host-X.mk is included, so that
DEPENDENCIES_HOST_PREREQ is filled in a fixed order.
Alternatively, if we make an assumption that we only need such
dependencies on extract-tools (like tar, xzcat) then we can introduce
another level of dependencies.
Yet another alternative is to explicitly express the dependencies like:
host-ccache: host-tar
but this is cumbersome, and should also take into account the outcome
of suitable-host-package.


>
>> Loosely related to this: it seems there also is a HOSTCPP variable,
>> but it is never changed in the context of ccache settings (see e.g.
>> Makefile), while it is passed to configure scripts of autotools
>> packages. Shouldn't there be a HOSTCPP_NOCCACHE as well, to be
>> correct?
>
> Does executing cpp through ccache makes sense? For now, HOSTCPP is
> always 'cpp', regardless of whether ccache is enabled or not, so there
> should not be any problem.

Ah, sorry, my mistake. cpp is the pre-processor but I was confusing it with g++
(very bad naming IMO: the acronym CPP is also often used in the
context of the C++ language)

Thanks,
Thomas
Thomas Petazzoni Aug. 1, 2013, 11:57 a.m. UTC | #3
Dear Thomas De Schampheleire,

On Thu, 1 Aug 2013 12:08:30 +0200, Thomas De Schampheleire wrote:

> >> What do you think of this solution?
> >
> > This indeed looks like a much better solution. I think it would be even
> > better if host-ccache was moved from BASE_TARGETS to
> > DEPENDENCIES_HOST_PREREQ. I.e, the main Makefile chunk
> >
> > ifeq ($(BR2_CCACHE),y)
> > BASE_TARGETS += host-ccache
> > endif
> >
> > would be moved to support/dependencies/dependencies.mk as:
> >
> > ifeq ($(BR2_CCACHE),y)
> > DEPENDENCIES_HOST_PREREQ += host-ccache
> > endif
> >
> > and then we can also remove the usage of HOSTCC_NOCCACHE from
> > package/ccache/ccache.mk.
> >
> > This way, all the package that should not rely on ccache availability
> > are built as part of the 'dependencies' target, and as soon as we enter
> > $(BASE_TARGETS), we now that we have ccache available.
> 
> We will have to start expressing some dependencies between
> dependencies, I think: in order to build ccache, the package has to be
> extracted, but for this one would need host-tar (or host-xzcat for
> example).
> 
> This could be done by creating an order in which
> support/dependencies/check-host-X.mk is included, so that
> DEPENDENCIES_HOST_PREREQ is filled in a fixed order.
> Alternatively, if we make an assumption that we only need such
> dependencies on extract-tools (like tar, xzcat) then we can introduce
> another level of dependencies.
> Yet another alternative is to explicitly express the dependencies like:
> host-ccache: host-tar
> but this is cumbersome, and should also take into account the outcome
> of suitable-host-package.

I believe we don't have to worry about this for now. The
dependencies.mk code looks like this (with ccache added as proposed):

define suitable-host-package
$(shell support/dependencies/check-host-$(1).sh $(2))
endef
-include support/dependencies/check-host-*.mk

ifeq ($(BR2_STRIP_sstrip),y)
DEPENDENCIES_HOST_PREREQ+=host-sstrip
endif

ifeq ($(BR2_CCACHE),y)
DEPENDENCIES_HOST_PREREQ+=host-ccache
endif

So the dependencies such as host-tar and host-xzcat are always pulled
before host-sstrip and host-ccache, so we should be all set. For sure,
this is the case because we don't do top-level parallel build, but
there are already many places in Buildroot that rely on ordering of
dependencies.

> > Does executing cpp through ccache makes sense? For now, HOSTCPP is
> > always 'cpp', regardless of whether ccache is enabled or not, so there
> > should not be any problem.
> 
> Ah, sorry, my mistake. cpp is the pre-processor but I was confusing it with g++
> (very bad naming IMO: the acronym CPP is also often used in the
> context of the C++ language)

Right.

Thomas
Thomas De Schampheleire Aug. 1, 2013, 12:03 p.m. UTC | #4
Hi Thomas,

On Thu, Aug 1, 2013 at 1:57 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Thu, 1 Aug 2013 12:08:30 +0200, Thomas De Schampheleire wrote:
>
>> >> What do you think of this solution?
>> >
>> > This indeed looks like a much better solution. I think it would be even
>> > better if host-ccache was moved from BASE_TARGETS to
>> > DEPENDENCIES_HOST_PREREQ. I.e, the main Makefile chunk
>> >
>> > ifeq ($(BR2_CCACHE),y)
>> > BASE_TARGETS += host-ccache
>> > endif
>> >
>> > would be moved to support/dependencies/dependencies.mk as:
>> >
>> > ifeq ($(BR2_CCACHE),y)
>> > DEPENDENCIES_HOST_PREREQ += host-ccache
>> > endif
>> >
>> > and then we can also remove the usage of HOSTCC_NOCCACHE from
>> > package/ccache/ccache.mk.
>> >
>> > This way, all the package that should not rely on ccache availability
>> > are built as part of the 'dependencies' target, and as soon as we enter
>> > $(BASE_TARGETS), we now that we have ccache available.
>>
>> We will have to start expressing some dependencies between
>> dependencies, I think: in order to build ccache, the package has to be
>> extracted, but for this one would need host-tar (or host-xzcat for
>> example).
>>
>> This could be done by creating an order in which
>> support/dependencies/check-host-X.mk is included, so that
>> DEPENDENCIES_HOST_PREREQ is filled in a fixed order.
>> Alternatively, if we make an assumption that we only need such
>> dependencies on extract-tools (like tar, xzcat) then we can introduce
>> another level of dependencies.
>> Yet another alternative is to explicitly express the dependencies like:
>> host-ccache: host-tar
>> but this is cumbersome, and should also take into account the outcome
>> of suitable-host-package.
>
> I believe we don't have to worry about this for now. The
> dependencies.mk code looks like this (with ccache added as proposed):
>
> define suitable-host-package
> $(shell support/dependencies/check-host-$(1).sh $(2))
> endef
> -include support/dependencies/check-host-*.mk
>
> ifeq ($(BR2_STRIP_sstrip),y)
> DEPENDENCIES_HOST_PREREQ+=host-sstrip
> endif
>
> ifeq ($(BR2_CCACHE),y)
> DEPENDENCIES_HOST_PREREQ+=host-ccache
> endif
>
> So the dependencies such as host-tar and host-xzcat are always pulled
> before host-sstrip and host-ccache, so we should be all set. For sure,
> this is the case because we don't do top-level parallel build, but
> there are already many places in Buildroot that rely on ordering of
> dependencies.
>

You're right, this order is fine.

But the solution of moving host-ccache to dependencies seems so
surprisingly simple... I wonder why no-one thought of it before,
surely we are missing something?

Thanks,
Thomas
Thomas Petazzoni Aug. 1, 2013, 12:07 p.m. UTC | #5
Dear Thomas De Schampheleire,

On Thu, 1 Aug 2013 14:03:12 +0200, Thomas De Schampheleire wrote:

> > So the dependencies such as host-tar and host-xzcat are always pulled
> > before host-sstrip and host-ccache, so we should be all set. For sure,
> > this is the case because we don't do top-level parallel build, but
> > there are already many places in Buildroot that rely on ordering of
> > dependencies.
> >
> 
> You're right, this order is fine.
> 
> But the solution of moving host-ccache to dependencies seems so
> surprisingly simple... I wonder why no-one thought of it before,
> surely we are missing something?

I think a lot of the things with those "main" targets in the main
Makefile and the BASE_TARGETS, TARGETS variables and so on is just a
big mess that no one ever cleaned up and clarified with good rules
about what each variable is.

That's why I've NAKed additional 'hacks' such as
http://patchwork.ozlabs.org/patch/257372/ in the past. BTW, I think
moving ccache into the dependencies will avoid the need for this patch
257372, and then we can apply http://patchwork.ozlabs.org/patch/257374/
and http://patchwork.ozlabs.org/patch/257375/.

Best regards,

Thomas
Thomas De Schampheleire Aug. 1, 2013, 12:51 p.m. UTC | #6
On Thu, Aug 1, 2013 at 2:07 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Thu, 1 Aug 2013 14:03:12 +0200, Thomas De Schampheleire wrote:
>
>> > So the dependencies such as host-tar and host-xzcat are always pulled
>> > before host-sstrip and host-ccache, so we should be all set. For sure,
>> > this is the case because we don't do top-level parallel build, but
>> > there are already many places in Buildroot that rely on ordering of
>> > dependencies.
>> >
>>
>> You're right, this order is fine.
>>
>> But the solution of moving host-ccache to dependencies seems so
>> surprisingly simple... I wonder why no-one thought of it before,
>> surely we are missing something?
>
> I think a lot of the things with those "main" targets in the main
> Makefile and the BASE_TARGETS, TARGETS variables and so on is just a
> big mess that no one ever cleaned up and clarified with good rules
> about what each variable is.
>
> That's why I've NAKed additional 'hacks' such as
> http://patchwork.ozlabs.org/patch/257372/ in the past. BTW, I think
> moving ccache into the dependencies will avoid the need for this patch
> 257372, and then we can apply http://patchwork.ozlabs.org/patch/257374/
> and http://patchwork.ozlabs.org/patch/257375/.

Ok, sounds good.
I'll try to create a patch soon, or if you want to do it that's fine
by me as well.

Best regards,
Thomas
Thomas Petazzoni Aug. 1, 2013, 12:54 p.m. UTC | #7
Dear Thomas De Schampheleire,

On Thu, 1 Aug 2013 14:51:32 +0200, Thomas De Schampheleire wrote:

> > That's why I've NAKed additional 'hacks' such as
> > http://patchwork.ozlabs.org/patch/257372/ in the past. BTW, I think
> > moving ccache into the dependencies will avoid the need for this patch
> > 257372, and then we can apply http://patchwork.ozlabs.org/patch/257374/
> > and http://patchwork.ozlabs.org/patch/257375/.
> 
> Ok, sounds good.
> I'll try to create a patch soon, or if you want to do it that's fine
> by me as well.

If you can do it, that'd be great. I'd like to keep some time to merge
a few more things, and then do the -rc1 release after updating the
CHANGES file. So I won't have much time to look at this myself.

Thanks!

Thomas
Thomas De Schampheleire Aug. 1, 2013, 9:29 p.m. UTC | #8
On Thu, Aug 1, 2013 at 2:54 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Thu, 1 Aug 2013 14:51:32 +0200, Thomas De Schampheleire wrote:
>
>> > That's why I've NAKed additional 'hacks' such as
>> > http://patchwork.ozlabs.org/patch/257372/ in the past. BTW, I think
>> > moving ccache into the dependencies will avoid the need for this patch
>> > 257372, and then we can apply http://patchwork.ozlabs.org/patch/257374/
>> > and http://patchwork.ozlabs.org/patch/257375/.
>>
>> Ok, sounds good.
>> I'll try to create a patch soon, or if you want to do it that's fine
>> by me as well.
>
> If you can do it, that'd be great. I'd like to keep some time to merge
> a few more things, and then do the -rc1 release after updating the
> CHANGES file. So I won't have much time to look at this myself.

I will send a patch tomorrow morning...
diff mbox

Patch

diff --git a/support/dependencies/dependencies.mk
b/support/dependencies/dependencies.mk
index a2d229c..6fd1df1 100644
--- a/support/dependencies/dependencies.mk
+++ b/support/dependencies/dependencies.mk
@@ -25,6 +25,7 @@  core-dependencies:
                DL_TOOLS="$(sort $(DL_TOOLS_DEPENDENCIES))" \
                $(TOPDIR)/support/dependencies/dependencies.sh

+dependencies: HOSTCC=$(HOSTCC_NOCCACHE) HOSTCXX=$(HOSTCXX_NOCCACHE)
 dependencies: core-dependencies $(DEPENDENCIES_HOST_PREREQ)

 dependencies-source: