Add ability to specify a port number from cli for start and stop serv…

…er, instead of config or env vars
mozilla · Jul 31, 2024 · 3f7e105 · 3f7e105
1 parent 9958e7c
commit 3f7e105
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 18 deletions.
diff --git a/README.md b/README.md
@@ -114,9 +114,9 @@ sccache supports gcc, clang, MSVC, rustc, [NVCC](https://docs.nvidia.com/cuda/cu
 
 If you don't [specify otherwise](#storage-options), sccache will use a local disk cache.
 
-sccache works using a client-server model, where the server runs locally on the same machine as the client. The client-server model allows the server to be more efficient by keeping some state in memory. The sccache command will spawn a server process if one is not already running, or you can run `sccache --start-server` to start the background server process without performing any compilation.
+sccache works using a client-server model, where the server runs locally on the same machine as the client. The client-server model allows the server to be more efficient by keeping some state in memory. The sccache command will spawn a server process if one is not already running, or you can run `sccache --start-server[=port]` to start the background server process without performing any compilation.
 
-You can run `sccache --stop-server` to terminate the server. It will also terminate after (by default) 10 minutes of inactivity.
+You can run `sccache --stop-server[=port]` to terminate the server. It will also terminate after (by default) 10 minutes of inactivity.
 
 Running `sccache --show-stats` will print a summary of cache statistics.
 

diff --git a/src/cmdline.rs b/src/cmdline.rs
@@ -62,9 +62,9 @@ pub enum Command {
     /// Run background server.
     InternalStartServer,
     /// Start background server as a subprocess.
-    StartServer,
+    StartServer(Option<u16>),
     /// Stop background server.
-    StopServer,
+    StopServer(Option<u16>),
     /// Zero cache statistics and exit.
     ZeroStats,
     /// Show the status of the distributed client.
@@ -136,13 +136,19 @@ fn get_clap_command() -> clap::Command {
                 .action(ArgAction::SetTrue),
             flag_infer_long("start-server")
                 .help("start background server")
-                .action(ArgAction::SetTrue),
+                .value_name("port")
+                .num_args(0..=1)
+                .default_missing_value("None")
+                .action(ArgAction::Append),
             flag_infer_long("debug-preprocessor-cache")
                 .help("show all preprocessor cache entries")
                 .action(ArgAction::SetTrue),
             flag_infer_long("stop-server")
                 .help("stop background server")
-                .action(ArgAction::SetTrue),
+                .value_name("port")
+                .num_args(0..=1)
+                .default_missing_value("None")
+                .action(ArgAction::Append),
             flag_infer_long_and_short("zero-stats")
                 .help("zero statistics counters")
                 .action(ArgAction::SetTrue),
@@ -268,12 +274,18 @@ pub fn try_parse() -> Result<Command> {
                     .cloned()
                     .expect("There is a default value");
                 Ok(Command::ShowStats(fmt, true))
-            } else if matches.get_flag("start-server") {
-                Ok(Command::StartServer)
+            } else if matches.contains_id("start-server") {
+                let port = matches
+                    .get_one::<String>("start-server")
+                    .and_then(|s| s.parse::<u16>().ok());
+                Ok(Command::StartServer(port))
             } else if matches.get_flag("debug-preprocessor-cache") {
                 Ok(Command::DebugPreprocessorCacheEntries)
-            } else if matches.get_flag("stop-server") {
-                Ok(Command::StopServer)
+            } else if matches.contains_id("stop-server") {
+                let port = matches
+                    .get_one::<String>("stop-server")
+                    .and_then(|s| s.parse::<u16>().ok());
+                Ok(Command::StopServer(port))
             } else if matches.get_flag("zero-stats") {
                 Ok(Command::ZeroStats)
             } else if matches.get_flag("dist-auth") {

diff --git a/src/commands.rs b/src/commands.rs
@@ -82,7 +82,10 @@ async fn read_server_startup_status<R: AsyncReadExt + Unpin>(
 /// Re-execute the current executable as a background server, and wait
 /// for it to start up.
 #[cfg(not(windows))]
-fn run_server_process(startup_timeout: Option<Duration>) -> Result<ServerStartup> {
+fn run_server_process(
+    startup_timeout: Option<Duration>,
+    _port: Option<u16>,
+) -> Result<ServerStartup> {
     trace!("run_server_process");
     let tempdir = tempfile::Builder::new().prefix("sccache").tempdir()?;
     let socket_path = tempdir.path().join("sock");
@@ -98,11 +101,14 @@ fn run_server_process(startup_timeout: Option<Duration>) -> Result<ServerStartup
         tokio::net::UnixListener::bind(&socket_path)?
     };
 
+    let port = _port.unwrap_or_else(get_port);
+
     let _child = process::Command::new(&exe_path)
         .current_dir(workdir)
         .env("SCCACHE_START_SERVER", "1")
         .env("SCCACHE_STARTUP_NOTIFY", &socket_path)
         .env("RUST_BACKTRACE", "1")
+        .env("SCCACHE_SERVER_PORT", port.to_string())
         .spawn()?;
 
     let startup = async move {
@@ -169,7 +175,10 @@ fn redirect_error_log(f: File) -> Result<()> {
 
 /// Re-execute the current executable as a background server.
 #[cfg(windows)]
-fn run_server_process(startup_timeout: Option<Duration>) -> Result<ServerStartup> {
+fn run_server_process(
+    startup_timeout: Option<Duration>,
+    _port: Option<u16>,
+) -> Result<ServerStartup> {
     use futures::StreamExt;
     use std::mem;
     use std::os::windows::ffi::OsStrExt;
@@ -190,6 +199,7 @@ fn run_server_process(startup_timeout: Option<Duration>) -> Result<ServerStartup
     let pipe_name = &format!(r"\\.\pipe\{}", Uuid::new_v4().as_simple());
 
     // Spawn a server which should come back and connect to us
+    let port = _port.unwrap_or_else(get_port);
     let exe_path = env::current_exe()?;
     let mut exe = OsStr::new(&exe_path)
         .encode_wide()
@@ -204,6 +214,10 @@ fn run_server_process(startup_timeout: Option<Duration>) -> Result<ServerStartup
                 OsString::from(&pipe_name),
             ),
             (OsString::from("RUST_BACKTRACE"), OsString::from("1")),
+            (
+                OsString::from("SCCACHE_SERVER_PORT"),
+                OsString::from(port.to_string()),
+            ),
         ];
         for (key, val) in env::vars_os().chain(extra_vars) {
             v.extend(
@@ -309,7 +323,7 @@ fn connect_or_start_server(
         {
             // If the connection was refused we probably need to start
             // the server.
-            match run_server_process(startup_timeout)? {
+            match run_server_process(startup_timeout, Some(port))? {
                 ServerStartup::Ok { port: actualport } => {
                     if port != actualport {
                         // bail as the next connect_with_retry will fail
@@ -662,11 +676,11 @@ pub fn run_command(cmd: Command) -> Result<i32> {
             }
             server::start_server(config, get_port())?;
         }
-        Command::StartServer => {
+        Command::StartServer(_port) => {
             trace!("Command::StartServer");
             println!("sccache: Starting the server...");
-            let startup =
-                run_server_process(startup_timeout).context("failed to start server process")?;
+            let startup = run_server_process(startup_timeout, _port)
+                .context("failed to start server process")?;
             match startup {
                 ServerStartup::Ok { port } => {
                     if port != DEFAULT_PORT {
@@ -678,10 +692,11 @@ pub fn run_command(cmd: Command) -> Result<i32> {
                 ServerStartup::Err { reason } => bail!("Server startup failed: {}", reason),
             }
         }
-        Command::StopServer => {
+        Command::StopServer(port) => {
             trace!("Command::StopServer");
             println!("Stopping sccache server...");
-            let server = connect_to_server(get_port()).context("couldn't connect to server")?;
+            let server = connect_to_server(port.unwrap_or_else(get_port))
+                .context("couldn't connect to server")?;
             let stats = request_shutdown(server)?;
             stats.print(false);
         }

diff --git a/src/test/tests.rs b/src/test/tests.rs
@@ -320,3 +320,29 @@ fn test_server_port_in_use() {
         s
     );
 }
+
+#[test]
+#[serial]
+// test fails intermittently on macos:
+// 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)
+        .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
+    );
+}