From 2a429e59bf4fc0b06a1b6496e94b45352a28b456 Mon Sep 17 00:00:00 2001 From: brianheineman Date: Tue, 16 Jul 2024 18:52:09 -0700 Subject: [PATCH 1/4] fix: improve commands on windows to return stdout and stderr --- postgresql_commands/src/traits.rs | 96 +++++++++++++++---------------- 1 file changed, 47 insertions(+), 49 deletions(-) diff --git a/postgresql_commands/src/traits.rs b/postgresql_commands/src/traits.rs index 3c173a6..718cadc 100644 --- a/postgresql_commands/src/traits.rs +++ b/postgresql_commands/src/traits.rs @@ -1,7 +1,9 @@ +use std::env; use crate::error::{Error, Result}; use std::ffi::{OsStr, OsString}; use std::fmt::Debug; use std::path::PathBuf; +use std::process::ExitStatus; use std::time::Duration; use tracing::debug; @@ -140,46 +142,41 @@ impl CommandExecutor for std::process::Command { /// Execute the command and return the stdout and stderr fn execute(&mut self) -> Result<(String, String)> { debug!("Executing command: {}", self.to_command_string()); - #[cfg(not(target_os = "windows"))] - { - let output = self.output()?; - let stdout = String::from_utf8_lossy(&output.stdout).into_owned(); - let stderr = String::from_utf8_lossy(&output.stderr).into_owned(); - - debug!( - "Result: {}\nstdout: {}\nstderr: {}", - output - .status - .code() - .map_or("None".to_string(), |c| c.to_string()), - stdout, - stderr - ); - - if output.status.success() { - Ok((stdout, stderr)) - } else { - Err(Error::CommandError { stdout, stderr }) - } - } - - // TODO: Processes can hang on Windows when attempting to get stdout/stderr using code - // that works for Linux/MacOS; this implementation should be updated to retrieve the - // values of stdout/stderr without hanging - #[cfg(target_os = "windows")] - { + let program = self.get_program().to_string_lossy().to_string(); + let stdout: String; + let stderr: String; + let status: ExitStatus; + + if env::consts::OS == "windows" && program.as_str().ends_with("pg_ctl") { + // TODO: Processes can hang on Windows when attempting to get stdout/stderr using code + // that works for Linux/MacOS; this implementation should be updated to retrieve the + // values of stdout/stderr without hanging let mut process = self .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) .spawn()?; - let status = process.wait()?; - let stdout = String::new(); - let stderr = String::new(); - if status.success() { - Ok((stdout, stderr)) - } else { - Err(Error::CommandError { stdout, stderr }) - } + stdout = String::new(); + stderr = String::new(); + status = process.wait()?; + } else { + let output = self.output()?; + stdout = String::from_utf8_lossy(&output.stdout).into_owned(); + stderr = String::from_utf8_lossy(&output.stderr).into_owned(); + status = output.status; + } + debug!( + "Result: {}\nstdout: {}\nstderr: {}", + status + .code() + .map_or("None".to_string(), |c| c.to_string()), + stdout, + stderr + ); + + if status.success() { + Ok((stdout, stderr)) + } else { + Err(Error::CommandError { stdout, stderr }) } } } @@ -194,19 +191,20 @@ impl AsyncCommandExecutor for tokio::process::Command { Some(duration) => tokio::time::timeout(duration, self.output()).await?, None => self.output().await, }?; - - #[cfg(not(target_os = "windows"))] - let stdout = String::from_utf8_lossy(&output.stdout).into_owned(); - #[cfg(not(target_os = "windows"))] - let stderr = String::from_utf8_lossy(&output.stderr).into_owned(); - - // TODO: Processes can hang on Windows when attempting to get stdout/stderr using code - // that works for Linux/MacOS; this implementation should be updated to retrieve the - // values of stdout/stderr without hanging - #[cfg(target_os = "windows")] - let stdout = String::new(); - #[cfg(target_os = "windows")] - let stderr = String::new(); + let program = self.get_program().to_string_lossy().to_string(); + let stdout: String; + let stderr: String; + + if std::env::OS == "windows" && program.as_str().ends_with("pg_ctl") { + // TODO: Processes can hang on Windows when attempting to get stdout/stderr using code + // that works for Linux/MacOS; this implementation should be updated to retrieve the + // values of stdout/stderr without hanging + stdout = String::new(); + stderr = String::new(); + } else { + stdout = String::from_utf8_lossy(&output.stdout).into_owned(); + stderr = String::from_utf8_lossy(&output.stderr).into_owned(); + } debug!( "Result: {}\nstdout: {}\nstderr: {}", From fa15224faa32cf8199e99c63c76a2d9900a10046 Mon Sep 17 00:00:00 2001 From: brianheineman Date: Tue, 16 Jul 2024 20:08:19 -0600 Subject: [PATCH 2/4] fix: correct linting errors --- postgresql_commands/src/traits.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/postgresql_commands/src/traits.rs b/postgresql_commands/src/traits.rs index 718cadc..93b807d 100644 --- a/postgresql_commands/src/traits.rs +++ b/postgresql_commands/src/traits.rs @@ -1,5 +1,5 @@ -use std::env; use crate::error::{Error, Result}; +use std::env::consts::OS; use std::ffi::{OsStr, OsString}; use std::fmt::Debug; use std::path::PathBuf; @@ -147,7 +147,7 @@ impl CommandExecutor for std::process::Command { let stderr: String; let status: ExitStatus; - if env::consts::OS == "windows" && program.as_str().ends_with("pg_ctl") { + if OS == "windows" && program.as_str().ends_with("pg_ctl") { // TODO: Processes can hang on Windows when attempting to get stdout/stderr using code // that works for Linux/MacOS; this implementation should be updated to retrieve the // values of stdout/stderr without hanging @@ -166,9 +166,7 @@ impl CommandExecutor for std::process::Command { } debug!( "Result: {}\nstdout: {}\nstderr: {}", - status - .code() - .map_or("None".to_string(), |c| c.to_string()), + status.code().map_or("None".to_string(), |c| c.to_string()), stdout, stderr ); @@ -191,11 +189,11 @@ impl AsyncCommandExecutor for tokio::process::Command { Some(duration) => tokio::time::timeout(duration, self.output()).await?, None => self.output().await, }?; - let program = self.get_program().to_string_lossy().to_string(); + let program = self.as_std().get_program().to_string_lossy().to_string(); let stdout: String; let stderr: String; - if std::env::OS == "windows" && program.as_str().ends_with("pg_ctl") { + if OS == "windows" && program.as_str().ends_with("pg_ctl") { // TODO: Processes can hang on Windows when attempting to get stdout/stderr using code // that works for Linux/MacOS; this implementation should be updated to retrieve the // values of stdout/stderr without hanging From af2f21224cfedaf5058b4c565d5f3b2d2fe82005 Mon Sep 17 00:00:00 2001 From: brianheineman Date: Tue, 16 Jul 2024 20:13:03 -0600 Subject: [PATCH 3/4] build: remove clear caches action --- .github/workflows/clear-caches.yml | 22 ---------------------- postgresql_commands/src/traits.rs | 8 ++------ 2 files changed, 2 insertions(+), 28 deletions(-) delete mode 100644 .github/workflows/clear-caches.yml diff --git a/.github/workflows/clear-caches.yml b/.github/workflows/clear-caches.yml deleted file mode 100644 index 3078fe2..0000000 --- a/.github/workflows/clear-caches.yml +++ /dev/null @@ -1,22 +0,0 @@ -# Clearing caches regularly takes care of caches growing to problematic size over time - -name: Clear caches - -on: - schedule: - - cron: '0 4 * * MON' - workflow_dispatch: - -permissions: - contents: read - -jobs: - clear-caches: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - - name: Clear all caches - run: gh cache delete --all - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/postgresql_commands/src/traits.rs b/postgresql_commands/src/traits.rs index 93b807d..6c12f36 100644 --- a/postgresql_commands/src/traits.rs +++ b/postgresql_commands/src/traits.rs @@ -148,9 +148,7 @@ impl CommandExecutor for std::process::Command { let status: ExitStatus; if OS == "windows" && program.as_str().ends_with("pg_ctl") { - // TODO: Processes can hang on Windows when attempting to get stdout/stderr using code - // that works for Linux/MacOS; this implementation should be updated to retrieve the - // values of stdout/stderr without hanging + // The pg_ctl process can hang on Windows when attempting to get stdout/stderr. let mut process = self .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) @@ -194,9 +192,7 @@ impl AsyncCommandExecutor for tokio::process::Command { let stderr: String; if OS == "windows" && program.as_str().ends_with("pg_ctl") { - // TODO: Processes can hang on Windows when attempting to get stdout/stderr using code - // that works for Linux/MacOS; this implementation should be updated to retrieve the - // values of stdout/stderr without hanging + // The pg_ctl process can hang on Windows when attempting to get stdout/stderr. stdout = String::new(); stderr = String::new(); } else { From 14863d5a995888925834185012d6e2147c4595cb Mon Sep 17 00:00:00 2001 From: brianheineman Date: Tue, 16 Jul 2024 20:40:48 -0600 Subject: [PATCH 4/4] test: correct windows test failure --- postgresql_commands/src/traits.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/postgresql_commands/src/traits.rs b/postgresql_commands/src/traits.rs index 6c12f36..dbd6df5 100644 --- a/postgresql_commands/src/traits.rs +++ b/postgresql_commands/src/traits.rs @@ -363,10 +363,7 @@ mod test { command.args(["/C", "echo foo"]); let (stdout, stderr) = command.execute()?; - #[cfg(not(target_os = "windows"))] assert!(stdout.starts_with("foo")); - #[cfg(target_os = "windows")] - assert!(stdout.is_empty()); assert!(stderr.is_empty()); Ok(()) }