Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 278761 - sys-apps/sandbox log integration with PM
Summary: sys-apps/sandbox log integration with PM
Status: CONFIRMED
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Sandbox (show other bugs)
Hardware: All Linux
: High normal (vote)
Assignee: Portage team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 431638
  Show dependency tree
 
Reported: 2009-07-22 20:12 UTC by Mounir Lamouri (volkmar) (RETIRED)
Modified: 2024-01-27 06:39 UTC (History)
3 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mounir Lamouri (volkmar) (RETIRED) gentoo-dev 2009-07-22 20:12:04 UTC
According to zmedico, sandbox now can call a binary eerror and produce a normal die message.
Comment 1 Zac Medico gentoo-dev 2009-07-22 20:44:26 UTC
Specifically, it would be nice if the "ACCESS VIOLATION SUMMARY" is logged via eerror. Currently, there's no convenient way (afaik) to extract an error message from an ebuild that dies due to a sandbox violation.
Comment 3 SpanKY gentoo-dev 2012-12-17 21:55:30 UTC
this won't work for a couple of reasons

(1) portage helpers require $T be set in the environment so they can update their logs.  some environments (such as scons) remove unknown vars from the env.  i could add logic to sandbox to preserve this variable, but i also vaguely recall seeing other packages re-use $T for its own purposes.

(2) the e{error,warn,etc...} helpers always write to fd 2 (stderr).  that's unacceptable from the sandbox PoV -- it can't indiscriminately write to any fd.  that's why it has a log file in the first place and why it's been dumping straight to /dev/tty.  i have larger plans to sort that latter point out, but it still wouldn't fix the former (which i think PMS mandates?).

(3) relying on an external program "eerror" and "ewarn" and such existing isn't portable.  afaik, PMS says these should exist so the ebuild env can run them (which they do w/portage -- both as functions & programs in $PATH), but there's no requirement that they be available via $PATH.  i think Brian said he doesn't have them in $PATH.

(4) sandbox typically runs in the same context as the process it is sandboxing.  running portage helpers to give it information requires a fork+exec to run the helper (and then sandbox would write its data to it).  since the process we're running in the context could already be using SIGCHLD for its purposes, we get into nasty race semantics: if it's threaded & forking & watching for SIGCHLD, that means we can't have our SIGCHLD go up the stack, and we can't have our local SIGCHLD swallow all signals.  technically, it's possible, but it's a huge hassle i'd rather just avoid.

what i propose instead is that portage set SANDBOX_ERROR_LOG to point to $T/logging/sandbox and then watch for that file.  i'll update sandbox to guarantee that it only writes errors to it that way you can abort the build if the file exists and is non-zero in size.  atm, sandbox has SANDBOX_LOG and SANDBOX_DEBUG_LOG -- the former isn't currently useful as it can also write informational/non-fatal messages.  i'll also make sure that these env vars get preserved in cases where packages try to unset them.
Comment 4 Zac Medico gentoo-dev 2012-12-18 00:56:41 UTC
(In reply to comment #3)
> what i propose instead is that portage set SANDBOX_ERROR_LOG to point to
> $T/logging/sandbox and then watch for that file.  i'll update sandbox to
> guarantee that it only writes errors to it that way you can abort the build
> if the file exists and is non-zero in size.

Sounds good.

> atm, sandbox has SANDBOX_LOG
> and SANDBOX_DEBUG_LOG -- the former isn't currently useful as it can also
> write informational/non-fatal messages.  i'll also make sure that these env
> vars get preserved in cases where packages try to unset them.

Current versions of portage assume that non-empty SANDBOX_LOG is an error, so I guess they're broken in this assumption. The relevant code is at the end of __ebuild_main in bin/phase-functions.sh, where it uses [[ ! -s $SANDBOX_LOG ]] for exit status.
Comment 5 SpanKY gentoo-dev 2012-12-18 06:54:03 UTC
(In reply to comment #4)

sorry, let me clarify.  relying on SANDBOX_LOG is currently correct as sandbox only writes access violations to it.  in my head, i had other things planned.  but rather than break that (since it's working), i'll just relocate the non-fatal messages.

the desirable behavior is for the short sandbox messages to show up at the same time they get executed (so they get interleaved in the general log file at the right place), so writing to just a log file won't work.

i guess what's the driving desire here ?  you want sandbox to trigger portage to kill the build immediately when a violation occurs rather than waiting until the end of a specific phase ?
Comment 6 Zac Medico gentoo-dev 2012-12-18 07:24:03 UTC
(In reply to comment #5)
> sorry, let me clarify.  relying on SANDBOX_LOG is currently correct as
> sandbox only writes access violations to it.  in my head, i had other things
> planned.  but rather than break that (since it's working), i'll just
> relocate the non-fatal messages.

Great. Given that, I guess portage can easily take the log and convert it to an eerror message.

> the desirable behavior is for the short sandbox messages to show up at the
> same time they get executed (so they get interleaved in the general log file
> at the right place), so writing to just a log file won't work.
> 
> i guess what's the driving desire here ?

The main issue is that we want to be able to show a useful eerror message without having to display the raw build log. This is important when the bulk of the build output needs to be hidden due to --quiet or --jobs.

> you want sandbox to trigger
> portage to kill the build immediately when a violation occurs rather than
> waiting until the end of a specific phase ?

Not necessarily. It's fine if we wait until the phase reaches completion. At that point checking the size of the sandbox log has been a reliable way to see if a sandbox violation has occurred. The reason that we need to detect that at the end of __ebuild_main in bin/phase-functions.sh is so that we do not report a "false success" via ipc. We return success/failure via ipc since experience has show that the bash/sandbox return code is not always reliable. By returning it via ipc, we can confirm that bash has executed our script all the way to the expected return point.
Comment 7 Zac Medico gentoo-dev 2012-12-18 07:41:26 UTC
(In reply to comment #6)
> We return success/failure via ipc
> since experience has show that the bash/sandbox return code is not always
> reliable. By returning it via ipc, we can confirm that bash has executed our
> script all the way to the expected return point.

Additionally, the ipc callback triggers a timeout which so that emerge will stop waiting when there are orphan subprocesses (as in bug 278895).
Comment 8 SpanKY gentoo-dev 2012-12-25 04:33:46 UTC
(In reply to comment #6)

ferringb said the log format has been standard for quite some time.  i haven't mucked with it since i started maintaining things, so i can document it.  although the current log file is self documenting.  let's stick with that.  so i think that puts the ball back in portage's court (other than me documenting things on the sandbox side as a stable API).

VERSION 1.0
FORMAT: F - Function called
FORMAT: S - Access Status
FORMAT: P - Path as passed to function
FORMAT: A - Absolute Path (not canonical)
FORMAT: R - Canonical Path
FORMAT: C - Command Line

F: open_wr
S: deny
P: /
A: /
R: /
C: touch /

F: utimensat
S: deny
P: /
A: /
R: /
C: touch /
Comment 9 SpanKY gentoo-dev 2013-02-25 04:25:18 UTC
i've finished gutting the messaging system.  when the sandbox process starts up, it'll stick the path to the process's /dev/stderr in $SANDBOX_MESSAGE_PATH.  then all children will look up that path and write to that rather than stdout or stderr or tty.

http://git.overlays.gentoo.org/gitweb/?p=proj/sandbox.git;a=commitdiff;h=e12fee192ac8b0343a468e5a8f7811a7b029ff9a
Comment 10 Brian Harring 2024-01-27 06:39:56 UTC
Anything left here?  It's unclear exactly what this ticket is asking for.