From 25814ed5cbbabd8c496643604e0deb9167653d23 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Fri, 29 Nov 2024 01:41:53 -0600 Subject: [PATCH] scripts: Fixed failed subprocess stderr, unconditionally forward It looks like the failure case in our scripts' subprocess stderr handling was not tested well during a fix to stderr blocking (a735bcd). This code was attempting to print stderr only if an error occured, but with stderr=None this just results in a NoneType TypeError. In retrospect, completely hiding stderr is kind of shitty if a subprocess fails, but it doesn't seem possible to read from both stdin and stderr with Python's APIs without getting stuck when the stderr's buffer is full. It might be possible to work around this with either multithreading, select calls, or a temp file, but I'm not sure slightly less verbose scripts are worth the added complexity in every single subprocess call. For now just reverting to unconditionally forwarding stderr from the child process. This is the simplest/most robust option. --- scripts/bench.py | 16 ---------------- scripts/code.py | 12 ------------ scripts/cov.py | 4 ---- scripts/data.py | 12 ------------ scripts/perf.py | 12 ------------ scripts/perfbd.py | 8 -------- scripts/structs.py | 8 -------- scripts/test.py | 16 ---------------- 8 files changed, 88 deletions(-) diff --git a/scripts/bench.py b/scripts/bench.py index f067f89b..4da66294 100755 --- a/scripts/bench.py +++ b/scripts/bench.py @@ -722,7 +722,6 @@ def find_perms(runner, bench_ids=[], **args): print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -743,9 +742,6 @@ def find_perms(runner, bench_ids=[], **args): total_perms += perms proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) sys.exit(-1) # get which suite each case belongs to via paths @@ -754,7 +750,6 @@ def find_perms(runner, bench_ids=[], **args): print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -773,9 +768,6 @@ def find_perms(runner, bench_ids=[], **args): case_suites[m.group('case')] = suite proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) sys.exit(-1) # figure out expected suite perms @@ -800,7 +792,6 @@ def find_path(runner, id, **args): print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -816,9 +807,6 @@ def find_path(runner, id, **args): path = (path_, lineno) proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) sys.exit(-1) return path @@ -831,7 +819,6 @@ def find_defines(runner, id, **args): print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -845,9 +832,6 @@ def find_defines(runner, id, **args): defines[define] = value proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) sys.exit(-1) return defines diff --git a/scripts/code.py b/scripts/code.py index 987694ee..119487ab 100755 --- a/scripts/code.py +++ b/scripts/code.py @@ -176,7 +176,6 @@ def collect_dwarf_files(obj_path, *, print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -200,9 +199,6 @@ def collect_dwarf_files(obj_path, *, files[int(m.group('no'))] = m.group('path') proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) raise sp.CalledProcessError(proc.returncode, proc.args) # simplify paths @@ -264,7 +260,6 @@ def collect_dwarf_info(obj_path, filter=None, *, print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -293,9 +288,6 @@ def collect_dwarf_info(obj_path, filter=None, *, m.group('v').strip()) proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) raise sp.CalledProcessError(proc.returncode, proc.args) return entries @@ -318,7 +310,6 @@ def collect_sizes(obj_path, *, print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -330,9 +321,6 @@ def collect_sizes(obj_path, *, sizes[func] = size proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) raise sp.CalledProcessError(proc.returncode, proc.args) return sizes diff --git a/scripts/cov.py b/scripts/cov.py index b26a3928..8e245ec5 100755 --- a/scripts/cov.py +++ b/scripts/cov.py @@ -272,16 +272,12 @@ def collect_cov(gcda_path, *, print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) cov = json.load(proc.stdout) proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) raise sp.CalledProcessError(proc.returncode, proc.args) return cov diff --git a/scripts/data.py b/scripts/data.py index b7dacc1e..1aafac67 100755 --- a/scripts/data.py +++ b/scripts/data.py @@ -176,7 +176,6 @@ def collect_dwarf_files(obj_path, *, print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -200,9 +199,6 @@ def collect_dwarf_files(obj_path, *, files[int(m.group('no'))] = m.group('path') proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) raise sp.CalledProcessError(proc.returncode, proc.args) # simplify paths @@ -264,7 +260,6 @@ def collect_dwarf_info(obj_path, filter=None, *, print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -293,9 +288,6 @@ def collect_dwarf_info(obj_path, filter=None, *, m.group('v').strip()) proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) raise sp.CalledProcessError(proc.returncode, proc.args) return entries @@ -318,7 +310,6 @@ def collect_sizes(obj_path, *, print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -330,9 +321,6 @@ def collect_sizes(obj_path, *, sizes[func] = size proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) raise sp.CalledProcessError(proc.returncode, proc.args) return sizes diff --git a/scripts/perf.py b/scripts/perf.py index 6d59983f..4df12e14 100755 --- a/scripts/perf.py +++ b/scripts/perf.py @@ -277,7 +277,6 @@ def collect_syms(obj_path, *, print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -297,9 +296,6 @@ def collect_syms(obj_path, *, sym_at.append((addr, name, size)) proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) raise sp.CalledProcessError(proc.returncode, proc.args) # sort and keep largest/first when duplicates @@ -349,7 +345,6 @@ def collect_dwarf_lines(obj_path, *, print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -390,9 +385,6 @@ def collect_dwarf_lines(obj_path, *, op_addr = 0 proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) raise sp.CalledProcessError(proc.returncode, proc.args) # sort and keep first when duplicates @@ -446,7 +438,6 @@ def collect_decompressed(path, *, print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -594,9 +585,6 @@ def collect_decompressed(path, *, proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) raise sp.CalledProcessError(proc.returncode, proc.args) # rearrange results into result type diff --git a/scripts/perfbd.py b/scripts/perfbd.py index 6976fd9d..751cda85 100755 --- a/scripts/perfbd.py +++ b/scripts/perfbd.py @@ -183,7 +183,6 @@ def collect_syms(obj_path, *, print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -203,9 +202,6 @@ def collect_syms(obj_path, *, sym_at.append((addr, name, size)) proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) raise sp.CalledProcessError(proc.returncode, proc.args) # sort and keep largest/first when duplicates @@ -254,7 +250,6 @@ def collect_dwarf_lines(obj_path, *, print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -295,9 +290,6 @@ def collect_dwarf_lines(obj_path, *, op_addr = 0 proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) raise sp.CalledProcessError(proc.returncode, proc.args) # sort and keep first when duplicates diff --git a/scripts/structs.py b/scripts/structs.py index 263f8f19..7a6e3160 100755 --- a/scripts/structs.py +++ b/scripts/structs.py @@ -175,7 +175,6 @@ def collect_dwarf_files(obj_path, *, print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -199,9 +198,6 @@ def collect_dwarf_files(obj_path, *, files[int(m.group('no'))] = m.group('path') proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) raise sp.CalledProcessError(proc.returncode, proc.args) # simplify paths @@ -263,7 +259,6 @@ def collect_dwarf_info(obj_path, filter=None, *, print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -292,9 +287,6 @@ def collect_dwarf_info(obj_path, filter=None, *, m.group('v').strip()) proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) raise sp.CalledProcessError(proc.returncode, proc.args) return entries diff --git a/scripts/test.py b/scripts/test.py index 36d4d275..207d06fb 100755 --- a/scripts/test.py +++ b/scripts/test.py @@ -742,7 +742,6 @@ def find_perms(runner, test_ids=[], **args): print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -763,9 +762,6 @@ def find_perms(runner, test_ids=[], **args): total_perms += perms proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) sys.exit(-1) # get which suite each case belongs to via paths @@ -774,7 +770,6 @@ def find_perms(runner, test_ids=[], **args): print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -793,9 +788,6 @@ def find_perms(runner, test_ids=[], **args): case_suites[m.group('case')] = suite proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) sys.exit(-1) # figure out expected suite perms @@ -820,7 +812,6 @@ def find_path(runner, id, **args): print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -836,9 +827,6 @@ def find_path(runner, id, **args): path = (path_, lineno) proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) sys.exit(-1) return path @@ -851,7 +839,6 @@ def find_defines(runner, id, **args): print(' '.join(shlex.quote(c) for c in cmd)) proc = sp.Popen(cmd, stdout=sp.PIPE, - stderr=None if args.get('verbose') else sp.DEVNULL, universal_newlines=True, errors='replace', close_fds=False) @@ -865,9 +852,6 @@ def find_defines(runner, id, **args): defines[define] = value proc.wait() if proc.returncode != 0: - if not args.get('verbose'): - for line in proc.stderr: - sys.stderr.write(line) sys.exit(-1) return defines