From f4d7ada6082f4db829b626107b2df703fc64f968 Mon Sep 17 00:00:00 2001 From: Matt Clarkson Date: Wed, 30 Oct 2024 19:26:35 +0000 Subject: [PATCH 1/7] chore: run `black` --- labgrid/run/run.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/labgrid/run/run.py b/labgrid/run/run.py index 9027dce0..6c4e837a 100644 --- a/labgrid/run/run.py +++ b/labgrid/run/run.py @@ -14,8 +14,7 @@ from labgrid import Environment, Target class Shell(Protocol): - def run(cmd: str) -> Tuple[Collection[str], Collection[str], int]: - ... + def run(cmd: str) -> Tuple[Collection[str], Collection[str], int]: ... def arguments(prsr: ArgumentParser) -> None: -- GitLab From 6d65c99d3beb9f3eb77f000e5e7c9dace5d418d3 Mon Sep 17 00:00:00 2001 From: Matt Clarkson Date: Wed, 30 Oct 2024 19:29:31 +0000 Subject: [PATCH 2/7] refactor: move `bazel.labgrid.{protocol.,}util` --- bazel/labgrid/{protocol => }/util/BUILD.bazel | 0 bazel/labgrid/{protocol => }/util/target.py | 0 labgrid/run/BUILD.bazel | 2 +- labgrid/run/run.py | 2 +- 4 files changed, 2 insertions(+), 2 deletions(-) rename bazel/labgrid/{protocol => }/util/BUILD.bazel (100%) rename bazel/labgrid/{protocol => }/util/target.py (100%) diff --git a/bazel/labgrid/protocol/util/BUILD.bazel b/bazel/labgrid/util/BUILD.bazel similarity index 100% rename from bazel/labgrid/protocol/util/BUILD.bazel rename to bazel/labgrid/util/BUILD.bazel diff --git a/bazel/labgrid/protocol/util/target.py b/bazel/labgrid/util/target.py similarity index 100% rename from bazel/labgrid/protocol/util/target.py rename to bazel/labgrid/util/target.py diff --git a/labgrid/run/BUILD.bazel b/labgrid/run/BUILD.bazel index 80f170ab..0a2da3e6 100644 --- a/labgrid/run/BUILD.bazel +++ b/labgrid/run/BUILD.bazel @@ -27,7 +27,7 @@ py_binary( tags = ["manual"], visibility = ["//visibility:public"], deps = [ - "//bazel/labgrid/protocol/util:target", + "//bazel/labgrid/util:target", "//labgrid:pkg", "//labgrid/config:deps", "@rules_python//python/runfiles", diff --git a/labgrid/run/run.py b/labgrid/run/run.py index 6c4e837a..9d62bff7 100644 --- a/labgrid/run/run.py +++ b/labgrid/run/run.py @@ -7,9 +7,9 @@ from subprocess import PIPE, run from sys import argv, stderr, stdout from typing import Collection, Protocol, Tuple +from bazel.labgrid.target import TemporaryDirectory from python.runfiles import Runfiles -from bazel.labgrid.protocol.util.target import TemporaryDirectory from labgrid import Environment, Target -- GitLab From 7a6f983c056c6caa4d95fa8716ef5b746ab9fe58 Mon Sep 17 00:00:00 2001 From: Matt Clarkson Date: Wed, 30 Oct 2024 19:32:56 +0000 Subject: [PATCH 3/7] refactor(TemporaryDirectory): import `uutil.getnode` directly --- bazel/labgrid/util/target.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/bazel/labgrid/util/target.py b/bazel/labgrid/util/target.py index 1ec7b420..25873592 100644 --- a/bazel/labgrid/util/target.py +++ b/bazel/labgrid/util/target.py @@ -1,4 +1,4 @@ -import uuid +from uuid import getnode from contextlib import contextmanager from pathlib import Path, PurePath from shlex import join @@ -36,9 +36,7 @@ def TemporaryDirectory( mkdir: Path, ) -> Iterator[PurePath]: # Transfer mktemp - mktemp_remote = ( - f"/tmp/{uuid.getnode()}" - ) # avoid race-condition if there are parallel executions + mktemp_remote = f"/tmp/{getnode()}" transfer.put(mktemp, mktemp_remote) target = Subprocess(shell) result = target.run(f"{mktemp_remote} -d", check=True, text=True) -- GitLab From 26085d325dd7483fd75567c02d593fce9d50e3ff Mon Sep 17 00:00:00 2001 From: Matt Clarkson Date: Wed, 30 Oct 2024 19:33:38 +0000 Subject: [PATCH 4/7] fix(TemporaryDirectory): remove root on exceptions --- bazel/labgrid/util/target.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bazel/labgrid/util/target.py b/bazel/labgrid/util/target.py index 25873592..64e864ac 100644 --- a/bazel/labgrid/util/target.py +++ b/bazel/labgrid/util/target.py @@ -60,6 +60,7 @@ def TemporaryDirectory( # Delete mktemp target.run(f"{tools / 'rm'} {mktemp_remote}", check=True, text=True) - yield execroot - - target.run(f"{tools / 'rm'} -rf {root}", check=True, text=True) + try: + yield execroot + finally: + target.run(f"{tools / 'rm'} -rf {root}", check=True, text=True) -- GitLab From be249f08df8e29c2881cdf897e1c3a9458cb61f3 Mon Sep 17 00:00:00 2001 From: Matt Clarkson Date: Wed, 30 Oct 2024 19:49:45 +0000 Subject: [PATCH 5/7] fix(TemporaryDirectory): correctly stringify paths and use tuples for commands Simplifies the remote setup by removing an unnecessary `tools` directory. --- bazel/labgrid/util/target.py | 54 +++++++++++++++++++----------------- labgrid/run/run.py | 2 +- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/bazel/labgrid/util/target.py b/bazel/labgrid/util/target.py index 64e864ac..d65641d2 100644 --- a/bazel/labgrid/util/target.py +++ b/bazel/labgrid/util/target.py @@ -1,14 +1,19 @@ -from uuid import getnode from contextlib import contextmanager from pathlib import Path, PurePath from shlex import join from subprocess import CalledProcessError, CompletedProcess -from typing import Iterator, Mapping +from threading import get_native_id +from typing import Iterable, Iterator +from uuid import getnode from labgrid.protocol.commandprotocol import CommandProtocol from labgrid.protocol.filetransferprotocol import FileTransferProtocol +class TargetCalledProcessError(CalledProcessError): + pass + + class Subprocess: def __init__(self, shell: CommandProtocol): self.__shell = shell @@ -18,12 +23,16 @@ class Subprocess: return self.__shell def run( - self, args: tuple[str, ...], *, check: bool = False, text: bool = False + self, args: Iterable[str], *, check: bool = False, text: bool = False ) -> CompletedProcess: assert text, "Non-text target execution is not supported" - out, err, code = self.shell.run(args) + assert not isinstance(args, str), "`run` accepts iterables but not strings" + joined = join(args) + out, err, code = self.shell.run(joined) if check and code: - raise CalledProcessError(cmd=args, returncode=code, output=out, stderr=err) + raise TargetCalledProcessError( + cmd=joined, returncode=code, output=out, stderr=err + ) return CompletedProcess(returncode=code, stdout=out, stderr=err, args=args) @@ -35,32 +44,27 @@ def TemporaryDirectory( rm: Path, mkdir: Path, ) -> Iterator[PurePath]: - # Transfer mktemp - mktemp_remote = f"/tmp/{getnode()}" - transfer.put(mktemp, mktemp_remote) + # Get a unique temporary directory, avoiding concurrent `TemporaryDirectory` managers + unique = f".{getnode()}.{get_native_id()}" + transfer.put(f"{mktemp}", unique) target = Subprocess(shell) - result = target.run(f"{mktemp_remote} -d", check=True, text=True) + result = target.run((f"./{unique}", "-d"), check=True, text=True) root = PurePath(result.stdout[0].rstrip()) - tools = root / "tools" - execroot = root / "execroot" - # Transfer mkdir - transfer.put(mkdir, f"{root}") - # Create tools dir - target.run(f"{root / 'mkdir'} {tools}", check=True, text=True) - # Create execroot dir - target.run(f"{root / 'mkdir'} {execroot}", check=True, text=True) - # Transfer rm - transfer.put(rm, f"{tools}") + # Transfer some basic tools + transfer.put(f"{mkdir}", f"{root / 'mkdir'}") + transfer.put(f"{rm}", f"{root / 'rm'}") - ##cleanup + # Create the execution root + execroot = root / "execroot" + target.run((f"{root / 'mkdir'}", f"{execroot}"), check=True, text=True) - # Delete mkdir - target.run(f"{tools / 'rm'} {root / 'mkdir'}", check=True, text=True) - # Delete mktemp - target.run(f"{tools / 'rm'} {mktemp_remote}", check=True, text=True) + # Remove unneeded tools + target.run((f"{root / 'rm'}", f"{root / 'mkdir'}"), check=True, text=True) + target.run((f"{root / 'rm'}", f"{unique}"), check=True, text=True) + # Clean up the root directory when complete try: yield execroot finally: - target.run(f"{tools / 'rm'} -rf {root}", check=True, text=True) + target.run((f"{root / 'rm'}", "-rf", f"{root}"), check=True, text=True) diff --git a/labgrid/run/run.py b/labgrid/run/run.py index 9d62bff7..f0ee21bb 100644 --- a/labgrid/run/run.py +++ b/labgrid/run/run.py @@ -7,7 +7,7 @@ from subprocess import PIPE, run from sys import argv, stderr, stdout from typing import Collection, Protocol, Tuple -from bazel.labgrid.target import TemporaryDirectory +from bazel.labgrid.util.target import TemporaryDirectory from python.runfiles import Runfiles from labgrid import Environment, Target -- GitLab From 6b7a9467b37eb18defc9162bb1ee05a2f3679047 Mon Sep 17 00:00:00 2001 From: Matt Clarkson Date: Wed, 30 Oct 2024 21:16:54 +0000 Subject: [PATCH 6/7] refactor(run): refactor into a `run` function --- labgrid/run/run.py | 133 +++++++++++++++++++++++++++++++++------------ 1 file changed, 99 insertions(+), 34 deletions(-) diff --git a/labgrid/run/run.py b/labgrid/run/run.py index f0ee21bb..c21ce9f8 100644 --- a/labgrid/run/run.py +++ b/labgrid/run/run.py @@ -1,20 +1,30 @@ #! /usr/bin/env python3 +from __future__ import annotations + from argparse import ArgumentParser +from dataclasses import dataclass from os import environ, linesep from pathlib import Path, PurePath from shlex import join -from subprocess import PIPE, run +from subprocess import CalledProcessError from sys import argv, stderr, stdout -from typing import Collection, Protocol, Tuple -from bazel.labgrid.util.target import TemporaryDirectory from python.runfiles import Runfiles -from labgrid import Environment, Target +from bazel.labgrid.util.target import TemporaryDirectory +from labgrid import Environment -class Shell(Protocol): - def run(cmd: str) -> Tuple[Collection[str], Collection[str], int]: ... +class RunfileNotFoundError(FileNotFoundError): + pass + + +def runfile(path: str) -> Path: + runfiles = Runfiles.Create() + resolved = Path(runfiles.Rlocation(path)) + if not resolved.exists(): + raise RunfileNotFoundError(f"runfile not found: {path}") + return resolved def arguments(prsr: ArgumentParser) -> None: @@ -40,58 +50,113 @@ def arguments(prsr: ArgumentParser) -> None: help="The state to transition the LabGrid strategy to at the end", default=environ["BZL_LG_FINAL_STATE"], ) + prsr.add_argument( + "--mktemp", + help="A `mktemp` binary for the target.", + type=runfile, + default=runfile("ape/ape/assimilate/mktemp.ape/mktemp"), + ) + prsr.add_argument( + "--rm", + help="A `rm` binary for the target.", + type=runfile, + default=runfile("ape/ape/assimilate/rm.ape/rm"), + ) + prsr.add_argument( + "--mkdir", + help="A `mkdir` binary for the target.", + type=runfile, + default=runfile("ape/ape/assimilate/mkdir.ape/mkdir"), + ) -def main(exe: Path, *args: str) -> int: - # Allow subprocesses to have their own runfiles - runfiles = Runfiles.Create() - - del environ["RUNFILES_DIR"] +@dataclass(frozen=True) +class State: + desired: str + final: str | None = None - prsr = ArgumentParser( - prog=str(exe), description="Runs a command over SSH to a LabGrid Docker device." - ) - arguments(prsr) - - parsed = prsr.parse_args(args) +# TODO: move this out to `bazel.labgrid.run` on day to allow downstream to re-use +def run( + program: Path, + *arguments: str, + config: Path, + state: State, + mktemp: Path, + mkdir: Path, + rm: Path, +) -> int: + # Allow any Bazel binaries used in the configuration to work + try: + del environ["RUNFILES_DIR"] + except KeyError: + pass # Start up Docker container with LabGrid - env = Environment(str(parsed.config)) + env = Environment(str(config)) target = env.get_target() strategy = target.get_driver("Strategy") try: - strategy.transition(parsed.state) + strategy.transition(state.desired) # Retrieve the communication protocols shell = target.get_driver("CommandProtocol") transfer = target.get_driver("FileTransferProtocol") - # utils - mktemp = runfiles.Rlocation("ape/ape/assimilate/mktemp.ape/mktemp") - mkdir = runfiles.Rlocation("ape/ape/assimilate/mkdir.ape/mkdir") - rm = runfiles.Rlocation("ape/ape/assimilate/rm.ape/rm") - with TemporaryDirectory(shell, transfer, mktemp, rm, mkdir) as temp_dir: - # Transfer the provided program over to the Docker image - program = "{}/{}".format(temp_dir, parsed.program.name) - transfer.put(parsed.program, program) + with TemporaryDirectory(shell, transfer, mktemp, rm, mkdir) as temp: + # Transfer the provided program over to the device + remote = temp / program.name + transfer.put(f"{program}", f"{remote}") # Transfer runfiles - src_runfiles = parsed.program.with_suffix(".runfiles") - dest_runfiles = Path(program).with_suffix(".runfiles") + # TODO: implement a `BazelFileTransfer` that transfers runfiles implicitly in `put` + # Make that the default file transfer implementation so _any_ file transfer automatically gets the runfiles as well + src_runfiles = program.with_suffix(".runfiles") + dest_runfiles = remote.with_suffix(".runfiles") if src_runfiles.exists(): - transfer.put(src_runfiles, dest_runfiles) + transfer.put(f"{src_runfiles}", f"{dest_runfiles}") # Run the transferred program - out, err, code = shell.run(join((program, *parsed.arguments))) + out, err, code = shell.run(join((f"{remote}", *arguments))) for line in out: stdout.write(f"{line}{linesep}") for line in err: stderr.write(f"{line}{linesep}") - + return code finally: - strategy.transition(parsed.final_state) + strategy.transition(state.final) + + +def main(exe: Path, *args: str) -> int: + prsr = ArgumentParser( + prog=str(exe), description="Runs a command on a LabGrid device." + ) + + arguments(prsr) - return code + parsed = prsr.parse_args(args) + + # TODO: implement an argument parser action to do this + state = State(final=parsed.final_state, desired=parsed.state) + + try: + return run( + parsed.program, + *parsed.arguments, + state=state, + config=parsed.config, + mktemp=parsed.mktemp, + rm=parsed.rm, + mkdir=parsed.mkdir, + ) + except CalledProcessError as e: + print(f"fatal: subprocess failed: {e}", file=stderr) + if e.stdout is not None: + print(e.stdout, file=stderr) + if e.stderr is not None: + print(e.stderr, file=stderr) + return e.returncode + except KeyboardInterrupt: + return 130 def entry(): -- GitLab From a159d21c877d791558bad77967d088c7a0cd7294 Mon Sep 17 00:00:00 2001 From: Matt Clarkson Date: Wed, 30 Oct 2024 21:36:18 +0000 Subject: [PATCH 7/7] fix(Subprocess): join `std{out,err}` lines --- bazel/labgrid/util/target.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/bazel/labgrid/util/target.py b/bazel/labgrid/util/target.py index d65641d2..5ef2a148 100644 --- a/bazel/labgrid/util/target.py +++ b/bazel/labgrid/util/target.py @@ -1,4 +1,5 @@ from contextlib import contextmanager +from os import linesep from pathlib import Path, PurePath from shlex import join from subprocess import CalledProcessError, CompletedProcess @@ -29,11 +30,15 @@ class Subprocess: assert not isinstance(args, str), "`run` accepts iterables but not strings" joined = join(args) out, err, code = self.shell.run(joined) + stdout = linesep.join(out) + stderr = linesep.join(err) if check and code: raise TargetCalledProcessError( - cmd=joined, returncode=code, output=out, stderr=err + cmd=joined, returncode=code, output=stdout, stderr=stderr ) - return CompletedProcess(returncode=code, stdout=out, stderr=err, args=args) + return CompletedProcess( + returncode=code, stdout=stdout, stderr=stderr, args=args + ) @contextmanager @@ -49,7 +54,7 @@ def TemporaryDirectory( transfer.put(f"{mktemp}", unique) target = Subprocess(shell) result = target.run((f"./{unique}", "-d"), check=True, text=True) - root = PurePath(result.stdout[0].rstrip()) + root = PurePath(result.stdout.rstrip()) # Transfer some basic tools transfer.put(f"{mkdir}", f"{root / 'mkdir'}") -- GitLab