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

[Emil Velikov]

On 24 June 2016 at 03:59, Dave Airlie <airlied at gmail.com> wrote:
> On 23 June 2016 at 23:25, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 23 June 2016 at 03:49, Michel Dänzer <michel at daenzer.net> wrote:
>>> On 22.06.2016 21:04, Emil Velikov wrote:
>>>> From: Emil Velikov <emil.velikov at collabora.com>
>>>>
>>>> Do not rely on the git sha1:
>>>>  - its current truncated form makes it less unique
>>>>  - it does not attribute for local (Vulkand or otherwise) changes
>>>>
>>>> Use a timestamp produced at the time of build. It's perfectly unique,
>>>> unless someone explicitly thinkers with their system clock. Even then
>>>> chances of producing the exact same one are very small, if not zero.
>>>>
>>>> Cc: Jason Ekstrand <jason at jlekstrand.net>
>>>> Cc: mesa-stable at lists.freedesktop.org
>>>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>>>> ---
>>>> Current approach uses seconds since Epoch, but if people prefer we
>>>> can use nano seconds, combination of the two and/or other.
>>>> ---
>>>>  src/intel/vulkan/.gitignore   |  2 ++
>>>>  src/intel/vulkan/Makefile.am  | 15 ++++++++++++++-
>>>>  src/intel/vulkan/anv_device.c |  4 ++--
>>>>  3 files changed, 18 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/intel/vulkan/.gitignore b/src/intel/vulkan/.gitignore
>>>> index a496146..7ef6a48 100644
>>>> --- a/src/intel/vulkan/.gitignore
>>>> +++ b/src/intel/vulkan/.gitignore
>>>> @@ -2,3 +2,5 @@
>>>>  /anv_entrypoints.c
>>>>  /anv_entrypoints.h
>>>>  /dev_icd.json
>>>> +/anv_timestamp.h.tmp
>>>> +/anv_timestamp.h
>>>> 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`" > $@
>>>
>>> This conflicts with efforts to make builds reproducible, see e.g.
>>> https://wiki.debian.org/ReproducibleBuilds . One technique used by such
>>> efforts is to replace any timestamps with all 0s.
>>>
>> Thanks to letting me know before things got swapped with 0s. Is there
>> a way to contact/check how other projects have handled/resolved this ?
>> The page linked looks like a policy/guidelines one and I'm struggling
>> to see any examples of how things were resolved for project X or Y :-\
>>
>>>
>>> Would it be possible to generate a hash over all source files listed in
>>> any Makefile.sources, or something like that?
>>>
>> If you want to pursue that task please do.
>>
>> A couple of humble requests a) can you get some very rough measurement
>> how long all the hashing takes, b) please try to avoid adding extra
>> external dependencies (not sure how many people will like that).
>>
>
> I think you should be taking that as a NAK for this approach.
>
Hmm possibly. I would really appreciate if people say so from the beginning.

> If one of the reasons for not using the git_sha1 is we can't detect
> patches from distros, and one distro is knowingly going to have to
> patch the approach out, then the approach isn't really suitable.
>
There are multiple reasons for reworking the git_sha1.h dependency,
here's a few:
 - multiple reports that git_sha1.h broke my workflow', with people
resisting the suggested one line fix
 - some people ignore the suggestion and place something else "which
should be enough"
 - others outright revert the patch(es) that mandate git_sha1.h
 - provides uniqueness, even if one patches the sources.

> I'm also not sure but does running make install on the same tree twice
> re-generate this file? or did we take that into a/c?
>
I did consider that - thus my 'use a define' suggestions above. Guess
I should have explicitly mentioned that it will be added _only_ to the
file(s) that need it. Thus the whole anv won't get rebuilt, as per
Jason's concern.

To sum it up: the goal is to minimise complains/reverts/other from
builders while saving Jason, you, others time debugging, when old
cache(s) are used.

-Emil