On 23 August 2016 at 04:33, Nicholas Miell <nmi...@gmail.com> wrote: > On 08/22/2016 04:10 PM, Julien Cristau wrote: >> >> On Mon, Aug 22, 2016 at 14:18:51 -0700, Jason Ekstrand wrote: >> >>> On Mon, Aug 22, 2016 at 2:06 PM, Julien Cristau <jcris...@debian.org> >>> wrote: >>> >>>> On Fri, Aug 19, 2016 at 09:04:14 -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. >>>>> >>>> If a user is going to install stuff in .local, don't they have >>>> LD_LIBRARY_PATH pointing there too? >>>> >>> >>> Actually, no. The loader will look for ICD files in >>> .local/share/vulkan/icd.d and the ICD file will point to the right .so. >>> It >>> should work out-of-the-box unless you either have a broken loader or >>> we're >>> installing something wrong. My understanding of the following section was that the user is responsible of placing the ICD in such a way that dlopen finds it. Thus it was rather confusing to me how/why this patch was needed since an initial assumption of using LD_LIBRARY_PATH (as you would if you install anything into the non-standard prefix) was taken as granted.
Although I see your point - asking people to fiddle with LD_LIBRARY_PATH is a bit annoying. "If the ICD is specified via a filename, the loader will attempt to open that file as a shared object using dlopen(), and the file must be in a directory that dlopen is _configured_ to look in (Note: various distributions are configured differently)" * My emphasis on configured >> >> >> So somehow they're only building the vulkan driver but not libGL or >> anything else? Still, I guess a bunch of people will need both a 32bit >> and a 64bit version of the driver. How is the 64-bit >> ~/.local/share/vulkan/icd.d/intel_icd.json not going to clash with the >> 32-bit ~/.local/share/vulkan/icd.d/intel_icd.json? I'm just not seeing >> how this solves the problem... >> > > To recap the IRC discussion: > > ld-linux.so will expand $LIB to lib or lib64 as appropriate. > > Unfortunately, Debian patches glibc to make $LIB expand to the target triple > and there's no convenient way to figure out how $LIB expands at build time. > > So you can just put $LIB in your architecture-independent ICD file and > everything will work just fine, you just have to manually put the shared > libraries in the appropriate location for your distribution. > Skimmed through the discussion and I'm not sure the above will be enough. Since the user is free to place json files in $HOME/.local ... this implies that they may _not_ have access to /usr or /etc. Thus as they install the file (to say $HOME/foo/lib) the Vulkan loader will not be able to pick it up. In theory one should be able to have a varying .json (one with and one without the path) although the heuristics will be fragile due to the varying $LIB (like the case of Debian and derivatives) and $DESTDIR. So I see the following options: - new configure option - have to agree with Dave, please don't. Furthermore we already have more than 50! of them. - fold the "w or w/o full path" under and existing option (--enable-debug ?) - somewhat picky/fragile as well. Kind of OK for short term solution. - use LD_LIBRARY_PATH - slightly annoying yet add once and forget about it. OK-ish short term solution. - json update - the better long term solution imho. Json update: - the same file _cannot_ be provided by multiple packages - thus we should use differently named files, but we should not rely on the filename to determine arch/triple. There has been no such recommendations/restrictions about the name so starting now feels wrong. - so let's bump "file_format_version" - 1.0.1 or 1.1.0 ? - add ICD::arch tag and teach the loader to honour it. For json files missing it we'll assume a "can be used on this platform" behaviour. - should we bother with the other parts of the triple or add it only as need arise ? With the above in place, one can have full path + filename for the mutlilib setups. Old loaders will just get an error upon dlopen of incompatible/wrong arch ICDs. How do the above short/long term solutions sound ? Thanks Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
