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 Peter Bennett

Owner: changed from Peter Bennett to Stuart Auchterlonie
Status: newassigned

I don't understand what the code is doing, assigning to Stuarta as he added that line on 2016-01-27

comment:2 Changed 5 years ago by Stuart Auchterlonie

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 Mac Michaels

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:

https://github.com/MythTV/mythtv/pull/182

comment:4 Changed 4 years ago by Bill Meek

Resolution: Won't Fix
Status: assignedclosed

The test was removed in v31-Pre in 1df343e9ab7defa284a73390210a65cf2112f17e .

Note: See TracTickets for help on using tickets.