[Mesa-dev] [PATCH 2/4] anv: use cache uuid based on the build timestamp.

[Emil Velikov]

On 22 June 2016 at 17:07, Eric Engestrom <eric.engestrom at imgtec.com> wrote:
> On Wed, Jun 22, 2016 at 04:57:39PM +0100, Eric Engestrom wrote:
>> On Wed, Jun 22, 2016 at 01:04:41PM +0100, Emil Velikov wrote:
>> > From: Emil Velikov <emil.velikov at collabora.com>
>
> [...]
>
>> > diff --git a/src/intel/vulkan/Makefile.am b/src/intel/vulkan/Makefile.am
>> > index 4d9ff90..8332ae5 100644
>> > --- a/src/intel/vulkan/Makefile.am
>> > +++ b/src/intel/vulkan/Makefile.am
>> > @@ -131,7 +131,20 @@ anv_entrypoints.c : anv_entrypoints_gen.py $(vulkan_include_HEADERS)
>> >     $(AM_V_GEN) cat $(vulkan_include_HEADERS) |\
>> >     $(PYTHON2) $(srcdir)/anv_entrypoints_gen.py code > $@
>> >
>> > -BUILT_SOURCES = $(VULKAN_GENERATED_FILES)
>> > +
>> > +.PHONY: anv_timestamp.h.tmp
>> > +anv_timestamp.h.tmp:
>> > +   $(AM_V_GEN) echo "#define ANV_TIMESTAMP `date +%s`" > $@
>> > +
>> > +anv_timestamp.h: anv_timestamp.h.tmp
>> > +   @echo "Updating anv_timestamp.h"
>> > +   @if ! cmp -s anv_timestamp.h.tmp anv_timestamp.h; then \
>> > +           mv anv_timestamp.h.tmp anv_timestamp.h ;\
>> > +   else \
>> > +           rm anv_timestamp.h.tmp ;\
>> > +   fi
>>
>> For the else branch to be taken, the target would have to be generated
>> twice within the same second, and I don't see why we would want to avoid
>> modifying anv_timestamp.h's timestamp in this case, since that would be
>> the only thing that would be changed, and said timestamp would be within
>> the same second anyway.
>> I suggest to drop the `if` and unconditionally do the move.
>>
>> Since this wouldn't change the behaviour anyway, with or without my
>> suggestion this patch is:
>> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>
>
>
> Actually, the .tmp target isn't needed either, you just have to tag the
> .h target as PHONY:
>
>   .PHONY: anv_timestamp.h
>   anv_timestamp.h:
>         @echo "Updating anv_timestamp.h"
>         $(AM_V_GEN) echo "#define ANV_TIMESTAMP `date +%s`" > $@
>
> Unless I'm missing something, this has the exact same behaviour.
Hmm yes, we don't need the temporary here since we _want_ the .h to be
regenerated on each make invocation (unlike git_sha1.h where this came
from).
In practise this patch does the same, although it is an overkill :-)

Thanks
Emil