From f4cf2eb234d9c7768162b53249bcfb068546f1cc Mon Sep 17 00:00:00 2001 From: just-an-engineer Date: Tue, 13 Aug 2024 11:09:45 -0400 Subject: [PATCH] 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 | 17 +++++-- src/config.rs | 1 - src/test/tests.rs | 42 +++++++++++------- src/util.rs | 111 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 21 deletions(-) diff --git a/src/commands.rs b/src/commands.rs index 189880ba6..33efb3727 100644 --- a/src/commands.rs +++ b/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( #[cfg(not(windows))] fn run_server_process( startup_timeout: Option, - _port: Option, + cli_port: Option, ) -> Result { 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) diff --git a/src/config.rs b/src/config.rs index 1b26bd6e1..af283d84a 100644 --- a/src/config.rs +++ b/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!( diff --git a/src/test/tests.rs b/src/test/tests.rs index fdd0424e7..aea833521 100644 --- a/src/test/tests.rs +++ b/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::() % 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()); } diff --git a/src/util.rs b/src/util.rs index 4fc45af2e..1a2fffbe4 100644 --- a/src/util.rs +++ b/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> { + 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); + } }