From b0a5ec5c1a84fb02bbe25ab985d9ca1efc6bd676 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 25 May 2018 10:00:32 +0200 Subject: [PATCH 1/5] workaround crash in diffoscope when verifying APKs On versions of diffoscope before 87, like the version included in Ubuntu xenial LTS, it would crash saying: ValueError: max_diff_block_lines (100) cannot be smaller than max_page_diff_block_lines (128) https://bugs.debian.org/875451 --- fdroidserver/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index f99f2824..51fef39b 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -2788,7 +2788,7 @@ def compare_apks(apk1, apk2, tmp_dir, log_dir=None): htmlfile = logfilename + '.diffoscope.html' textfile = logfilename + '.diffoscope.txt' if subprocess.call([config['diffoscope'], - '--max-report-size', '12345678', '--max-diff-block-lines', '100', + '--max-report-size', '12345678', '--max-diff-block-lines', '128', '--html', htmlfile, '--text', textfile, absapk1, absapk2]) != 0: return("Failed to run diffoscope " + apk1) From a3a0b8dcf0a22bbe05733a9b2d71aec0a1a4c7cd Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 25 May 2018 11:29:44 +0200 Subject: [PATCH 2/5] verify: --reuse-remote-apk to reuse local APKs For something like a verification server, this avoids having `fdroid verify` redownload the remote APK from f-droid.org every time its run. For normal users, it should download a fresh copy each time to avoid false errors based on confusion over anything that might have changed the local copy of the remote APK. This patch has been used on verification.f-droid.org for a while now. It is the last thing keeping verification.f-droid.org from using fdroidserver straight from stretch-backports. --- fdroidserver/verify.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/fdroidserver/verify.py b/fdroidserver/verify.py index 73b94725..aac0a515 100644 --- a/fdroidserver/verify.py +++ b/fdroidserver/verify.py @@ -40,6 +40,8 @@ def main(): parser = ArgumentParser(usage="%(prog)s [options] [APPID[:VERCODE] [APPID[:VERCODE] ...]]") common.setup_global_opts(parser) parser.add_argument("appid", nargs='*', help=_("applicationId with optional versionCode in the form APPID[:VERCODE]")) + parser.add_argument("--reuse-remote-apk", action="store_true", default=False, + help=_("Verify against locally cached copy rather than redownloading.")) options = parser.parse_args() config = common.read_config(options) @@ -74,18 +76,19 @@ def main(): logging.info("Processing {apkfilename}".format(apkfilename=apkfilename)) remoteapk = os.path.join(tmp_dir, apkfilename) - if os.path.exists(remoteapk): - os.remove(remoteapk) - url = 'https://f-droid.org/repo/' + apkfilename - logging.info("...retrieving " + url) - try: - net.download_file(url, dldir=tmp_dir) - except requests.exceptions.HTTPError as e: + if not options.reuse_remote_apk or not os.path.exists(remoteapk): + if os.path.exists(remoteapk): + os.remove(remoteapk) + url = 'https://f-droid.org/repo/' + apkfilename + logging.info("...retrieving " + url) try: - net.download_file(url.replace('/repo', '/archive'), dldir=tmp_dir) + net.download_file(url, dldir=tmp_dir) except requests.exceptions.HTTPError as e: - raise FDroidException(_('Downloading {url} failed. {error}') - .format(url=url, error=e)) + try: + net.download_file(url.replace('/repo', '/archive'), dldir=tmp_dir) + except requests.exceptions.HTTPError as e: + raise FDroidException(_('Downloading {url} failed. {error}') + .format(url=url, error=e)) compare_result = common.verify_apks( remoteapk, From 5ff1b5ef37f44a812720c2f296235443367d5f42 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 25 May 2018 11:56:13 +0200 Subject: [PATCH 3/5] verify: exit with error code if any APK fails to verify --- fdroidserver/verify.py | 8 +++++--- tests/run-tests | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/fdroidserver/verify.py b/fdroidserver/verify.py index aac0a515..03787179 100644 --- a/fdroidserver/verify.py +++ b/fdroidserver/verify.py @@ -104,9 +104,11 @@ def main(): logging.info("...NOT verified - {0}".format(e)) notverified += 1 - logging.info(_("Finished")) - logging.info("{0} successfully verified".format(verified)) - logging.info("{0} NOT verified".format(notverified)) + if verified > 0: + logging.info("{0} successfully verified".format(verified)) + if notverified > 0: + logging.info("{0} NOT verified".format(notverified)) + sys.exit(notverified) if __name__ == "__main__": diff --git a/tests/run-tests b/tests/run-tests index e0ea2aa1..f17f933b 100755 --- a/tests/run-tests +++ b/tests/run-tests @@ -447,6 +447,21 @@ test -e repo/com.politedroid_5.apk ! test -e repo/com.politedroid_6.apk +#------------------------------------------------------------------------------# +echo_header 'test that verify can succeed and fail' + +REPOROOT=`create_test_dir` +cd $REPOROOT +test -d tmp || mkdir tmp +test -d unsigned || mkdir unsigned +cp $WORKSPACE/tests/repo/com.politedroid_6.apk tmp/ +cp $WORKSPACE/tests/repo/com.politedroid_6.apk unsigned/ +$fdroid verify --reuse-remote-apk --verbose com.politedroid +# force a fail +cp $WORKSPACE/tests/repo/com.politedroid_5.apk unsigned/com.politedroid_6.apk +! $fdroid verify --reuse-remote-apk --verbose com.politedroid + + #------------------------------------------------------------------------------# echo_header 'test allowing disabled signatures in repo and archive' From 14127bf4180f022be8facef6f980306416ff2d93 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 25 May 2018 12:12:40 +0200 Subject: [PATCH 4/5] gitlab-ci: combine all lint/syntax/safety checks into a single job This should make it easier to accept merge requests where there are only cosmetic problems with them. pep8/pylint/pyflakes runs can then be disabled in the 'test' job by not installing the in the ci-images-server base image. --- .gitlab-ci.yml | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 3cbb4b08..e5b9597f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -121,24 +121,17 @@ pip_install: - fdroid readmeta - fdroid update --help -pyup_io_safety_check: +lint_format_safety_checks: image: alpine:3.7 variables: LANG: C.UTF-8 script: - - apk add --no-cache ca-certificates python3 + - apk add --no-cache bash dash ca-certificates python3 - python3 -m ensurepip - - pip3 install safety - - safety check --full-report - -pylint: - image: alpine:3.7 - variables: - LANG: C.UTF-8 - script: - - apk add --no-cache ca-certificates python3 - - python3 -m ensurepip - - pip3 install pylint + - pip3 install pep8 pyflakes pylint safety + - export EXITVALUE=0 + - ./hooks/pre-commit || export EXITVALUE=1 + - safety check --full-report || export EXITVALUE=1 - pylint --rcfile=.pylint-rcfile --output-format=colorized --reports=n fdroid makebuildserver @@ -146,6 +139,8 @@ pylint: fdroidserver/*.py tests/*.py tests/*.TestCase + || export EXITVALUE=1 + - exit $EXITVALUE fedora_latest: image: fedora:latest From fb02073cab67d3182f53fc1819bc59db75a46a0e Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 25 May 2018 12:32:34 +0200 Subject: [PATCH 5/5] fix "local variable 'e' is assigned to but never used" --- fdroid | 2 +- fdroidserver/verify.py | 2 +- makebuildserver | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fdroid b/fdroid index 4147119d..d0924b4d 100755 --- a/fdroid +++ b/fdroid @@ -156,7 +156,7 @@ def main(): # str(e) often doesn't contain a reason, so just show the backtrace except Exception as e: logging.critical(_("Unknown exception found!")) - raise + raise e sys.exit(0) diff --git a/fdroidserver/verify.py b/fdroidserver/verify.py index 03787179..9e335563 100644 --- a/fdroidserver/verify.py +++ b/fdroidserver/verify.py @@ -83,7 +83,7 @@ def main(): logging.info("...retrieving " + url) try: net.download_file(url, dldir=tmp_dir) - except requests.exceptions.HTTPError as e: + except requests.exceptions.HTTPError: try: net.download_file(url.replace('/repo', '/archive'), dldir=tmp_dir) except requests.exceptions.HTTPError as e: diff --git a/makebuildserver b/makebuildserver index 11e68e9c..5b859a04 100755 --- a/makebuildserver +++ b/makebuildserver @@ -74,7 +74,7 @@ config = { if os.path.isfile('/usr/bin/systemd-detect-virt'): try: virt = subprocess.check_output('/usr/bin/systemd-detect-virt').strip().decode('utf-8') - except subprocess.CalledProcessError as e: + except subprocess.CalledProcessError: virt = 'none' if virt == 'qemu' or virt == 'kvm' or virt == 'bochs': logger.info('Running in a VM guest, defaulting to QEMU/KVM via libvirt')