Add unversioned shebangs check

Dockerfile

@@ -2,7 +2,8 @@ FROM fedora
 
 RUN dnf -y install --setopt=install_weak_deps=false --setopt=tsflags=nodocs \
     --setopt=deltarpm=false python2-rpm libtaskotron-core libtaskotron-fedora \
-    python3-rpm tox python2 python3 python2-dnf python3-dnf && dnf clean all
+    python3-rpm tox python2 python3 python2-dnf python3-dnf \
+    python2-libarchive-c && dnf clean all
 
 ENV LANG=C.UTF-8 LC_ALL=C.UTF-8
 

README.rst

@@ -15,6 +15,8 @@ Currently the following checks are available:
 
 -  Whether only Python 2 version of the package contains the executables.
 
+-  Whether the package uses versioned shebangs in its executables.
+
 Running
 -------
 
@@ -23,8 +25,8 @@ You can run the checks locally with
 install it (you can
 follow the
 `Quickstart <https://qa.fedoraproject.org/docs/libtaskotron/latest/quickstart.html>`__).
-You'll also need the ``rpm`` and ``dnf`` Python 2 modules (``python2-rpm``,
-``python2-dnf``).
+You'll also need the ``rpm``, ``dnf`` and ``libarchive-c`` Python 2 modules
+(``python2-rpm``, ``python2-dnf``, ``python2-libarchive-c``).
 Note that Taskotron unfortunately runs on Python 2, but the code in
 this repository is Python 3 compatible as well.
 

python_versions_check.py

@@ -18,6 +18,7 @@
     task_naming_scheme,
     task_requires_naming_scheme,
     task_executables,
+    task_unversioned_shebangs,
 )
 from taskotron_python_versions.common import log, Package, PackageException
 
@@ -57,6 +58,7 @@ def run(koji_build, workdir='.', artifactsdir='artifacts'):
     details.append(task_requires_naming_scheme(
         srpm_packages + packages, koji_build, artifact))
     details.append(task_executables(packages, koji_build, artifact))
+    details.append(task_unversioned_shebangs(packages, koji_build, artifact))
 
     # finally, the main detail with overall results
     outcome = 'PASSED'

runtask.yml

@@ -14,6 +14,7 @@ environment:
     rpm:
         - rpm-python
         - python2-dnf
+        - python2-libarchive-c
 
 actions:
     - name: download rpms from koji

setup.py

@@ -14,11 +14,13 @@
     description=description,
     long_description=long_description,
     keywords='taskotron fedora python rpm',
-    author='Miro Hrončok, Iryna Shcherbina',
-    author_email='mhroncok@redhat.com, ishcherb@redhat.com',
+    author='Miro Hrončok, Iryna Shcherbina, Michal Cyprian',
+    author_email=('mhroncok@redhat.com, ishcherb@redhat.com, '
+                  'mcyprian@redhat.com'),
     url='https://github.com/fedora-python/taskotron-python-versions',
     license='Public Domain',
     packages=find_packages(),
