PHD: allow patched Crucible dependencies (#778)

`phd-runner` uses the dependency on Crucible specified in the
workspace's Cargo.toml to determine which downstairs artifact to
download from Buildomat. This only works when the Crucible dependency is
a Git dep on the `oxidecomputer/crucible` GitHub repo. In some cases,
developers may patch Crucible to a local checkout or a different
repository, such as for testing changes that haven't been merged to
`oxidecomputer/crucible`. Unfortunately, the phd-runner build script
fails in this case, which makes running any PHD tests impossible.

This is unfortunate. PHD tests _can_ still be run when the Crucible
dependency is patched, either by disabling the Crucible-based tests, or
by manually providing a Crucible downstairs binary path. However,
because the build script fails in this case, it's impossible to build
`phd-runner`, which sucks. Instead, we should allow the build to succeed
and just disable the `--crucible-downstairs-commit auto` CLI option when
the dependency isn't pointed at the Crucible GitHub repo.

This commit does that. Now, we just log a warning and set the env var to
a sentinel value that means we couldn't determine the SHA for auto mode.
The `phd-runner` binary will then exit with an error if
`--crucible-downstairs-commit auto` is selected when compiled without a
known Git SHA.

To test this, I patched Crucible with a local checkout:

```toml
[patch."https://github.com/oxidecomputer/crucible"]
crucible = { path = "../crucible/upstairs" }
crucible-client-types = { path = "../crucible/crucible-client-types" }
```

Running `cargo build -p phd-runner` now succeeds:

```console
eliza@theseus ~/Code/oxide/propolis $ cargo build -p phd-runner
     Locking 4 packages to latest compatible versions
      Adding crucible v0.0.1 (/home/eliza/Code/oxide/crucible/upstairs)
      Adding crucible-client-types v0.1.0 (/home/eliza/Code/oxide/crucible/crucible-client-types)
      Adding crucible-common v0.0.1 (/home/eliza/Code/oxide/crucible/common)
      Adding crucible-protocol v0.0.0 (/home/eliza/Code/oxide/crucible/protocol)
   Compiling propolis-client v0.1.0 (/home/eliza/Code/oxide/propolis/lib/propolis-client)
   Compiling phd-runner v0.1.0 (/home/eliza/Code/oxide/propolis/phd-tests/runner)
warning: [email protected]: Crucible dependency is patched with a local checkout, so the `--crucible-downstairs-commit auto` flag will be disabled in this PHD build
   Compiling phd-framework v0.1.0 (/home/eliza/Code/oxide/propolis/phd-tests/framework)
   Compiling phd-testcase v0.1.0 (/home/eliza/Code/oxide/propolis/phd-tests/testcase)
   Compiling phd-tests v0.1.0 (/home/eliza/Code/oxide/propolis/phd-tests/tests)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 11.63s
```

Fixes #770

phd-tests/runner/build.rs

@@ -12,25 +12,24 @@ fn main() -> anyhow::Result<()> {
 }
 
 fn set_crucible_git_rev() -> anyhow::Result<()> {
+    const CRUCIBLE_REPO: &str = "https://github.com/oxidecomputer/crucible";
     fn extract_crucible_dep_sha(
         src: &cargo_metadata::Source,
     ) -> anyhow::Result<&str> {
-        const CRUCIBLE_REPO: &str = "https://github.com/oxidecomputer/crucible";
-
         let src = src.repr.strip_prefix("git+").ok_or_else(|| {
-            anyhow::anyhow!("Crucible package's source should be from git")
+            anyhow::anyhow!("Crucible is not a Git dependency")
         })?;
 
         if !src.starts_with(CRUCIBLE_REPO) {
             println!("cargo:warning=expected Crucible package's source to be {CRUCIBLE_REPO:?}, but is {src:?}");
         }
 
         let rev = src.split("?rev=").nth(1).ok_or_else(|| {
-            anyhow::anyhow!("Crucible package's source should have a revision")
+            anyhow::anyhow!("Crucible package's source did not have a revision")
         })?;
         let mut parts = rev.split('#');
         let sha = parts.next().ok_or_else(|| {
-            anyhow::anyhow!("Crucible package's source should have a revision")
+            anyhow::anyhow!("Crucible package's source did not have a revision")
         })?;
         assert_eq!(Some(sha), parts.next());
         Ok(sha)
@@ -48,16 +47,24 @@ fn set_crucible_git_rev() -> anyhow::Result<()> {
             anyhow::anyhow!("Failed to find Crucible package in cargo metadata")
         })?;
 
-    let crucible_src = crucible_pkg.source.as_ref().ok_or_else(|| {
-            anyhow::anyhow!("Crucible package should not be a workspace member, and therefore should have source metadata")
-        })?;
-
-    let crucible_sha =
-        extract_crucible_dep_sha(crucible_src).with_context(|| {
-            format!(
-                "Failed to extract Crucible source SHA from {crucible_src:?}"
+    let mut errmsg = String::new();
+    let crucible_sha = crucible_pkg
+        .source
+        .as_ref()
+        .ok_or_else(|| {
+            anyhow::anyhow!(
+                "Crucible dependency is patched with a local checkout"
             )
-        })?;
+        })
+        .and_then(extract_crucible_dep_sha)
+        .unwrap_or_else(|err| {
+            println!(
+                "cargo:warning={err}, so the `--crucible-downstairs-commit auto` \
+                 flag will be disabled in this PHD build",
+            );
+            errmsg = format!("CANT_GET_YE_CRUCIBLE_SHA{err}");
+            &errmsg
+        });
 
     println!("cargo:rustc-env=PHD_CRUCIBLE_GIT_REV={crucible_sha}");
 

phd-tests/runner/src/config.rs

@@ -265,9 +265,22 @@ impl RunOptions {
                 // The Git revision of Crucible we depend on is determined when building
                 // `phd-runner` by the build script, so that the `phd-runner` binary can
                 // be run even after moving it out of the Propolis cargo workspace.
-                let commit = env!("PHD_CRUCIBLE_GIT_REV").parse().context(
-                "PHD_CRUCIBLE_GIT_REV must be set to a valid Git revision by the build script",
-            )?;
+                let commit = env!("PHD_CRUCIBLE_GIT_REV");
+                if let Some(reason) =
+                    commit.strip_prefix("CANT_GET_YE_CRUCIBLE_SHA")
+                {
+                    anyhow::bail!(
+                        "Because {reason}, phd-runner's build script could not determine \
+                         the Crucible Git SHA, so the `--crucible-downstairs-commit auto` \
+                         option has been disabled.\n\tYou can provide a local Crucible \
+                         binary using `--crucible-downstairs-cmd`.",
+                    )
+                }
+
+                let commit = commit.parse().context(
+                    "PHD_CRUCIBLE_GIT_REV must be set to a valid Git \
+                        revision by the build script",
+                )?;
                 Ok(Some(CrucibleDownstairsSource::BuildomatGitRev(commit)))
             }
             None => Ok(None),