From 462baddf1790dc82c6862d3a0ade9a8537a4aa83 Mon Sep 17 00:00:00 2001
From: Dustin Carlino <dabreegster@gmail.com>
Date: Tue, 13 Apr 2021 09:45:48 -0700
Subject: [PATCH] Robustify running the importer from the UI, for #602 and #82:

- Echo process output to the main UI's stdout, for easier debugging
- Remove the timing breakdown from elevation import, since it breaks
  when the function bails out early
---
 convert_osm/src/elevation.rs | 16 +++-------------
 convert_osm/src/lib.rs       |  4 +++-
 map_gui/src/tools/command.rs |  2 ++
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/convert_osm/src/elevation.rs b/convert_osm/src/elevation.rs
index e9c764c847..a5f9c09eae 100644
--- a/convert_osm/src/elevation.rs
+++ b/convert_osm/src/elevation.rs
@@ -4,20 +4,14 @@ use std::process::Command;
 
 use anyhow::Result;
 
-use abstutil::Timer;
 use geom::{Distance, PolyLine};
 use map_model::raw::{OriginalRoad, RawMap};
 
-pub fn add_data(map: &mut RawMap, timer: &mut Timer) -> Result<()> {
-    timer.start("add elevation data");
-
-    // TODO General problem: If we bail out early, the timer gets screwed up and later crashes. I
-    // think we need to start using scoped objects that call timer.stop() when dropped.
-    timer.start("generate input");
+pub fn add_data(map: &mut RawMap) -> Result<()> {
+    // TODO It'd be nice to include more timing breakdown here, but if we bail out early,
+    // it's tedious to call timer.stop().
     let ids = generate_input(map)?;
-    timer.stop("generate input");
 
-    timer.start("run elevation_lookups");
     std::fs::create_dir_all("elevation_output")?;
     std::fs::create_dir_all(abstio::path_shared_input("elevation"))?;
     let pwd = std::env::current_dir()?.display().to_string();
@@ -56,11 +50,8 @@ pub fn add_data(map: &mut RawMap, timer: &mut Timer) -> Result<()> {
     if !status.success() {
         bail!("Command failed: {}", status);
     }
-    timer.stop("run elevation_lookups");
 
-    timer.start("grab output");
     scrape_output(map, ids)?;
-    timer.stop("grab output");
 
     // Clean up temporary files
     std::fs::remove_file("elevation_input/query")?;
@@ -68,7 +59,6 @@ pub fn add_data(map: &mut RawMap, timer: &mut Timer) -> Result<()> {
     std::fs::remove_file("elevation_output/query")?;
     std::fs::remove_dir("elevation_output")?;
 
-    timer.stop("add elevation data");
     Ok(())
 }
 
diff --git a/convert_osm/src/lib.rs b/convert_osm/src/lib.rs
index 4126863302..ba32f9044c 100644
--- a/convert_osm/src/lib.rs
+++ b/convert_osm/src/lib.rs
@@ -111,9 +111,11 @@ pub fn convert(opts: Options, timer: &mut abstutil::Timer) -> RawMap {
     parking::apply_parking(&mut map, &opts, timer);
 
     // TODO Make this bail out on failure, after the new dependencies are clearly explained.
-    if let Err(err) = elevation::add_data(&mut map, timer) {
+    timer.start("add elevation data");
+    if let Err(err) = elevation::add_data(&mut map) {
         error!("No elevation data: {}", err);
     }
+    timer.stop("add elevation data");
     if let Some(ref path) = opts.extra_buildings {
         add_extra_buildings(&mut map, path).unwrap();
     }
diff --git a/map_gui/src/tools/command.rs b/map_gui/src/tools/command.rs
index 307896a2c6..a5039237fb 100644
--- a/map_gui/src/tools/command.rs
+++ b/map_gui/src/tools/command.rs
@@ -75,6 +75,7 @@ impl<A: AppLike + 'static> RunCommand<A> {
             // This is almost always a timeout.
             Err(err) => err.capture,
         };
+        // TODO This doesn't interleave stdout and stderr as expected.
         for raw in vec![stdout, stderr] {
             if let Some(bytes) = raw {
                 if let Ok(string) = String::from_utf8(bytes) {
@@ -97,6 +98,7 @@ impl<A: AppLike + 'static> RunCommand<A> {
                     self.lines
                         .push_back(line.split('\r').last().unwrap().to_string());
                 } else {
+                    println!("> {}", line);
                     self.lines.push_back(line);
                 }
             }