+    install_requires=['libarchive-c'],
     setup_requires=['setuptools', 'pytest-runner'],
     tests_require=['pytest', 'pyyaml'],
     classifiers=[

taskotron_python_versions/__init__.py

@@ -2,11 +2,13 @@
 from .naming_scheme import task_naming_scheme
 from .requires import task_requires_naming_scheme
 from .two_three import task_two_three
+from .unversioned_shebangs import task_unversioned_shebangs
 
 
 __all__ = (
     'task_two_three',
     'task_naming_scheme',
     'task_requires_naming_scheme',
     'task_executables',
+    'task_unversioned_shebangs',
 )

taskotron_python_versions/common.py

@@ -46,6 +46,7 @@ def __init__(self, path):
         the RPM package header containing its metadata.
         """
         self.filename = os.path.basename(path)
+        self.path = path
         # To be populated in the first check.
         self.py_versions = None
 

taskotron_python_versions/unversioned_shebangs.py

@@ -0,0 +1,99 @@
+import libarchive
+
+from .common import log, write_to_artifact
+
+MESSAGE = """These RPMs contain problematic shebang in some of the scripts:
+{}
+This is discouraged and should be avoided. Please check the shebangs
+and use either `#!/usr/bin/python2` or `#!/usr/bin/python3`.
+"""
+
+# TODO: update to real doc relevant to shebangs
+INFO_URL = ' https://pagure.io/packaging-committee/issue/698'
+
+FORBIDDEN_SHEBANGS = ['#!/usr/bin/python', '#!/usr/bin/env python']
+
+
+def matches(line, query):
+    """Both arguments must be of a type bytes"""
+    return line == query or line.startswith(query + b' ')
+
+
+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.
+    """
+    problematic = set()
+    with libarchive.file_reader(archive) as a:
+        for entry in a:
+            try:
+                first_line = next(entry.get_blocks(), '').splitlines()[0]
+            except IndexError:
+                continue  # file is empty
+            if matches(first_line, query.encode()):
+                problematic.add(entry.pathname.lstrip('.'))
+
+    return problematic
+
+
+def shebang_to_require(shebang):
+    """Convert shebang to the format of requirement."""
+    return shebang.split()[0][2:].encode()
+
+
+def get_scripts_summary(package):
+    """Collect problematic scripts data for given RPM package.
+    Content of archive is processed only if package requires
+    unversioned python binary or env.
+    """
+    scripts_summary = {}
+
+    for shebang in FORBIDDEN_SHEBANGS:
+        if shebang_to_require(shebang) in package.require_names:
+            scripts_summary[shebang] = get_problematic_files(
+                package.path, shebang)
+    return scripts_summary
+
+
+def task_unversioned_shebangs(packages, koji_build, artifact):
+    """Check if some of the binaries contains '/usr/bin/python'
+    shebang or '/usr/bin/env python' shebang.
+    """
+    # libtaskotron is not available on Python 3, so we do it inside
+    # to make the above functions testable anyway
+    from libtaskotron import check
+
+    outcome = 'PASSED'
+
+    problem_rpms = {}
+    shebang_message = ''
+
+    for package in packages:
+        log.debug('Checking shebangs of {}'.format(package.filename))
+        problem_rpms[package.nvr] = get_scripts_summary(package)
+
+    for package, pkg_summary in problem_rpms.items():
+        for shebang, scripts in pkg_summary.items():
+            outcome = 'FAILED'
+            shebang_message += \
+                '{}\n * Scripts containing `{}` shebang:\n   {}'.format(
+                    package, shebang, '\n   '.join(sorted(scripts)))
+
+    detail = check.CheckDetail(
+        checkname='python-versions.unversioned_shebangs',
+        item=koji_build,
+        report_type=check.ReportType.KOJI_BUILD,
+        outcome=outcome)
+
+    if outcome == 'FAILED':
+        detail.artifact = artifact
+        write_to_artifact(artifact, MESSAGE.format(shebang_message), INFO_URL)
+    else:
+        shebang_message = 'No problems found.'
+
+    log.info('python-versions.unversioned_shebangs {} for {}. {}'.format(
+        outcome, koji_build, shebang_message))
+
+    return detail

test/functional/common.py

@@ -4,9 +4,16 @@
 from taskotron_python_versions.common import Package
 
 
-def pkg(filename):
+def pkg_path(filename):
+    """Returns a path for given rpm in fixture"""
     return os.path.join(os.path.dirname(__file__), '..', 'fixtures', filename)
 
 
+def gpkg_path(pkgglob):
+    """Returns a path for the first rpm satisfying given glob"""
+    return glob.glob(pkg_path(pkgglob))[0]
+
+
 def gpkg(pkgglob):
-    return Package(glob.glob(pkg(pkgglob))[0])
+    """Returns a Package object for the first rpm satisfying given glob"""
+    return Package(gpkg_path(pkgglob))

test/functional/test_unversioned_shebangs.py

@@ -0,0 +1,63 @@
+import pytest
+
+from taskotron_python_versions.unversioned_shebangs import (
+    matches,
+    get_problematic_files,
+    shebang_to_require,
+    get_scripts_summary,
+)
+from .common import gpkg, gpkg_path
+
+
+@pytest.mark.parametrize(('line', 'query', 'expected'), (
+    (b'#!/usr/bin/python', b'#!/usr/bin/python', True),
+    (b'#!/usr/bin/python ', b'#!/usr/bin/python', True),
+    (b'#!/usr/bin/python -I', b'#!/usr/bin/python', True),
+    (b'#!/usr/bin/python #comment', b'#!/usr/bin/python', True),
+    (b'#!/usr/bin/python3', b'#!/usr/bin/python', False),
+    (b'#!/usr/bin/python2', b'#!/usr/bin/python', False),
+    (b'#!/usr/bin/env python', b'#!/usr/bin/env python', True),
+    (b'#!/usr/bin/env python -I', b'#!/usr/bin/env python', True),
+    (b'#!/usr/bin/env python3', b'#!/usr/bin/env python', False),
+    (b'#!/usr/bin/env python2', b'#!/usr/bin/env python', False),
+    (b'#!/usr/bin/env perl', b'#!/usr/bin/env python', False),
+))
+def test_matches(line, query, expected):
+    assert matches(line, query) == expected
+
+
+@pytest.mark.parametrize(('archive', 'query', 'expected'), (
+    ('tracer*',
+     '#!/usr/bin/python', {'/usr/bin/tracer'}),
+    ('python3-django*', '#!/usr/bin/env python',
+     {'/usr/lib/python3.6/site-packages/django/bin/django-admin.py',
+      ('/usr/lib/python3.6/site-packages/'
+       'django/conf/project_template/manage.py-tpl')}),
+    ('python3-django*', '#!/usr/bin/python', set()),
+    ('pyserial*', '#!/usr/bin/python', set()),
+))
+def test_get_problematic_files(archive, query, expected):
+    assert get_problematic_files(gpkg_path(archive), query) == expected
+
+
+@pytest.mark.parametrize(('shebang', 'expected'), (
+    ("#!/foo", b"/foo"),
+    ("#!/usr/bin/python", b"/usr/bin/python"),
+    ("#!/usr/bin/env python", b"/usr/bin/env"),
+))
+def test_shebang_to_require(shebang, expected):
+    assert shebang_to_require(shebang) == expected
+
+
+@pytest.mark.parametrize(('glob', 'expected'), (
+    ('tracer*',
+     {'#!/usr/bin/python': {'/usr/bin/tracer'}}),
+    ('python3-django*',
+     {'#!/usr/bin/env python':
+      {'/usr/lib/python3.6/site-packages/django/bin/django-admin.py',
+       ('/usr/lib/python3.6/site-packages/'
+        'django/conf/project_template/manage.py-tpl')}}),
+    ('pyserial*', {}),
+))
+def test_get_scripts_summary(glob, expected):
+    assert get_scripts_summary(gpkg(glob)) == expected

test/integration/test_integration.py

@@ -123,7 +123,7 @@ def test_number_of_results(results, request):
     results = request.getfixturevalue(results)
 
     # Each time a new check is added, this number needs to be increased
-    assert len(results) == 5
+    assert len(results) == 6
 
 
 @pytest.mark.parametrize('results', ('eric', 'six', 'admesh',
@@ -280,3 +280,34 @@ def test_artifact_contains_executables_and_looks_as_expected(
         In case the Python version matter, also create an additional
         executables for Python 3.
     """).strip() in artifact.strip()
+
+
+@pytest.mark.parametrize('results', ('eric', 'six', 'admesh',
+                                     'copr', 'epub', 'twine'))
+def test_unvesioned_shebangs_passed(results, request):
+    results = request.getfixturevalue(results)
+    assert results['python-versions.unversioned_shebangs'].outcome == 'PASSED'
+
+
+@pytest.mark.parametrize('results', ('yum', 'tracer'))
+def test_unvesioned_shebangs_failed(results, request):
+    results = request.getfixturevalue(results)
+    assert results['python-versions.unversioned_shebangs'].outcome == 'FAILED'
+
+
+def test_artifact_contains_unversioned_shebangs_and_looks_as_expected(
+        tracer):
+    result = tracer['python-versions.unversioned_shebangs']
+    with open(result.artifact) as f:
+        artifact = f.read()
+
+    print(artifact)
+
+    assert dedent("""
+        These RPMs contain problematic shebang in some of the scripts:
+        tracer-0.6.9-1.fc23
+         * Scripts containing `#!/usr/bin/python` shebang:
+           /usr/bin/tracer
+        This is discouraged and should be avoided. Please check the shebangs
+        and use either `#!/usr/bin/python2` or `#!/usr/bin/python3`.
+   """).strip() in artifact.strip()

tox.ini

@@ -3,7 +3,9 @@ envlist = py27,py3,integration,style
 skipsdist = True
 
 [testenv]
-deps = pytest
+deps =
+    pytest
+    libarchive-c
 commands = python -m pytest -v {posargs} test/functional
 sitepackages = True