[Mesa-dev] [PATCH] gbm: Replace GBM_DRIVERS_PATH with LIBGL_DRIVERS_PATH

[Ilia Mirkin]

On Mon, Jul 21, 2014 at 11:23 AM, Kristian Høgsberg <krh at bitplanet.net> wrote:
> On Sun, Jul 20, 2014 at 11:36 PM, Eric Anholt <eric at anholt.net> wrote:
>> Emil Velikov <emil.l.velikov at gmail.com> writes:
>>
>>> On 18/07/14 17:27, Kenneth Graunke wrote:
>>>> On Friday, July 18, 2014 07:41:57 AM Dylan Baker wrote:
>>>>> Currently mesa searches for two different environment variables,
>>>>> LIBGL_DRIVERS_PATH and GBM_DRIVERS_PATH. The first is used for search
>>>>> for DRI drivers in every case except GBM, and the latter is used
>>>>> exclusively for setting GBM drivers. This patch simplifies things by
>>>>> having just one variable to set.
>>>>>
>>>>> Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
>>>>> ---
>>>>>  src/gbm/backends/dri/gbm_dri.c | 4 ++--
>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
>>>>> index 347bc99..9d9d1c4 100644
>>>>> --- a/src/gbm/backends/dri/gbm_dri.c
>>>>> +++ b/src/gbm/backends/dri/gbm_dri.c
>>>>> @@ -212,8 +212,8 @@ dri_load_driver(struct gbm_dri_device *dri)
>>>>>
>>>>>     search_paths = NULL;
>>>>>     if (geteuid() == getuid()) {
>>>>> -      /* don't allow setuid apps to use GBM_DRIVERS_PATH */
>>>>> -      search_paths = getenv("GBM_DRIVERS_PATH");
>>>>> +      /* don't allow setuid apps to use LIBGL_DRIVERS_PATH */
>>>>> +      search_paths = getenv("LIBGL_DRIVERS_PATH");
>>>>>     }
>>>>>     if (search_paths == NULL)
>>>>>        search_paths = DEFAULT_DRIVER_DIR;
>>>>>
>>>>
>>>> I'm definitely a fan of moving to LIBGL_DRIVERS_PATH for everything - GBM_DRIVERS_PATH is just another environment variable to forget to set properly.
>>>>
>>>> As is, this is:
>>>> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>>>>
>>>> Are people okay with just moving to LIBGL_DRIVERS_PATH completely like this?  Or do people want it to check GBM_DRIVERS_PATH then fall back to LIBGL_DRIVERS_PATH?  Or use $GBM_DRIVERS_PATH:$LIBGL_DRIVERS_PATH?
>>>>
>>> A few thoughts of a blurry mind (sorry I'm out of coffee):
>>>
>>> I can envision at least at couple of use-cases where either of the above will
>>> cause more harm than good, yet I feel that affected people will come here
>>> after spending hours of debugging/bisecting.
>>>
>>> IMHO the "one variable to rule them all" while convenient (set only
>>> LIBGL_DRIVERS_PATH in your debug scripts/session), is not at all useful. If
>>> the only issue is "I always forget to set it", I believe that it's not a thing
>>> that should be addressed in mesa.
>>
>> I definitely lost out recently because there are two different
>> variables, and I can't come up with any reason to have two.  What cases
>> are you thinking of?
>
> It's a historical accident more than anything.  It wasn't designed to
> work this way for any particular reason.  My only comment is that we
> should try getenv("GBM..") if getenv("LIBGL..) returns NULL.  That
> doesn't seem like a big maintenance burden and avoids breaking
> anybodys scripts.

Probably a good idea, at least for a while. I think Emil's concern was
if you're doing a bisect, depending on which side of this commit
you're on, you'd have to use one or the other variable. This would be
very annoying.

[Of course I never quite understood the point of this in the first
place... doesn't LD_LIBRARY_PATH just work for everyone? There's
probably some use-case I'm not thinking of...]

  -ilia