Split main.cpp into cli_args, commands, and self_test
main.cpp had grown to ~910 lines holding CLI parsing, every command
implementation, and the -t/--test handler all in one file. Split it:
- cli_args.{h,cpp}: getopt_long parsing and all post-loop validation
(parse_args()), producing a ParsedArgs the rest of the program consumes.
- commands.{h,cpp}: every command implementation plus dispatch_command(),
an exhaustive switch over Mode with no default -- so -Wswitch (this
project's warning_level=3) now catches a future Mode value added without
a matching dispatch case, instead of silently falling through. Confirmed
by temporarily adding an unhandled enumerator and observing the warning.
- self_test.{h,cpp}: -t/--test's own file, ahead of real tests landing here.
Also fixes Mode::mount, which previously had no explicit dispatch check at
all -- it ran only because it was whatever fell off the end of main()'s
if/else chain when nothing else matched. It's now mount_command(), a real
case in dispatch_command() like every other mode.
main.cpp itself shrinks to ~30 lines: load config, apply its log level,
parse_args(), dispatch_command().
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv3s5jckJKzh6JkMoi2Akz
This commit is contained in:
@@ -15,42 +15,83 @@ kernel actually supports. See `README.md` for the human-facing overview (build/u
|
||||
status); this file stays the dense, file-by-file reference. Still early-stage.
|
||||
|
||||
Source layout (all under `src/`):
|
||||
- `main.cpp` — CLI entry point, dependency checks, orchestration (`mount_image()`,
|
||||
`run_container()`, `cleanup_image()`, `unmount_image()`, `list_images_command()`,
|
||||
`inspect_image_command()`, `create_volume_command()`, `list_volumes_command()`,
|
||||
`delete_volume_command()`, `list_processes_command()`, `clean_processes_command()`).
|
||||
`list_processes_command()` implements `--list-processes` (long-option only):
|
||||
calls `list_sessions()` (`pid_file.{h,cpp}`, see below) and prints one
|
||||
tab-aligned `pid`, `container name`, `running`/`exited` row per entry (same
|
||||
two-column tab-alignment scheme as `list_images_command()`/
|
||||
- `main.cpp` — the CLI entry point only, and deliberately tiny (~30 lines):
|
||||
loads the config file, applies its `global.log-level` (`apply_log_level()`,
|
||||
`cli_args.h`) before CLI parsing so an explicit `--log-level` can still
|
||||
override it afterward, calls `parse_args()` (`cli_args.h`) and returns its
|
||||
exit code immediately if it gives one (covers `-h`/`-V` and every parse
|
||||
error), otherwise calls `dispatch_command()` (`commands.h`) and returns its
|
||||
result. All of the actual option-parsing and command logic that used to live
|
||||
here moved out into `cli_args.{h,cpp}`/`commands.{h,cpp}`/`self_test.{h,cpp}`
|
||||
(see below) specifically to keep this file from re-growing into a dumping
|
||||
ground as more commands (docker-compose support, etc.) get added.
|
||||
- `cli_args.{h,cpp}` — command-line parsing only, nothing else. `Mode` (the
|
||||
enum of every CLI action) and `ParsedArgs` (everything `parse_args()`
|
||||
extracts from `argv`) live in the header since `commands.h`'s
|
||||
`dispatch_command()` consumes them; `print_usage()`/`print_version()`, the
|
||||
`options` namespace of getopt long-option codes, and the `long_options`
|
||||
array itself are `.cpp`-local. `parse_args(argc, argv, out)` runs the
|
||||
`getopt_long` loop plus all of the post-loop validation that used to live at
|
||||
the top of `main()`: mode/`--volume` interaction (`-v` alone vs. combined
|
||||
with `-r`, see below), `--group` requires `--user`, no leftover positional
|
||||
args outside `-r`/`-e`, and (moved here from what used to be inline in the
|
||||
`Mode::exec` dispatch arm) `-e/--exec <pid>`'s own pid parsing/validation
|
||||
(`ParsedArgs::exec_pid`, a positive integer or a hard error) and its
|
||||
trailing-command requirement (`ParsedArgs::command`, required non-empty).
|
||||
Returns an exit code `main()` should return immediately (`0` for `-h`/`-V`,
|
||||
`1` for any parse error) when set; `nullopt` means `out` is ready for
|
||||
`dispatch_command()`. `-D/--daemonize` (has a short form; `'D'` was free) is
|
||||
a plain boolean flag (`ParsedArgs::daemonize_flag`, set in its own `case
|
||||
'D':`, same pattern as `-n/--no-nsenter`). `--hostname <name>`/`--env
|
||||
VAR=VALUE`/`--env-file <file>` (all long-option only, `--env`/`--env-file`
|
||||
both repeatable) are collected here into `ParsedArgs::hostname_flag`/
|
||||
`env_specs` — `--env` pushes `{false, optarg}`, `--env-file` pushes `{true,
|
||||
optarg}` into the *same* ordered `std::vector<EnvSpec>` (not two separate
|
||||
lists), preserving their exact relative command-line order across both
|
||||
flags, since `resolve_env_specs()` (`env_spec.{h,cpp}`, see below) needs
|
||||
that order to let a later one override an earlier one for the same variable
|
||||
name — actually resolving them happens later, in `commands.cpp`'s
|
||||
`run_container()`. `-v/--volume` is dual-purpose: used alone it's a
|
||||
standalone `Mode::volume` request; combined with `-r/--run` it's repeatable
|
||||
and requests a volume mount instead (resolved later by
|
||||
`resolve_volume_mount()`, `volume_mount.{h,cpp}`, see below). Since `-v`
|
||||
must be repeatable with `-r` but each occurrence still takes two
|
||||
space-separated tokens, the getopt loop doesn't let `'v'` set `Mode`
|
||||
directly: it accumulates `(spec, path)` pairs into `ParsedArgs::volume_specs`
|
||||
(consuming the second token manually, with a guard against swallowing the
|
||||
next flag if there isn't one), and only *after* the loop decides whether
|
||||
that means one standalone `Mode::volume` call or, together with `-r`,
|
||||
passes `volume_specs` through unresolved for `dispatch_command()`/
|
||||
`run_container()` to handle.
|
||||
- `commands.{h,cpp}` — every command's implementation, plus the dispatcher.
|
||||
`dispatch_command(args, config_path, config)` (the only externally-linked
|
||||
function; everything else in this file is `.cpp`-local) is a `switch
|
||||
(args.mode)` with **one explicit `case` per `Mode` enumerator and no
|
||||
`default:`**, so `-Wswitch` (this project builds at `warning_level=3`)
|
||||
forces a compile warning/error if a future `Mode` value is ever added
|
||||
without a matching dispatch case, instead of silently falling through to
|
||||
the wrong command — confirmed by testing (temporarily adding an unhandled
|
||||
enumerator triggered exactly the expected `-Wswitch` warning). `Mode::mount`
|
||||
has its own explicit case (`mount_command()`) for the same reason: it used
|
||||
to be handled only by *falling off the end* of a long `if`/`else` chain in
|
||||
`main()` with no explicit check at all — the very kind of implicit,
|
||||
easy-to-silently-break behavior this dispatcher redesign exists to close
|
||||
off, especially with more `Mode` values (docker-compose support) expected
|
||||
soon. `list_processes_command()` implements `--list-processes` (long-option
|
||||
only): calls `list_sessions()` (`pid_file.{h,cpp}`, see below) and prints
|
||||
one tab-aligned `pid`, `container name`, `running`/`exited` row per entry
|
||||
(same two-column tab-alignment scheme as `list_images_command()`/
|
||||
`list_volumes_command()`, extended to a third column), no header row, silent
|
||||
success on an empty list. `clean_processes_command()` implements
|
||||
`--clean-processes` (also long-option only): calls `clean_stale_sessions()`
|
||||
(`pid_file.{h,cpp}`) and prints one `removed stale pid file for '<name>' (pid
|
||||
<pid>)` line per file actually removed — nothing is printed for sessions still
|
||||
running, and an empty result (nothing stale) is silent success, same
|
||||
convention as the rest of this file's list/delete commands. `-e/--exec <pid>`
|
||||
(has a short form, unlike the rest of the process-tracking flags) dispatches
|
||||
straight to `exec_in_session()` (`exec_session.{h,cpp}`, see below): `pid`
|
||||
lands in `mode_arg` (parsed as a positive integer, erroring out otherwise) the
|
||||
same way `-r`'s image path does, and the trailing command
|
||||
(`argv[optind:]`, required — errors out if empty) is collected the same way
|
||||
`-r`'s own command is, sharing that mode's exemption from the "no leftover
|
||||
positional args" check.
|
||||
`-D/--daemonize` (has a short form; `'D'` was free) is a plain boolean flag
|
||||
(`daemonize_flag`, set in its own `case 'D':`, same pattern as
|
||||
`-n/--no-nsenter`) threaded through to `run_container()`, which computes
|
||||
`container_name` *before* `mount_image()` now (moved up from right before
|
||||
the `run_bwrap()` call — it only ever depended on `image_tar`, a parameter
|
||||
available from the start, so this is a pure reordering) and, if daemonizing,
|
||||
calls `daemonize(container_name)` (`daemonize.{h,cpp}`, see below)
|
||||
immediately after: a returned value means this is the original (parent)
|
||||
process (or a hard daemonize failure) — print it and `return` right away;
|
||||
`nullopt` means this is the now-detached child, which falls through into the
|
||||
rest of `run_container()`'s existing body completely unchanged, including
|
||||
the unmount/cleanup that already runs after `run_bwrap()` returns (no
|
||||
separate watcher/reaper — the daemonized child *is* what runs the whole
|
||||
session, start to finish).
|
||||
convention as the rest of this file's list/delete commands. `Mode::exec`'s
|
||||
dispatch case is a one-line call to `exec_in_session(*args.exec_pid,
|
||||
args.command)` (`exec_session.{h,cpp}`, see below) — the pid/command
|
||||
parsing and validation now happens in `cli_args.cpp`'s `parse_args()`
|
||||
instead (see above).
|
||||
`inspect_image_command()` implements `-i/--inspect
|
||||
<image.tar>`: prints every `OciImageConfig` field (user/group, exposed ports, env,
|
||||
volumes, default command) without mounting or running the image — extend it
|
||||
@@ -67,38 +108,45 @@ Source layout (all under `src/`):
|
||||
(also `std::filesystem::remove_all()`s the host directory — errors out before
|
||||
touching the config if that fails, warns instead of failing if the directory was
|
||||
already gone) — see `config_file.{h,cpp}` below for what a "volume" means here (a
|
||||
distinct concept from `OciImageConfig::volumes`). `-v/--volume` is dual-purpose:
|
||||
used alone it's `create_volume_command()`; combined with `-r/--run` it instead
|
||||
requests a volume mount (repeatable) and is resolved by `resolve_volume_mount()`
|
||||
(see `volume_mount.{h,cpp}` below) instead. Since `-v` must be repeatable with
|
||||
`-r` but each occurrence still takes two space-separated tokens, `main()`'s
|
||||
getopt loop no longer lets `'v'` set `Mode` itself: it accumulates
|
||||
`(spec, path)` pairs into `volume_specs` (consuming the second token manually,
|
||||
with a guard against swallowing the next flag if there isn't one), and only
|
||||
*after* the loop decides whether that means one standalone `Mode::kVolume` call
|
||||
or, together with `-r`, threads `volume_specs` through to `run_container()`.
|
||||
`run_container()` resolves each spec (erroring out, `ok = false`, same as a
|
||||
failed `--user` resolution — `bwrap` is skipped but unmount/cleanup still runs)
|
||||
into a `ResolvedVolumeMount`, rejecting a duplicate or non-absolute container
|
||||
path first, and passes the resolved list to `run_bwrap()`. `--hostname <name>`
|
||||
(long-option only, no short form) is likewise threaded straight through
|
||||
`run_container()` into `run_bwrap()`/`build_bwrap_args()` (`bwrap.{h,cpp}`) —
|
||||
see there for how/when it actually takes effect. `run_container()` also derives
|
||||
a `container_name` for the session-tracking pid file (see `pid_file.{h,cpp}`
|
||||
below): `read_image_ref()` (`oci_image.{h,cpp}`) applied to the single image
|
||||
tar being run, formatted as `name:tag`, falling back to the tar's own filename
|
||||
stem if `read_image_ref()` can't determine one — passed through to
|
||||
`run_bwrap()` alongside everything else. `--env VAR=VALUE`/`--env-file <file>`
|
||||
(both long-option only, both repeatable) accumulate into a single ordered
|
||||
`std::vector<EnvSpec>` — `--env` pushes `{false, optarg}`, `--env-file` pushes
|
||||
`{true, optarg}` — preserving their exact relative command-line order across
|
||||
*both* flags (not two separate lists), since `resolve_env_specs()`
|
||||
(`env_spec.{h,cpp}`, see below) needs that order to let a later one override
|
||||
an earlier one for the same variable name. `run_container()` calls it once
|
||||
(same `ok = false`-on-failure pattern as volume/user resolution) and passes
|
||||
the resolved list to `run_bwrap()` as `extra_env`.
|
||||
distinct concept from `OciImageConfig::volumes`). `dispatch_command()`'s
|
||||
`Mode::volume`/`Mode::delete_volume`/`Mode::delete_volume_full` cases call these.
|
||||
`run_container()` (the `Mode::run` dispatch case) resolves each `-v` spec
|
||||
(erroring out, `ok = false`, same as a failed `--user` resolution — `bwrap`
|
||||
is skipped but unmount/cleanup still runs) into a `ResolvedVolumeMount`,
|
||||
rejecting a duplicate or non-absolute container path first, and passes the
|
||||
resolved list to `run_bwrap()`. `--hostname <name>` is likewise threaded
|
||||
straight through `run_container()` into `run_bwrap()`/`build_bwrap_args()`
|
||||
(`bwrap.{h,cpp}`) — see there for how/when it actually takes effect.
|
||||
`run_container()` also derives a `container_name` for the session-tracking
|
||||
pid file (see `pid_file.{h,cpp}` below): `read_image_ref()`
|
||||
(`oci_image.{h,cpp}`) applied to the single image tar being run, formatted
|
||||
as `name:tag`, falling back to the tar's own filename stem if
|
||||
`read_image_ref()` can't determine one — passed through to `run_bwrap()`
|
||||
alongside everything else. `--env`/`--env-file`'s already-ordered
|
||||
`env_specs` (see `cli_args.{h,cpp}` above) are resolved here, once, via
|
||||
`resolve_env_specs()` (`env_spec.{h,cpp}`, see below) — same `ok =
|
||||
false`-on-failure pattern as volume/user resolution — and the resolved list
|
||||
is passed to `run_bwrap()` as `extra_env`. `-D/--daemonize`'s
|
||||
`daemonize_flag` is also consumed here: `run_container()` computes
|
||||
`container_name` *before* `mount_image()` (needed so `daemonize()` below can
|
||||
use the real container name for the log file from its very first line, not
|
||||
just after a later rename) and, if daemonizing, calls
|
||||
`daemonize(container_name)` (`daemonize.{h,cpp}`, see below) immediately
|
||||
after: a returned value means this is the original (parent) process (or a
|
||||
hard daemonize failure) — print it and `return` right away; `nullopt` means
|
||||
this is the now-detached child, which falls through into the rest of
|
||||
`run_container()`'s existing body completely unchanged, including the
|
||||
unmount/cleanup that already runs after `run_bwrap()` returns (no separate
|
||||
watcher/reaper — the daemonized child *is* what runs the whole session,
|
||||
start to finish).
|
||||
- `self_test.{h,cpp}` — `run_self_tests()` implements `-t/--test`, this
|
||||
project's own built-in self-test mode (distinct from the Meson-driven
|
||||
fixture smoke test under `tests/`, described in "Build & test commands"
|
||||
below). Currently just reports `detect_bwrap_unshare_args()`'s output
|
||||
(`bwrap.{h,cpp}`); deliberately its own small file since real tests are
|
||||
expected here soon.
|
||||
- `env_spec.{h,cpp}` — `resolve_env_specs()` turns an ordered list of
|
||||
`EnvSpec {is_file, value}` (see `main.cpp` above) into a flat, ordered list of
|
||||
`EnvSpec {is_file, value}` (see `cli_args.{h,cpp}` above) into a flat, ordered list of
|
||||
`(key, value)` pairs. A literal (`--env`) is split at its *first* `=` (the
|
||||
value may itself contain `=`; the key must be non-empty). A file (`--env-file`)
|
||||
is read line by line: blank/whitespace-only lines and lines whose first
|
||||
@@ -120,14 +168,14 @@ Source layout (all under `src/`):
|
||||
annotations (`io.containerd.image.name` preferred, else
|
||||
`org.opencontainers.image.ref.name`), falling back to the archive's filename and
|
||||
`"latest"` respectively. `read_image_ref()` is public (not just an internal
|
||||
helper of `list_oci_images()`) precisely so `run_container()` (`main.cpp`) can
|
||||
helper of `list_oci_images()`) precisely so `run_container()` (`commands.cpp`) can
|
||||
reuse the exact same logic to name a *single* image tar's session pid file (see
|
||||
`pid_file.{h,cpp}` below) instead of duplicating it.
|
||||
`read_oci_image_config()` reads the image config blob referenced by the manifest
|
||||
and extracts `User` (split on `:` into `OciImageConfig::user`/`group`),
|
||||
`ExposedPorts`, `Env`, `Volumes`, and the effective default command
|
||||
(`Entrypoint ++ Cmd`). `user`/`group` and the default command are consumed by
|
||||
`-r/--run`, and every field is displayed by `-i/--inspect` (see `main.cpp` above)
|
||||
`-r/--run`, and every field is displayed by `-i/--inspect` (see `commands.{h,cpp}` above)
|
||||
— `ExposedPorts`/`Env`/`Volumes` are otherwise still just captured for when
|
||||
networking/volumes are implemented.
|
||||
- `containers_storage.{h,cpp}` — wraps the `containers-storage` CLI (`import-layer`,
|
||||
@@ -235,7 +283,7 @@ Source layout (all under `src/`):
|
||||
`"/root"` for uid 0 or `"/"` otherwise when there's no matching row.
|
||||
`build_sandbox_env()` (`bwrap.cpp`) sets the sandboxed process's `HOME` from
|
||||
this — `"/root"` only when no user override applies at all (no `--user`, no
|
||||
image-declared `config.User`). `run_container()` (`main.cpp`) calls `resolve_user_and_group()`
|
||||
image-declared `config.User`). `run_container()` (`commands.cpp`) calls `resolve_user_and_group()`
|
||||
with either the explicit `--user`/`--group` flags, or, when `--user` wasn't given,
|
||||
the image's own declared `config.User` (`OciImageConfig::user`/`group`) — so a
|
||||
container defaults to running as whatever user the image itself declares, not
|
||||
@@ -288,7 +336,7 @@ Source layout (all under `src/`):
|
||||
failure path here (can't create the directory/file, can't lock, can't remove)
|
||||
is a `spdlog::warn`, never fatal — session tracking is best-effort and must
|
||||
never block or fail `-r/--run` itself. `list_sessions()` implements
|
||||
`--list-processes` (`main.cpp`'s `list_processes_command()`): scans the same
|
||||
`--list-processes` (`commands.cpp`'s `list_processes_command()`): scans the same
|
||||
`run/` directory and reports one `SessionInfo {pid, container_name, running}`
|
||||
per readable pid file. `pid` is read from the file's own contents, not parsed
|
||||
from the filename (ambiguous for names that themselves contain `-`);
|
||||
@@ -336,7 +384,7 @@ Source layout (all under `src/`):
|
||||
`--daemonize` was asked to capture would defeat the point of the flag. The
|
||||
child reports `"LOG <path>\n"` over the pipe immediately (so the parent can
|
||||
show a useful location even on failure) and returns `nullopt` to its caller
|
||||
(`run_container()`, `main.cpp`), which then falls through into the rest of
|
||||
(`run_container()`, `commands.cpp`), which then falls through into the rest of
|
||||
that function's existing body completely unchanged — **the daemonized child
|
||||
is what runs the whole rest of `run_container()`, including the unmount/
|
||||
cleanup that already existed after `run_bwrap()` returns; no separate
|
||||
@@ -410,7 +458,7 @@ Source layout (all under `src/`):
|
||||
overwrites it afterward — same precedence pattern already used for `SPDLOG_LEVEL`.
|
||||
`write_config_file()` writes the whole file back out (via libyaml's
|
||||
document-building/emitter API, symmetric to the read side) — used by
|
||||
`-v/--volume` (`create_volume_command()`, `main.cpp`) to persist a new
|
||||
`-v/--volume` (`create_volume_command()`, `commands.cpp`) to persist a new
|
||||
`VolumeEntry {name, directory}` into the `volumes` section, preserving `global`
|
||||
untouched. **`VolumeEntry`/the `volumes` section is a distinct concept from
|
||||
`OciImageConfig::volumes`**: this is a user-defined `name -> host directory`
|
||||
@@ -506,7 +554,7 @@ Build directory is `buildDir/` (already configured).
|
||||
the enum's own name (e.g. `Mode::run`, `OciPortProtocol::tcp`), so plain snake_case
|
||||
enumerators are enough on their own. Free-standing constants also use plain snake_case;
|
||||
when several are conceptually related, group them under a named `namespace` instead of
|
||||
relying on a shared prefix to imply the grouping (e.g. `main.cpp`'s `getopt_long` long-option
|
||||
relying on a shared prefix to imply the grouping (e.g. `cli_args.cpp`'s `getopt_long` long-option
|
||||
codes live in `namespace options { constexpr int log_level = ...; }`, and `bwrap.cpp`'s
|
||||
priv-drop-helper path/binary-name pair live in `namespace priv_drop { ... }`) — nest the named
|
||||
namespace inside the file's existing anonymous namespace where one is already present, so
|
||||
|
||||
Reference in New Issue
Block a user