Add a client cache to store most recently used port to use for future…

… operations (such as a compile request) without having to specify the non-default port every time

src/commands.rs

@@ -21,7 +21,7 @@ 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, ServerStats};
-use crate::util::daemonize;
+use crate::util::{self, daemonize};
 use byteorder::{BigEndian, ByteOrder};
 use fs::{File, OpenOptions};
 use fs_err as fs;
@@ -55,7 +55,13 @@ fn get_port() -> u16 {
         env::var("SCCACHE_SERVER_PORT")
             .ok()
             .and_then(|s| s.parse().ok())
-            .unwrap_or(DEFAULT_PORT)
+            .unwrap_or(
+                util::get_from_client_cache("recent_port")
+                    .ok()
+                    .and_then(|port| port)
+                    .and_then(|port| port.parse().ok())
+                    .unwrap_or(DEFAULT_PORT),
+            )
     };
     match &Config::load() {
         Ok(config) => config.port.unwrap_or_else(fallback),
@@ -90,7 +96,7 @@ async fn read_server_startup_status<R: AsyncReadExt + Unpin>(
 #[cfg(not(windows))]
 fn run_server_process(
     startup_timeout: Option<Duration>,
-    _port: Option<u16>,
+    cli_port: Option<u16>,
 ) -> Result<ServerStartup> {
     trace!("run_server_process");
     let tempdir = tempfile::Builder::new().prefix("sccache").tempdir()?;
@@ -107,7 +113,10 @@ fn run_server_process(
         tokio::net::UnixListener::bind(&socket_path)?
     };
 
-    let port = _port.unwrap_or_else(get_port);
+    let port = cli_port.unwrap_or_else(get_port);
+    if cli_port.is_some() {
+        util::write_to_client_cache("recent_port", port.to_string().as_str())?;
+    }
 
     let _child = process::Command::new(&exe_path)
         .current_dir(workdir)

src/config.rs

@@ -1591,7 +1591,6 @@ no_credentials = true
 
 #[test]
 fn test_port_config() {
-    // just set up a config file with just port, then have it read it in, and ensure port is defined in the struct
     const CONFIG_STR: &str = "port = 8080";
     let file_config: FileConfig = toml::from_str(CONFIG_STR).expect("Is valid toml.");
     assert_eq!(

src/test/tests.rs

@@ -299,7 +299,6 @@ fn test_server_compile() {
 // https://github.com/mozilla/sccache/issues/234
 #[cfg(not(target_os = "macos"))]
 fn test_server_port_in_use() {
-    // Bind an arbitrary free port.
     let listener = TcpListener::bind("127.0.0.1:0").unwrap();
     let sccache = find_sccache_binary();
     let output = Command::new(sccache)
@@ -327,22 +326,33 @@ fn test_server_port_in_use() {
 // https://github.com/mozilla/sccache/issues/234
 #[cfg(not(target_os = "macos"))]
 fn test_server_port_from_cli() {
-    // Bind an arbitrary free port.
-    let listener = TcpListener::bind("127.0.0.1:0").unwrap();
     let sccache = find_sccache_binary();
-    let port = listener.local_addr().unwrap().port().to_string();
-    let output = Command::new(sccache)
-        .arg("--start-server")
-        .arg(port)
+    loop {
+        let port = 10_000 + rand::random::<u16>() % 30_000;
+        let output = Command::new(sccache.clone())
+            .arg("--start-server")
+            .arg(port.to_string())
+            .output()
+            .unwrap();
+        let s = String::from_utf8_lossy(&output.stderr);
+        const PORT_IN_USE: &str = "Address in use";
+        if s.contains(PORT_IN_USE) {
+            continue;
+        }
+        assert!(output.status.success());
+        break;
+    }
+    // try to compile something to ensure our compile requests use the most recent port
+    let output = Command::new(sccache.clone())
+        .arg("gcc")
+        .arg("-c")
+        .arg("test.c")
+        .arg("-o")
+        .arg("test.o")
         .output()
         .unwrap();
-    assert!(!output.status.success());
-    let s = String::from_utf8_lossy(&output.stderr);
-    const MSG: &str = "Server startup failed:";
-    assert!(
-        s.contains(MSG),
-        "Output did not contain '{}':\n========\n{}\n========",
-        MSG,
-        s
-    );
+    assert!(output.status.success());
+
+    let output = Command::new(sccache).arg("--stop-server").output().unwrap();
+    assert!(output.status.success());
 }

src/util.rs

@@ -15,6 +15,7 @@
 use crate::mock_command::{CommandChild, RunCommand};
 use blake3::Hasher as blake3_Hasher;
 use byteorder::{BigEndian, ByteOrder};
+use directories::BaseDirs;
 use fs::File;
 use fs_err as fs;
 use object::{macho, read::archive::ArchiveFile, read::macho::FatArch};
@@ -939,6 +940,104 @@ pub fn new_reqwest_blocking_client() -> reqwest::blocking::Client {
         .expect("http client must build with success")
 }
 
+pub fn write_to_client_cache(key: &str, value: &str) -> Result<()> {
+    if let Some(base_dirs) = BaseDirs::new() {
+        let cache_dir = base_dirs.cache_dir();
+        let cache_dir = cache_dir.join("sccache");
+        std::fs::create_dir_all(&cache_dir)?;
+        let cache_file = cache_dir.join("client_cache");
+        let mut file = std::fs::OpenOptions::new()
+            .read(true)
+            .write(true)
+            .create(true)
+            .truncate(true)
+            .open(cache_file)?;
+        let mut contents = String::new();
+        file.read_to_string(&mut contents)?;
+
+        let mut new_contents = String::new();
+        let mut found = false;
+        for line in contents.lines() {
+            if line.starts_with(key) {
+                found = true;
+                new_contents.push_str(&format!("{} = \"{}\"\n", key, value));
+            } else {
+                new_contents.push_str(line);
+                new_contents.push('\n');
+            }
+        }
+        if !found {
+            new_contents.push_str(&format!("{} = \"{}\"\n", key, value));
+        }
+        file.write_all(new_contents.as_bytes())?;
+        Ok(())
+    } else {
+        Err(anyhow!(
+            "no valid home directory path could be retrieved from OS"
+        ))
+    }
+}
+
+pub fn get_from_client_cache(key: &str) -> Result<Option<String>> {
+    if let Some(base_dirs) = BaseDirs::new() {
+        let cache_dir = base_dirs.cache_dir();
+        let cache_dir = cache_dir.join("sccache");
+        let cache_file = cache_dir.join("client_cache");
+        if !cache_file.exists() {
+            return Ok(None);
+        }
+        let mut file = std::fs::File::open(&cache_file)?;
+        let mut contents = String::new();
+        file.read_to_string(&mut contents)?;
+        for line in contents.lines() {
+            let mut parts = line.splitn(2, '=');
+            if let Some(k) = parts.next() {
+                if k.trim() == key {
+                    if let Some(v) = parts.next() {
+                        return Ok(Some(v.trim().trim_matches('"').to_string()));
+                    }
+                }
+            }
+        }
+        Ok(None)
+    } else {
+        Err(anyhow!(
+            "no valid home directory path could be retrieved from OS"
+        ))
+    }
+}
+
+pub fn remove_key_from_client_cache(key: &str) -> Result<()> {
+    if let Some(base_dirs) = BaseDirs::new() {
+        let cache_dir = base_dirs.cache_dir();
+        let cache_dir = cache_dir.join("sccache");
+        let cache_file = cache_dir.join("client_cache");
+        if !cache_file.exists() {
+            return Ok(());
+        }
+        let mut file = std::fs::OpenOptions::new()
+            .read(true)
+            .write(true)
+            .truncate(true)
+            .open(cache_file)?;
+        let mut contents = String::new();
+        file.read_to_string(&mut contents)?;
+        let mut new_contents = String::new();
+        for line in contents.lines() {
+            if !line.starts_with(key) {
+                new_contents.push_str(line);
+                new_contents.push('\n');
+            }
+        }
+        file.write_all(new_contents.as_bytes())?;
+        Ok(())
+    } else {
+        Err(anyhow!(
+            "no valid home directory path could be retrieved from OS"
+        ))
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::{OsStrExt, TimeMacroFinder};
@@ -1049,4 +1148,16 @@ mod tests {
         finder.find_time_macros(b"TIMESTAMP__ This is larger than the haystack");
         assert!(finder.found_timestamp());
     }
+
+    #[test]
+    fn test_client_cache() {
+        let key = "test_key";
+        let value = "test_value";
+        super::write_to_client_cache(key, value).unwrap();
+        let result = super::get_from_client_cache(key).unwrap();
+        assert_eq!(result, Some(value.to_string()));
+        super::remove_key_from_client_cache(key).unwrap();
+        let result = super::get_from_client_cache(key).unwrap();
+        assert_eq!(result, None);
+    }
 }