Opened 5 years ago
Closed 4 years ago
#13401 closed Bug Report - General (Won't Fix)
[v30] configure script uses wrong logic operator on result of test is_python3
Reported by: | jgmiller | Owned by: | Stuart Auchterlonie |
---|---|---|---|
Priority: | major | Milestone: | 30.1 |
Component: | MythTV - configure script | Version: | v30-fixes |
Severity: | medium | Keywords: | python version logic |
Cc: | Ticket locked: | no |
Description
The configure script allows setting the python executable path with --python=path_to_specific_version
The configure script tests the python executable at line 6956
# Check for python dependencies if enabled bindings_python; then
is_python3 && python=python2
The is_python3 function tests the python version with
check_cmd $python << EOF
import sys if sys.version_info > (3,0):
sys.exit(0)
else:
sys.exit(1)
EOF
So if the python version is greater than (3,0) viz the python version is a pyhthon3 variant. the test function exits with 0 to return true/success.
on return to the calling function
is_python3 && python=python2
because is_python3 returns 0 ie is successful/true, the statement then sets python to python2 because of the "&&" operator.
The operator "&&" should in fact be the OR operator, two vertical pipes, "\|\|" (without the backslashes) meaning if not true/not success, ie only set python to python2 if the python version is not 3.
I think somebody forgot that in Bourne shell scripts, functions which return 0 are successful, other values are failure, as opposed to other scripting languages (eg PERL) where 0 is false.
Change History (4)
comment:1 Changed 5 years ago by
Owner: | changed from Peter Bennett to Stuart Auchterlonie |
---|---|
Status: | new → assigned |
comment:2 Changed 5 years ago by
This was added in https://github.com/MythTV/mythtv/commit/e5b211f0397696f952a4262c506a88c30dcf06f7
The reasoning behind this, is when I implemented this we did not support python3, and this was to force the use of python2, on distributions which had switched the default python from 2 -> 3
So the test is correct from that perspective, however we have done some work on python3 support, so the check may now need changing to allow python3.
Will check with our python gurus
Regards Stuart
comment:3 Changed 4 years ago by
This ticket should move to version 31.0. Leave the commit as is on v30-fixes and version 30.1 since only python 2 is supported by version 30.1
Test should should be removed as python 3 is supported by editing git pull request:
comment:4 Changed 4 years ago by
Resolution: | → Won't Fix |
---|---|
Status: | assigned → closed |
The test was removed in v31-Pre in 1df343e9ab7defa284a73390210a65cf2112f17e .
I don't understand what the code is doing, assigning to Stuarta as he added that line on 2016-01-27