From dc755a2374d589b3cf88a662336fb3b6d1c5d2dc Mon Sep 17 00:00:00 2001 From: Joshua Thayer Date: Tue, 11 Sep 2018 16:00:30 -0700 Subject: [PATCH 1/5] makes things happy about python3 --- sd-journalist/dev-monitor.py | 4 ++-- sd-journalist/do-not-open-here | 2 +- sd-journalist/move-to-svs | 2 +- sd-journalist/pipereader.py | 8 ++++---- sd-journalist/sd-process-download | 12 ++++++------ sd-journalist/sd-process-feedback | 2 +- tests/base.py | 4 ++-- tests/integration/test_integration | 2 +- 8 files changed, 18 insertions(+), 18 deletions(-) diff --git a/sd-journalist/dev-monitor.py b/sd-journalist/dev-monitor.py index 04070c20..25ec6848 100755 --- a/sd-journalist/dev-monitor.py +++ b/sd-journalist/dev-monitor.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 import datetime import pipereader @@ -12,7 +12,7 @@ def poller_cb(poller, msg, err): if msg in sd_process_display.messages: longer = sd_process_display.messages[msg] - print "[{}] {}: {}".format(datetime.datetime.now(), msg, longer) + print("[{}] {}: {}".format(datetime.datetime.now(), msg, longer)) reader = pipereader.PipeReader("sdfifo", poller_cb) diff --git a/sd-journalist/do-not-open-here b/sd-journalist/do-not-open-here index 2135c968..39096d65 100755 --- a/sd-journalist/do-not-open-here +++ b/sd-journalist/do-not-open-here @@ -1,4 +1,4 @@ -#!/usr/bin/python +#!/usr/bin/env python3 import sys from PyQt4 import Qt diff --git a/sd-journalist/move-to-svs b/sd-journalist/move-to-svs index a4447796..b572ccda 100755 --- a/sd-journalist/move-to-svs +++ b/sd-journalist/move-to-svs @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 import glob import re diff --git a/sd-journalist/pipereader.py b/sd-journalist/pipereader.py index d7322d0d..9fbdc93a 100755 --- a/sd-journalist/pipereader.py +++ b/sd-journalist/pipereader.py @@ -1,4 +1,4 @@ -#!/usr/bin/python +#!/usr/bin/env python3 import os import select @@ -60,7 +60,7 @@ def read(self): poller.register(fifo) elif event & select.EPOLLERR: - print "Error while polling." + print("Error while polling.") cb(None, "POLLING_ERROR") poller.unregister(fileno) os.close(fileno) @@ -68,7 +68,7 @@ def read(self): print("FIFO opened {}".format(fifo)) poller.register(fifo) elif event: - print "Totally unhandled event: {}".format(event) + print("Totally unhandled event: {}".format(event)) cb(None, "POLLING_ERROR") poller.unregister(fileno) os.close(fileno) @@ -77,7 +77,7 @@ def read(self): def reporter(poller, msg, err): - print "Got a message: {} (error: {})".format(msg.rstrip(), err) + print("Got a message: {} (error: {})".format(msg.rstrip(), err)) if msg.rstrip() == "quit": poller.quit() diff --git a/sd-journalist/sd-process-download b/sd-journalist/sd-process-download index 122632d5..9067780c 100755 --- a/sd-journalist/sd-process-download +++ b/sd-journalist/sd-process-download @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 import sys import time @@ -20,7 +20,7 @@ def spawn_monitor_process(sigfile): # congrats, new parent! return except OSError as e: - print >>sys.stderr, "fork #1 failed: %d (%s)" % (e.errno, e.strerror) + print("fork #1 failed: %d (%s)" % (e.errno, e.strerror), file=sys.stderr) sys.exit(1) os.setsid() @@ -30,8 +30,8 @@ def spawn_monitor_process(sigfile): if pid > 0: # exit from second parent. init process inherits child sys.exit(0) - except OSError, e: - print >>sys.stderr, "fork #2 failed: %d (%s)" % (e.errno, e.strerror) + except OSError as e: + print("fork #2 failed: %d (%s)" % (e.errno, e.strerror), file=sys.stderr) sys.exit(1) subprocess.Popen(["python", "/usr/local/bin/sd-process-display", sigfile]) @@ -43,11 +43,11 @@ def wait_for_sigfile(sigfile): while os.path.isfile(sigfile) is False: if int(time.time() - started > 30): - print "Waited too long for monitoring process!" + print("Waited too long for monitoring process!") sys.exit(1) time.sleep(0.5) - print "{} is still not a file".format(sigfile) + print("{} is still not a file".format(sigfile)) os.remove(sigfile) diff --git a/sd-journalist/sd-process-feedback b/sd-journalist/sd-process-feedback index 44744a41..314135a5 100755 --- a/sd-journalist/sd-process-feedback +++ b/sd-journalist/sd-process-feedback @@ -1,4 +1,4 @@ -#!/usr/bin/python +#!/usr/bin/env python3 import errno import posix diff --git a/tests/base.py b/tests/base.py index 4b4c97bd..4aab7f66 100644 --- a/tests/base.py +++ b/tests/base.py @@ -29,7 +29,7 @@ def _reboot(self): # * CalledProcessorError # * QubesVMError (from qubesadmin.base) # * QubesVMNotStartedError (from qubesadmin.base) - for v in self.vm.connected_vms.values(): + for v in list(self.vm.connected_vms.values()): if v.is_running(): msg = ("Need to halt connected VM {}" " before testing".format(v)) @@ -58,7 +58,7 @@ def assertFilesMatch(self, remote_path, local_path): with open(local_path) as f: content = f.read() import difflib - print "".join(difflib.unified_diff(remote_content, content)) + print("".join(difflib.unified_diff(remote_content, content))) self.assertTrue(remote_content == content) def assertFileHasLine(self, remote_path, wanted_line): diff --git a/tests/integration/test_integration b/tests/integration/test_integration index c86626b0..83050120 100755 --- a/tests/integration/test_integration +++ b/tests/integration/test_integration @@ -1,4 +1,4 @@ -#!/usr/bin/python +#!/usr/bin/env python3 import sys import pipereader From a62b029e90709584031c2599e33f594acc95ef63 Mon Sep 17 00:00:00 2001 From: Joshua Thayer Date: Tue, 11 Sep 2018 16:40:30 -0700 Subject: [PATCH 2/5] Make flake8 pass --- Makefile | 2 +- sd-journalist/move-to-svs | 2 +- sd-journalist/sd-process-download | 6 ++++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 6b3de2af..e8ba1a02 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -DEVVM=work +DEVVM=securedrop-dev DEVDIR=/home/user/projects/securedrop-workstation # important: no trailing slash HOST=$(shell hostname) diff --git a/sd-journalist/move-to-svs b/sd-journalist/move-to-svs index b572ccda..edbc50a9 100755 --- a/sd-journalist/move-to-svs +++ b/sd-journalist/move-to-svs @@ -19,7 +19,7 @@ probable_sd_downloads = [z.group(0) for z in matched_sd_pattern if z] # make a tarball of these zips fh = tempfile.NamedTemporaryFile(suffix=".sd-xfer", delete=False) -print "fh name " + fh.name +print("fh name " + fh.name) out_tar = tarfile.open(mode='w', fileobj=fh) diff --git a/sd-journalist/sd-process-download b/sd-journalist/sd-process-download index 9067780c..a6adccbb 100755 --- a/sd-journalist/sd-process-download +++ b/sd-journalist/sd-process-download @@ -20,7 +20,8 @@ def spawn_monitor_process(sigfile): # congrats, new parent! return except OSError as e: - print("fork #1 failed: %d (%s)" % (e.errno, e.strerror), file=sys.stderr) + print("fork #1 failed: %d (%s)" % (e.errno, e.strerror), + file=sys.stderr) sys.exit(1) os.setsid() @@ -31,7 +32,8 @@ def spawn_monitor_process(sigfile): # exit from second parent. init process inherits child sys.exit(0) except OSError as e: - print("fork #2 failed: %d (%s)" % (e.errno, e.strerror), file=sys.stderr) + print("fork #2 failed: %d (%s)" % (e.errno, e.strerror), + file=sys.stderr) sys.exit(1) subprocess.Popen(["python", "/usr/local/bin/sd-process-display", sigfile]) From bb4633b935424677d02092e2414915ad6a65c5d6 Mon Sep 17 00:00:00 2001 From: Joshua Thayer Date: Tue, 11 Sep 2018 16:41:02 -0700 Subject: [PATCH 3/5] Work in the work VM --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e8ba1a02..6b3de2af 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -DEVVM=securedrop-dev +DEVVM=work DEVDIR=/home/user/projects/securedrop-workstation # important: no trailing slash HOST=$(shell hostname) From 9af1c71a9cbac077b5bf231738a680efef63a327 Mon Sep 17 00:00:00 2001 From: Conor Schaefer Date: Tue, 11 Sep 2018 17:15:57 -0700 Subject: [PATCH 4/5] Run flake8 tests for Python3 in container Hacky implementation, I admit. As part of the migration from Python2 to Python3, we must update the flake8 logic to use Python3, as well. The most straightforward way to do so is in a container. In order to support volume mounting in the container, we must update CircleCI to use the "machine" type for builds. The wrapper script logic is a bit convoluted here, would prefer to fold in the 'file' package requirement in the underlying container (freedomofpress/ci-python), but deferring that work for now to unblock development on the workstation. As for why the file package is necessary, flake8 autodiscovery will only lint .py files, not the various scripts we have here. --- .circleci/config.yml | 12 +++--------- Makefile | 8 ++++---- scripts/flake8-linting | 26 ++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 13 deletions(-) create mode 100755 scripts/flake8-linting diff --git a/.circleci/config.yml b/.circleci/config.yml index 2afef769..6151d3c0 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -2,17 +2,11 @@ version: 2 jobs: lint: - docker: - - image: debian:stretch + machine: true steps: - checkout - - setup_remote_docker - # Check version output for debugging - - run: apt-get update - - run: apt-get install -y python-pip make - - run: pip install flake8 - - run: python --version - - run: flake8 --version + - run: sudo apt-get update + - run: sudo apt-get install -qq make - run: make flake8 workflows: diff --git a/Makefile b/Makefile index 6b3de2af..0e73ac2c 100644 --- a/Makefile +++ b/Makefile @@ -119,13 +119,13 @@ validate: assert-dom0 ## Checks for local requirements in dev env { echo "ERROR: missing 'config.json'!" && \ echo "Create from 'config.json.example'." && exit 1 ; } +.PHONY: flake8 flake8: ## Lints all Python files with flake8 # Not requiring dom0 since linting requires extra packages, # available only in the developer environment, i.e. Work VM. - @flake8 . - @find -type f -exec file -i {} + \ - | perl -F':\s+' -nE '$$F[1] =~ m/text\/x-python/ and say $$F[0]' \ - | xargs flake8 + @docker run -v $(PWD):/code -w /code --name sdw_flake8 --rm \ + --entrypoint /code/scripts/flake8-linting \ + quay.io/freedomofpress/ci-python \ update-fedora-templates: assert-dom0 ## Upgrade to Fedora 28 templates @./scripts/update-fedora-templates diff --git a/scripts/flake8-linting b/scripts/flake8-linting new file mode 100755 index 00000000..5fbd3e3c --- /dev/null +++ b/scripts/flake8-linting @@ -0,0 +1,26 @@ +#!/bin/bash +set -e +set -u +set -o pipefail + + +# Install 'file', used for discovering all Python scripts. +# This is a hack, we should fold in the 'file' package as a dependecy +# in the underlying container. +printf "Updating apt lists..." +apt-get update > /dev/null +printf " done.\n" + +apt-get install -qq file + +# Install flake8 inside container +pip install -q flake8 + +# First pass with flake8 autodiscovery +flake8 + +# Second pass to make sure we've found all Python scripts, +# since flake8's autodiscovery option can miss some. +find -type f -exec file -i {} + \ + | perl -F':\s+' -nE '$F[1] =~ m/text\/x-python/ and say $F[0]' \ + | xargs flake8 From 768cb4277457ec04e520b2af13e4274b1ddd6d92 Mon Sep 17 00:00:00 2001 From: Joshua Thayer Date: Wed, 12 Sep 2018 13:56:53 -0700 Subject: [PATCH 5/5] fix linting issue --- sd-journalist/sd-process-display | 1 + 1 file changed, 1 insertion(+) diff --git a/sd-journalist/sd-process-display b/sd-journalist/sd-process-display index e2c64751..75915485 100644 --- a/sd-journalist/sd-process-display +++ b/sd-journalist/sd-process-display @@ -35,6 +35,7 @@ class SDDialog(QDialog): self.buttons.accepted.connect(self.accept) self.buttons.rejected.connect(self.reject) + messages = { # bootstrapping 'EXISTING_SIGFILE': (