From 8268dc0e3d365772dadab890af31e35f5129bd28 Mon Sep 17 00:00:00 2001 From: Oliver Smith Date: Sat, 14 Jul 2018 01:13:28 +0000 Subject: [PATCH] pmbootstrap: kill process if silent for 5 minutes (rewrite logging) --- .gitlab/setup-pmos-environment.sh | 9 +- pmb/__init__.py | 15 +- pmb/aportgen/busybox_static.py | 4 +- pmb/aportgen/musl.py | 2 +- pmb/build/menuconfig.py | 7 +- pmb/build/other.py | 5 +- pmb/chroot/apk.py | 3 +- pmb/chroot/apk_static.py | 9 +- pmb/chroot/initfs.py | 2 +- pmb/chroot/root.py | 23 +-- pmb/chroot/shutdown.py | 4 +- pmb/chroot/user.py | 26 +-- pmb/chroot/zap.py | 6 +- pmb/flasher/run.py | 2 +- pmb/helpers/frontend.py | 16 +- pmb/helpers/git.py | 7 +- pmb/helpers/other.py | 2 +- pmb/helpers/run.py | 105 ++-------- pmb/helpers/run_core.py | 268 ++++++++++++++++++++++++++ pmb/install/_install.py | 6 +- pmb/install/blockdevice.py | 2 +- pmb/install/format.py | 5 +- pmb/install/losetup.py | 2 +- pmb/install/recovery.py | 2 +- pmb/parse/arguments.py | 11 +- pmb/parse/bootimg.py | 5 +- pmb/qemu/run.py | 4 +- test/test_chroot_interactive_shell.py | 11 +- test/test_qemu_running_processes.py | 9 +- test/test_run_core.py | 145 ++++++++++++++ test/test_shell_escape.py | 19 +- 31 files changed, 544 insertions(+), 192 deletions(-) create mode 100644 pmb/helpers/run_core.py create mode 100644 test/test_run_core.py diff --git a/.gitlab/setup-pmos-environment.sh b/.gitlab/setup-pmos-environment.sh index ee1959fa..372bdb5d 100755 --- a/.gitlab/setup-pmos-environment.sh +++ b/.gitlab/setup-pmos-environment.sh @@ -5,11 +5,18 @@ # all of these configurations to save time, at least for now. # Author: Clayton Craft +# skip non-shared runner [[ -d "/home/pmos" ]] && echo "pmos user already exists, assume running on pre-configured runner" && exit + +# mount binfmt_misc mount -t binfmt_misc none /proc/sys/fs/binfmt_misc + +# install dependencies (procps: /bin/kill) apt update -apt install -q -y git sudo shellcheck +apt install -q -y git sudo shellcheck procps pip3 install virtualenv + +# create pmos user echo "Creating pmos user" useradd pmos -m -s /bin/bash -b "/home" chown -R pmos:pmos . diff --git a/pmb/__init__.py b/pmb/__init__.py index ba4021fc..a3b8cde5 100644 --- a/pmb/__init__.py +++ b/pmb/__init__.py @@ -71,15 +71,16 @@ def main(): except Exception as e: logging.info("ERROR: " + str(e)) - if os.path.exists(args.log): - # Hint to read the log file (only gets printed to stdout) - print("Run 'pmbootstrap log' for details.") - else: - logging.info("Crashed before the log file was created.") - logging.info("Running init again like the following gives more details:") - logging.info(" pmbootstrap --details-to-stdout init") logging.info("See also: ") logging.debug(traceback.format_exc()) + + # Hints about the log file (print to stdout only) + if os.path.exists(args.log): + print("Run 'pmbootstrap log' for details.") + else: + print("Crashed before the log file was created.") + print("Running init again like the following gives more details:") + print(" pmbootstrap --details-to-stdout init") return 1 diff --git a/pmb/aportgen/busybox_static.py b/pmb/aportgen/busybox_static.py index 0cef481a..0b91ec60 100644 --- a/pmb/aportgen/busybox_static.py +++ b/pmb/aportgen/busybox_static.py @@ -54,8 +54,8 @@ def generate(args, pkgname): # Hash the distfile hashes = pmb.chroot.user(args, ["sha512sum", "busybox-static-" + version + "-" + arch + ".apk"], - "buildroot_" + arch, working_dir="/var/cache/distfiles", - return_stdout=True) + "buildroot_" + arch, "/var/cache/distfiles", + output_return=True) # Write the APKBUILD pmb.helpers.run.user(args, ["mkdir", "-p", args.work + "/aportgen"]) diff --git a/pmb/aportgen/musl.py b/pmb/aportgen/musl.py index a824f585..e74fbb53 100644 --- a/pmb/aportgen/musl.py +++ b/pmb/aportgen/musl.py @@ -60,7 +60,7 @@ def generate(args, pkgname): hashes = pmb.chroot.user(args, ["sha512sum", "musl-" + version + "-" + arch + ".apk", "musl-dev-" + version + "-" + arch + ".apk"], "buildroot_" + arch, - working_dir="/var/cache/distfiles", return_stdout=True) + "/var/cache/distfiles", output_return=True) # Write the APKBUILD pmb.helpers.run.user(args, ["mkdir", "-p", args.work + "/aportgen"]) diff --git a/pmb/build/menuconfig.py b/pmb/build/menuconfig.py index 810224e2..043c2ae6 100644 --- a/pmb/build/menuconfig.py +++ b/pmb/build/menuconfig.py @@ -77,7 +77,7 @@ def get_outputdir(args, pkgname): cmd = "srcdir=/home/pmos/build/src source APKBUILD; echo $builddir" ret = pmb.chroot.user(args, ["sh", "-c", cmd], "native", "/home/pmos/build", - return_stdout=True).rstrip() + output_return=True).rstrip() if os.path.exists(chroot + ret + "/.config"): return ret @@ -129,13 +129,14 @@ def menuconfig(args, pkgname): pmb.chroot.user(args, ["abuild", "unpack"], "native", "/home/pmos/build") logging.info("(native) apply patches") pmb.chroot.user(args, ["abuild", "prepare"], "native", - "/home/pmos/build", log=False, env={"CARCH": arch}) + "/home/pmos/build", output="interactive", + env={"CARCH": arch}) # Run make menuconfig outputdir = get_outputdir(args, pkgname) logging.info("(native) make " + kopt) pmb.chroot.user(args, ["make", kopt], "native", - outputdir, log=False, + outputdir, output="interactive", env={"ARCH": pmb.parse.arch.alpine_to_kernel(arch), "DISPLAY": os.environ.get("DISPLAY"), "XAUTHORITY": "/home/pmos/.Xauthority"}) diff --git a/pmb/build/other.py b/pmb/build/other.py index 8612c04a..b41d4813 100644 --- a/pmb/build/other.py +++ b/pmb/build/other.py @@ -84,13 +84,12 @@ def copy_to_buildpath(args, package, suffix="native"): # Clean up folder build = args.work + "/chroot_" + suffix + "/home/pmos/build" if os.path.exists(build): - pmb.chroot.root(args, ["rm", "-rf", "/home/pmos/build"], - suffix=suffix) + pmb.chroot.root(args, ["rm", "-rf", "/home/pmos/build"], suffix) # Copy aport contents pmb.helpers.run.root(args, ["cp", "-r", aport + "/", build]) pmb.chroot.root(args, ["chown", "-R", "pmos:pmos", - "/home/pmos/build"], suffix=suffix) + "/home/pmos/build"], suffix) def is_necessary(args, arch, apkbuild, indexes=None): diff --git a/pmb/chroot/apk.py b/pmb/chroot/apk.py index 112097d1..223403fd 100644 --- a/pmb/chroot/apk.py +++ b/pmb/chroot/apk.py @@ -52,8 +52,7 @@ def update_repository_list(args, suffix="native", check=False): for line in handle: lines_old.append(line[:-1]) else: - pmb.helpers.run.root(args, ["mkdir", "-p", os.path.dirname(path)], - suffix) + pmb.helpers.run.root(args, ["mkdir", "-p", os.path.dirname(path)]) # Up to date: Save cache, return lines_new = pmb.helpers.repo.urls(args) diff --git a/pmb/chroot/apk_static.py b/pmb/chroot/apk_static.py index b54485af..aa13b59d 100644 --- a/pmb/chroot/apk_static.py +++ b/pmb/chroot/apk_static.py @@ -102,7 +102,7 @@ def verify_signature(args, files, sigkey_path): pmb.helpers.run.user(args, ["openssl", "dgst", "-sha1", "-verify", sigkey_path, "-signature", files[ "sig"]["temp_path"], - files["apk"]["temp_path"]], check=True) + files["apk"]["temp_path"]]) except BaseException: os.unlink(files["sig"]["temp_path"]) os.unlink(files["apk"]["temp_path"]) @@ -133,7 +133,7 @@ def extract(args, version, apk_path): " (must match the package version " + version + ")") os.chmod(temp_path, os.stat(temp_path).st_mode | stat.S_IEXEC) version_bin = pmb.helpers.run.user(args, [temp_path, "--version"], - check=True, return_stdout=True) + output_return=True) version_bin = version_bin.split(" ")[1].split(",")[0] if not version.startswith(version_bin + "-r"): os.unlink(temp_path) @@ -179,6 +179,5 @@ def init(args): extract(args, version, apk_static) -def run(args, parameters, check=True): - pmb.helpers.run.root( - args, [args.work + "/apk.static"] + parameters, check=check) +def run(args, parameters): + pmb.helpers.run.root(args, [args.work + "/apk.static"] + parameters) diff --git a/pmb/chroot/initfs.py b/pmb/chroot/initfs.py index 571bb948..f07ca799 100644 --- a/pmb/chroot/initfs.py +++ b/pmb/chroot/initfs.py @@ -88,7 +88,7 @@ def ls(args, flavor, suffix, extra=False): if extra: tmp = "/tmp/initfs-extra-extracted" extract(args, flavor, suffix, extra) - pmb.chroot.root(args, ["ls", "-lahR", "."], suffix, tmp, log=False) + pmb.chroot.root(args, ["ls", "-lahR", "."], suffix, tmp, "stdout") pmb.chroot.root(args, ["rm", "-r", tmp], suffix) diff --git a/pmb/chroot/root.py b/pmb/chroot/root.py index 637d40ff..a46b5dd8 100644 --- a/pmb/chroot/root.py +++ b/pmb/chroot/root.py @@ -23,6 +23,7 @@ import pmb.config import pmb.chroot import pmb.chroot.binfmt import pmb.helpers.run +import pmb.helpers.run_core def executables_absolute_path(): @@ -39,23 +40,17 @@ def executables_absolute_path(): return ret -def root(args, cmd, suffix="native", working_dir="/", log=True, - auto_init=True, return_stdout=False, check=True, env={}): +def root(args, cmd, suffix="native", working_dir="/", output="log", + output_return=False, check=None, env={}, auto_init=True): """ Run a command inside a chroot as root. - :param cmd: command as list, e.g. ["echo", "string with spaces"] - :param suffix: of the chroot to execute code in - :param working_dir: path inside chroot where the command should run - :param log: when set to true, redirect all output to the logfile - :param auto_init: automatically initialize the chroot - :param return_stdout: write stdout to a buffer and return it as string when - the command is through - :param check: raise an exception, when the command fails :param env: dict of environment variables to be passed to the command, e.g. {"JOBS": "5"} - :returns: * stdout when return_stdout is True - * None otherwise + :param auto_init: automatically initialize the chroot + + See pmb.helpers.run_core.core() for a detailed description of all other + arguments and the return value. """ # Initialize chroot chroot = args.work + "/chroot_" + suffix @@ -90,4 +85,6 @@ def root(args, cmd, suffix="native", working_dir="/", log=True, pmb.helpers.run.flat_cmd(cmd, working_dir)] cmd_sudo = ["sudo", "env", "-i", executables["sh"], "-c", pmb.helpers.run.flat_cmd(cmd_chroot, env=env_all)] - return pmb.helpers.run.core(args, cmd_sudo, msg, log, return_stdout, check) + kill_as_root = output in ["log", "stdout"] + return pmb.helpers.run_core.core(args, msg, cmd_sudo, None, output, + output_return, check, kill_as_root) diff --git a/pmb/chroot/shutdown.py b/pmb/chroot/shutdown.py index 117351f1..5152877e 100644 --- a/pmb/chroot/shutdown.py +++ b/pmb/chroot/shutdown.py @@ -46,8 +46,8 @@ def shutdown_cryptsetup_device(args, name): if not os.path.exists(args.work + "/chroot_native/dev/mapper/" + name): return pmb.chroot.apk.install(args, ["cryptsetup"]) - status = pmb.chroot.root(args, ["cryptsetup", "status", name], check=False, - return_stdout=True) + status = pmb.chroot.root(args, ["cryptsetup", "status", name], + output_return=True, check=False) if not status: logging.warning("WARNING: Failed to run cryptsetup to get the status" " for " + name + ", assuming it is not mounted" diff --git a/pmb/chroot/user.py b/pmb/chroot/user.py index d8e8e708..cb5cb236 100644 --- a/pmb/chroot/user.py +++ b/pmb/chroot/user.py @@ -20,30 +20,24 @@ import pmb.chroot.root import pmb.helpers.run -def user(args, cmd, suffix="native", working_dir="/", log=True, - auto_init=True, return_stdout=False, check=True, env={}): +def user(args, cmd, suffix="native", working_dir="/", output="log", + output_return=False, check=None, env={}, auto_init=True): """ Run a command inside a chroot as "user". We always use the BusyBox implementation of 'su', because other implementations may override the PATH environment variable (#1071). - :param cmd: command as list, e.g. ["echo", "string with spaces"] - :param suffix: of the chroot to execute code in - :param working_dir: path inside chroot where the command should run - :param log: when set to true, redirect all output to the logfile - :param auto_init: automatically initialize the chroot - :param return_stdout: write stdout to a buffer and return it as string when - the command is through - :param check: raise an exception, when the command fails :param env: dict of environment variables to be passed to the command, e.g. {"JOBS": "5"} - :returns: * stdout when return_stdout is True - * None otherwise + :param auto_init: automatically initialize the chroot + + See pmb.helpers.run_core.core() for a detailed description of all other + arguments and the return value. """ flat_cmd = pmb.helpers.run.flat_cmd(cmd, env=env) cmd = ["busybox", "su", "pmos", "-c", flat_cmd] - return pmb.chroot.root(args, cmd, suffix, working_dir, log, - auto_init, return_stdout, check) + return pmb.chroot.root(args, cmd, suffix, working_dir, output, + output_return, check, {}, auto_init) def exists(args, username, suffix="native"): @@ -54,5 +48,5 @@ def exists(args, username, suffix="native"): :returns: bool """ output = pmb.chroot.root(args, ["getent", "passwd", username], - suffix, return_stdout=True, check=False) - return (output is not None) + suffix, output_return=True, check=False) + return len(output) > 0 diff --git a/pmb/chroot/zap.py b/pmb/chroot/zap.py index 0913f824..d78763f9 100644 --- a/pmb/chroot/zap.py +++ b/pmb/chroot/zap.py @@ -155,9 +155,9 @@ def zap_pkgs_online_mismatch(args, confirm=True, dry=False): # Iterate over existing apk caches for path in paths: arch = os.path.basename(path).split("_", 2)[2] - chroot = "native" if arch == args.arch_native else "buildroot_" + arch + suffix = "native" if arch == args.arch_native else "buildroot_" + arch # Clean the cache with apk - logging.info("(" + chroot + ") apk -v cache clean") + logging.info("(" + suffix + ") apk -v cache clean") if not dry: - pmb.chroot.root(args, ["apk", "-v", "cache", "clean"], chroot) + pmb.chroot.root(args, ["apk", "-v", "cache", "clean"], suffix) diff --git a/pmb/flasher/run.py b/pmb/flasher/run.py index 85c30b6c..6745f3ec 100644 --- a/pmb/flasher/run.py +++ b/pmb/flasher/run.py @@ -51,4 +51,4 @@ def run(args, action, flavor=None): command[i] = command[i].replace(key, value) # Run the action - pmb.chroot.root(args, command, log=False) + pmb.chroot.root(args, command, output="interactive") diff --git a/pmb/helpers/frontend.py b/pmb/helpers/frontend.py index 157afd6d..924741cb 100644 --- a/pmb/helpers/frontend.py +++ b/pmb/helpers/frontend.py @@ -134,10 +134,10 @@ def chroot(args): if args.user: logging.info("(" + suffix + ") % su pmos -c '" + " ".join(args.command) + "'") - pmb.chroot.user(args, args.command, suffix, log=False) + pmb.chroot.user(args, args.command, suffix, output=args.output) else: logging.info("(" + suffix + ") % " + " ".join(args.command)) - pmb.chroot.root(args, args.command, suffix, log=False) + pmb.chroot.root(args, args.command, suffix, output=args.output) def config(args): @@ -324,7 +324,7 @@ def stats(args): # Install ccache and display stats pmb.chroot.apk.install(args, ["ccache"], suffix) logging.info("(" + suffix + ") % ccache -s") - pmb.chroot.user(args, ["ccache", "-s"], suffix, log=False) + pmb.chroot.user(args, ["ccache", "-s"], suffix, output="stdout") def work_migrate(args): @@ -334,17 +334,17 @@ def work_migrate(args): def log(args): if args.clear_log: - pmb.helpers.run.user(args, ["truncate", "-s", "0", args.log], - log=False) + pmb.helpers.run.user(args, ["truncate", "-s", "0", args.log]) pmb.helpers.run.user(args, ["tail", "-f", args.log, "-n", args.lines], - log=False) + output="tui") def log_distccd(args): logpath = "/home/pmos/distccd.log" if args.clear_log: - pmb.chroot.user(args, ["truncate", "-s", "0", logpath], log=False) - pmb.chroot.user(args, ["tail", "-f", logpath, "-n", args.lines], log=False) + pmb.chroot.user(args, ["truncate", "-s", "0", logpath]) + pmb.chroot.user(args, ["tail", "-f", logpath, "-n", args.lines], + output="tui") def zap(args): diff --git a/pmb/helpers/git.py b/pmb/helpers/git.py index 1821992a..9bf300aa 100644 --- a/pmb/helpers/git.py +++ b/pmb/helpers/git.py @@ -33,14 +33,13 @@ def clone(args, repo_name): pmb.chroot.apk.install(args, ["git"]) logging.info("(native) git clone " + pmb.config.git_repos[repo_name]) pmb.chroot.user(args, ["git", "clone", "--depth=1", - pmb.config.git_repos[repo_name], repo_name], working_dir="/home/pmos/git/") + pmb.config.git_repos[repo_name], repo_name], + working_dir="/home/pmos/git/") def rev_parse(args, revision="HEAD"): rev = pmb.helpers.run.user(args, ["git", "rev-parse", revision], - working_dir=args.aports, - return_stdout=True, - check=False) + args.aports, output_return=True, check=False) if rev is None: logging.warning("WARNING: Failed to determine revision of git repository at " + args.aports) return "" diff --git a/pmb/helpers/other.py b/pmb/helpers/other.py index 5690478c..9c1d7211 100644 --- a/pmb/helpers/other.py +++ b/pmb/helpers/other.py @@ -35,7 +35,7 @@ def folder_size(args, path): output = pmb.helpers.run.root(args, ["du", "--summarize", "--apparent-size", "--block-size=1", - path], return_stdout=True) + path], output_return=True) ret = int(output.split("\t")[0]) return ret diff --git a/pmb/helpers/run.py b/pmb/helpers/run.py index 374e9311..e0e7d254 100644 --- a/pmb/helpers/run.py +++ b/pmb/helpers/run.py @@ -17,75 +17,7 @@ You should have received a copy of the GNU General Public License along with pmbootstrap. If not, see . """ import shlex -import subprocess -import logging -import os - - -def core(args, cmd, log_message, log, return_stdout, check=True, - working_dir=None, background=False): - """ - Run the command and write the output to the log. - - :param cmd: command as list, e.g. ["echo", "string with spaces"] - :param log_message: simplified and more readable form of the command, e.g. - "(native) % echo test" instead of the full command with - entering the chroot and more escaping - :param log: * True: write stdout and stderr of the running process into - the log file (read with "pmbootstrap log"). - * False: redirect stdout and stderr to pmbootstrap stdout - :param return_stdout: write stdout to a buffer and return it as string when - the command is through - :param check: raise an exception, when the command fails - :param working_dir: path in host system where the command should run - :param background: run the process in the background and return the process - handler - :returns: * stdout when return_stdout is True - * process handler when background is True - * None otherwise - """ - logging.debug(log_message) - logging.verbose("run: " + str(cmd)) - - if working_dir: - working_dir_old = os.getcwd() - os.chdir(working_dir) - - ret = None - if background: - if log: - ret = subprocess.Popen(cmd, stdout=args.logfd, stderr=args.logfd) - else: - ret = subprocess.Popen(cmd) - logging.debug("Started process in background with PID " + str(ret.pid)) - else: - try: - if log: - if return_stdout: - ret = subprocess.check_output(cmd).decode("utf-8") - args.logfd.write(ret) - else: - subprocess.check_call(cmd, stdout=args.logfd, - stderr=args.logfd) - args.logfd.flush() - else: - logging.debug("*** output passed to pmbootstrap stdout, not" + - " to this log ***") - subprocess.check_call(cmd) - - except subprocess.CalledProcessError as exc: - if check: - if log: - logging.debug("^" * 70) - logging.info("NOTE: The failed command's output is above" - " the ^^^ line in the logfile: " + args.log) - raise RuntimeError("Command failed: " + log_message) from exc - else: - pass - - if working_dir: - os.chdir(working_dir_old) - return ret +import pmb.helpers.run_core def flat_cmd(cmd, working_dir=None, env={}): @@ -117,24 +49,16 @@ def flat_cmd(cmd, working_dir=None, env={}): return ret -def user(args, cmd, log=True, working_dir=None, return_stdout=False, - check=True, background=False, env={}): +def user(args, cmd, working_dir=None, output="log", output_return=False, + check=None, env={}, kill_as_root=False): """ Run a command on the host system as user. - :param cmd: command as list, e.g. ["echo", "string with spaces"] - :param log: when set to true, redirect all output to the logfile - :param working_dir: path in host system where the command should run - :param return_stdout: write stdout to a buffer and return it as string when - the command is through - :param check: raise an exception, when the command fails - :param background: run the process in the background and return the process - handler :param env: dict of environment variables to be passed to the command, e.g. {"JOBS": "5"} - :returns: * stdout when return_stdout is True - * process handler when background is True - * None otherwise + + See pmb.helpers.run_core.core() for a detailed description of all other + arguments and the return value. """ # Readable log message (without all the escaping) msg = "% " @@ -147,18 +71,23 @@ def user(args, cmd, log=True, working_dir=None, return_stdout=False, # Add environment variables and run if env: cmd = ["sh", "-c", flat_cmd(cmd, env=env)] - return core(args, cmd, msg, log, return_stdout, check, working_dir, - background) + return pmb.helpers.run_core.core(args, msg, cmd, working_dir, output, + output_return, check, kill_as_root) -def root(args, cmd, log=True, working_dir=None, return_stdout=False, - check=True, background=False, env={}): +def root(args, cmd, working_dir=None, output="log", output_return=False, + check=None, env={}): """ Run a command on the host system as root, with sudo. - NOTE: See user() above for parameter descriptions. + :param env: dict of environment variables to be passed to the command, e.g. + {"JOBS": "5"} + + See pmb.helpers.run_core.core() for a detailed description of all other + arguments and the return value. """ if env: cmd = ["sh", "-c", flat_cmd(cmd, env=env)] cmd = ["sudo"] + cmd - return user(args, cmd, log, working_dir, return_stdout, check, background) + return user(args, cmd, working_dir, output, output_return, check, env, + True) diff --git a/pmb/helpers/run_core.py b/pmb/helpers/run_core.py new file mode 100644 index 00000000..9b97ac6e --- /dev/null +++ b/pmb/helpers/run_core.py @@ -0,0 +1,268 @@ +""" +Copyright 2018 Oliver Smith + +This file is part of pmbootstrap. + +pmbootstrap is free software: you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +pmbootstrap is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with pmbootstrap. If not, see . +""" +import fcntl +import logging +import selectors +import subprocess +import sys +import time +import os +import pmb.helpers.run + +""" For a detailed description of all output modes, read the description of + core() at the bottom. All other functions in this file get (indirectly) + called by core(). """ + + +def sanity_checks(output="log", output_return=False, check=None, + kill_as_root=False): + """ + Raise an exception if the parameters passed to core() don't make sense + (all parameters are described in core() below). + """ + if output not in ["log", "stdout", "interactive", "tui", "background"]: + raise RuntimeError("Invalid output value: " + str(output)) + + # Prevent setting the check parameter with output="background". + # The exit code won't be checked when running in background, so it would + # always by check=False. But we prevent it from getting set to check=False + # as well, so it does not look like you could change it to check=True. + if check is not None and output == "background": + raise RuntimeError("Can't use check with output: background") + + if output_return and output in ["tui", "background"]: + raise RuntimeError("Can't use output_return with output: " + output) + + if kill_as_root and output in ["interactive", "tui", "background"]: + raise RuntimeError("Can't use kill_as_root with output: " + output) + + +def background(args, cmd, working_dir=None): + """ Run a subprocess in background and redirect its output to the log. """ + ret = subprocess.Popen(cmd, stdout=args.logfd, stderr=args.logfd, + cwd=working_dir) + logging.debug("Started process in background with PID " + str(ret.pid)) + return ret + + +def pipe_read(args, process, output_to_stdout=False, output_return=False, + output_return_buffer=False): + """ + Read all available output from a subprocess and copy it to the log and + optionally stdout and a buffer variable. This is only meant to be called by + foreground_pipe() below. + + :param process: subprocess.Popen instance + :param output_to_stdout: copy all output to pmbootstrap's stdout + :param output_return: when set to True, output_return_buffer will be + extended + :param output_return_buffer: list of bytes that gets extended with the + current output in case output_return is True. + """ + while True: + # Copy available output + out = process.stdout.readline() + if len(out): + args.logfd.buffer.write(out) + if output_to_stdout: + sys.stdout.buffer.write(out) + if output_return: + output_return_buffer.append(out) + continue + + # No more output (flush buffers) + args.logfd.flush() + if output_to_stdout: + sys.stdout.flush() + return + + +def foreground_pipe(args, cmd, working_dir=None, output_to_stdout=False, + output_return=False, output_timeout=True, + kill_as_root=False): + """ + Run a subprocess in foreground with redirected output and optionally kill + it after being silent for too long. + + :param cmd: command as list, e.g. ["echo", "string with spaces"] + :param working_dir: path in host system where the command should run + :param output_to_stdout: copy all output to pmbootstrap's stdout + :param output_return: return the output of the whole program + :param output_timeout: kill the process when it doesn't print any output + after a certain time (configured with --timeout) + and raise a RuntimeError exception + :param kill_as_root: use sudo to kill the process when it hits the timeout + :returns: (code, output) + * code: return code of the program + * output: "" + * output: full program output string (output_return is True) + """ + # Start process in background (stdout and stderr combined) + process = subprocess.Popen(cmd, stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, cwd=working_dir) + + # Make process.stdout non-blocking + handle = process.stdout.fileno() + flags = fcntl.fcntl(handle, fcntl.F_GETFL) + fcntl.fcntl(handle, fcntl.F_SETFL, flags | os.O_NONBLOCK) + + # While process exists wait for output (with timeout) + output_buffer = [] + sel = selectors.DefaultSelector() + sel.register(process.stdout, selectors.EVENT_READ) + timeout = args.timeout if output_timeout else None + while process.poll() is None: + wait_start = time.perf_counter() if output_timeout else None + sel.select(timeout) + + # On timeout raise error (we need to measure time on our own, because + # select() may exit early even if there is no data to read and the + # timeout was not reached.) + if output_timeout: + wait_end = time.perf_counter() + if wait_end - wait_start >= args.timeout: + logging.info("Process did not write any output for " + + str(args.timeout) + " seconds. Killing it.") + logging.info("NOTE: The timeout can be increased with" + " 'pmbootstrap -t'.") + if kill_as_root: + pmb.helpers.run.root(args, ["kill", "-9", + str(process.pid)]) + else: + process.kill() + continue + + # Read all currently available output + pipe_read(args, process, output_to_stdout, output_return, + output_buffer) + + # There may still be output after the process quit + pipe_read(args, process, output_to_stdout, output_return, output_buffer) + + # Return the return code and output (the output gets built as list of + # output chunks and combined at the end, this is faster than extending the + # combined string with each new chunk) + return (process.returncode, b"".join(output_buffer).decode("utf-8")) + + +def foreground_tui(cmd, working_dir=None): + """ + Run a subprocess in foreground without redirecting any of its output. + + This is the only way text-based user interfaces (ncurses programs like + vim, nano or the kernel's menuconfig) work properly. + """ + + logging.debug("*** output passed to pmbootstrap stdout, not to this log" + " ***") + process = subprocess.Popen(cmd, cwd=working_dir) + return process.wait() + + +def core(args, log_message, cmd, working_dir=None, output="log", + output_return=False, check=None, kill_as_root=False): + """ + Run a command and create a log entry. + + This is a low level function not meant to be used directly. Use one of the + following instead: pmb.helpers.run.user(), pmb.helpers.run.root(), + pmb.chroot.user(), pmb.chroot.root() + + :param log_message: simplified and more readable form of the command, e.g. + "(native) % echo test" instead of the full command with + entering the chroot and more escaping + :param cmd: command as list, e.g. ["echo", "string with spaces"] + :param working_dir: path in host system where the command should run + :param output: where to write the output (stdout and stderr) of the + process. We almost always write to the log file, which can + be read with "pmbootstrap log" (output values: "log", + "stdout", "interactive", "background"), so it's easy to + trace what pmbootstrap does. + + The exception is "tui" (text-based user interface), where + it does not make sense to write to the log file (think of + ncurses UIs, such as "menuconfig"). + + When the output is not set to "interactive", "tui" or + "background", we kill the process if it does not output + anything for 5 minutes (time can be set with "pmbootstrap + --timeout"). + + The table below shows all possible values along with + their properties. "wait" indicates that we wait for the + process to complete. + + output value | timeout | out to log | out to stdout | wait + ----------------------------------------------------------- + "log" | x | x | | x + "stdout" | x | x | x | x + "interactive" | | x | x | x + "tui" | | | x | x + "background" | | x | | + + :param output_return: in addition to writing the program's output to the + destinations above in real time, write to a buffer + and return it as string when the command has + completed. This is not possible when output is + "background" or "tui". + :param check: an exception will be raised when the command's return code + is not 0. Set this to False to disable the check. This + parameter can not be used when the output is "background". + :param kill_as_root: use sudo to kill the process when it hits the timeout. + :returns: * program's return code (default) + * subprocess.Popen instance (output is "background") + * the program's entire output (output_return is True) + """ + sanity_checks(output, output_return, check, kill_as_root) + + # Log simplified and full command (pmbootstrap -v) + logging.debug(log_message) + logging.verbose("run: " + str(cmd)) + + # Background + if output == "background": + return background(args, cmd, working_dir) + + # Foreground + output_after_run = "" + if output == "tui": + # Foreground TUI + code = foreground_tui(cmd, working_dir) + else: + # Foreground pipe (always redirects to the error log file) + output_to_stdout = False + if not args.details_to_stdout and output in ["stdout", "interactive"]: + output_to_stdout = True + + output_timeout = output in ["log", "stdout"] + (code, output_after_run) = foreground_pipe(args, cmd, working_dir, + output_to_stdout, + output_return, + output_timeout, + kill_as_root) + + # Check the return code + if code and check is not False: + logging.debug("^" * 70) + logging.info("NOTE: The failed command's output is above the ^^^ line" + " in the log file: " + args.log) + raise RuntimeError("Command failed: " + log_message) + + # Return (code or output string) + return output_after_run if output_return else code diff --git a/pmb/install/_install.py b/pmb/install/_install.py index 05fa33b3..b0d9c264 100644 --- a/pmb/install/_install.py +++ b/pmb/install/_install.py @@ -199,7 +199,8 @@ def setup_login(args): suffix = "rootfs_" + args.device while True: try: - pmb.chroot.root(args, ["passwd", args.user], suffix, log=False) + pmb.chroot.root(args, ["passwd", args.user], suffix, + output="interactive") break except RuntimeError: logging.info("WARNING: Failed to set the password. Try it" @@ -254,7 +255,8 @@ def setup_keymap(args): args.keymap is not None and args.keymap in options): layout, variant = args.keymap.split("/") - pmb.chroot.root(args, ["setup-keymap", layout, variant], suffix, log=False) + pmb.chroot.root(args, ["setup-keymap", layout, variant], suffix, + output="interactive") else: logging.info("NOTE: No valid keymap specified for device") diff --git a/pmb/install/blockdevice.py b/pmb/install/blockdevice.py index 49d2afe1..34499404 100644 --- a/pmb/install/blockdevice.py +++ b/pmb/install/blockdevice.py @@ -39,7 +39,7 @@ def previous_install(args): pmb.helpers.mount.bind_blockdevice(args, blockdevice_outside, args.work + "/chroot_native" + blockdevice_inside) label = pmb.chroot.root(args, ["blkid", "-s", "LABEL", "-o", "value", - blockdevice_inside], return_stdout=True) + blockdevice_inside], output_return=True) pmb.helpers.run.root(args, ["umount", args.work + "/chroot_native" + blockdevice_inside]) return "pmOS_boot" in label diff --git a/pmb/install/format.py b/pmb/install/format.py index fbe3ad68..dd75bc5d 100644 --- a/pmb/install/format.py +++ b/pmb/install/format.py @@ -47,9 +47,10 @@ def format_and_mount_root(args): " *** TYPE IN THE FULL DISK ENCRYPTION PASSWORD (TWICE!) ***") pmb.chroot.root(args, ["cryptsetup", "luksFormat", "--use-urandom", "--cipher", args.cipher, "-q", device, - "--iter-time", args.iter_time], log=False) + "--iter-time", args.iter_time], + output="interactive") pmb.chroot.root(args, ["cryptsetup", "luksOpen", device, - "pm_crypt"], log=False) + "pm_crypt"], output="interactive") if not os.path.exists(args.work + "/chroot_native" + mountpoint): raise RuntimeError("Failed to open cryptdevice!") diff --git a/pmb/install/losetup.py b/pmb/install/losetup.py index 1b693f8f..405a637b 100644 --- a/pmb/install/losetup.py +++ b/pmb/install/losetup.py @@ -51,7 +51,7 @@ def device_by_back_file(args, back_file): # Get list from losetup losetup_output = pmb.chroot.root(args, ["losetup", "--json", - "--list"], return_stdout=True) + "--list"], output_return=True) if not losetup_output: return None diff --git a/pmb/install/recovery.py b/pmb/install/recovery.py index 5046e079..d146cb94 100644 --- a/pmb/install/recovery.py +++ b/pmb/install/recovery.py @@ -74,4 +74,4 @@ def create_zip(args, suffix): ["gzip", "-f1", "rootfs.tar"], ["build-recovery-zip", args.device]] for command in commands: - pmb.chroot.root(args, command, suffix, working_dir=zip_root) + pmb.chroot.root(args, command, suffix, zip_root) diff --git a/pmb/parse/arguments.py b/pmb/parse/arguments.py index 5bf55d58..e96a450c 100644 --- a/pmb/parse/arguments.py +++ b/pmb/parse/arguments.py @@ -242,6 +242,9 @@ def arguments(): parser.add_argument("-s", "--skip-initfs", dest="skip_initfs", help="do not re-generate the initramfs", action="store_true") + parser.add_argument("-t", "--timeout", help="seconds after which processes" + " get killed that stopped writing any output (default:" + " 300)", default=300, type=float) parser.add_argument("-w", "--work", help="folder where all data" " gets stored (chroots, caches, built packages)") parser.add_argument("-y", "--assume-yes", help="Assume 'yes' to all" @@ -337,7 +340,13 @@ def arguments(): " packages in the chroot before entering it") chroot.add_argument("--user", help="run the command as user, not as root", action="store_true") - chroot.add_argument("command", default=["sh"], help="command" + chroot.add_argument("--output", choices=["log", "stdout", "interactive", + "tui", "background"], help="how the output of the" + " program should be handled, choose from: 'log'," + " 'stdout', 'interactive', 'tui' (default)," + " 'background'. Details: pmb/helpers/run_core.py", + default="tui") + chroot.add_argument("command", default=["sh", "-i"], help="command" " to execute inside the chroot. default: sh", nargs='*') for action in [build_init, chroot]: suffix = action.add_mutually_exclusive_group() diff --git a/pmb/parse/bootimg.py b/pmb/parse/bootimg.py index f630fdbf..80116d6a 100644 --- a/pmb/parse/bootimg.py +++ b/pmb/parse/bootimg.py @@ -35,8 +35,9 @@ def bootimg(args, path): # Copy the boot.img into the chroot temporary folder pmb.helpers.run.root(args, ["cp", path, bootimg_path]) - file_output = pmb.chroot.user(args, ["file", "-b", "boot.img"], working_dir=temp_path, - return_stdout=True).rstrip() + file_output = pmb.chroot.user(args, ["file", "-b", "boot.img"], + working_dir=temp_path, + output_return=True).rstrip() if "android bootimg" not in file_output.lower(): if "force" in args and args.force: logging.warning("WARNING: boot.img file seems to be invalid, but" diff --git a/pmb/qemu/run.py b/pmb/qemu/run.py index 878c26fa..e793a77a 100644 --- a/pmb/qemu/run.py +++ b/pmb/qemu/run.py @@ -295,8 +295,8 @@ def run(args): process = None try: signal.signal(signal.SIGTERM, sigterm_handler) - process = pmb.helpers.run.user(args, qemu, - background=spice_enabled, env=env) + output = "background" if spice_enabled else "interactive" + process = pmb.helpers.run.user(args, qemu, output=output, env=env) if spice: pmb.helpers.run.user(args, spice, env=env) except KeyboardInterrupt: diff --git a/test/test_chroot_interactive_shell.py b/test/test_chroot_interactive_shell.py index 70d4d43a..67c12861 100644 --- a/test/test_chroot_interactive_shell.py +++ b/test/test_chroot_interactive_shell.py @@ -26,8 +26,9 @@ def test_chroot_interactive_shell(): """ pmb_src = os.path.realpath(os.path.join(os.path.dirname(__file__) + "/..")) os.chdir(pmb_src) - ret = subprocess.check_output(["./pmbootstrap.py", "-q", "chroot"], timeout=300, - input="echo hello_world\n", universal_newlines=True, + ret = subprocess.check_output(["./pmbootstrap.py", "-q", "chroot", "sh"], + timeout=300, input="echo hello_world\n", + universal_newlines=True, stderr=subprocess.STDOUT) assert ret == "hello_world\n" @@ -39,7 +40,7 @@ def test_chroot_interactive_shell_user(): pmb_src = os.path.realpath(os.path.join(os.path.dirname(__file__) + "/..")) os.chdir(pmb_src) ret = subprocess.check_output(["./pmbootstrap.py", "-q", "chroot", - "--user"], timeout=300, input="id -un", + "--user", "sh"], timeout=300, input="id -un", universal_newlines=True, stderr=subprocess.STDOUT) assert ret == "pmos\n" @@ -54,8 +55,8 @@ def test_chroot_arguments(): os.chdir(pmb_src) for arch in ["armhf", "aarch64", "x86_64"]: - ret = subprocess.check_output(["./pmbootstrap.py", "-q", "chroot", "-b", arch], - timeout=300, input="uname -m\n", + ret = subprocess.check_output(["./pmbootstrap.py", "-q", "chroot", "-b", arch, + "sh"], timeout=300, input="uname -m\n", universal_newlines=True, stderr=subprocess.STDOUT) if arch == "armhf": assert ret == "armv7l\n" diff --git a/test/test_qemu_running_processes.py b/test/test_qemu_running_processes.py index f26811f3..fa4d04dd 100644 --- a/test/test_qemu_running_processes.py +++ b/test/test_qemu_running_processes.py @@ -60,11 +60,11 @@ def ssh_create_askpass_script(args): pmb.chroot.root(args, ["chmod", "+x", "/tmp/y.sh"]) -def pmbootstrap_run(args, config, parameters, background=False): +def pmbootstrap_run(args, config, parameters, output="log"): """Execute pmbootstrap.py with a test pmbootstrap.conf.""" return pmb.helpers.run.user(args, ["./pmbootstrap.py", "-c", config] + parameters, working_dir=pmb_src, - background=background) + output=output) def pmbootstrap_yes(args, config, parameters): @@ -111,7 +111,7 @@ class Qemu(object): # Create and run system image pmbootstrap_yes(args, config, ["install", "--no-fde"]) self.process = pmbootstrap_run(args, config, ["qemu", "--display", - "none"], background=True) + "none"], "background") @pytest.fixture @@ -130,8 +130,7 @@ def ssh_run(args, command): "-o", "UserKnownHostsFile=/dev/null", "-o", "StrictHostKeyChecking=no", "-p", "2222", "testuser@localhost", "--", - command], - check=False, return_stdout=True) + command], output_return=True, check=False) return ret diff --git a/test/test_run_core.py b/test/test_run_core.py new file mode 100644 index 00000000..a3b0b16c --- /dev/null +++ b/test/test_run_core.py @@ -0,0 +1,145 @@ +""" +Copyright 2018 Oliver Smith + +This file is part of pmbootstrap. + +pmbootstrap is free software: you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +pmbootstrap is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with pmbootstrap. If not, see . +""" + +""" +This file tests functions from pmb.helpers.run_core +""" + +import os +import sys +import pytest + +# Import from parent directory +pmb_src = os.path.realpath(os.path.join(os.path.dirname(__file__) + "/..")) +sys.path.append(pmb_src) +import pmb.helpers.run_core + + +@pytest.fixture +def args(request): + import pmb.parse + sys.argv = ["pmbootstrap.py", "chroot"] + args = pmb.parse.arguments() + args.log = args.work + "/log_testsuite.txt" + pmb.helpers.logging.init(args) + request.addfinalizer(args.logfd.close) + return args + + +def test_sanity_checks(): + func = pmb.helpers.run_core.sanity_checks + + # Invalid output + with pytest.raises(RuntimeError) as e: + func("invalid-output") + assert str(e.value).startswith("Invalid output value") + + # Background and check + func("background", check=None) + for check in [True, False]: + with pytest.raises(RuntimeError) as e: + func("background", check=check) + assert str(e.value).startswith("Can't use check with") + + # output_return + func("log", output_return=True) + with pytest.raises(RuntimeError) as e: + func("tui", output_return=True) + assert str(e.value).startswith("Can't use output_return with") + + # kill_as_root + func("log", kill_as_root=True) + with pytest.raises(RuntimeError) as e: + func("tui", kill_as_root=True) + assert str(e.value).startswith("Can't use kill_as_root with") + + +def test_background(args): + # Sleep in background + process = pmb.helpers.run_core.background(args, ["sleep", "1"], "/") + + # Check if it is still running + assert process.poll() is None + + +def test_foreground_pipe(args): + func = pmb.helpers.run_core.foreground_pipe + cmd = ["echo", "test"] + + # Normal run + assert func(args, cmd) == (0, "") + + # Return output + assert func(args, cmd, output_return=True) == (0, "test\n") + + # Kill with output timeout + cmd = ["sh", "-c", "echo first; sleep 2; echo second"] + args.timeout = 0.3 + ret = func(args, cmd, output_return=True, output_timeout=True) + assert ret == (-9, "first\n") + + # Kill with output timeout as root + cmd = ["sudo", "sh", "-c", "printf first; sleep 2; printf second"] + args.timeout = 0.3 + ret = func(args, cmd, output_return=True, output_timeout=True, + kill_as_root=True) + assert ret == (-9, "first") + + # Finish before timeout + cmd = ["sh", "-c", "echo first; sleep 0.1; echo second; sleep 0.1;" + "echo third; sleep 0.1; echo fourth"] + args.timeout = 0.2 + ret = func(args, cmd, output_return=True, output_timeout=True) + assert ret == (0, "first\nsecond\nthird\nfourth\n") + + +def test_foreground_tui(): + func = pmb.helpers.run_core.foreground_tui + assert func(["echo", "test"]) == 0 + + +def test_core(args): + # Background + func = pmb.helpers.run_core.core + msg = "test" + process = func(args, msg, ["sleep", "1"], output="background") + assert process.poll() is None + + # Foreground (TUI) + ret = func(args, msg, ["echo", "test"], output="tui") + assert ret == 0 + + # Foreground (pipe) + ret = func(args, msg, ["echo", "test"], output="log") + assert ret == 0 + + # Return output + ret = func(args, msg, ["echo", "test"], output="log", output_return=True) + assert ret == "test\n" + + # Check the return code + with pytest.raises(RuntimeError) as e: + func(args, msg, ["false"], output="log") + assert str(e.value).startswith("Command failed:") + + # Kill with timeout + args.timeout = 0.2 + with pytest.raises(RuntimeError) as e: + func(args, msg, ["sleep", "1"], output="log") + assert str(e.value).startswith("Command failed:") diff --git a/test/test_shell_escape.py b/test/test_shell_escape.py index 33afb655..e7c94433 100644 --- a/test/test_shell_escape.py +++ b/test/test_shell_escape.py @@ -50,23 +50,24 @@ def test_shell_escape(args): "hello world\n": ["printf", "%s world\n", "hello"]} for expected, cmd in cmds.items(): copy = list(cmd) - core = pmb.helpers.run.core(args, cmd, str(cmd), True, True) + core = pmb.helpers.run_core.core(args, str(cmd), cmd, + output_return=True) assert expected == core assert cmd == copy - user = pmb.helpers.run.user(args, cmd, return_stdout=True) + user = pmb.helpers.run.user(args, cmd, output_return=True) assert expected == user assert cmd == copy - root = pmb.helpers.run.root(args, cmd, return_stdout=True) + root = pmb.helpers.run.root(args, cmd, output_return=True) assert expected == root assert cmd == copy - chroot_root = pmb.chroot.root(args, cmd, return_stdout=True) + chroot_root = pmb.chroot.root(args, cmd, output_return=True) assert expected == chroot_root assert cmd == copy - chroot_user = pmb.chroot.user(args, cmd, return_stdout=True) + chroot_user = pmb.chroot.user(args, cmd, output_return=True) assert expected == chroot_user assert cmd == copy @@ -80,19 +81,19 @@ def test_shell_escape_env(args): copy = list(cmd) func = pmb.helpers.run.user - assert func(args, cmd, return_stdout=True, env=env) == ret + assert func(args, cmd, output_return=True, env=env) == ret assert cmd == copy func = pmb.helpers.run.root - assert func(args, cmd, return_stdout=True, env=env) == ret + assert func(args, cmd, output_return=True, env=env) == ret assert cmd == copy func = pmb.chroot.root - assert func(args, cmd, return_stdout=True, env=env) == ret + assert func(args, cmd, output_return=True, env=env) == ret assert cmd == copy func = pmb.chroot.user - assert func(args, cmd, return_stdout=True, env=env) == ret + assert func(args, cmd, output_return=True, env=env) == ret assert cmd == copy