Revamp how attaching works (#652)

## Overview
The `CommandKind` type has confusing, non-orthogonal behavior.

It combines two ideas:

- Do we need a `HubrisArchive`, and if so, does it need to be fully loaded
  ("cooked") or do we just need the raw ZIP data?
- Do we need to attach to a system (called a "core")?
  - If so, what kind of attachments are valid?
    - Live system (debugger)
    - Live system (network)
    - Dump (RAM)
    - Archive (flash only)
  - How should we validate any attached system?

These two big ideas ("do we need an archive" and "do we need a core") are
combined in unintuitive ways.  For example, `CommandKind::Unattached { archive:
Archive::Required }` tries to attach to both a dump and an archive, but does not
fail if the archive is absent (see
#564 (comment)).

It's also unfortunate that it's defined at the `Command` level, because commands
may not have consistent behavior across all of their subcommands.  For example,
`humility hiffy -l` just lists Hiffy APIs from an archive, and does not require
attaching to a system.  Indeed, if you run `humility hiffy -l` with an archive
**and** you have a probe attached to an SP running a different image, it will
fail:

```console
$ humility --archive=build-cosmo-b-dev-image-default.zip hiffy -l
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.15s
     Running `target/debug/humility hiffy -l`
humility: attached via ST-Link V3
humility hiffy failed: image ID in archive ([bd, 1b, fb, 72, 35, 4b, a7, 48]) does not equal ID at 0x8004f50 ([7c, 56, 0, 8, 38, 54, 0, 8])
# this is an attached Grapefruit, why is it interfering with hiffy -l?
```

Some commands work around this by doing an extra parse of the arguments in their
`init` function, then return a different `Command` depending on arguments:
https://github.com/oxidecomputer/humility/blob/27d1f1549e58a54538657ab5359be9a5ae856de1/cmd/monorail/src/lib.rs#L1186-L1221

This works, but is annoying to have to do!

I have one more complaint, which is improved but not entirely fixed in this PR:
the `HubrisArchive` is default-constructed, then _actually_ populated based on
`archive` or `dump` arguments; if neither is present, then archive is not valid!
Commands have to work around this (with varying degrees of incredulity), e.g.

https://github.com/oxidecomputer/humility/blob/27d1f1549e58a54538657ab5359be9a5ae856de1/cmd/reset/src/lib.rs#L33-L37

## So, what's to be done?

This PR removes `CommandKind` entirely.  It is replaced with a set of functions
on `Cli` to build archives and cores, which can be called by commands when they
are needed (instead of before the command is invoked).

### Archive handling
- `archive` returns the archive (if available); it **does not** ever return a
  default-constructed archive
- `try_archive` returns an optional archive
- `raw_archive` returns the bytes of the ZIP file

All of these return a full `HubrisArchive`; it is the caller's responsibility to
store it somewhere and hand out references.

### Attachment
- `attach_live` attaches to a live system (debugger or `net`-based)
- `attach_live_or_dump` attaches to a live system or dump
- `attach_archive` attaches to an archive

There are also additional small wrapper functions with suffixes for validation,
e.g. `attach_live_booted` checks that the live system has booted.  Each of these
functions returns a `Box<dyn Core>` directly, instead of stashing it in the
`ExecutionContext`.

## Changes to commands
Each command is updated to construct its archive and/or core on-demand.  It's
mostly mechanical porting, e.g. 
```rust
CommandKind::Attached {
    archive: Archive::Required,
    attach: Attach::LiveOnly,
    validate: Validate::Booted,
}
```
becomes
```rust
    let hubris = &context.cli.archive()?;
    let core = &mut *context.cli.attach_live_booted(hubris)?;
```

...but there are a few weird edge cases – for example, `humility probe` now uses
`attach_probe` directly.

Author: mkeeter

Cargo.lock

@@ -113,8 +113,8 @@ humility = { path = "./humility-core", package = "humility-core" }
 humility-arch-arm = { path = "./humility-arch-arm" }
 humility-auxflash = { path = "./humility-auxflash" }
 humility-cortex = { path = "./humility-arch-cortex" }
-humility-cmd = { path = "./humility-cmd", default-features = false }
-humility-cli = { path = "./humility-cli" }
+humility-cmd = { path = "./humility-cmd" }
+humility-cli = { path = "./humility-cli", default-features = false }
 humility-dump-agent = { path = "./humility-dump-agent" }
 humility-doppel = { path = "./humility-doppel" }
 humility-hiffy = { path = "./humility-hiffy" }

Cargo.toml

@@ -38,8 +38,7 @@ So you've decided to add a subcommand to Humility. Here's what you need to do.
 
 2. Review the settings in the `init` routine you cribbed from to make sure they
    reflect your subcommand. In particular, the name must appear a second time in
-   the `name:` field, and `kind` should reflect whether you need an attached
-   target and archive, or not.
+   the `name:` field.
 
 3. Cite your new subcommand from the root `Cargo.toml`. This needs to happen in
    three places using two different names: in the `workspace.members` element

DEVELOPING.mkdn

@@ -13,10 +13,9 @@ use anyhow::Result;
 use clap::{CommandFactory, Parser};
 use colored::Colorize;
 use humility_cli::ExecutionContext;
-use humility_cmd::CommandKind;
 
 use humility_auxflash::AuxFlashHandler;
-use humility_cmd::{Archive, Attach, Command, Validate};
+use humility_cmd::Command;
 
 #[derive(Parser, Debug)]
 #[clap(name = "auxflash", about = env!("CARGO_PKG_DESCRIPTION"))]
@@ -99,9 +98,9 @@ fn auxflash_status(mut worker: AuxFlashHandler, verbose: bool) -> Result<()> {
 }
 
 fn auxflash(context: &mut ExecutionContext) -> Result<()> {
-    let core = &mut **context.core.as_mut().unwrap();
     let subargs = AuxFlashArgs::try_parse_from(&context.cli.cmd)?;
-    let hubris = context.archive.as_ref().unwrap();
+    let hubris = &context.cli.archive()?;
+    let core = &mut *context.cli.attach_live_booted(hubris)?;
     let mut worker = AuxFlashHandler::new(hubris, core, subargs.timeout)?;
 
     match subargs.cmd {
@@ -132,14 +131,5 @@ fn auxflash(context: &mut ExecutionContext) -> Result<()> {
 }
 
 pub fn init() -> Command {
-    Command {
-        app: AuxFlashArgs::command(),
-        name: "auxflash",
-        run: auxflash,
-        kind: CommandKind::Attached {
-            archive: Archive::Required,
-            attach: Attach::LiveOnly,
-            validate: Validate::Booted,
-        },
-    }
+    Command { app: AuxFlashArgs::command(), name: "auxflash", run: auxflash }
 }

cmd/auxflash/src/lib.rs

@@ -10,7 +10,7 @@ use std::path::PathBuf;
 
 use clap::{CommandFactory, Parser};
 
-use humility_cmd::{Archive, Attach, Command, CommandKind, Validate};
+use humility_cmd::Command;
 
 #[cfg(not(windows))]
 mod posix;
@@ -113,10 +113,5 @@ pub fn init() -> Command {
         app: UartConsoleArgs::command(),
         name: "console-proxy",
         run: console_proxy,
-        kind: CommandKind::Attached {
-            archive: Archive::Required,
-            attach: Attach::LiveOnly,
-            validate: Validate::Booted,
-        },
     }
 }

cmd/console-proxy/src/lib.rs

@@ -314,9 +314,9 @@ impl UnrawTermiosGuard {
 }
 
 pub(super) fn console_proxy(context: &mut ExecutionContext) -> Result<()> {
-    let core = &mut **context.core.as_mut().unwrap();
     let subargs = UartConsoleArgs::try_parse_from(&context.cli.cmd)?;
-    let hubris = context.archive.as_ref().unwrap();
+    let hubris = &context.cli.archive()?;
+    let core = &mut *context.cli.attach_live_booted(hubris)?;
     let mut worker = UartConsoleHandler::new(
         hubris,
         core,

cmd/console-proxy/src/posix.rs

@@ -206,7 +206,7 @@ use humility::core::Core;
 use humility::hubris::*;
 use humility::reflect::{self, Load, Value};
 use humility_cli::ExecutionContext;
-use humility_cmd::{Archive, Attach, Command, CommandKind, Validate};
+use humility_cmd::Command;
 use humility_doppel::{CountedRingbuf, CounterVariant, Counters};
 use indexmap::IndexMap;
 use std::collections::BTreeMap;
@@ -324,12 +324,11 @@ const LIST_HINT: &str = "use `humility counters list` to list all \
     available counters";
 
 fn counters(context: &mut ExecutionContext) -> Result<()> {
-    let core = &mut **context.core.as_mut().unwrap();
-    let hubris = context.archive.as_ref().unwrap();
-
+    let hubris = &context.cli.archive()?;
     let subargs = CountersArgs::try_parse_from(&context.cli.cmd)?;
 
     if let Some(Subcmd::Ipc(ipc)) = subargs.command {
+        let core = &mut *context.cli.attach_live_or_dump_match(hubris)?;
         return ipc.ipc_counter_dump(hubris, core);
     }
     let name = subargs.name();
@@ -428,6 +427,7 @@ fn counters(context: &mut ExecutionContext) -> Result<()> {
     }
 
     let mut json: IndexMap<&str, IndexMap<_, _>> = IndexMap::new();
+    let core = &mut *context.cli.attach_live_or_dump_match(hubris)?;
     for (t, ctrs) in counters {
         // Try not to use `?` here, because it causes one bad counter to make
         // them all unavailable. Instead, construct an iterator of
@@ -623,14 +623,5 @@ fn hint() -> impl std::fmt::Display {
 }
 
 pub fn init() -> Command {
-    Command {
-        app: CountersArgs::command(),
-        name: "counters",
-        run: counters,
-        kind: CommandKind::Attached {
-            archive: Archive::Required,
-            attach: Attach::Any,
-            validate: Validate::Match,
-        },
-    }
+    Command { app: CountersArgs::command(), name: "counters", run: counters }
 }

cmd/counters/src/lib.rs

@@ -29,7 +29,7 @@ use hif::*;
 use humility::core::Core;
 use humility::hubris::*;
 use humility_cli::ExecutionContext;
-use humility_cmd::{Archive, Attach, Command, CommandKind, Validate};
+use humility_cmd::Command;
 use humility_hiffy::*;
 use humility_idol::{self as idol, HubrisIdol};
 use ratatui::{
@@ -736,11 +736,10 @@ where
 }
 
 fn dashboard(context: &mut ExecutionContext) -> Result<()> {
-    let hubris = context.archive.as_ref().unwrap();
-
-    let core = &mut **context.core.as_mut().unwrap();
-
     let subargs = DashboardArgs::try_parse_from(&context.cli.cmd)?;
+    let hubris = &context.cli.archive()?;
+    let core = &mut *context.cli.attach_live_booted(hubris)?;
+
     let dashboard = Dashboard::new(hubris, core, &subargs)?;
 
     // setup terminal
@@ -767,16 +766,7 @@ fn dashboard(context: &mut ExecutionContext) -> Result<()> {
 }
 
 pub fn init() -> Command {
-    Command {
-        app: DashboardArgs::command(),
-        name: "dashboard",
-        run: dashboard,
-        kind: CommandKind::Attached {
-            archive: Archive::Required,
-            attach: Attach::LiveOnly,
-            validate: Validate::Booted,
-        },
-    }
+    Command { app: DashboardArgs::command(), name: "dashboard", run: dashboard }
 }
 
 fn sensor_ops(

cmd/dashboard/src/lib.rs

@@ -32,7 +32,7 @@ use anyhow::{Context, Result, bail};
 use byteorder::{ByteOrder, LittleEndian, WriteBytesExt};
 use clap::{CommandFactory, Parser};
 use humility_cli::ExecutionContext;
-use humility_cmd::{Archive, Command, CommandKind};
+use humility_cmd::Command;
 use probe_rs::{
     DebugProbeError, DebugProbeSelector, Probe,
     architecture::arm::{ApAddress, ArmProbeInterface, DapError, DpAddress},
@@ -464,6 +464,5 @@ pub fn init() -> Command {
         app: DebugMailboxArgs::command(),
         name: "debugmailbox",
         run: debugmailboxcmd,
-        kind: CommandKind::Unattached { archive: Archive::Ignored },
     }
 }

cmd/debugmailbox/src/lib.rs

@@ -19,24 +19,14 @@ use clap::{CommandFactory, Parser};
 use humility::core::Core;
 use humility::hubris::*;
 use humility_cli::ExecutionContext;
-use humility_cmd::CommandKind;
-use humility_cmd::{Archive, Attach, Command, Validate};
+use humility_cmd::Command;
 use humility_doppel::{GenOrRestartCount, Task, TaskDesc, TaskState};
 use std::num::NonZeroU32;
 use std::time::Duration;
 
 /// Command registration.
 pub fn init() -> Command {
-    Command {
-        app: DiagnoseArgs::command(),
-        name: "diagnose",
-        run: diagnose,
-        kind: CommandKind::Attached {
-            archive: Archive::Required,
-            attach: Attach::Any,
-            validate: Validate::Booted,
-        },
-    }
+    Command { app: DiagnoseArgs::command(), name: "diagnose", run: diagnose }
 }
 
 #[derive(Parser, Debug)]
@@ -83,10 +73,9 @@ fn section(title: &str) {
 }
 
 fn diagnose(context: &mut ExecutionContext) -> Result<()> {
-    let core = &mut **context.core.as_mut().unwrap();
-    let hubris = context.archive.as_ref().unwrap();
-
     let subargs = DiagnoseArgs::try_parse_from(&context.cli.cmd)?;
+    let hubris = &context.cli.archive()?;
+    let core = &mut *context.cli.attach_live_or_dump_booted(hubris)?;
 
     section("Initial Inspection");