Skip to content

fix: Add package prefix to uv output #843

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+19 −3
Open

fix: Add package prefix to uv output #843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions src/fromager/external_commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,16 @@ def run(
)
output = completed.stdout.decode("utf-8") if completed.stdout else ""
if completed.returncode != 0:
logger.error("%s failed with %s", cmd, output)
# Log the command failure first
logger.error(
"command failed with exit code %d: %s",
completed.returncode,
" ".join(shlex.quote(str(s)) for s in cmd),
)
# Log command output with visual prefix to distinguish from fromager output
if output:
logger.error(" > " + output.replace("\n", "\n > "))
Copy link
Member

@dhellmann dhellmann Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we get the package name in there, too?

My goal is to be able to do something like `grep "foo:" and find all of the lines in the log related to that package. They will be spread out because we resolve and build the sdist early in the bootstrap step, but then may not build the wheel until way later.

Copy link
Collaborator

@tiran tiran Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • use shlex.join instead. The function returns a string
  • use a single logger call to log the command, exit code, and output.


err_type = subprocess.CalledProcessError
if network_isolation:
# Look for a few common messages that mean there is a network
Expand All @@ -113,5 +122,9 @@ def run(
if substr in output:
err_type = NetworkIsolationError
raise err_type(completed.returncode, cmd, output)
logger.debug("output: %s", output)

# Log command output with visual prefix to distinguish from fromager output
if output:
logger.debug(" > " + output.replace("\n", "\n > "))
Copy link
Member

@dhellmann dhellmann Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, include the package name on each line.


return output
5 changes: 4 additions & 1 deletion tests/test_external_commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ def test_external_commands_log_file(tmp_path):
assert "test\n" == file_contents


@mock.patch("subprocess.run", return_value=mock.Mock(returncode=0))
@mock.patch(
"subprocess.run",
return_value=mock.Mock(returncode=0, stdout=b"test output\n"),
)
@mock.patch(
"fromager.external_commands.network_isolation_cmd",
return_value=["/bin/unshare", "--net", "--map-current-user"],
Expand Down
Loading