[Mesa-dev] [PATCH] anv: Give the installed intel_icd.json file an absolute path

[Emil Velikov]

On 20 August 2016 at 07:25, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On Aug 19, 2016 18:05, "Eric Engestrom" <eric.engestrom at imgtec.com> wrote:
>>
>> On Fri, Aug 19, 2016 at 09:04:14AM -0700, Jason Ekstrand wrote:
>> > Not providing a path allows the ICD to work on multi-arch systems but
>> > breaks it if you install anywhere other than /usr/lib.  Given that users
>> > may be installing locally in .local or similar, we probably do want to
>> > provide a filename.  Distros can carry a revert of this commit if they
>> > want
>> > an intel_icd.json file without the path.
>> >
Not sure I understand what this patch fixes that the Vulkan spec does
not have a recommendation or two for. Can anyone elaborate step by
step what we're having here, please ?

Furthermore this sounds like a developer only thing, so perhaps we
should restrict it there ? As-is we're effectively annoying some/most
distros.

>> > Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
>> > Cc: Mark Janes <mark.a.janes at intel.com>
>>
>> I have one question (below), but this patch is good regardless:
>> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>
>>
>> > ---
>> >  src/intel/vulkan/.gitignore                            | 1 +
>> >  src/intel/vulkan/Makefile.am                           | 7 ++++++-
>> >  src/intel/vulkan/{intel_icd.json => intel_icd.json.in} | 2 +-
>> >  3 files changed, 8 insertions(+), 2 deletions(-)
>> >  rename src/intel/vulkan/{intel_icd.json => intel_icd.json.in} (59%)
>> >
>> > diff --git a/src/intel/vulkan/.gitignore b/src/intel/vulkan/.gitignore
>> > index bde5cd8..a099ff6 100644
>> > --- a/src/intel/vulkan/.gitignore
>> > +++ b/src/intel/vulkan/.gitignore
>> > @@ -3,3 +3,4 @@
>> >  /anv_entrypoints.h
>> >  /anv_timestamp.h
>> >  /dev_icd.json
>> > +/intel_icd.json
>> > diff --git a/src/intel/vulkan/Makefile.am b/src/intel/vulkan/Makefile.am
>> > index ad0148d..9fef960 100644
>> > --- a/src/intel/vulkan/Makefile.am
>> > +++ b/src/intel/vulkan/Makefile.am
>> > @@ -141,7 +141,7 @@ anv_timestamp.h:
>> >       $(AM_V_GEN) echo "#define ANV_TIMESTAMP \"$(TIMESTAMP_CMD)\"" > $@
>> >
>> >  BUILT_SOURCES = $(VULKAN_GENERATED_FILES)
>> > -CLEANFILES = $(BUILT_SOURCES) dev_icd.json
>> > +CLEANFILES = $(BUILT_SOURCES) dev_icd.json intel_icd.json
>> >  EXTRA_DIST = \
>> >       $(top_srcdir)/include/vulkan/vk_icd.h \
>> >       anv_entrypoints_gen.py \
>> > @@ -170,6 +170,11 @@ dev_icd.json : dev_icd.json.in
>> >               -e "s#@build_libdir@#${abs_top_builddir}/${LIB_DIR}#" \
>> >               < $(srcdir)/dev_icd.json.in > $@
>> >
>> > +intel_icd.json : intel_icd.json.in
>> > +     $(AM_V_GEN) $(SED) \
>> > +             -e "s#@install_libdir@#${libdir}#" \
>> > +             < $(srcdir)/intel_icd.json.in > $@
>>
>> I think I may have already asked when dev_icd.json was added, but why
>> use a relative path for the dependency and a full path when reading it?
>> Why not use the full path for the dep and read `< $<` instead?
>
> No good reason that I know of other than copy+paste of the one above. We
> could probably make that change.  Emil?
>
IIRC '< $<' was only applicable in some cases according to POSIX make.
One the relative/full path, I think that non-gnumake implementations
were having issues with the proposed solution. If anyone can confirm
that it works I'm ok with changing it.

Thanks
Emil