Decode early and only deal with bytes when reading arbitrary files

Author: hroncok

taskotron_python_versions/common.py

@@ -108,11 +108,11 @@ def nvr(self):
 
     @property
     def require_names(self):
-        return self.hdr[rpm.RPMTAG_REQUIRENAME]
+        return [r.decode() for r in self.hdr[rpm.RPMTAG_REQUIRENAME]]
 
     @property
     def require_nevrs(self):
-        return self.hdr[rpm.RPMTAG_REQUIRENEVRS]
+        return [r.decode() for r in self.hdr[rpm.RPMTAG_REQUIRENEVRS]]
 
     @property
     def files(self):

taskotron_python_versions/python_usage.py

@@ -34,8 +34,6 @@ def task_python_usage(packages, koji_build, artifact):
         log.debug('Checking {}'.format(package.filename))
 
         for name in package.require_names:
-            name = name.decode()
-
             if name in PYTHON_COMMAND:
                 log.error(
                     '{} requires {}'.format(package.filename, name))

taskotron_python_versions/requires.py

@@ -107,8 +107,6 @@ def check_requires_naming_scheme(package, repoquery):
     misnamed_requires = set()
 
     for name in package.require_names:
-        name = name.decode()
-
         if is_unversioned(name):
             versioned = get_versioned_name(name, repoquery)
 

taskotron_python_versions/two_three.py

@@ -2,52 +2,52 @@
 
 
 NEVRS_STARTS = {
-    2: (b'python(abi) = 2.',),
-    3: (b'python(abi) = 3.',)
+    2: ('python(abi) = 2.',),
+    3: ('python(abi) = 3.',)
 }
 
 NAME_STARTS = {
     2: (
-        b'python-',
-        b'python2',
-        b'/usr/bin/python2',
-        b'libpython2',
-        b'pygtk2',
-        b'pygobject2',
-        b'pycairo',
-        b'py-',
+        'python-',
+        'python2',
+        '/usr/bin/python2',
+        'libpython2',
+        'pygtk2',
+        'pygobject2',
+        'pycairo',
+        'py-',
     ),
     3: (
-        b'python3',
-        b'/usr/bin/python3',
-        b'libpython3',
-        b'system-python'
+        'python3',
+        '/usr/bin/python3',
+        'libpython3',
+        'system-python'
     )
 }
 
 NAME_EXACTS = {
     2: (
-        b'/usr/bin/python',
-        b'python',
+        '/usr/bin/python',
+        'python',
     )
 }
 
 NAME_NOTS = (
-    b'python-rpm-macros',
-    b'python-srpm-macros',
-    b'python-sphinx-locale',
-    b'python-multilib-conf',
-    b'python-ldb-devel-common',
-    b'python-qt5-rpm-macros',
-    b'python-btchip-common',
-    b'python-matplotlib-data',
-    b'python-matplotlib-data-fonts',
-    b'python-cartopy-common',
-    b'python-django-bash-completion',
-    b'python-jupyter-filesystem',
-    b'python-pip-wheel',
-    b'python-setuptools-wheel',
-    b'python-wheel-wheel',
+    'python-rpm-macros',
+    'python-srpm-macros',
+    'python-sphinx-locale',
+    'python-multilib-conf',
+    'python-ldb-devel-common',
+    'python-qt5-rpm-macros',
+    'python-btchip-common',
+    'python-matplotlib-data',
+    'python-matplotlib-data-fonts',
+    'python-cartopy-common',
+    'python-django-bash-completion',
+    'python-jupyter-filesystem',
+    'python-pip-wheel',
+    'python-setuptools-wheel',
+    'python-wheel-wheel',
 )
 
 
@@ -80,22 +80,22 @@ def check_two_three(package):
     for nevr in package.require_nevrs:
         for py_version, starts in NEVRS_STARTS.items():
             if nevr.startswith(starts):
