[Draft] Fix BackgroundCommand.send_signal() and add trace-cmd record support #689
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
douglas-raillard-arm
changed the title
|
Upon deeper inspection, the |
douglas-raillard-arm
force-pushed
the
fix_signals
branch
from
9696cdc
to
8d55c17
Compare
|
PR rebased |
Support sending any signal to background commands, instead of only
supporting properly SIGKILL/SIGTERM/SIGQUIT.
The main issue is figuring out what PID to send the signal to, as the
devlib API allows running a whole snippet of shell script that typically
is wrapped under many layers of sh -c and sudo calls. In order to lift
the ambiguity, the user has access to a "devlib-signal-target" command
that points devlib at what process should be the target of signals:
# Run a "setup" command, then the main command that will receive the
# signals
cmd = 'echo setup; devlib-signal-target echo hello world'
with target.background(cmd) as bg:
bg.communicate()
The devlib-signal-target script can only be used once per background
command, so that it is never ambiguous what process is targeted, and so
that the Python code can cache the target PID. Subsequent invocations
of devlib-signal-target will fail.
Allow using trace-cmd record that continuously dump the trace to disk, allowing to overcome the buffer size limitations when recording for extended periods of time.
douglas-raillard-arm
force-pushed
the
fix_signals
branch
from
8d55c17
to
8fb315f
Compare
|
@marcbonnici Updated with rework of temp folder management, so it works on old versions of android as well. I can split that in a separate PR if needed but left it on top for now to ease testing of the combination. |
douglas-raillard-arm
force-pushed
the
fix_signals
branch
from
8fb315f
to
852f2d6
Compare
metin-arm
approved these changes
Nov 20, 2024
* Make use of Target.make_temp() in Target._xfer_cache_path() to
deduplicate temp folder creation code
* Introduce Target.tmp_directory attribute for the root of temporary
files.
* Make Target.tempfile() use Target.tmp_directory
* Rework the Target._resolve_paths() implementations:
* Target.tmp_directory is set to a default returned by "mktemp -d".
This way, if "mktemp -d" works out of of the box, all devlib
temporary files will be located in the expected location for the
that operating system.
* Target._resolve_paths() must set Target.working_directory and that
is checked with an assert. Other directories have defaults based
on this if _resolve_paths() does not set them.
Also avoid a "None" prefix when no prefix is asked for, and set None as the default prefix value. Remove the "devlib-test" default value as make_temp() has nothing to do with tests.
Align the parameters between the 2 methods: * Use "None" as default value * Do not add suffix or prefix if not asked for. * Separate components with "-" instead of "_"
… keyword-only Prevent Target argument soup and allow future non-breaking evolutions of the Target API by restricting what parameters can be positional. Only "connection_settings", "platform" and "working_directory" can now be passed positionally, all the other ones must be passed as keyword arguments.
douglas-raillard-arm
force-pushed
the
fix_signals
branch
from
852f2d6
to
765d6b7
Compare
|
Updated with a fix to |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #645
Supersedes #643
TODO: