Skip to content

Commit 55c47b0

Browse files
committed
fix: prefix external command output for better log clarity
Most log output uses the package name as a prefix (via FromagerLogRecord) so it is possible to tell which package is being processed when a message is logged. However, external command output (like uv) lacked both: 1. Visual indication that it came from an external command 2. Package name on continuation lines for grep This fix adds the package name to every line of external command output and uses a visual prefix (' > ') to distinguish it from fromager's own messages. The package name is retrieved from the log context and manually added to each continuation line, enabling grep to find all lines related to a specific package (e.g., `grep 'numpy:'`). Implementation: - Use shlex.join() for command formatting - Single logger call combining command info and output - str.replace() to add prefixes to each line - Package name manually added to continuation lines for greppability Benefits: - Every line has package name prefix for grep - Visual clarity: '> ' prefix distinguishes external command output - Parallel builds: Single log call prevents line interleaving - Simple implementation with no code duplication Example output: numpy: command failed with exit code 1: uv pip install numpy numpy: > Resolving dependencies... numpy: > Error: Could not find package Fixes: #753 Co-authored-by: Claude <claude@anthropic.com> Signed-off-by: Ioannis Angelakopoulos <iangelak@redhat.com>
1 parent 1fd83c3 commit 55c47b0

2 files changed

Lines changed: 35 additions & 3 deletions

File tree

src/fromager/external_commands.py

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
import typing
88
from io import TextIOWrapper
99

10+
from . import log
11+
1012
logger = logging.getLogger(__name__)
1113

1214
HERE = pathlib.Path(__file__).absolute().parent
@@ -98,8 +100,31 @@ def run(
98100
stdin=stdin,
99101
)
100102
output = completed.stdout.decode("utf-8") if completed.stdout else ""
103+
104+
# Get package name from context to add to each line for greppability
105+
try:
106+
req = log.requirement_ctxvar.get()
107+
line_prefix = f"{req.name}: "
108+
except LookupError:
109+
line_prefix = ""
110+
111+
# Prefix output lines with visual marker and package name for greppability
112+
formatted_output = None
113+
if output:
114+
output_prefix = line_prefix + " > "
115+
prefixed_output = " > " + output.replace("\n", f"\n{output_prefix}")
116+
formatted_output = f"{line_prefix}{prefixed_output}"
117+
101118
if completed.returncode != 0:
102-
logger.error("%s failed with %s", cmd, output)
119+
# Log complete error message with command, exit code, and output
120+
output_to_log = "\n" + formatted_output if formatted_output else ""
121+
logger.error(
122+
"command failed with exit code %d: %s%s",
123+
completed.returncode,
124+
shlex.join(cmd),
125+
output_to_log,
126+
)
127+
103128
err_type = subprocess.CalledProcessError
104129
if network_isolation:
105130
# Look for a few common messages that mean there is a network
@@ -113,5 +138,9 @@ def run(
113138
if substr in output:
114139
err_type = NetworkIsolationError
115140
raise err_type(completed.returncode, cmd, output)
116-
logger.debug("output: %s", output)
141+
142+
# Log command output with visual prefix to distinguish from fromager output
143+
if formatted_output:
144+
logger.debug(formatted_output)
145+
117146
return output

tests/test_external_commands.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ def test_external_commands_log_file(tmp_path: pathlib.Path) -> None:
2929
assert "test\n" == file_contents
3030

3131

32-
@mock.patch("subprocess.run", return_value=mock.Mock(returncode=0))
32+
@mock.patch(
33+
"subprocess.run",
34+
return_value=mock.Mock(returncode=0, stdout=b"test output\n"),
35+
)
3336
@mock.patch(
3437
"fromager.external_commands.network_isolation_cmd",
3538
return_value=["/bin/unshare", "--net", "--map-current-user"],

0 commit comments

Comments
 (0)