`compile_rustfmt` rewrite

[benluiwj]

[ytmimi]

I've been thinking about what the best return type is for build_rustfmt_from_src, and this is what I've come up with. Happy to change the name, but ultimately we need to associate the LD_LIBRARY_PATH with the binary's path, and I think this would work fairly well:

pub struct RustfmtRunner {
    ld_library_path: String,
    binary_path: PathBuf,
}

impl RustfmtRunner {
    fn run(&self, args: &[&str]) -> std::io::Result<std::process::Output> {
        Command::new(&self.binary_path)
            .env("LD_LIBRARY_PATH", &self.ld_library_path)
            .args(args)
            .output()
    }
}

pub fn build_rustfmt_from_src(output_path: PathBuf) -> Result<RustfmtRunner, CheckDiffError> {
    let ld_library_path = get_ld_lib_path()?;

    Command::new("cargo")
        .args(["build", "-q", "--release", "--bin", "rustfmt"])
        .output()?;

    std::fs::copy("target/release/rustfmt", &output_path)?;

    Ok(RustfmtRunner {
        ld_library_path,
        binary_path: output_path,
    })
}

[benluiwj]

@ytmimi i think the approach on build_rustfmt_from_src with RustfmtRunner is promising.

I was wondering if the error for run be an IO error or do you think its better to have a custom error for RustfmtRunner? I was thinking maybe something like GitError but for RustfmtRunner. What do you think?

Also, instead of returning Result<Output>, we return something like Result<(), RustfmtRunnerError>. How do you feel about this?

[ytmimi]

I was wondering if the error for run be an IO error or do you think its better to have a custom error for RustfmtRunner? I was thinking maybe something like GitError but for RustfmtRunner. What do you think?

Also, instead of returning std::io::Result<std::process::Output>, we return something like Result<(), RustfmtRunnerError>. How do you feel about this?

Personally, I don't think we need to get extremely specific with the errors. At the end of the day we're calling Command::output, and I think it's fine to just return std::io::Result<std::process::Output>.

If we ever need to convert the IO error we should be able to do that easily from the caller with the ? operator. Also, having access to the Output is going to be useful since we'll want access to stdout and stderr once we start formatting files with rustfmt.