Skip to content

Commit c7ddc85

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, the output of external commands (like uv) was logged without any visual indication that it came from an external command rather than fromager itself, making logs harder to read. This fix adds a visual prefix (' > ') to each line of external command output using str.replace(), then logs the entire output in a single call. Benefits: - Visual clarity: The '> ' prefix clearly distinguishes external command output from fromager's own log messages - Parallel builds: Logging the entire output as one block prevents line interleaving when building multiple packages in parallel - Package context: The package name prefix is still applied to the whole output block via FromagerLogRecord - Simple implementation: Uses a single str.replace() call Example output: numpy: command failed with exit code 1: uv pip install numpy numpy: > Resolving dependencies... > Error: Could not find package Fixes: #753 Co-authored-by: Claude <claude@anthropic.com> Signed-off-by: Ioannis Angelakopoulos <iangelak@redhat.com>
1 parent 222f3f8 commit c7ddc85

File tree

2 files changed

+19
-3
lines changed

2 files changed

+19
-3
lines changed

src/fromager/external_commands.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,16 @@ def run(
9999
)
100100
output = completed.stdout.decode("utf-8") if completed.stdout else ""
101101
if completed.returncode != 0:
102-
logger.error("%s failed with %s", cmd, output)
102+
# Log the command failure first
103+
logger.error(
104+
"command failed with exit code %d: %s",
105+
completed.returncode,
106+
" ".join(shlex.quote(str(s)) for s in cmd),
107+
)
108+
# Log command output with visual prefix to distinguish from fromager output
109+
if output:
110+
logger.error(" > " + output.replace("\n", "\n > "))
111+
103112
err_type = subprocess.CalledProcessError
104113
if network_isolation:
105114
# Look for a few common messages that mean there is a network
@@ -113,5 +122,9 @@ def run(
113122
if substr in output:
114123
err_type = NetworkIsolationError
115124
raise err_type(completed.returncode, cmd, output)
116-
logger.debug("output: %s", output)
125+
126+
# Log command output with visual prefix to distinguish from fromager output
127+
if output:
128+
logger.debug(" > " + output.replace("\n", "\n > "))
129+
117130
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):
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)