[Piglit] [PATCH v2 3/7] wflinfo: Reimplement wflinfo separating the various API's

[Eric Anholt]

Dylan Baker <dylan at pnwbakers.com> writes:

> Currently fast skipping is implemented such that it assumes there's a
> single version of ES, a single version of desktop, and all extensions
> are shared between them. This has basically worked because 1) there are
> basically no gles1 tests, and 2) piglit didn't have compat profile. But
> worked and correct are two different things.
>
> With the addition of compat profiles it's time to re-evaluate how fast
> skipping works. Namely we need to have different attributes for ES1,
> ES1+, core, compat, and I've added on for "legacy" (pre-profile), since
> waffle supports that.
>
> This maintains legacy interfaces so that existing code continues to
> work.
>
> v2: - Fix versions < 3.1 on implementations without core profile
> ---

I haven't wanted to look at this series because I don't like python, but
I had a few minutes today so I started to review.

>  framework/wflinfo.py                | 320 ++++++++++++++++------------
>  unittests/framework/test_wflinfo.py |  23 +-
>  2 files changed, 201 insertions(+), 142 deletions(-)
>
> diff --git a/framework/wflinfo.py b/framework/wflinfo.py
> index 8c7da084a..d3a79437d 100644
> --- a/framework/wflinfo.py
> +++ b/framework/wflinfo.py
> @@ -25,12 +25,14 @@ import errno
>  import os
>  import subprocess
>  import sys
> +import threading
>  
>  import six
>  
> -from framework import exceptions, core
> +from framework import exceptions
> +from framework.core import lazy_property
>  from framework.options import OPTIONS
> -from framework.test import piglit_test
> +# from framework.test import piglit_test

Commented out line meant to be removed?

>  class StopWflinfo(exceptions.PiglitException):
> @@ -40,6 +42,15 @@ class StopWflinfo(exceptions.PiglitException):
>          self.reason = reason
>  
>  
> +class ProfileInfo(object):
> +    """Information about a single profile (core, compat, es1, es2, etc)."""
> +
> +    def __init__(self, shader_version, language_version, extensions):
> +        self.shader_version = shader_version
> +        self.api_version = language_version
> +        self.extensions = extensions
> +
> +
>  class WflInfo(object):
>      """Class representing platform information as provided by wflinfo.
>  
> @@ -56,6 +67,15 @@ class WflInfo(object):
>  
>      """
>      __shared_state = {}
> +    __core_init = False
> +    __core_lock = threading.Lock()
> +    __compat_init = False
> +    __compat_lock = threading.Lock()
> +    __es1_init = False
> +    __es1_lock = threading.Lock()
> +    __es2_init = False
> +    __es2_lock = threading.Lock()
> +

All this init/locking stuff feels tremendously excessive, and if it was
necessary it ought to be part of lazy_property.  But I expect that you
could just accept the race to create two of the object representing the
driver's capabilities.

>      def __new__(cls, *args, **kwargs): # Implement the borg pattern:
> #
> https://code.activestate.com/recipes/66531-singleton-we-dont-need-no-stinkin-singleton-the-bo/
> @@ -94,9 +114,9 @@ class WflInfo(object):
>  
>                  # setup execution environment where we extend the PATH env var
>                  # to include the piglit TEST_BIN_DIR
> -                new_env = os.environ
> -                new_env['PATH'] = ':'.join([piglit_test.TEST_BIN_DIR,
> -                                            os.environ['PATH']])
> +                new_env = os.environ.copy()
> +                # new_env['PATH'] = ':'.join([piglit_test.TEST_BIN_DIR,
> +                                            # os.environ['PATH']])

What's going on with this being commented out?  Why is it changing in
this patch?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20180727/ae572863/attachment.sig>