From 535efdb0b8ecd4d5a57c070bbc9cdb7889c1f528 Mon Sep 17 00:00:00 2001 From: David Poole Date: Mon, 17 Mar 2025 11:12:05 -0700 Subject: [PATCH 1/2] fix several problems with stdout/stderr - subprocess calls should always send stderr outwards (no capture) - add option to capture or not capture stdout so $(shell) will be captured but recipes will go outwards (no capture) - set stdout to flush on every print() to match GNU Make which uses setbuf() (which isn't available to python) because otherwise stdout can race between pymake process and subprocess stdout - start tests on $(SHELLFLAGS) which is what led to finding these problems with stdout --- pymake/functions.py | 4 ++-- pymake/pymake.py | 9 ++++---- pymake/shell.py | 44 +++++++++++++++++++++++++++------------- tests/test_export.py | 8 ++++---- tests/test_shellflags.py | 28 +++++++++++++++++++++++++ 5 files changed, 69 insertions(+), 24 deletions(-) create mode 100644 tests/test_shellflags.py diff --git a/pymake/functions.py b/pymake/functions.py index 63fb4b6..75e0ed2 100644 --- a/pymake/functions.py +++ b/pymake/functions.py @@ -84,9 +84,9 @@ def eval(self, symbol_table): if self.fmt: t = self.token_list[0] filename, linenumber = t.get_pos() - print(self.fmt.format(filename, linenumber, msg), file=self.fh) + print(self.fmt.format(filename, linenumber, msg), file=self.fh, flush=True) else: - print("%s" % msg, file=self.fh) + print("%s" % msg, file=self.fh, flush=True) return "" diff --git a/pymake/pymake.py b/pymake/pymake.py index f0bd6bf..301fa08 100644 --- a/pymake/pymake.py +++ b/pymake/pymake.py @@ -502,14 +502,14 @@ def check_prefixes(s): s, ignore_failure, silent = check_prefixes(s) if not silent and not args.silent: - print(s) + print(s,flush=True) if args.dry_run: - print(s) + print(s, flush=True) continue exit_code = 0 - ret = shell.execute(s, symtable) + ret = shell.execute(s, symtable, capture=False) # # !!! Run a Sub-Make !!! @@ -535,7 +535,8 @@ def check_prefixes(s): ret.stdout = "" exit_code = ret.exit_code - print(ret.stdout,end="") +# print(ret.stderr,end="") +# print(ret.stdout,end="") if exit_code != 0: print("make:", ret.stderr, file=sys.stderr, end="") print("make: *** [%r: %s] Error %d %s" % (recipe.get_pos(), rule.target, exit_code, "(ignored)" if ignore_failure else ""), file=sys.stderr) diff --git a/pymake/shell.py b/pymake/shell.py index eb3a280..2c80dac 100644 --- a/pymake/shell.py +++ b/pymake/shell.py @@ -30,7 +30,7 @@ def __init__(self): self.is_submake = False -def execute(cmd_str, symbol_table, use_default_shell=True): +def execute(cmd_str, symbol_table, use_default_shell=True, capture=True): """execute a string with the shell, returning a bunch of useful info""" # capture a timestamp so we can match shell debug messages @@ -79,7 +79,7 @@ def execute(cmd_str, symbol_table, use_default_shell=True): if shell: cmd.extend(shell_list) if shellflags: - cmd.append(shellflags) + cmd.extend([f for f in shellflags.split()]) cmd.append(cmd_str) env = symbol_table.get_exports() @@ -115,21 +115,36 @@ def execute(cmd_str, symbol_table, use_default_shell=True): # outfile.write(" ".join(cmd)) # outfile.write("\n\n\n") + # definitely need to capture stdout when we're running a sub-make because + # that's how we determine the shell arguments to the actual sub-make + if cmd_str.startswith(submake.getname()): + capture = True + + logger.debug("cmd=>>>%s<<<", cmd) + try: - p = subprocess.run(cmd, - shell=False, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - universal_newlines=True, - check=False, # we'll check returncode ourselves - env=env - ) + if capture: + p = subprocess.run(cmd, + shell=False, + stdout=subprocess.PIPE, + universal_newlines=True, + check=False, # we'll check returncode ourselves + env=env + ) + else: + p = subprocess.run(cmd, + shell=False, + universal_newlines=True, + check=False, # we'll check returncode ourselves + env=env + ) + logger.debug("shell ts=%f exit status=%r", ts, p.returncode) # if p.returncode != 0: # breakpoint() return_status.exit_code = p.returncode return_status.stdout = p.stdout - return_status.stderr = p.stderr +# return_status.stderr = p.stderr except OSError as err: logger.error("shell ts=%f error=\"%s\"", ts, err) return_status.exit_code = 127 @@ -165,9 +180,10 @@ def execute_tokens(token_list, symbol_table): symbol_table.allow_recursion() # GNU Make returns one whitespace separated string, no CR/LF - # "all other newlines are replaced by spaces." gnu_make.pdf + # "If the result of the execution ends in a newline, that one newline is + # removed; all other newlines are replaced by spaces." GNU Make PDF exe_result.stdout = exe_result.stdout.strip().replace("\n", " ") - exe_result.stderr = exe_result.stderr.strip().replace("\n", " ") +# exe_result.stderr = exe_result.stderr.strip().replace("\n", " ") # save shell status pos = token_list[0].get_pos() @@ -188,7 +204,7 @@ def execute_tokens(token_list, symbol_table): # otherwise report stderr (if any) if exe_result.stderr: logger.error("command at %r failed with exit_code=%d", pos, exe_result.exit_code) - error_message(pos, exe_result.stderr) +# error_message(pos, exe_result.stderr) else: logger.error("command at %r failed with exit_code=%d (but stderr empty)", pos, exe_result.exit_code) diff --git a/tests/test_export.py b/tests/test_export.py index 1515085..0e9ef7f 100644 --- a/tests/test_export.py +++ b/tests/test_export.py @@ -12,13 +12,13 @@ # The subprocess.run_test() will raise error on non-zero exit. def run_test(makefile, expect, extra_args=None, extra_env=None): - output = run.pymake_string(makefile, extra_args, extra_env) + output = run.gnumake_string(makefile, extra_args, extra_env) if _debug: - print("pymake output=",output) + print("gnumake output=>>>",output, "<<<") run.verify(output,expect) - output = run.gnumake_string(makefile, extra_args, extra_env) + output = run.pymake_string(makefile, extra_args, extra_env) if _debug: - print("gnumake output=",output) + print("pymake output=>>>",output, "<<<") run.verify(output,expect) def test1(): diff --git a/tests/test_shellflags.py b/tests/test_shellflags.py new file mode 100644 index 0000000..4318dd1 --- /dev/null +++ b/tests/test_shellflags.py @@ -0,0 +1,28 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0 + +import run + +def test_simple(): + makefile=""" +$(info .SHELLFLAGS=$(.SHELLFLAGS)) +@:;@: +""" + a = run.gnumake_string(makefile) + + b = run.pymake_string(makefile) + + assert a==b + +def test_x_flag(): + makefile=""" +.SHELLFLAGS+=-x +$(info .SHELLFLAGS=$(.SHELLFLAGS)) +@:;@: +""" + a = run.gnumake_string(makefile) + + b = run.pymake_string(makefile) + + assert a==b + From 1024179d390211952d0fd670beaa598813a0cbe6 Mon Sep 17 00:00:00 2001 From: David Poole Date: Sat, 22 Mar 2025 07:56:14 -0700 Subject: [PATCH 2/2] small clean-ups to shell capture - remove stderr entirely from ShellReturn - clean up calling subprocess.run() in shell.execute() --- pymake/pymake.py | 6 ++---- pymake/shell.py | 42 ++++++++++++++-------------------------- tests/test_shellflags.py | 5 ++++- 3 files changed, 21 insertions(+), 32 deletions(-) diff --git a/pymake/pymake.py b/pymake/pymake.py index 301fa08..b66ee71 100644 --- a/pymake/pymake.py +++ b/pymake/pymake.py @@ -535,11 +535,9 @@ def check_prefixes(s): ret.stdout = "" exit_code = ret.exit_code -# print(ret.stderr,end="") -# print(ret.stdout,end="") if exit_code != 0: - print("make:", ret.stderr, file=sys.stderr, end="") - print("make: *** [%r: %s] Error %d %s" % (recipe.get_pos(), rule.target, exit_code, "(ignored)" if ignore_failure else ""), file=sys.stderr) + print("make: *** [%r: %s] Error %d %s" % (recipe.get_pos(), rule.target, + exit_code, "(ignored)" if ignore_failure else ""), file=sys.stderr) if not ignore_failure: break diff --git a/pymake/shell.py b/pymake/shell.py index 2c80dac..13c78df 100644 --- a/pymake/shell.py +++ b/pymake/shell.py @@ -23,8 +23,8 @@ class ShellReturn: def __init__(self): + # note: stderr is never captured self.stdout = None - self.stderr = None self.exit_code = None self.errmsg = None self.is_submake = False @@ -122,35 +122,28 @@ def execute(cmd_str, symbol_table, use_default_shell=True, capture=True): logger.debug("cmd=>>>%s<<<", cmd) - try: - if capture: - p = subprocess.run(cmd, - shell=False, - stdout=subprocess.PIPE, - universal_newlines=True, - check=False, # we'll check returncode ourselves - env=env - ) - else: - p = subprocess.run(cmd, - shell=False, - universal_newlines=True, - check=False, # we'll check returncode ourselves - env=env - ) + kwargs= { + "shell":False, + "universal_newlines":True, + "check":False, # we'll check returncode ourselves + "env":env + } + if capture: + kwargs["stdout"] = subprocess.PIPE + try: + p = subprocess.run(cmd, **kwargs) +# logger.debug("shell ts=%f exit status=%r", ts, p.returncode) # if p.returncode != 0: # breakpoint() return_status.exit_code = p.returncode return_status.stdout = p.stdout -# return_status.stderr = p.stderr except OSError as err: logger.error("shell ts=%f error=\"%s\"", ts, err) + # match gnu make's output return_status.exit_code = 127 return_status.stdout = "" - # match gnu make's output - return_status.stderr = err.strerror if cmd_str.startswith(submake.getname()): return_status.is_submake = True @@ -183,7 +176,6 @@ def execute_tokens(token_list, symbol_table): # "If the result of the execution ends in a newline, that one newline is # removed; all other newlines are replaced by spaces." GNU Make PDF exe_result.stdout = exe_result.stdout.strip().replace("\n", " ") -# exe_result.stderr = exe_result.stderr.strip().replace("\n", " ") # save shell status pos = token_list[0].get_pos() @@ -201,12 +193,8 @@ def execute_tokens(token_list, symbol_table): if exe_result.errmsg: error_message(pos, exe_result.errmsg) else: - # otherwise report stderr (if any) - if exe_result.stderr: - logger.error("command at %r failed with exit_code=%d", pos, exe_result.exit_code) -# error_message(pos, exe_result.stderr) - else: - logger.error("command at %r failed with exit_code=%d (but stderr empty)", pos, exe_result.exit_code) + # stderr already sent to the world + logger.error("command at %r failed with exit_code=%d", pos, exe_result.exit_code) return exe_result.stdout diff --git a/tests/test_shellflags.py b/tests/test_shellflags.py index 4318dd1..c2c8a0d 100644 --- a/tests/test_shellflags.py +++ b/tests/test_shellflags.py @@ -14,11 +14,14 @@ def test_simple(): assert a==b +# the -x flag will echo the command to stderr before executing it +# (very useful when debugging) def test_x_flag(): makefile=""" .SHELLFLAGS+=-x $(info .SHELLFLAGS=$(.SHELLFLAGS)) -@:;@: +top: + echo .SHELLFLAGS=$(.SHELLFLAGS) """ a = run.gnumake_string(makefile)