~soywod/pimalaya

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
3 2

[PATCH 1/2] fix: Fix `pipe()` to return exit code correctly

Details
Message ID
<20230221185145.90120-1-me@djha.skin>
DKIM signature
pass
Download raw message
Patch: +16 -3
Previously, this function didn't return exit code of the called
command; rather it returned the size of the output the command wrote to
stdout[1]. Now, this function returns the exit code, as originally
intended.

Technically, this breaks API, as the function signature type has
changed (`usize` became `i32`). However, reading through the code, it is
only used in the file `./src/domain/email/email.rs`. Because of type
inference, the code does not break.

1: https://doc.rust-lang.org/std/io/trait.Read.html#method.read_to_end
---
 CHANGELOG.md   |  1 +
 src/process.rs | 18 +++++++++++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 04e81a9..2ad6d07 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Fixed the process.rs' `pipe` function so it returns exit code correctly.
- Fixed date parsing using the [mail-parser] crate [#44].
- Fixed Cc addresses when replying all [#46].
- Clarified header/value trace logs [patch#2].
diff --git a/src/process.rs b/src/process.rs
index a294799..eb484d1 100644
--- a/src/process.rs
+++ b/src/process.rs
@@ -22,6 +22,10 @@ pub enum Error {
    SpawnProcessError(#[source] io::Error, String),
    #[error("cannot get standard input")]
    GetStdinError,
    #[error("cannot get exit status code")]
    GetExitStatusCodeAttemptFailedError(#[source] io::Error, String),
    #[error("exit status code unavailable")]
    GetExitStatusCodeNotAvailableError,
    #[error("cannot write data to standard input")]
    WriteStdinError(#[source] io::Error),
    #[error("cannot get standard output")]
@@ -35,7 +39,6 @@ pub enum Error {
/// Runs the given command and returns the output as UTF8 string.
pub fn run(cmd: &str, input: &[u8]) -> Result<Vec<u8>> {
    let mut output = input.to_owned();

    for cmd in cmd.split('|') {
        debug!("running command: {}", cmd);
        (output, _) = pipe(cmd.trim(), &output)?;
@@ -45,7 +48,7 @@ pub fn run(cmd: &str, input: &[u8]) -> Result<Vec<u8>> {
}

/// Runs the given command in a pipeline and returns the raw output.
pub fn pipe(cmd: &str, input: &[u8]) -> Result<(Vec<u8>, usize)> {
pub fn pipe(cmd: &str, input: &[u8]) -> Result<(Vec<u8>, i32)> {
    let mut output = Vec::new();

    let windows = cfg!(target_os = "windows")
@@ -53,7 +56,7 @@ pub fn pipe(cmd: &str, input: &[u8]) -> Result<(Vec<u8>, usize)> {
            .map(|env| env.starts_with("MINGW"))
            .unwrap_or_default());

    let pipeline = if windows {
    let mut pipeline = if windows {
        Command::new("cmd")
            .args(&["/C", cmd])
            .stdin(Stdio::piped())
@@ -73,11 +76,20 @@ pub fn pipe(cmd: &str, input: &[u8]) -> Result<(Vec<u8>, usize)> {

    pipeline
        .stdin
        .as_mut()
        .ok_or_else(|| Error::GetStdinError)?
        .write_all(input)
        .map_err(Error::WriteStdinError)?;

    let exit_code = pipeline
        .wait()
        .map_err(|err| {
            Error::GetExitStatusCodeAttemptFailedError(err, cmd.to_string())
        })?
        .code()
        .ok_or_else(|| Error::GetExitStatusCodeNotAvailableError)?;

    pipeline
        .stdout
        .ok_or_else(|| Error::GetStdoutError)?
        .read_to_end(&mut output)
-- 
2.39.1

[PATCH 2/2] feat: Add message for non-zero exits in pipe()

Details
Message ID
<20230221185145.90120-2-me@djha.skin>
In-Reply-To
<20230221185145.90120-1-me@djha.skin> (view parent)
DKIM signature
pass
Download raw message
Patch: +10 -2
Add a warning message for non-zero exit codes in pipe(). On most
platforms, a non-zero exit status indicates a failure, and even on
windows, it strongly suggests one.

This patch has direct bearing on the himalaya command `himalaya watch`.
This message will show up in the logs of that command, allowing the
operator to understand whether or not the watch command failed or if it
needs debugging.
---
 src/process.rs | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/process.rs b/src/process.rs
index eb484d1..bd08558 100644
--- a/src/process.rs
+++ b/src/process.rs
@@ -3,7 +3,8 @@
//! This module contains cross platform helpers around the
//! `std::process` crate.

use log::debug;
use log::{debug, warn};

use std::{
    env,
    io::{self, prelude::*},
@@ -39,9 +40,16 @@ pub enum Error {
/// Runs the given command and returns the output as UTF8 string.
pub fn run(cmd: &str, input: &[u8]) -> Result<Vec<u8>> {
    let mut output = input.to_owned();
    let mut exit_code;
    for cmd in cmd.split('|') {
        debug!("running command: {}", cmd);
        (output, _) = pipe(cmd.trim(), &output)?;
        (output, exit_code) = pipe(cmd.trim(), &output)?;
        if exit_code != 0 {
            warn!(
                "command `{}` returned non-zero status exit code `{}`",
                cmd, exit_code
            );
        }
    }

    Ok(output)
-- 
2.39.1
Details
Message ID
<87ttzcu3qr.fsf@posteo.net>
In-Reply-To
<20230221185145.90120-1-me@djha.skin> (view parent)
DKIM signature
pass
Download raw message
Patch applied [6e054a1], thank you!

[6e054a1]: https://git.sr.ht/~soywod/himalaya-lib/commit/6e054a112ecf831e31d741cce0aa72809c8676db

Re: [PATCH 2/2] feat: Add message for non-zero exits in pipe()

Details
Message ID
<87sfewu34w.fsf@posteo.net>
In-Reply-To
<20230221185145.90120-2-me@djha.skin> (view parent)
DKIM signature
pass
Download raw message
Patched applied [c7228a0], thank you!

[c7228a0]: https://git.sr.ht/~soywod/himalaya-lib/commit/c7228a045c0b435e07cdf634178cc3ad06c9acec
Reply to thread Export thread (mbox)