Re: [PATCH] python/sepolicy: call segenxml.py with python3

[Petr Lautrbach <plautrba@redhat.com>]

Stephen Smalley <sds@tycho.nsa.gov> writes:

> On 9/26/19 5:58 PM, Nicolas Iooss wrote:
>> On Thu, Sep 26, 2019 at 2:52 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>>
>>> Fixes: https://github.com/SELinuxProject/selinux/issues/61
>>> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
>>> ---
>>>   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