[Mesa-dev] [PATCH] clover: add --with-icd-file-dir option

[Tom Stellard]

On Thu, May 07, 2015 at 04:59:41PM +0900, Michel Dänzer wrote:
> On 05.05.2015 01:47, Tom Stellard wrote:
> > On Mon, May 04, 2015 at 10:13:19AM -0400, Ilia Mirkin wrote:
> >> On Mon, May 4, 2015 at 10:04 AM, Tom Stellard <tom at stellard.net> wrote:
> >>> On Sat, May 02, 2015 at 01:31:41PM -0400, Ilia Mirkin wrote:
> >>>> On Sat, May 2, 2015 at 1:19 PM, EdB <edb+mesa at sigluy.net> wrote:
> >>>>> The standard ICD file path is /etc/OpenCL/vendor/.
> >>>>> However it doesn't fit well with custom build.
> >>>>> This option allow ICD vendor file installation path override
> >>>>> ---
> >>>>>  configure.ac                           | 6 ++++++
> >>>>>  src/gallium/targets/opencl/Makefile.am | 2 +-
> >>>>>  2 files changed, 7 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/configure.ac b/configure.ac
> >>>>> index 095e23e..bf08d76 100644
> >>>>> --- a/configure.ac
> >>>>> +++ b/configure.ac
> >>>>> @@ -2005,6 +2005,12 @@ AC_ARG_WITH([d3d-libdir],
> >>>>>      [D3D_DRIVER_INSTALL_DIR="$withval"],
> >>>>>      [D3D_DRIVER_INSTALL_DIR="${libdir}/d3d"])
> >>>>>  AC_SUBST([D3D_DRIVER_INSTALL_DIR])
> >>>>> +AC_ARG_WITH([icd-file-dir],
> >>>>> +    [AS_HELP_STRING([--with-icd-file-dir=DIR],
> >>>>> +        [directory for the OpenCL ICD vendor file @<:@/etc/OpenCL/vendors@:>@])],
> >>>>> +    [ICD_FILE_INSTALL_DIR="$withval"],
> >>>>> +    [ICD_FILE_INSTALL_DIR="/etc/OpenCL/vendors"])
> >>>>
> >>>> What about making this default to ${sysconfdir}/OpenCL/vendors ? That
> >>>> way using --prefix should auto-make it go into the prefix instead of
> >>>> unexpectedly installing things outside of the specified prefix? That
> >>>> way a distro build which specifies --sysconfdir as /etc will get it in
> >>>> the right place, while by default it'll go into /usr/local/etc and a
> >>>> user can override the icd loader's default behaviour with
> >>>> OPENCL_VENDOR_PATH?
> >>>>
> >>>
> >>> I would prefer not to make this the default behavior, because it violates the spec
> >>> and there could potentially be multiple icd implementations, which may or may not have
> >>> the overrides.
> >>>
> >>> I think the best solution would be to rename the option to something like
> >>> --enable-ocl-icd-respect-prefix (suggestions for other names encouraged).
> >>> and have the option enable the behavior that Ilia is describing.
> >>>
> >>> This will give distros and advanced users a way to setup their system
> >>> the way they want.
> >>
> >> It's just a very anti-autoconf thing to do to have "make install" fail
> >> by default unless you specify some "hey, i actually want make install
> >> to work" option.
> >>
> >> I think it's crazy to expect that, by default, people will want to
> >> write over their system installs, and having things go outside of the
> >> specified --prefix is very surprising (unless you force some other
> >> option). And asking the user to run "make install" as root is even
> >> crazier.
> >>
> > 
> > My expectation is that, by default, when people specify --enable-opencl-icd
> > they want an implementation that conforms to the specification.
> > Unfortunately, this means installing icd files to /etc.
> > 
> > There is no good solution here, but I'd rather have users specify a flag
> > to get a sane build system, than requiring them to set a flag and set
> > an environment variable just to get working OpenCL with the ICD loader.
> > 
> >> I guess I haven't hit this yet because there's no OpenCL support in
> >> nouveau or freedreno, but I made the same stink about vdpau when Emil
> >> tried to make it install to some system location by default. At least
> >> a few people seemed to agree with me back then...
> >>
> > 
> > Does the vdpau spec also require installation to a specific system director
> > (e.g. /etc/) ?
> 
> Tom, I think ensuring that the OpenCL ICD loader can pick up the
> mesa.icd file is something for the distributor / administrator / user to
> worry about, not Mesa upstream.
> 

I don't really disagree with this in general.  My position is that when
there is a situation where it is impossible to follow both the API spec
and build system best practices that it is more important to follow the
API spec.

I realize some people disagree with this, and I completely understand
their rationale.

For this particular situation, I'm happy with any solution that:

1. Allows a user to install the icd file to /etc if he or she wants to.
and
2. Does not require the user to read the spec to know that /etc is the
correct place to install it.

I think EdB's latest patch is a good solution:
http://lists.freedesktop.org/archives/mesa-dev/2015-May/083661.html

-Tom

> There's a similar situation with the drirc file, which is installed
> inside the prefix by default but only read from /etc/.
> 
> 
> -- 
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer