From 93b572f2466890d711870c50b3f240de80fa56dc Mon Sep 17 00:00:00 2001 From: Michal Berger Date: Mon, 13 Feb 2023 15:33:34 +0100 Subject: [PATCH] autorun_post: Adjust .check_call() arguments Don't set std{out,err} to a PIPE to make sure a child process (lcov and/or genhtml) doesn't block indefinitely as per subprocess doc: "Note: Do not use stdout=PIPE or stderr=PIPE with this function. The child process will block if it generates enough output to a pipe to fill up the OS pipe buffer as the pipes are not being read from." The above scenario may happen even when genhtml is not failing, but still reporting warnings to stderr. To avoid that, keep std{out,err} connected to the active tty - this way we also won't miss any warnings lcov, genhtml might report. Also, drop the shell=True - there's simply no need to execute lcov, genhtml under an extra sh process. Signed-off-by: Michal Berger Change-Id: I01372c2d7d8b70c90639419859fa76ad2b7ebd5c Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/16772 Reviewed-by: Tomasz Zawadzki Tested-by: SPDK CI Jenkins Reviewed-by: Konrad Sztyber --- autorun_post.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/autorun_post.py b/autorun_post.py index 0166b56b6..c9cbfd4ad 100755 --- a/autorun_post.py +++ b/autorun_post.py @@ -7,6 +7,7 @@ import shutil import subprocess import argparse +import itertools import os import sys import glob @@ -61,19 +62,23 @@ def generateCoverageReport(output_dir, repo_dir): if len(covfiles) == 0: return lcov_opts = [ - '--rc lcov_branch_coverage=1', - '--rc lcov_function_coverage=1', - '--rc genhtml_branch_coverage=1', - '--rc genhtml_function_coverage=1', - '--rc genhtml_legend=1', - '--rc geninfo_all_blocks=1', + '--rc', 'lcov_branch_coverage=1', + '--rc', 'lcov_function_coverage=1', + '--rc', 'genhtml_branch_coverage=1', + '--rc', 'genhtml_function_coverage=1', + '--rc', 'genhtml_legend=1', + '--rc', 'geninfo_all_blocks=1', ] + + # HACK: This is a workaround for some odd CI assumptions + details = '--show-details' + cov_total = os.path.abspath(os.path.join(output_dir, 'cov_total.info')) coverage = os.path.join(output_dir, 'coverage') - lcov = 'lcov' + ' ' + ' '.join(lcov_opts) + ' -q -a ' + ' -a '.join(covfiles) + ' -o ' + cov_total - genhtml = 'genhtml' + ' ' + ' '.join(lcov_opts) + ' -q ' + cov_total + ' --legend' + ' -t "Combined" --show-details -o ' + coverage + lcov = ['lcov', *lcov_opts, '-q', *itertools.chain(*[('-a', f) for f in covfiles]), '-o', cov_total] + genhtml = ['genhtml', *lcov_opts, '-q', cov_total, '--legend', '-t', 'Combined', *details.split(), '-o', coverage] try: - subprocess.check_call([lcov], shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + subprocess.check_call(lcov) except subprocess.CalledProcessError as e: print("lcov failed") print(e) @@ -89,7 +94,7 @@ def generateCoverageReport(output_dir, repo_dir): Line = re.sub("^SF:.*/repo", replacement, Line) file.write(Line + '\n') try: - subprocess.check_call([genhtml], shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + subprocess.check_call(genhtml) except subprocess.CalledProcessError as e: print("genhtml failed") print(e)