Stephen Smalley <sds@xxxxxxxxxxxxx> writes: > On 9/26/19 5:58 PM, Nicolas Iooss wrote: >> On Thu, Sep 26, 2019 at 2:52 PM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote: >>> >>> Fixes: https://github.com/SELinuxProject/selinux/issues/61 >>> Signed-off-by: Stephen Smalley <sds@xxxxxxxxxxxxx> >>> --- >>> python/sepolicy/sepolicy/interface.py | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/python/sepolicy/sepolicy/interface.py b/python/sepolicy/sepolicy/interface.py >>> index 583091ae18aa..b1b39a492d73 100644 >>> --- a/python/sepolicy/sepolicy/interface.py >>> +++ b/python/sepolicy/sepolicy/interface.py >>> @@ -196,7 +196,7 @@ def get_xml_file(if_file): >>> from subprocess import getstatusoutput >>> basedir = os.path.dirname(if_file) + "/" >>> filename = os.path.basename(if_file).split(".")[0] >>> - rc, output = getstatusoutput("python /usr/share/selinux/devel/include/support/segenxml.py -w -m %s" % basedir + filename) >>> + rc, output = getstatusoutput("/usr/bin/python3 /usr/share/selinux/devel/include/support/segenxml.py -w -m %s" % basedir + filename) >>> if rc != 0: >>> sys.stderr.write("\n Could not proceed selected interface file.\n") >>> sys.stderr.write("\n%s" % output) >> >> Considering that Python's "command" module was removed in Python 3 >> (according to https://docs.python.org/2/library/commands.html), and >> that Python 3's subprocess.getstatusoutput() supports using a list of >> arguments instead of a string, it seems better to change this code to >> something like: I think this is not correct: Execute the string 'cmd' in a shell with 'check_output' and return a 2-tuple (status, output). The locale encoding is used to decode the output and process newlines. >>> subprocess.getstatusoutput(["echo", "hey"]) (0, '') >> subprocess.getstatusoutput("echo hey") (0, 'hey') >> >> from subprocess import getstatusoutput >> basedir = os.path.dirname(if_file) >> filename = os.path.basename(if_file).split(".")[0] >> rc, output = getstatusoutput(["python3", >> "/usr/share/selinux/devel/include/support/segenxml.py", "-w", "-m", >> os.path.join(basedir, filename)]) >> >> The code that I suggest is not compatible with Python 2 (which does >> not support using list of arguments). Therefore, doing so makes >> sepolicy really Python-3 only. I do not consider this to be an issue, >> but others may prefer to wait for 3.0 to be released before dropping >> support for Python 2 completely. > > Anyone else have an opinion on whether we should fix this in a > python2-compatible manner? I'd stay with python2 compatible for now. > Also, should it be just "python3" or "/usr/bin/python3"? It would be great if it could use $(PYTHON) from Makefile. >> >> By the way, the current code is quite misleading because ("%s" % a + >> b) is interpreted as (("%s" % a) + b), not ("%s" % (a + b)). >> Thankfully the "%s" is at the end of the format string, but if you >> want to keep Python 2 compatibility, I suggest adding parentheses >> somewhere. -- () ascii ribbon campaign - against html e-mail /\ www.asciiribbon.org - against proprietary attachments
