builder/BuildResults: restrict named_drvs to just two fields

The nix code in `./src/logged_evaluation.nix` only ever prints two
derivations, but `named_drvs` was generalized to a HashMap which
supports any amount of derivations.

We are better off with a struct in this case, because we can fail
early if we don’t find one of the expected derivations in the nix
output (or find one more than once). It also acts as documentation of
what exactly we expect from the output of `logged_evaluation.nix`.

src/build_loop.rs

@@ -27,8 +27,8 @@ pub enum Event {
 pub struct BuildResults {
     /// See `build::Info.drvs`
     drvs: HashMap<usize, roots::RootPath>,
-    /// See `build::Info.drvs`
-    pub named_drvs: HashMap<String, roots::RootPath>,
+    /// See `build::Info.outputPaths
+    pub output_paths: builder::OutputPaths<roots::RootPath>,
 }
 
 /// Results of a single, failing build.
@@ -105,17 +105,19 @@ impl<'a> BuildLoop<'a> {
         let paths = reduce_paths(&paths);
         debug!("  -> reduced to: {:?}", paths.len());
 
-        debug!("named drvs: {:#?}", build.named_drvs);
+        debug!("named drvs: {:#?}", build.output_paths);
+
+        // create root for every field in OutputPaths
+        let output_paths = build
+            .output_paths
+            .map_with_attr_name(|attr_name, store_path| {
+                roots.add(&format!("attr-{}", attr_name), &store_path)
+            })?;
 
         let mut event = BuildResults {
             drvs: HashMap::new(),
-            named_drvs: HashMap::new(),
+            output_paths,
         };
-        for (name, drv) in build.named_drvs.iter() {
-            event
-                .named_drvs
-                .insert(name.clone(), roots.add(&format!("attr-{}", name), drv)?);
-        }
 
         for (i, drv) in build.drvs.iter().enumerate() {
             event

src/builder.rs

@@ -10,13 +10,15 @@
 use cas::ContentAddressable;
 use regex::Regex;
 use std::any::Any;
-use std::collections::HashMap;
 use std::ffi::{OsStr, OsString};
 use std::path::PathBuf;
 use std::process::{Command, Stdio};
 use {NixFile, StorePath};
 
-fn instrumented_build(root_nix_file: &NixFile, cas: &ContentAddressable) -> Result<Info, Error> {
+fn instrumented_build(
+    root_nix_file: &NixFile,
+    cas: &ContentAddressable,
+) -> Result<Info<StorePath>, Error> {
     // We're looking for log lines matching:
     //
     //     copied source '...' -> '/nix/store/...'
@@ -54,27 +56,71 @@ fn instrumented_build(root_nix_file: &NixFile, cas: &ContentAddressable) -> Resu
 
     let produced_drvs = ::nix::parse_nix_output(&output.stdout, StorePath::from);
 
-    let (paths, named_drvs, log_lines): (Vec<PathBuf>, HashMap<String, StorePath>, Vec<OsString>) =
-        stderr_results.into_iter().fold(
-            (vec![], HashMap::new(), vec![]),
-            |(mut paths, mut named_drvs, mut log_lines), result| {
+    // iterate over all lines, parsing out the ones we are interested in
+    let (paths, output_paths, log_lines): (
+        Vec<PathBuf>,
+        // `None` if the field was not seen before, `Some` if it was
+        OutputPaths<Option<StorePath>>,
+        Vec<OsString>
+    ) =
+    stderr_results.into_iter().fold(
+        (vec![], OutputPaths { shell: None, shell_gc_root: None }, vec![]),
+        |(mut paths, mut output_paths, mut log_lines), result| {
                 match result {
                     LogDatum::Source(src) => {
                         paths.push(src);
                     }
-                    LogDatum::AttrDrv(name, drv) => {
-                        named_drvs.insert(name, StorePath(drv));
-                    }
+                    LogDatum::ShellDrv(drv) => {
+                        // check whether we have seen this field before
+                        match output_paths.shell {
+                            None => { output_paths.shell = Some(StorePath(drv)); }
+                            // programming error
+                            Some(StorePath(old)) => panic!(
+                                "`lorri read` got attribute `{}` a second time, first path was {:?} and second {:?}",
+                                "shell", old, drv)
+                        }
+                    },
+                    LogDatum::ShellGcRootDrv(drv) => {
+                        // check whether we have seen this field before
+                        match output_paths.shell_gc_root {
+                            None => { output_paths.shell_gc_root = Some(StorePath(drv)); }
+                            // programming error
+                            Some(StorePath(old)) => panic!(
+                                "`lorri read` got attribute `{}` a second time, first path was {:?} and second {:?}",
+                                "shell_gc_root", old, drv)
+                        }
+                    },
                     LogDatum::Text(line) => log_lines.push(line),
                 };
 
-                (paths, named_drvs, log_lines)
+                (paths, output_paths, log_lines)
             },
         );
+
+    // check whether we got all required `OutputPaths`
+    let output_paths = match output_paths {
+        // programming error
+        OutputPaths { shell: None, .. } => {
+            panic!("`lorri read` never got required attribute `shell`")
+        }
+        // programming error
+        OutputPaths {
+            shell_gc_root: None,
+            ..
+        } => panic!("`lorri read` never got required attribute `shell_gc_root`"),
+        OutputPaths {
+            shell: Some(shell),
+            shell_gc_root: Some(shell_gc_root),
+        } => OutputPaths {
+            shell,
+            shell_gc_root,
+        },
+    };
+
     Ok(Info {
         exec_result: output.status,
         drvs: produced_drvs,
-        named_drvs,
+        output_paths,
         paths,
         log_lines,
     })
@@ -84,14 +130,15 @@ fn instrumented_build(root_nix_file: &NixFile, cas: &ContentAddressable) -> Resu
 ///
 /// Instruments the nix file to gain extra information,
 /// which is valuable even if the build fails.
-pub fn run(root_nix_file: &NixFile, cas: &ContentAddressable) -> Result<Info, Error> {
+pub fn run(root_nix_file: &NixFile, cas: &ContentAddressable) -> Result<Info<StorePath>, Error> {
     instrumented_build(root_nix_file, cas)
 }
 
 #[derive(Debug, PartialEq)]
 enum LogDatum {
     Source(PathBuf),
-    AttrDrv(String, PathBuf),
+    ShellDrv(PathBuf),
+    ShellGcRootDrv(PathBuf),
     Text(OsString),
 }
 
@@ -124,10 +171,16 @@ fn parse_evaluation_line(line: &OsStr) -> LogDatum {
             } else if let Some(matches) = LORRI_READ.captures(&linestr) {
                 LogDatum::Source(PathBuf::from(&matches["source"]))
             } else if let Some(matches) = LORRI_ATTR_DRV.captures(&linestr) {
-                LogDatum::AttrDrv(
-                    String::from(&matches["attribute"]),
-                    PathBuf::from(&matches["drv"]),
-                )
+                let drv = &matches["drv"];
+                let attr = &matches["attribute"];
+                match attr {
+                    "shell" => LogDatum::ShellDrv(PathBuf::from(drv)),
+                    "shell_gc_root" => LogDatum::ShellGcRootDrv(PathBuf::from(drv)),
+                    _ => panic!(
+                        "`lorri read` trace was `{} -> {}`, unknown attribute `{}`! (add to `builder.rs`)",
+                        attr, drv, attr
+                    ),
+                }
             } else {
                 LogDatum::Text(line.to_owned())
             }
@@ -140,16 +193,14 @@ fn parse_evaluation_line(line: &OsStr) -> LogDatum {
 /// valuable information in the output, like new paths
 /// to watch.
 #[derive(Debug)]
-pub struct Info {
+pub struct Info<T> {
     /// The result of executing Nix
     pub exec_result: std::process::ExitStatus,
 
-    // TODO: what?
-    // are those actual drv files?
-    /// All the attributes in the default expression which belong to
-    /// attributes.
-    pub named_drvs: HashMap<String, StorePath>,
+    /// See `OutputPaths`
+    pub output_paths: OutputPaths<T>,
 
+    // TODO: this is redundant with shell_gc_root
     /// A list of the evaluation's result derivations
     pub drvs: Vec<StorePath>,
 
@@ -161,6 +212,28 @@ pub struct Info {
     pub log_lines: Vec<OsString>,
 }
 
+/// Output derivations generated by `logged-evaluation.nix`
+#[derive(Debug, Clone)]
+pub struct OutputPaths<T> {
+    /// Original shell derivation
+    pub shell: T,
+    /// Shell derivation modified to work as a gc root
+    pub shell_gc_root: T,
+}
+
+impl<T> OutputPaths<T> {
+    /// Similar to other `map` functions, but also provides the name of the field
+    pub fn map_with_attr_name<U, F, E>(self, f: F) -> Result<OutputPaths<U>, E>
+    where
+        F: Fn(&str, T) -> Result<U, E>,
+    {
+        Ok(OutputPaths {
+            shell: f("shell", self.shell)?,
+            shell_gc_root: f("shell_gc_root", self.shell_gc_root)?,
+        })
+    }
+}
+
 /// Possible errors from an individual evaluation
 #[derive(Debug)]
 pub enum Error {
@@ -210,7 +283,11 @@ mod tests {
 
         assert_eq!(
             parse_evaluation_line(&OsString::from("trace: lorri attribute: 'shell' -> '/nix/store/q3ngidzvincycjjvlilf1z6vj1w4wnas-lorri.drv'")),
-            LogDatum::AttrDrv(String::from("shell"), PathBuf::from("/nix/store/q3ngidzvincycjjvlilf1z6vj1w4wnas-lorri.drv"))
+            LogDatum::ShellDrv(PathBuf::from("/nix/store/q3ngidzvincycjjvlilf1z6vj1w4wnas-lorri.drv"))
+        );
+        assert_eq!(
+            parse_evaluation_line(&OsString::from("trace: lorri attribute: 'shell_gc_root' -> '/nix/store/q3ngidzvincycjjvlilf1z6vj1w4wnas-lorri-keep-env-hack-foo.drv'")),
+            LogDatum::ShellGcRootDrv(PathBuf::from("/nix/store/q3ngidzvincycjjvlilf1z6vj1w4wnas-lorri-keep-env-hack-foo.drv"))
         );
 
         assert_eq!(

src/ops/shell.rs

@@ -51,10 +51,7 @@ pub fn main(project: Project) -> OpResult {
 
     // the `shell` derivation is required in oder to start a shell
     // TODO: is this actually a derivation? Or an attribute?
-    let shell_drv = first_build
-        .named_drvs
-        .get("shell")
-        .expect("Failed to start the shell: no \"shell\" derivation found");
+    let shell_drv = first_build.output_paths.shell;
 
     let build_thread = {
         thread::spawn(move || {