Skip to content
Closed
Changes from all commits
Commits
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
21 changes: 13 additions & 8 deletions mkosi/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,22 @@
raise subprocess.CalledProcessError(rc, ["self"])


def log_sandbox_issue(sandbox: Sequence[str], cmdline: Sequence[str], errmsg: str) -> None:
if ARG_DEBUG.get():
# Complete but very long and hard to read. It could be reusable.
logging.error(f"{shlex.join(['mkosi-sandbox', *sandbox, *cmdline])}")
else:
logging.error("sandbox command omitted, rerun with --debug to see full command with bind mounts")
logging.error(f"{shlex.join(cmdline)}")
Comment on lines +110 to +115
Copy link
Contributor

Choose a reason for hiding this comment

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

Please stick to a single call to logging.error() and use newlines instead. I would keep the original log message the same and if --debug is not set, simply append another line that says the following:

"sandbox details omitted, rerun with --debug to see the full sandbox command."

Copy link
Contributor Author

@marc-hb marc-hb Jan 20, 2026

Choose a reason for hiding this comment

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

... and use newlines instead. I would keep the original log message the same...

So are you ok with printing the error message on a separate line? Adding just one newline. Not 100% clear to me from the above, sorry.

I tested this PR heavily (especially compared to how small of a code change it is...) and NOT having the error message trailing a 5-10 lines-long command made a massive difference. It's quite painful to find before this PR.

I don't mind the code style and the number of logging statements, the terminal output and "user experience" is really the purpose of this.


logging.error(errmsg)


def log_process_failure(sandbox: Sequence[str], cmdline: Sequence[str], returncode: int) -> None:
if -returncode in (signal.SIGINT, signal.SIGTERM):
logging.error(f"Interrupted by {signal.Signals(-returncode).name} signal")
elif returncode < 0:
logging.error(
f'"{shlex.join(["mkosi-sandbox", *sandbox, *cmdline] if ARG_DEBUG.get() else cmdline)}"'
f" was killed by {signal.Signals(-returncode).name} signal."
)
log_sandbox_issue(sandbox, cmdline, f" was killed by {signal.Signals(-returncode).name} signal.")
elif returncode == 127 and cmdline[0] != "mkosi":
# Anything invoked beneath /work is a script that we mount into place (so we know it exists). If one
# of these scripts fails with exit code 127, it's either because the script interpreter was not
Expand All @@ -126,10 +134,7 @@
else:
logging.error(f"{cmdline[0]} not found.")
else:
logging.error(
f'"{shlex.join(["mkosi-sandbox", *sandbox, *cmdline] if ARG_DEBUG.get() else cmdline)}"'
f" returned non-zero exit code {returncode}."
)
log_sandbox_issue(sandbox, cmdline, f" returned non-zero exit code {returncode}.")


def run(
Expand Down