| Message ID | 1381427457-25627-1-git-send-email-tjlee@ambarella.com |
|---|---|
| State | Accepted |
| Headers | show |
Hi Tzu-Jung, Tzu-Jung Lee <roylee17@gmail.com> wrote: >usage: > # set cache limit size > make CCACHE_OPTIONS="--max-size=5G" ccache-options > > # zero statistics counters > make CCACHE_OPTIONS="--zero-stats" ccache-options > >Signed-off-by: Tzu-Jung Lee <tjlee@ambarella.com> >--- >Include the useful warning message suggested by Thomas. > >We might need a separate patch to obsolete the ccache-stats later. > > docs/manual/ccache-support.txt | 11 +++++++++++ > package/ccache/ccache.mk | 11 +++++++++++ > 2 files changed, 22 insertions(+) > >diff --git a/docs/manual/ccache-support.txt b/docs/manual/ccache-support.txt >index 4969180..fe06a01 100644 >--- a/docs/manual/ccache-support.txt >+++ b/docs/manual/ccache-support.txt >@@ -23,3 +23,14 @@ remove this directory. > > You can get statistics on the cache (its size, number of hits, > misses, etc.) by running +make ccache-stats+. >+ >+The make target +ccache-options+ and the +CCACHE_OPTIONS+ variable >+provide more generic access to the ccache. For example >+ >+----------------- >+# set cache limit size >+make CCACHE_OPTIONS="--max-size=5G" ccache-options >+ >+# zero statistics counters >+make CCACHE_OPTIONS="--zero-stats" ccache-options >+----------------- >diff --git a/package/ccache/ccache.mk b/package/ccache/ccache.mk >index c5e9385..7b6155d 100644 >--- a/package/ccache/ccache.mk >+++ b/package/ccache/ccache.mk >@@ -45,3 +45,14 @@ ifeq ($(BR2_CCACHE),y) > ccache-stats: host-ccache > $(Q)$(CCACHE) -s > endif >+ >+ifeq ($(BR2_CCACHE),y) >+ccache-options: host-ccache >+ifeq ($(CCACHE_OPTIONS),) >+ $(Q)echo "Usage: make ccache-options CCACHE_OPTIONS=\"opts\"" >+ $(Q)echo "where 'opts' corresponds to one or more valid ccache options" \ >+ "(see ccache help text below)" >+ $(Q)echo >+endif >+ $(Q)$(CCACHE) $(CCACHE_OPTIONS) >+endif Did you see my other comment about combining the ifeq statements of ccache-stats and ccache-options? Best regards, Thomas
Hi Thomas, On Thu, Oct 10, 2013 at 12:45 PM, Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote: > Hi Tzu-Jung, > > Tzu-Jung Lee <roylee17@gmail.com> wrote: >>usage: >> # set cache limit size >> make CCACHE_OPTIONS="--max-size=5G" ccache-options >> >> # zero statistics counters >> make CCACHE_OPTIONS="--zero-stats" ccache-options >> >>Signed-off-by: Tzu-Jung Lee <tjlee@ambarella.com> >>--- >>Include the useful warning message suggested by Thomas. >> >>We might need a separate patch to obsolete the ccache-stats later. >> >> docs/manual/ccache-support.txt | 11 +++++++++++ >> package/ccache/ccache.mk | 11 +++++++++++ >> 2 files changed, 22 insertions(+) >> >>diff --git a/docs/manual/ccache-support.txt b/docs/manual/ccache-support.txt >>index 4969180..fe06a01 100644 >>--- a/docs/manual/ccache-support.txt >>+++ b/docs/manual/ccache-support.txt >>@@ -23,3 +23,14 @@ remove this directory. >> >> You can get statistics on the cache (its size, number of hits, >> misses, etc.) by running +make ccache-stats+. >>+ >>+The make target +ccache-options+ and the +CCACHE_OPTIONS+ variable >>+provide more generic access to the ccache. For example >>+ >>+----------------- >>+# set cache limit size >>+make CCACHE_OPTIONS="--max-size=5G" ccache-options >>+ >>+# zero statistics counters >>+make CCACHE_OPTIONS="--zero-stats" ccache-options >>+----------------- >>diff --git a/package/ccache/ccache.mk b/package/ccache/ccache.mk >>index c5e9385..7b6155d 100644 >>--- a/package/ccache/ccache.mk >>+++ b/package/ccache/ccache.mk >>@@ -45,3 +45,14 @@ ifeq ($(BR2_CCACHE),y) >> ccache-stats: host-ccache >> $(Q)$(CCACHE) -s >> endif >>+ >>+ifeq ($(BR2_CCACHE),y) >>+ccache-options: host-ccache >>+ifeq ($(CCACHE_OPTIONS),) >>+ $(Q)echo "Usage: make ccache-options CCACHE_OPTIONS=\"opts\"" >>+ $(Q)echo "where 'opts' corresponds to one or more valid ccache options" \ >>+ "(see ccache help text below)" >>+ $(Q)echo >>+endif >>+ $(Q)$(CCACHE) $(CCACHE_OPTIONS) >>+endif > > Did you see my other comment about combining the ifeq statements of ccache-stats and ccache-options? Yes, but I think it probably would be better as a separate patch, which: 1. either remove the ccache-stats code & and manual section 2. or issue warning about the obsolete usage. So this patch alone don't affect those are currently using ccache-stats before they migrate to ccache-options. thanks. Roy
Dear Tzu-Jung Lee, On Thu, 10 Oct 2013 12:51:27 -0700, Tzu-Jung Lee wrote: > > Did you see my other comment about combining the ifeq statements of > > ccache-stats and ccache-options? > > Yes, but I think it probably would be better as a separate patch, > which: > > 1. either remove the ccache-stats code & and manual section > 2. or issue warning about the obsolete usage. > > So this patch alone don't affect those are currently using > ccache-stats before they migrate to ccache-options. I believe there might be a misunderstanding here. You seem to think Thomas suggested to remove ccache-stats because ccache-options now allows to do the same. But in fact, Thomas only suggested to change: """ ifeq ($(BR2_CCACHE),y) ccache-stats: ... endif ifeq ($(BR2_CCACHE),y) ccache-options: ... endif """ by """ ifeq ($(BR2_CCACHE),y) ccache-stats: ... ccache-options: ... endif """ i.e, ccache-stats continues to exist, the only difference with your implementation is that it sits under the same ifeq ($(BR2_CCACHE),y) condition. It's just a minor nit. Best regards, Thomas
On Thu, Oct 10, 2013 at 1:57 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Tzu-Jung Lee, > > On Thu, 10 Oct 2013 12:51:27 -0700, Tzu-Jung Lee wrote: >> > Did you see my other comment about combining the ifeq statements of >> > ccache-stats and ccache-options? >> >> Yes, but I think it probably would be better as a separate patch, >> which: >> >> 1. either remove the ccache-stats code & and manual section >> 2. or issue warning about the obsolete usage. >> >> So this patch alone don't affect those are currently using >> ccache-stats before they migrate to ccache-options. > > I believe there might be a misunderstanding here. You seem to think > Thomas suggested to remove ccache-stats because ccache-options now > allows to do the same. > > But in fact, Thomas only suggested to change: > > """ > ifeq ($(BR2_CCACHE),y) > ccache-stats: > ... > endif > > ifeq ($(BR2_CCACHE),y) > ccache-options: > ... > endif > """ > > by > > """ > ifeq ($(BR2_CCACHE),y) > ccache-stats: > ... > > ccache-options: > ... > endif > """ > > i.e, ccache-stats continues to exist, the only difference with your > implementation is that it sits under the same ifeq ($(BR2_CCACHE),y) > condition. It's just a minor nit. Oops, my bad. Could you help do the inline editing when you merge it? thanks. Roy > > Best regards, > > Thomas > -- > Thomas Petazzoni, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com
Dear Tzu-Jung Lee, On Thu, 10 Oct 2013 14:01:40 -0700, Tzu-Jung Lee wrote: > > i.e, ccache-stats continues to exist, the only difference with your > > implementation is that it sits under the same ifeq ($(BR2_CCACHE),y) > > condition. It's just a minor nit. > > Oops, my bad. > Could you help do the inline editing when you merge it? I'm not the maintainer, so I can't merge patches. But most likely, Peter Korsgaard can do it when merging your patch. Thanks a lot for all your quick updates to this patch! Best regards, Thomas
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: >Dear Tzu-Jung Lee, > >On Thu, 10 Oct 2013 14:01:40 -0700, Tzu-Jung Lee wrote: > >> > i.e, ccache-stats continues to exist, the only difference with your >> > implementation is that it sits under the same ifeq ($(BR2_CCACHE),y) >> > condition. It's just a minor nit. >> >> Oops, my bad. >> Could you help do the inline editing when you merge it? > >I'm not the maintainer, so I can't merge patches. But most likely, >Peter Korsgaard can do it when merging your patch. > >Thanks a lot for all your quick updates to this patch! Yes, I certainly agree with that! Acked-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>>>>> "Tzu-Jung" == Tzu-Jung Lee <roylee17@gmail.com> writes: > usage: > # set cache limit size > make CCACHE_OPTIONS="--max-size=5G" ccache-options > # zero statistics counters > make CCACHE_OPTIONS="--zero-stats" ccache-options > Signed-off-by: Tzu-Jung Lee <tjlee@ambarella.com> Committed with the redundant ifeq removed as pointed out by Thomas, thanks.
diff --git a/docs/manual/ccache-support.txt b/docs/manual/ccache-support.txt index 4969180..fe06a01 100644 --- a/docs/manual/ccache-support.txt +++ b/docs/manual/ccache-support.txt @@ -23,3 +23,14 @@ remove this directory. You can get statistics on the cache (its size, number of hits, misses, etc.) by running +make ccache-stats+. + +The make target +ccache-options+ and the +CCACHE_OPTIONS+ variable +provide more generic access to the ccache. For example + +----------------- +# set cache limit size +make CCACHE_OPTIONS="--max-size=5G" ccache-options + +# zero statistics counters +make CCACHE_OPTIONS="--zero-stats" ccache-options +----------------- diff --git a/package/ccache/ccache.mk b/package/ccache/ccache.mk index c5e9385..7b6155d 100644 --- a/package/ccache/ccache.mk +++ b/package/ccache/ccache.mk @@ -45,3 +45,14 @@ ifeq ($(BR2_CCACHE),y) ccache-stats: host-ccache $(Q)$(CCACHE) -s endif + +ifeq ($(BR2_CCACHE),y) +ccache-options: host-ccache +ifeq ($(CCACHE_OPTIONS),) + $(Q)echo "Usage: make ccache-options CCACHE_OPTIONS=\"opts\"" + $(Q)echo "where 'opts' corresponds to one or more valid ccache options" \ + "(see ccache help text below)" + $(Q)echo +endif + $(Q)$(CCACHE) $(CCACHE_OPTIONS) +endif
usage: # set cache limit size make CCACHE_OPTIONS="--max-size=5G" ccache-options # zero statistics counters make CCACHE_OPTIONS="--zero-stats" ccache-options Signed-off-by: Tzu-Jung Lee <tjlee@ambarella.com> --- Include the useful warning message suggested by Thomas. We might need a separate patch to obsolete the ccache-stats later. docs/manual/ccache-support.txt | 11 +++++++++++ package/ccache/ccache.mk | 11 +++++++++++ 2 files changed, 22 insertions(+)