jaymzh / pius

PGP Individual User Signer
Other
97 stars 25 forks source link

Incomplete Python 3 support: not taking into account bytes vs strings #124

Closed dllud closed 5 years ago

dllud commented 5 years ago

pius has #!/usr/bin/python as shebang which means it should work with both Python 2 and 3, depending on the system's default. Python 3 support was supposedly added to pius some years ago through #26. However, pius isn't working with Python 3.7.2 on my Arch-based machine. The main reason is due to the introduction of the bytes() object in Python 3, which changed the way that things like subprocess.PIPE read and write data. subprocess.PIPE now outputs a bytes() object and also expects that same object as input.

Throughout pius codebase objects returned from subprocess's stdout are handled as if they were instances of string which causes several TypeErrors while parsing stdout. Example:

DEBUG: Running: /usr/bin/gpg2 --version
Traceback (most recent call last):
  File "/home/david/pius/pius", line 333, in <module>
    main()
  File "/home/david/pius/pius", line 265, in main
    options.mail_host
  File "/home/david/pius/libpius/signer.py", line 89, in __init__
    self.gpg2 = self._is_gpg2()
  File "/home/david/pius/libpius/signer.py", line 121, in _is_gpg2
    m = re.match(r'^gpg \(GnuPG.*\) ([0-9\.]+)$', line)
  File "/usr/lib/python3.7/re.py", line 173, in match
    return _compile(pattern, flags).match(string)
TypeError: cannot use a string pattern on a bytes-like object

This can be easily fixed by adding the universal_newlines=True option on all subprocess.Popen calls that have their stdout used in some way.

On the other hand, fixing the stdin usage requires some more effort. One must either manually encode all strings before writing to stdin or use a buffered text interface that does the encoding. One of these solutions should be chosen and applied.

Meanwhile, the shebang should be changed to #!/usr/bin/python2, because pius does not run on Python 3.

jaymzh commented 5 years ago

Yeah that's unfortunate. It's currently developed on py2 and py3 support was contributed, as you pointed out. I anticipate pius 3 being python3 only, there's a task for it. As of this point, I will accept PRs to make pius 2 be py3-compatible (or change the shebang line), but my focus is on the 3 branch.

dllud commented 5 years ago

Here's a simple PR to change the shebang line: #125 Better leave the full Python 3 support for someone proficient in Python (unlike me).

jaymzh commented 5 years ago

attached PR merged. Full py3 support tracked in #2 , so I'm gonna close this.