[Mesa-dev] [PATCH v2 12/23] glsl/tests/optimization-test: correctly manage srcdir/builddir

[Emil Velikov]

On 19 March 2017 at 15:07, Eric Engestrom <eric at engestrom.ch> wrote:
> On Friday, 2017-03-17 13:19:38 +0000, Emil Velikov wrote:
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> At the moment we look for generator script(s) in builddir while they
>> are in srcdir, and we proceed to generate the tests and expected output
>> in srcdir, which is not allowed.
>>
>> To untangle:
>>  - look for the generator script in the correct place
>>  - generate the files in builddir, by extending create_test_cases.py to
>> use --outdir
>>
>> With this in place the test passes `make check' for OOT builds - would
>> that be as standalone or part of `make distcheck'
>>
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>> ---
>>  src/compiler/glsl/tests/lower_jumps/create_test_cases.py | 13 +++++++++++--
>>  src/compiler/glsl/tests/optimization-test.sh             | 10 ++++++++--
>>  2 files changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/compiler/glsl/tests/lower_jumps/create_test_cases.py b/src/compiler/glsl/tests/lower_jumps/create_test_cases.py
>> index 3be1079bc14..defff2ed34f 100644
>> --- a/src/compiler/glsl/tests/lower_jumps/create_test_cases.py
>> +++ b/src/compiler/glsl/tests/lower_jumps/create_test_cases.py
>> @@ -21,6 +21,7 @@
>>  # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>>  # DEALINGS IN THE SOFTWARE.
>>
>> +import argparse
>>  import os
>>  import os.path
>>  import re
>> @@ -30,6 +31,7 @@ import sys
>>  sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) # For access to sexps.py, which is in parent dir
>>  from sexps import *
>>
>> +outdir = "."
>>  def make_test_case(f_name, ret_type, body):
>>      """Create a simple optimization test case consisting of a single
>>      function with the given name, return type, and body.
>> @@ -289,14 +291,14 @@ def create_test_case(doc_string, input_sexp, expected_sexp, test_name,
>>              pull_out_jumps, lower_sub_return, lower_main_return,
>>              lower_continue, lower_break))
>>      args = ['../../glsl_test', 'optpass', '--quiet', '--input-ir', optimization]
>> -    test_file = '{0}.opt_test'.format(test_name)
>> +    test_file = os.path.join(outdir, '{0}.opt_test'.format(test_name))
>>      with open(test_file, 'w') as f:
>>          f.write('#!/usr/bin/env bash\n#\n# This file was generated by create_test_cases.py.\n#\n')
>>          f.write(doc_string)
>>          f.write('{0} <<EOF\n'.format(bash_quote(*args)))
>>          f.write('{0}\nEOF\n'.format(input_str))
>>      os.chmod(test_file, 0774)
>> -    expected_file = '{0}.opt_test.expected'.format(test_name)
>> +    expected_file = os.path.join(outdir, '{0}.opt_test.expected'.format(test_name))
>>      with open(expected_file, 'w') as f:
>>          f.write('{0}\n'.format(expected_output))
>>
>> @@ -623,6 +625,13 @@ def test_lower_return_non_void_at_end_of_loop():
>>                       lower_sub_return=True, lower_break=True)
>>
>>  if __name__ == '__main__':
>> +    parser = argparse.ArgumentParser()
>> +    parser.add_argument('--outdir',
>> +                        help='Directory to put the generated files in',
>> +                        required=True)
>> +    args = parser.parse_args()
>> +    outdir = args.outdir
>> +
>>      test_lower_returns_main()
>>      test_lower_returns_sub()
>>      test_lower_returns_1()
>> diff --git a/src/compiler/glsl/tests/optimization-test.sh b/src/compiler/glsl/tests/optimization-test.sh
>> index 47970c6be29..1113cb1f17c 100755
>> --- a/src/compiler/glsl/tests/optimization-test.sh
>> +++ b/src/compiler/glsl/tests/optimization-test.sh
>> @@ -28,13 +28,19 @@ compare_ir=$srcdir/glsl/tests/compare_ir.py
>>  total=0
>>  pass=0
>>
>> +# Store our location before we start diving into subdirectories.
>> +ORIGDIR=`pwd`
>>  echo "======       Generating tests      ======"
>> -for dir in tests/*/; do
>> +for dir in $srcdir/glsl/tests/*/; do
>>      if [ -e "${dir}create_test_cases.py" ]; then
>> -        cd $dir; $PYTHON2 create_test_cases.py; cd ..
>> +        # construct the correct builddir
>> +        completedir="$abs_builddir/glsl/tests/`echo ${dir} | sed 's|.*/glsl/tests/||g'`"
>
> Nit: s/completedir/testoutdir/ ?
>
>> +        mkdir -p $completedir
>> +        cd $dir; $PYTHON2 create_test_cases.py --outdir $completedir; cd ..
>
> I haven't ran this script, but it looks like `cd ..` is kinda pointless
> here, it doesn't get you back anywhere useful?
>
This was there since the original - iirc the gist was that "dir" is
relative, so we should go up/down the same amount of we end up in ...

> Also, any reason not to use pushd/popd here?
> (I think I asked this already asked this, but I can't remember/find the
> answer.)
>
pushd/popd are bashisms

>>      fi
>>      echo "$dir"
>>  done
>> +cd "$ORIGDIR"
>
> This shouldn't be needed? I can't see how this could be useful, unless
> you use `cd` back and forth and mess it up.
>
It's also required I'm afraid. As we do OOT builds builddir != srcdir
so we'll end up in the wrong place otherwise.

I might be missing something, but please splinkle a few "echo `pwd`"
and you'll see what I'm talking about.

Thanks
Emil