[Piglit] [PATCH] core.py: don't overwrite user's PIGLIT_SOURCE_DIR env var

[Brian Paul]

On 09/22/2014 05:41 PM, Dylan Baker wrote:
> On Monday, September 22, 2014 11:52:11 AM Brian Paul wrote:
>> When creating an Options object, use the environment's PIGLIT_SOURCE_DIR
>> value when available.
>>
>> Fixes spec/ARB_ES3_compatibility/oes_compressed_etc2_texture-miptree test
>> failures in Cywin.
> cygwin
>>
>> Also, remove the MESA_DEBUG=silent setting.  Piglit shouldn't care if
>> debug output is on/off (we stopped looking for Mesa error messages a
>> long time ago).
>
> My understanding from Eric's comment (That I think I deleted on
> accident) was that with mesa debug not set to silent some tests would
> generate so much stdout/stderr chatter that it could crash piglit.

I don't think there's that much debug output (unless some drivers are 
really verbose).  In any case, I think it's better for users to set 
their env vars as needed rather than have piglit muck with them.


>
>> ---
>>   framework/core.py |   12 ++++++------
>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/framework/core.py b/framework/core.py
>> index f7d6f71..2ca17df 100644
>> --- a/framework/core.py
>> +++ b/framework/core.py
>> @@ -107,12 +107,12 @@ class Options(object):
>>           self.sync = sync
>>           # env is used to set some base environment variables that are not going
>>           # to change across runs, without sending them to os.environ which is
>> -        # fickle as easy to break
>> -        self.env = {
>> -            'PIGLIT_SOURCE_DIR': os.path.abspath(
>> -                os.path.join(os.path.dirname(__file__), '..')),
>> -            'MESA_DEBUG': 'silent',
>> -        }
>> +        # fickle and easy to break
>> +        self.env = {}
>> +        if 'PIGLIT_SOURCE_DIR' in os.environ:
>> +            self.env['PIGLIT_SOURCE_DIR'] = os.environ['PIGLIT_SOURCE_DIR']
>> +        else:
>> +            self.env['PIGLIT_SOURCE_DIR'] = os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))
>
> Instead of an if/else could we use dict's get() method? It takes a value
> to get, and then a fallback value if that value doesn't exist (default
> is None)
>
> self.env = {
>      'PIGLIT_SOURCE_DIR': os.environ.get(
> 		'PIGLIT_SOURCE_DIR',
> 		os.path.abspath(os.path.join(...))
> 	)
> }

Looks good.  My python knowledge is a bit shallow.  I'll post a v2 after 
testing.

Thanks for the review!

-Brian