| Message ID | 20170726160040.6516-2-derodat@adacore.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 2017-07-26 at 18:00 +0200, Pierre-Marie de Rodat wrote: [...snip...] > diff --git a/gcc/testsuite/python/testutils.py > b/gcc/testsuite/python/testutils.py > new file mode 100644 > index 00000000000..503105ad9d0 > --- /dev/null > +++ b/gcc/testsuite/python/testutils.py > @@ -0,0 +1,45 @@ > +# Copyright (C) 2017 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or > modify > +# it under the terms of the GNU General Public License as published > by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with GCC; see the file COPYING3. If not see > +# <http://www.gnu.org/licenses/>. > + > +# Helpers to drive a testcase > + > +def print_pass(message): > + """Emit a PASS message. > + > + :param str message: Message to emit. > + """ > + print('PASS: {}'.format(message)) str.format was introduced in Python 2.6, so presumably the minimum python 2 version here is at least 2.6+; for Python 3 I believe it was present in Python 3.0 onwards. > + > +def print_fail(message): > + """Emit a FAIL message. > + > + :param str message: Message to emit. > + """ > + print('FAIL: {}'.format(message)) > + > + > +def check(predicate, message): > + """ > + If `predicate` is True, emit a PASS message, otherwise emit a > FAIL one. A very nitpicky nitpick: this comment should be spelled as "is true" (lowercase), rather than "is True" since the requirement is that predicate's "truth value" is true, rather than predicate *is* the boolean "True" singleton; e.g. if someone passes in an int as predicate, its nonzero-ness would be used, rather than always being false (since no int *is* the boolean singleton "True"). > + > + :param bool predicate: Whether the test should pass. > + :param str message: Message to emit. > + """ > + if predicate: > + print_pass(message) > + else: > + print_fail(message)
On 07/26/2017 06:25 PM, David Malcolm wrote: > str.format was introduced in Python 2.6, so presumably the minimum > python 2 version here is at least 2.6+; for Python 3 I believe it was > present in Python 3.0 onwards. Hm… Python 2.6 is fairly old: last binary release was ages ago, last source release was in 2013. Do you think it’s worth supporting it? >> +def check(predicate, message): >> + """ >> + If `predicate` is True, emit a PASS message, otherwise emit a >> FAIL one. > > A very nitpicky nitpick: this comment should be spelled as "is true" > (lowercase), rather than "is True" since the requirement is that > predicate's "truth value" is true, rather than predicate *is* the > boolean "True" singleton; e.g. if someone passes in an int as > predicate, its nonzero-ness would be used, rather than always being > false (since no int *is* the boolean singleton "True"). I agree with you: I updated the patch on my machine. Thank you!
On Wed, 2017-07-26 at 18:35 +0200, Pierre-Marie de Rodat wrote: > On 07/26/2017 06:25 PM, David Malcolm wrote: > > str.format was introduced in Python 2.6, so presumably the minimum > > python 2 version here is at least 2.6+; for Python 3 I believe it > > was > > present in Python 3.0 onwards. > > Hm… Python 2.6 is fairly old: last binary release was ages ago, last > source release was in 2013. Do you think it’s worth supporting it? IIRC RHEL 6 has Python 2.6 as its /usr/bin/python (but Python 2.7 is available as a "software collection" add-on). I don't know if gcc as a project would want to support 2.6+ or simply 2.7 for Python 2. > > > +def check(predicate, message): > > > + """ > > > + If `predicate` is True, emit a PASS message, otherwise emit > > > a > > > FAIL one. > > > > A very nitpicky nitpick: this comment should be spelled as "is > > true" > > (lowercase), rather than "is True" since the requirement is that > > predicate's "truth value" is true, rather than predicate *is* the > > boolean "True" singleton; e.g. if someone passes in an int as > > predicate, its nonzero-ness would be used, rather than always being > > false (since no int *is* the boolean singleton "True"). > > I agree with you: I updated the patch on my machine. Thank you! Thanks. I saw at least one other instance of this in the other patch; I'll look over it again. Dave
On 07/26/2017 06:48 PM, David Malcolm wrote: > IIRC RHEL 6 has Python 2.6 as its /usr/bin/python (but Python 2.7 is > available as a "software collection" add-on). > > I don't know if gcc as a project would want to support 2.6+ or simply > 2.7 for Python 2. I don’t know neither: let’s wait for further feedback, then. If needed I’ll turn all the .format into % operations.
you are unconditionally hard coding python as the interpreter, which on most distributions points to 2.7. Please check python3 as well and make that the preferred interpreter if available. python 2.7 is now EOL'd for 2020. Matthias On 26.07.2017 18:00, Pierre-Marie de Rodat wrote: > gcc/testsuite/ > > * lib/gcc-python.exp: New test library. > * python/testutils.py: New Python helper. > --- > gcc/testsuite/lib/gcc-python.exp | 95 +++++++++++++++++++++++++++++++++++++++ > gcc/testsuite/python/testutils.py | 45 +++++++++++++++++++ > 2 files changed, 140 insertions(+) > create mode 100644 gcc/testsuite/lib/gcc-python.exp > create mode 100644 gcc/testsuite/python/testutils.py > > diff --git a/gcc/testsuite/lib/gcc-python.exp b/gcc/testsuite/lib/gcc-python.exp > new file mode 100644 > index 00000000000..30cf74a87ac > --- /dev/null > +++ b/gcc/testsuite/lib/gcc-python.exp > @@ -0,0 +1,95 @@ > +# Copyright (C) 2017 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with GCC; see the file COPYING3. If not see > +# <http://www.gnu.org/licenses/>. > + > +# Helpers to run a Python interpreter > + > +load_lib "remote.exp" > + > +# Return whether a working Python interpreter is available. > + > +proc check-python-available { args } { > + set result [local_exec "python -c print(\"Hello\")" "" "" 300] > + > + set status [lindex $result 0] > + set output [string trim [lindex $result 1]] > + > + if { $status != 0 || $output != "Hello" } { > + return 0 > + } else { > + return 1 > + } > +} > + > +# Run the SCRIPT_PY Python script. Add one PASSing (FAILing) test per output > +# line that starts with "PASS: " ("FAIL: "). Also fail for any other output > +# line and for non-zero exit status code. > +# > +# The Python script can access Python modules and packages in the > +# $srcdir/python directory. > + > +proc python-test { script_py } { > + global srcdir > + > + set testname testname-for-summary > + > + # This assumes that we are three frames down from dg-test, and that > + # it still stores the filename of the testcase in a local variable "name". > + # A cleaner solution would require a new DejaGnu release. > + upvar 2 prog src_file > + > + set asm_file "[file rootname [file tail $src_file]].o" > + set script_py_path "[file dirname $src_file]/$script_py" > + > + set old_pythonpath [getenv "PYTHONPATH"] > + set support_dir "$srcdir/python" > + if { $old_pythonpath == "" } { > + setenv "PYTHONPATH" $support_dir > + } else { > + setenv "PYTHONPATH" "$support_dir:$PYTHONPATH" > + } > + > + set commandline "python $script_py_path $asm_file" > + set timeout 300 > + > + verbose -log "Executing: $commandline (timeout = $timeout)" 2 > + set result [local_exec $commandline "" "" $timeout] > + > + set status [lindex $result 0] > + set output [lindex $result 1] > + > + if { $status != 0 } { > + fail [concat "$testname: $script_py stopped with non-zero status" \ > + " code ($status)"] > + } > + > + foreach line [split $output "\n"] { > + if { $line == "" } { > + continue > + } > + if { [regexp "^PASS: (.*)" $line dummy message] } { > + pass "$testname/$script_py: $message" > + continue > + } > + if { [regexp "^FAIL: (.*)" $line dummy message] } { > + fail "$testname/$script_py: $message" > + continue > + } > + > + fail "$testname/$script_py: spurious output: $line" > + } > + > + setenv "PYTHONPATH" $old_pythonpath > +} > diff --git a/gcc/testsuite/python/testutils.py b/gcc/testsuite/python/testutils.py > new file mode 100644 > index 00000000000..503105ad9d0 > --- /dev/null > +++ b/gcc/testsuite/python/testutils.py > @@ -0,0 +1,45 @@ > +# Copyright (C) 2017 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with GCC; see the file COPYING3. If not see > +# <http://www.gnu.org/licenses/>. > + > +# Helpers to drive a testcase > + > +def print_pass(message): > + """Emit a PASS message. > + > + :param str message: Message to emit. > + """ > + print('PASS: {}'.format(message)) > + > + > +def print_fail(message): > + """Emit a FAIL message. > + > + :param str message: Message to emit. > + """ > + print('FAIL: {}'.format(message)) > + > + > +def check(predicate, message): > + """ > + If `predicate` is True, emit a PASS message, otherwise emit a FAIL one. > + > + :param bool predicate: Whether the test should pass. > + :param str message: Message to emit. > + """ > + if predicate: > + print_pass(message) > + else: > + print_fail(message) >
On 07/27/2017 10:50 AM, Matthias Klose wrote: > you are unconditionally hard coding python as the interpreter, which on most > distributions points to 2.7. Please check python3 as well and make that the > preferred interpreter if available. python 2.7 is now EOL'd for 2020. Understood, thank you. I’ll do this for the next patchset I’ll submit.
On Thu, 2017-07-27 at 10:49 +0200, Pierre-Marie de Rodat wrote: > On 07/26/2017 06:48 PM, David Malcolm wrote: > > IIRC RHEL 6 has Python 2.6 as its /usr/bin/python (but Python 2.7 > > is > > available as a "software collection" add-on). > > > > I don't know if gcc as a project would want to support 2.6+ or > > simply > > 2.7 for Python 2. > > I don’t know neither: let’s wait for further feedback, then. If > needed > I’ll turn all the .format into % operations. Note that str.format was introduced in Python 2.6, so even if we do support that old version, str.format is still good; no need to rewrite those ops. (sorry, I used to maintain Python for RHEL, so I'm perhaps over -sensitive to this kind of thing). Dave
On 07/26/2017 10:48 AM, David Malcolm wrote: > On Wed, 2017-07-26 at 18:35 +0200, Pierre-Marie de Rodat wrote: >> On 07/26/2017 06:25 PM, David Malcolm wrote: >>> str.format was introduced in Python 2.6, so presumably the minimum >>> python 2 version here is at least 2.6+; for Python 3 I believe it >>> was >>> present in Python 3.0 onwards. >> >> Hm… Python 2.6 is fairly old: last binary release was ages ago, last >> source release was in 2013. Do you think it’s worth supporting it? > > IIRC RHEL 6 has Python 2.6 as its /usr/bin/python (but Python 2.7 is > available as a "software collection" add-on). Given the age of RHEL 6, I wouldn't get too hung up on on supporting it for this stuff. Hell, it's EOL in just a few short years. > > I don't know if gcc as a project would want to support 2.6+ or simply > 2.7 for Python 2. If it was trivial, then let's support 2.6. But if it's nontrivial, I'd support stepping to something more modern. Jeff
On 08/02/2017 08:43 PM, Jeff Law wrote: > If it was trivial, then let's support 2.6. But if it's nontrivial, I'd > support stepping to something more modern. It is trivial. I’ve done it locally. :-)
diff --git a/gcc/testsuite/lib/gcc-python.exp b/gcc/testsuite/lib/gcc-python.exp new file mode 100644 index 00000000000..30cf74a87ac --- /dev/null +++ b/gcc/testsuite/lib/gcc-python.exp @@ -0,0 +1,95 @@ +# Copyright (C) 2017 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with GCC; see the file COPYING3. If not see +# <http://www.gnu.org/licenses/>. + +# Helpers to run a Python interpreter + +load_lib "remote.exp" + +# Return whether a working Python interpreter is available. + +proc check-python-available { args } { + set result [local_exec "python -c print(\"Hello\")" "" "" 300] + + set status [lindex $result 0] + set output [string trim [lindex $result 1]] + + if { $status != 0 || $output != "Hello" } { + return 0 + } else { + return 1 + } +} + +# Run the SCRIPT_PY Python script. Add one PASSing (FAILing) test per output +# line that starts with "PASS: " ("FAIL: "). Also fail for any other output +# line and for non-zero exit status code. +# +# The Python script can access Python modules and packages in the +# $srcdir/python directory. + +proc python-test { script_py } { + global srcdir + + set testname testname-for-summary + + # This assumes that we are three frames down from dg-test, and that + # it still stores the filename of the testcase in a local variable "name". + # A cleaner solution would require a new DejaGnu release. + upvar 2 prog src_file + + set asm_file "[file rootname [file tail $src_file]].o" + set script_py_path "[file dirname $src_file]/$script_py" + + set old_pythonpath [getenv "PYTHONPATH"] + set support_dir "$srcdir/python" + if { $old_pythonpath == "" } { + setenv "PYTHONPATH" $support_dir + } else { + setenv "PYTHONPATH" "$support_dir:$PYTHONPATH" + } + + set commandline "python $script_py_path $asm_file" + set timeout 300 + + verbose -log "Executing: $commandline (timeout = $timeout)" 2 + set result [local_exec $commandline "" "" $timeout] + + set status [lindex $result 0] + set output [lindex $result 1] + + if { $status != 0 } { + fail [concat "$testname: $script_py stopped with non-zero status" \ + " code ($status)"] + } + + foreach line [split $output "\n"] { + if { $line == "" } { + continue + } + if { [regexp "^PASS: (.*)" $line dummy message] } { + pass "$testname/$script_py: $message" + continue + } + if { [regexp "^FAIL: (.*)" $line dummy message] } { + fail "$testname/$script_py: $message" + continue + } + + fail "$testname/$script_py: spurious output: $line" + } + + setenv "PYTHONPATH" $old_pythonpath +} diff --git a/gcc/testsuite/python/testutils.py b/gcc/testsuite/python/testutils.py new file mode 100644 index 00000000000..503105ad9d0 --- /dev/null +++ b/gcc/testsuite/python/testutils.py @@ -0,0 +1,45 @@ +# Copyright (C) 2017 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with GCC; see the file COPYING3. If not see +# <http://www.gnu.org/licenses/>. + +# Helpers to drive a testcase + +def print_pass(message): + """Emit a PASS message. + + :param str message: Message to emit. + """ + print('PASS: {}'.format(message)) + + +def print_fail(message): + """Emit a FAIL message. + + :param str message: Message to emit. + """ + print('FAIL: {}'.format(message)) + + +def check(predicate, message): + """ + If `predicate` is True, emit a PASS message, otherwise emit a FAIL one. + + :param bool predicate: Whether the test should pass. + :param str message: Message to emit. + """ + if predicate: + print_pass(message) + else: + print_fail(message)