From 357238128030c89ac198cf78ac5c5e92f2543fb4 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Mon, 22 Apr 2019 09:45:14 -0400
Subject: [PATCH] nixenv: Return a different error if stats parse fails

---
 ofborg/src/nixenv.rs             | 51 +++++++++++++++++++++++++-------
 ofborg/src/outpathdiff.rs        |  2 +-
 ofborg/src/tasks/eval/nixpkgs.rs |  9 +++---
 3 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/ofborg/src/nixenv.rs b/ofborg/src/nixenv.rs
index 30be6e5..9ef5e5a 100644
--- a/ofborg/src/nixenv.rs
+++ b/ofborg/src/nixenv.rs
@@ -28,21 +28,23 @@ impl HydraNixEnv {
         }
     }
 
-    pub fn execute(&self) -> Result<(outpathdiff::PackageOutPaths, EvaluationStats), Error> {
+    pub fn execute_with_stats(
+        &self,
+    ) -> Result<(outpathdiff::PackageOutPaths, EvaluationStats), Error> {
         self.place_nix()?;
         let (status, stdout, mut stderr) = self.run_nix_env();
         self.remove_nix()?;
 
-        if !status {
-            Err(Error::Fd(stderr))
-        } else if let Ok(stats) = serde_json::from_reader(&mut stderr) {
+        if status {
             let outpaths = outpathdiff::parse_lines(&mut BufReader::new(stdout));
+            let stats = serde_json::from_reader(&mut stderr).map_err(|err| {
+                let seek = stderr.seek(SeekFrom::Start(0));
+
+                Error::StatsParse(stderr, seek, err)
+            })?;
             Ok((outpaths, stats))
         } else {
-            stderr
-                .seek(SeekFrom::Start(0))
-                .expect("Seeking to Start(0)");
-            Err(Error::Fd(stderr))
+            Err(Error::CommandFailed(stderr))
         }
     }
 
@@ -86,7 +88,8 @@ impl HydraNixEnv {
 
 pub enum Error {
     Io(std::io::Error),
-    Fd(File),
+    CommandFailed(File),
+    StatsParse(File, Result<u64, std::io::Error>, serde_json::Error),
 }
 
 impl From<std::io::Error> for Error {
@@ -99,7 +102,7 @@ impl Error {
     pub fn display(self) -> String {
         match self {
             Error::Io(e) => format!("Failed during the setup of executing nix-env: {:?}", e),
-            Error::Fd(mut fd) => {
+            Error::CommandFailed(mut fd) => {
                 let mut buffer = Vec::new();
                 let read_result = fd.read_to_end(&mut buffer);
                 let bufstr = String::from_utf8_lossy(&buffer);
@@ -112,6 +115,34 @@ impl Error {
                     ),
                 }
             }
+            Error::StatsParse(mut fd, seek, parse_err) => {
+                let mut buffer = Vec::new();
+                let read_result = fd.read_to_end(&mut buffer);
+                let bufstr = String::from_utf8_lossy(&buffer);
+
+                let mut lines =
+                    String::from("Parsing nix-env's performance statistics failed.\n\n");
+
+                if let Err(seek_err) = seek {
+                    lines.push_str(&format!(
+                        "Additionally, resetting to the beginning of the output failed with:\n{:?}\n\n",
+                        seek_err
+                    ));
+                }
+
+                if let Err(read_err) = read_result {
+                    lines.push_str(&format!(
+                        "Additionally, loading the output failed with:\n{:?}\n\n",
+                        read_err
+                    ));
+                }
+
+                lines.push_str(&format!("Parse error:\n{:?}\n\n", parse_err));
+
+                lines.push_str(&format!("Evaluation output:\n{}", bufstr));
+
+                lines
+            }
         }
     }
 }
diff --git a/ofborg/src/outpathdiff.rs b/ofborg/src/outpathdiff.rs
index cc25128..769b79e 100644
--- a/ofborg/src/outpathdiff.rs
+++ b/ofborg/src/outpathdiff.rs
@@ -95,7 +95,7 @@ impl OutPathDiff {
     }
 
     fn run(&mut self) -> Result<(PackageOutPaths, EvaluationStats), NixEnvError> {
-        self.calculator.execute()
+        self.calculator.execute_with_stats()
     }
 }
 
diff --git a/ofborg/src/tasks/eval/nixpkgs.rs b/ofborg/src/tasks/eval/nixpkgs.rs
index 5c24627..6324f19 100644
--- a/ofborg/src/tasks/eval/nixpkgs.rs
+++ b/ofborg/src/tasks/eval/nixpkgs.rs
@@ -136,7 +136,7 @@ impl<'a> NixpkgsStrategy<'a> {
              */
 
             Err(Error::FailWithGist(
-                String::from("The branch this PR will merge in to does not evaluate, and so this PR cannot be checked."),
+                String::from("The branch this PR will merge in to does not cleanly evaluate, and so this PR cannot be checked."),
                 String::from("Output path comparison"),
                 err.display(),
             ))
@@ -150,7 +150,7 @@ impl<'a> NixpkgsStrategy<'a> {
         if let Some(ref mut rebuildsniff) = self.outpath_diff {
             if let Err(mut err) = rebuildsniff.find_after() {
                 Err(Error::FailWithGist(
-                    String::from("This PR breaks listing of package outputs after merging."),
+                    String::from("This PR does not cleanly list of package outputs after merging."),
                     String::from("Output path comparison"),
                     err.display(),
                 ))
@@ -299,10 +299,9 @@ impl<'a> NixpkgsStrategy<'a> {
             status.set(hubcaps::statuses::State::Pending);
 
             let nixenv = HydraNixEnv::new(self.nix.clone(), dir.to_path_buf(), true);
-            match nixenv.execute() {
-                Ok(pkgs) => {
+            match nixenv.execute_with_stats() {
+                Ok((pkgs, _stats)) => {
                     let mut try_build: Vec<String> = pkgs
-                        .0
                         .keys()
                         .map(|pkgarch| pkgarch.package.clone())
                         .filter(|pkg| possibly_touched_packages.contains(&pkg))