On 8 September 2017 at 18:36, Eric Engestrom <eric.engest...@imgtec.com> wrote: > On Friday, 2017-09-08 08:13:54 -0700, Kenneth Graunke wrote: >> On Friday, September 8, 2017 6:33:44 AM PDT Emil Velikov wrote: >> > Hi Eric, >> > >> > On 8 September 2017 at 13:40, Eric Engestrom <eric.engest...@imgtec.com> >> > wrote: >> > > Instead of setting based on set/unset, allow users to use boolean values. >> > > In the docs, use `ALWAYS=true` instead of `ALWAYS=1` as it's clearer IMO. >> > > >> > I'm a bit weary about this series. In the past we had cases where >> > people will set the variable to empty string and/or 0 and expect >> > things to work. >> > >> > IIRC the older Intel QA team was using it, not sure about now. Either >> > way it's great to confirm with a few people before pushing this. >> >> That team is gone. I like the consistency of using env_var_as_boolean >> across the codebase - it might break a few things, but I think that's >> okay. Most people don't use this, and most of those that do use =1. > > Agreed. > >> >> > That aside, env_var_as_boolean() treats 'yes', '1' and 'true' in >> > identical manner. Perhaps we can state what a "boolean variable" is >> > and use that reference through the documentation? >> > >> > -Emil >> >> That seems like a good idea. > > Yes indeed, and I'll do that, but as a follow up patch. Sure thing.
> I'm thinking of adding something like this at the top of envvar.html: > > There are different kind of environment variables: > - BOOL: expects 1/true/yes or 0/false/no. > - INT: expects an integer value. > - ENUM: expects an string, but only a limited set of values is > accepted. > - PATH: expects a file or folder path. > - NAME: expects a string name (like a driver name), but not a full > path. > - NON-NULL: any value enables it (including empty string); use > `unset` to disable. > Note: some variables expect more complicated values, like > MESA_EXTENSION_OVERRIDE. > > and then each env var in the doc would get an annotation: > LIBGL_ALWAYS_SOFTWARE (BOOL) > LIBGL_SHOW_FPS (INT) This should really be a bool, but that's for another time. > EGL_LOG_LEVEL (ENUM: debug, info, warning, fatal) > EGL_DRIVERS_PATH (PATH) > EGL_DRIVER (PATH or NAME) > EGL_PLATFORM (NAME) > ... etc. > > Hopefully, this should be clear enough. The last option (NON-NULL) would > be applied to the old-style toggles. > Seems pretty clear over here. Although damn man... that's really nice and comprehensive. > For now, there seems to be another issue; I sent this series to travis, > and EGL is failing to link [1] because every exported symbol in mesautil > appears twice; some sort of double-link issue? > I don't know if I'm tired or thick, but I can't figure it out :] > > [1] https://travis-ci.org/1ace/mesa/jobs/273302410#L6797 > > I'd appreciate any help :) > I've spotted it - libEGL_common conditionally uses libmesautil.la (for the wayland code). I'd just make that unconditionally and omit it from libEGL/libEGL_mesa. With the above fix, the patch is Reviewed-by: Emil Velikov <emil.veli...@collabora.com> -Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
