Avoid spawning a new server for --show-stats

Fixes mozilla#2042

src/commands.rs

@@ -12,14 +12,15 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use crate::cache::storage_from_config;
 use crate::client::{connect_to_server, connect_with_retry, ServerConnection};
 use crate::cmdline::{Command, StatsFormat};
 use crate::compiler::ColorMode;
 use crate::config::{default_disk_cache_dir, Config};
 use crate::jobserver::Client;
 use crate::mock_command::{CommandChild, CommandCreatorSync, ProcessCommandCreator, RunCommand};
 use crate::protocol::{Compile, CompileFinished, CompileResponse, Request, Response};
-use crate::server::{self, DistInfo, ServerInfo, ServerStartup};
+use crate::server::{self, DistInfo, ServerInfo, ServerStartup, ServerStats};
 use crate::util::daemonize;
 use byteorder::{BigEndian, ByteOrder};
 use fs::{File, OpenOptions};
@@ -607,8 +608,16 @@ pub fn run_command(cmd: Command) -> Result<i32> {
     match cmd {
         Command::ShowStats(fmt, advanced) => {
             trace!("Command::ShowStats({:?})", fmt);
-            let srv = connect_or_start_server(get_port(), startup_timeout)?;
-            let stats = request_stats(srv).context("failed to get stats from server")?;
+            let stats = match connect_to_server(get_port()) {
+                Ok(srv) => request_stats(srv).context("failed to get stats from server")?,
+                // If there is no server, spawning a new server would start with zero stats
+                // anyways, so we can just return (mostly) empty stats directly.
+                Err(_) => {
+                    let runtime = Runtime::new()?;
+                    let storage = storage_from_config(config, runtime.handle()).ok();
+                    runtime.block_on(ServerInfo::new(ServerStats::default(), storage.as_deref()))?
+                }
+            };
             match fmt {
                 StatsFormat::Text => stats.print(advanced),
                 StatsFormat::Json => serde_json::to_writer(&mut io::stdout(), &stats)?,

src/server.rs

@@ -910,22 +910,7 @@ where
     /// Get info and stats about the cache.
     async fn get_info(&self) -> Result<ServerInfo> {
         let stats = self.stats.lock().await.clone();
-        let cache_location = self.storage.location();
-        let use_preprocessor_cache_mode = self
-            .storage
-            .preprocessor_cache_mode_config()
-            .use_preprocessor_cache_mode;
-        let version = env!("CARGO_PKG_VERSION").to_string();
-        futures::try_join!(self.storage.current_size(), self.storage.max_size()).map(
-            move |(cache_size, max_cache_size)| ServerInfo {
-                stats,
-                cache_location,
-                cache_size,
-                max_cache_size,
-                use_preprocessor_cache_mode,
-                version,
-            },
-        )
+        ServerInfo::new(stats, Some(&*self.storage)).await
     }
 
     /// Zero stats about the cache.
@@ -1678,6 +1663,35 @@ impl ServerStats {
 }
 
 impl ServerInfo {
+    pub async fn new(stats: ServerStats, storage: Option<&dyn Storage>) -> Result<Self> {
+        let cache_location;
+        let use_preprocessor_cache_mode;
+        let cache_size;
+        let max_cache_size;
+        if let Some(storage) = storage {
+            cache_location = storage.location();
+            use_preprocessor_cache_mode = storage
+                .preprocessor_cache_mode_config()
+                .use_preprocessor_cache_mode;
+            (cache_size, max_cache_size) =
+                futures::try_join!(storage.current_size(), storage.max_size())?;
+        } else {
+            cache_location = String::new();
+            use_preprocessor_cache_mode = false;
+            cache_size = None;
+            max_cache_size = None;
+        }
+        let version = env!("CARGO_PKG_VERSION").to_string();
+        Ok(ServerInfo {
+            stats,
+            cache_location,
+            cache_size,
+            max_cache_size,
+            use_preprocessor_cache_mode,
+            version,
+        })
+    }
+
     /// Print info to stdout in a human-readable format.
     pub fn print(&self, advanced: bool) {
         let (name_width, stat_width) = self.stats.print(advanced);

tests/harness/mod.rs

@@ -61,15 +61,14 @@ pub fn start_local_daemon(cfg_path: &Path, cached_cfg_path: &Path) {
     }
 }
 
-pub fn stop_local_daemon() {
+pub fn stop_local_daemon() -> bool {
     trace!("sccache --stop-server");
-    drop(
-        sccache_command()
-            .arg("--stop-server")
-            .stdout(Stdio::null())
-            .stderr(Stdio::null())
-            .status(),
-    );
+    sccache_command()
+        .arg("--stop-server")
+        .stdout(Stdio::null())
+        .stderr(Stdio::null())
+        .status()
+        .map_or(false, |status| status.success())
 }
 
 pub fn get_stats<F: 'static + Fn(ServerInfo)>(f: F) {

tests/sccache_cargo.rs

@@ -166,11 +166,12 @@ fn test_run_log() -> Result<()> {
         .context("Failed to create tempdir")?;
     let tmppath = tempdir.path().join("perm.log");
     let mut cmd = Command::new(SCCACHE_BIN.as_os_str());
-    cmd.arg("--show-stats")
+    cmd.arg("--start-server")
         .env("SCCACHE_ERROR_LOG", &tmppath) // Should not work
         .env("SCCACHE_LOG", "debug");
 
     cmd.assert().success();
+    stop_sccache()?;
     assert!(Path::new(&tmppath).is_file());
     Ok(())
 }

tests/system.rs

@@ -881,6 +881,18 @@ fn test_sccache_command(preprocessor_cache_mode: bool) {
     }
 }
 
+#[test]
+#[serial]
+fn test_stats_no_server() {
+    // Ensure there's no existing sccache server running.
+    stop_local_daemon();
+    get_stats(|_| {});
+    assert!(
+        !stop_local_daemon(),
+        "Server shouldn't be running after --show-stats"
+    );
+}
+
 #[test_case(true ; "with preprocessor cache")]
 #[test_case(false ; "without preprocessor cache")]
 #[serial]