-                log.debug('Found dependency {}'.format(nevr.decode()))
+                log.debug('Found dependency {}'.format(nevr))
                 log.debug('Requires Python {}'.format(py_version))
                 py_versions[py_version] = nevr
 
     for name in package.require_names:
         for py_version, starts in NAME_STARTS.items():
             if py_version not in py_versions:
                 if name.startswith(starts) and name not in NAME_NOTS:
-                    log.debug('Found dependency {}'.format(name.decode()))
+                    log.debug('Found dependency {}'.format(name))
                     log.debug('Requires Python {}'.format(py_version))
                     py_versions[py_version] = name
 
         for py_version, exacts in NAME_EXACTS.items():
             if py_version not in py_versions:
                 if name in exacts:
-                    log.debug('Found dependency {}'.format(name.decode()))
+                    log.debug('Found dependency {}'.format(name))
                     log.debug('Requires Python {}'.format(py_version))
                     py_versions[py_version] = name
 

taskotron_python_versions/unversioned_shebangs.py

@@ -42,7 +42,7 @@ def get_problematic_files(archive, query):
     """Search for the files inside archive with the first line
     matching given query. Some of the files can contain data, which
     are not in the plain text format. Bytes are read from the file and
-    the shebang query has to be of the same type.
+    the shebang query is encoded as well. We only test for ASCII shebangs.
     """
     problematic = set()
     with libarchive.file_reader(archive) as a:
@@ -51,21 +51,15 @@ def get_problematic_files(archive, query):
                 first_line = next(entry.get_blocks(), '').splitlines()[0]
             except IndexError:
                 continue  # file is empty
-            if matches(first_line, query.encode()):
+            if matches(first_line, query.encode('ascii')):
                 problematic.add(entry.pathname.lstrip('.'))
 
     return problematic
 
 
-def shebang_to_require(shebang, use_bytes=True):
-    """Convert shebang to the format of requirement.
-    If the use_bytes argument is set to False, executable path
-    is returned as a string instead of the default bytes type."""
-    executable_path = shebang.split()[0][2:]
-    if use_bytes:
-        return executable_path.encode()
-    else:
-        return executable_path
+def shebang_to_require(shebang):
+    """Convert shebang to the format of requirement."""
+    return shebang.split()[0][2:]
 
 
 def get_scripts_summary(package):
@@ -79,7 +73,7 @@ def get_scripts_summary(package):
         if shebang_to_require(shebang) in package.require_names:
             log.debug('Package {} requires {}'.format(
                 package.filename, shebang_to_require(
-                    shebang, use_bytes=False)))
+                    shebang)))
             problematic = get_problematic_files(package.path, shebang)
             if problematic:
                 log.debug('{} shebang was found in scripts: {}'.format(

test/functional/test_unversioned_shebangs.py

@@ -40,15 +40,13 @@ def test_get_problematic_files(archive, query, expected):
     assert get_problematic_files(gpkg_path(archive), query) == expected
 
 
-@pytest.mark.parametrize(('shebang', 'use_bytes', 'expected'), (
-    ("#!/foo", True, b"/foo"),
-    ("#!/usr/bin/python", True, b"/usr/bin/python"),
-    ("#!/usr/bin/env python", True, b"/usr/bin/env"),
-    ("#!/usr/bin/python", False, "/usr/bin/python"),
-    ("#!/usr/bin/env python", False, "/usr/bin/env"),
+@pytest.mark.parametrize(('shebang', 'expected'), (
+    ("#!/foo", "/foo"),
+    ("#!/usr/bin/python", "/usr/bin/python"),
+    ("#!/usr/bin/env python", "/usr/bin/env"),
 ))
-def test_shebang_to_require(shebang, use_bytes, expected):
-    assert shebang_to_require(shebang, use_bytes) == expected
+def test_shebang_to_require(shebang, expected):
+    assert shebang_to_require(shebang) == expected
 
 
 @pytest.mark.parametrize(('glob', 'expected'), (