Allow conversion of `std::process::Command` into a `openssh::Command` given a `openssh::Session`.

[aalekhpatel07]

The std::process::Command and the openssh::Command APIs are fairly similar, so I wonder if it is reasonable to allow an arbitrary std::process::Command to be run over ssh.

Example

use std::process;
use openssh::{OverSsh, Session, KnownHosts};

#[tokio::main]
pub async fn main() {
    let session = Session::connect_mux("me@ssh.example.com", KnownHosts::Strict).await.unwrap();

    let ls = process::Command::new("ls")
        .arg("-l")
        .arg("-h")
        .over_ssh(&session)
        .expect("no env vars or current_dir are specified")
        .output()
        .await
        .unwrap();

    println!("{}", String::from_utf8_lossy(&ls.stdout));
}

[jonhoo]

This change is 

[aalekhpatel07]

wow thank you for your kind words and a quick review. I'll clean it up right away! Thanks for your time.

[NobodyXu]

Ehhh we now have a rustc ICE.

@aalekhpatel07 Would you like to open an issue in rust-lang/rust?

[codecov-commenter]

Codecov Report

Merging #112 (f30d34e) into master (3319a81) will increase coverage by 0.32%. The diff coverage is 87.50%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

| [Files Changed](https://app.codecov.io/gh/openssh-rust/openssh/pull/112?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [src/error.rs](https://app.codecov.io/gh/openssh-rust/openssh/pull/112?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2Vycm9yLnJz) | `89.16% <ø> (ø)` | | | [src/command.rs](https://app.codecov.io/gh/openssh-rust/openssh/pull/112?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbW1hbmQucnM=) | `89.92% <69.23%> (-8.08%)` | :arrow_down: | | [src/escape.rs](https://app.codecov.io/gh/openssh-rust/openssh/pull/112?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2VzY2FwZS5ycw==) | `100.00% <100.00%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/openssh-rust/openssh/pull/112/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

[aalekhpatel07]

Ehhh we now have a rustc ICE.

@aalekhpatel07 Would you like to open an issue in rust-lang/rust?

Haha for sure. Any preference for a name?

[NobodyXu]

@aalekhpatel07 Looks like some of the ICEs happens in mir borrow checking, so perhaps "mir borrow checking ICE in openssh-rust" will be a good name.

Also I recommend you to create a separate branch, containing the commit causing the ICEs just so it's easier for others to reproduce the ICEs.

[aalekhpatel07]

I believe the new escape(s: &[u8]) -> String should do the job for us. lmk

(I'm sorry I think I accidentally removed the review request for jonhoo).

[NobodyXu]

(I'm sorry I think I accidentally removed the review request for jonhoo).

No worries, I've re-requested it.

[NobodyXu]

Aha, the CI reliably failed with ICEs from rustc.

[aalekhpatel07]

okay how about these changes? (thank you for being patient.)

[NobodyXu]

okay how about these changes? (thank you for being patient.)

It's much better, but we would still need @jonhoo 's approval on this. I'm not entirely sure about escaping since the utf encoding is a mess, so I just want to be safe and ask @jonhoo to also review this.

[NobodyXu]

@aalekhpatel07 BTW, don't forget to create a new issue on rust-lang/rust and linked it to this. We would also have to waiting for the compiler guys to investigate and fix the issues.

Since we are on rust 1.67, not nightly channel, this might requires a new minor release.

I think this ICE might be a regression.

[NobodyXu]

@aalekhpatel07 Hmmm, let me create the bug report for this.

[NobodyXu]

@aalekhpatel07 I've created an ICE bug report on rust-lang/rust.

Note that the lint CI did work as intended and you need to fix all cargo-clippy warnings.

[aalekhpatel07]

They have a PR for that ICE already??

Wow! I'm blown away by how responsive, knowledgeable, and supportive the contributors are in the Rust ecosystem.

Thank you for all your help, @NobodyXu, for helping me improve the PR and develop my understanding of Cows and OsStrs in Rust.

[NobodyXu]

They have a PR for that ICE already??

Yes, https://github.com/rust-lang/rust/pull/107532

Though I have no idea on whether it will land in 1.68 or will they have a backport it to 1.67.1 since whether a backport will happen depends on the severity of the bug.

Thank you for all your help, @NobodyXu, for helping me improve the PR and develop my understanding of Cows and OsStrs in Rust.

No worries!

[aalekhpatel07]

one more set of changes as suggested in the review. please lmk if I missed something or misinterpreted a suggestion.

[jonhoo]

Looks like rustfmt is complaining, and somehow we're also hitting a compiler ICE? https://github.com/openssh-rust/openssh/actions/runs/4153289081/jobs/7184837200

[aalekhpatel07]

Really appreciate yours and @NobodyXu's precious time reviewing this! I was able to learn a few things from this review, so thank you! Now hopefully the other PR in shell-escape also lands soon.

[aalekhpatel07]

yep the compiler ICE was pointed out by @NobodyXu last week and I believe there was some work happening in rust-lang/rust#107532 to fix it. Rustfmt should be happy now though.

[jonhoo]

I'll hold off on merging this until the fix for the ICE lands then 👍 Nothing more needed from your end at this point :)

[NobodyXu]

Since the ICE is fixed and the CI has passed, I will merge this PR.