From c2d50e3bcc7887d45c40f23376ca2e4b2356f82c Mon Sep 17 00:00:00 2001 From: Iryna Shcherbina Date: Tue, 6 Feb 2018 14:27:09 +0100 Subject: [PATCH 1/3] Move file_contains function to common The function is going to be needed by shebang check as well to work with log file. --- taskotron_python_versions/common.py | 25 +++++++++++++++++ taskotron_python_versions/python_usage.py | 28 +------------------ .../{test_python_usage.py => test_common.py} | 2 +- 3 files changed, 27 insertions(+), 28 deletions(-) rename test/functional/{test_python_usage.py => test_common.py} (95%) diff --git a/taskotron_python_versions/common.py b/taskotron_python_versions/common.py index 924545d..1ab64fd 100644 --- a/taskotron_python_versions/common.py +++ b/taskotron_python_versions/common.py @@ -2,6 +2,7 @@ import logging import os +import mmap import rpm log = logging.getLogger('python-versions') @@ -46,6 +47,30 @@ def packages_by_version(packages): return pkg_by_version +def file_contains(path, needle): + """Check if the file residing on the given path contains the given needle + """ + # Since we have no idea if build.log is valid utf8, let's convert our ASCII + # needle to bytes and use bytes everywhere. + # This also allow us to use mmap on Python 3. + # Be explicit here, to make it fail early if our needle is not ASCII. + needle = needle.encode('ascii') + + with open(path, 'rb') as f: + # build.logs tend to be laaaarge, so using a single read() is bad idea; + # let's optimize prematurely because practicality beats purity + + # Memory-mapped file object behaving like bytearray + mmf = mmap.mmap(f.fileno(), + length=0, # = determine automatically + access=mmap.ACCESS_READ) + try: + return mmf.find(needle) != -1 + finally: + # mmap context manager is Python 3 only + mmf.close() + + class PackageException(Exception): """Base Exception class for Package API.""" diff --git a/taskotron_python_versions/python_usage.py b/taskotron_python_versions/python_usage.py index ecfb955..8037647 100644 --- a/taskotron_python_versions/python_usage.py +++ b/taskotron_python_versions/python_usage.py @@ -1,6 +1,4 @@ -import mmap - -from .common import log, write_to_artifact +from .common import log, write_to_artifact, file_contains MESSAGE = """You've used /usr/bin/python during build on the following arches: @@ -16,30 +14,6 @@ WARNING = 'DEPRECATION WARNING: python2 invoked with /usr/bin/python' -def file_contains(path, needle): - """Check if the file residing on the given path contains the given needle - """ - # Since we have no idea if build.log is valid utf8, let's convert our ASCII - # needle to bytes and use bytes everywhere. - # This also allow us to use mmap on Python 3. - # Be explicit here, to make it fail early if our needle is not ASCII. - needle = needle.encode('ascii') - - with open(path, 'rb') as f: - # build.logs tend to be laaaarge, so using a single read() is bad idea; - # let's optimize prematurely because practicality beats purity - - # Memory-mapped file object behaving like bytearray - mmf = mmap.mmap(f.fileno(), - length=0, # = determine automatically - access=mmap.ACCESS_READ) - try: - return mmf.find(needle) != -1 - finally: - # mmap context manager is Python 3 only - mmf.close() - - def task_python_usage(logs, koji_build, artifact): """Parses the build.logs for /usr/bin/python invocation warning """ diff --git a/test/functional/test_python_usage.py b/test/functional/test_common.py similarity index 95% rename from test/functional/test_python_usage.py rename to test/functional/test_common.py index c5516ad..459c326 100644 --- a/test/functional/test_python_usage.py +++ b/test/functional/test_common.py @@ -1,6 +1,6 @@ import pytest -from taskotron_python_versions.python_usage import file_contains +from taskotron_python_versions.common import file_contains from .common import gpkg_path From b9ddff09714b5b4e30b01ee18a358d20e2679103 Mon Sep 17 00:00:00 2001 From: Iryna Shcherbina Date: Wed, 7 Feb 2018 17:55:33 +0100 Subject: [PATCH 2/3] Update unversioned_shebangs task to also check for automatic shebangs mangling in build logs Keep the ambiguous shebangs check in place, in case brp script is disabled, but also check the build logs for the warning generated when the shebangs are automatically mangled. Fail the shebang check in both cases. --- python_versions_check.py | 3 +- .../unversioned_shebangs.py | 79 +++++++++++++++---- 2 files changed, 65 insertions(+), 17 deletions(-) diff --git a/python_versions_check.py b/python_versions_check.py index 70ab96b..0af385c 100644 --- a/python_versions_check.py +++ b/python_versions_check.py @@ -69,7 +69,8 @@ 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)) + details.append(task_unversioned_shebangs( + packages, logs, koji_build, artifact)) details.append(task_py3_support( srpm_packages + packages, koji_build, artifact)) details.append(task_python_usage(logs, koji_build, artifact)) diff --git a/taskotron_python_versions/unversioned_shebangs.py b/taskotron_python_versions/unversioned_shebangs.py index 1ca5eef..1d22af2 100644 --- a/taskotron_python_versions/unversioned_shebangs.py +++ b/taskotron_python_versions/unversioned_shebangs.py @@ -1,6 +1,6 @@ import libarchive -from .common import log, write_to_artifact +from .common import log, write_to_artifact, file_contains MESSAGE = """These RPMs contain problematic shebang in some of the scripts: {} @@ -8,10 +8,22 @@ and use either `#!/usr/bin/python2` or `#!/usr/bin/python3`. """ +MANGLED_MESSAGE = """The package uses either `#!/usr/bin/python` or +`#!/usr/bin/env python` shebangs. They are forbidden by the guidelines +and have been automatically mangled during build on the following +arches: + + {} + +Please check the shebangs and use either `#!/usr/bin/python2` or +`#!/usr/bin/python3` explicitly. +""" + INFO_URL = \ 'https://fedoraproject.org/wiki/Packaging:Python#Multiple_Python_Runtimes' FORBIDDEN_SHEBANGS = ['#!/usr/bin/python', '#!/usr/bin/env python'] +WARNING = 'WARNING: mangling shebang in' def matches(line, query): @@ -69,29 +81,64 @@ def get_scripts_summary(package): 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' +def check_packages(packages): + """Check if the packages have executables with shebangs + forbidden by the guidelines. + Return: (str) problem packages along with file names + """ problem_rpms = {} - shebang_message = '' - for package in packages: log.debug('Checking shebangs of {}'.format(package.filename)) problem_rpms[package.nvr] = get_scripts_summary(package) + shebang_message = '' 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))) + return shebang_message + + +def check_logs(logs): + """Check the build log for the warning message + that the shebangs were automatically mangled. + + Return: (str) architectures where warning was found + """ + problem_arches = set() + for buildlog in logs: + if file_contains(buildlog, WARNING): + log.debug('{} contains our warning'.format(buildlog)) + _, _, arch = buildlog.rpartition('.') + problem_arches.add(arch) + return ', '.join(sorted(problem_arches)) + + +def task_unversioned_shebangs(packages, logs, koji_build, artifact): + """Check if some of the binaries contain '/usr/bin/python' + shebang or '/usr/bin/env python' shebang or whether those + shebangs were mangled during the build. + """ + # 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' + message = '' + problems = '' + + problems = check_packages(packages) + if problems: + outcome = 'FAILED' + message = MESSAGE.format(problems) + + mangled_on_arches = check_logs(logs) + if mangled_on_arches: + outcome = 'FAILED' + message = MANGLED_MESSAGE.format(mangled_on_arches) + problems = 'Shebangs mangled on: {}'.format(mangled_on_arches) detail = check.CheckDetail( checkname='unversioned_shebangs', @@ -101,11 +148,11 @@ def task_unversioned_shebangs(packages, koji_build, artifact): if outcome == 'FAILED': detail.artifact = artifact - write_to_artifact(artifact, MESSAGE.format(shebang_message), INFO_URL) + write_to_artifact(artifact, message, INFO_URL) else: - shebang_message = 'No problems found.' + problems = 'No problems found.' log.info('subcheck unversioned_shebangs {} for {}. {}'.format( - outcome, koji_build, shebang_message)) + outcome, koji_build, problems)) return detail From 7701c21dec34362c19c835102b63b8effd94bc7f Mon Sep 17 00:00:00 2001 From: Iryna Shcherbina Date: Fri, 9 Feb 2018 13:43:39 +0100 Subject: [PATCH 3/3] Add integration test for shebangs mangling update in unversioned_shebangs check Reuse python-bucky fixture with the latest version for the check, as it did not change much, except that naming scheme was fixed, but we have other fixtures for naming scheme check. --- test/integration/test_integration.py | 35 +++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/test/integration/test_integration.py b/test/integration/test_integration.py index 6a17f19..85a8c1e 100644 --- a/test/integration/test_integration.py +++ b/test/integration/test_integration.py @@ -159,7 +159,7 @@ def results(request): _nodejs = fixtures_factory('nodejs-semver-5.1.1-2.fc26') nodejs = fixtures_factory('_nodejs') -_bucky = fixtures_factory('python-bucky-2.2.2-7.fc27') +_bucky = fixtures_factory('python-bucky-2.2.2-9.fc28') bucky = fixtures_factory('_bucky') _jsonrpc = fixtures_factory('jsonrpc-glib-3.27.4-1.fc28') @@ -220,13 +220,14 @@ def test_artifact_contains_two_three_and_looks_as_expected(tracer): ''').strip().format(result.item) in artifact.strip() -@pytest.mark.parametrize('results', ('eric', 'epub', 'twine', 'vdirsyncer')) +@pytest.mark.parametrize('results', ('eric', 'epub', 'twine', 'vdirsyncer', + 'bucky')) def test_naming_scheme_passed(results, request): results = request.getfixturevalue(results) assert results['dist.python-versions.naming_scheme'].outcome == 'PASSED' -@pytest.mark.parametrize('results', ('copr', 'six', 'admesh', 'bucky')) +@pytest.mark.parametrize('results', ('copr', 'six', 'admesh')) def test_naming_scheme_failed(results, request): results = request.getfixturevalue(results) assert results['dist.python-versions.naming_scheme'].outcome == 'FAILED' @@ -369,6 +370,34 @@ def test_artifact_contains_unversioned_shebangs_and_looks_as_expected( """).strip() in artifact.strip() +@pytest.mark.parametrize('results', ('bucky',)) +def test_unvesioned_shebangs_mangled_failed(results, request): + results = request.getfixturevalue(results) + result = results['dist.python-versions.unversioned_shebangs'] + assert result.outcome == 'FAILED' + + +def test_artifact_contains_mangled_unversioned_shebangs_and_looks_as_expected( + bucky): + result = bucky['dist.python-versions.unversioned_shebangs'] + with open(result.artifact) as f: + artifact = f.read() + + print(artifact) + + assert dedent(""" + The package uses either `#!/usr/bin/python` or + `#!/usr/bin/env python` shebangs. They are forbidden by the guidelines + and have been automatically mangled during build on the following + arches: + + noarch + + Please check the shebangs and use either `#!/usr/bin/python2` or + `#!/usr/bin/python3` explicitly. + """).strip() in artifact.strip() + + @pytest.mark.parametrize('results', ('eric', 'six', 'admesh', 'tracer', 'copr', 'epub', 'twine', 'docutils')) def test_py3_support_passed(results, request):