Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
doy committed Dec 3, 2023
1 parent 1007cac commit 1895e6c
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 65 deletions.
12 changes: 6 additions & 6 deletions .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
steps:
- uses: actions/checkout@v3
- uses: dtolnay/rust-toolchain@stable
- run: cargo build --all-targets
- run: cargo build --all-targets --all-features

This comment has been minimized.

Copy link
@mightyiam

mightyiam Dec 10, 2023

Seems like this can already be committed to main, regardless.

This comment has been minimized.

Copy link
@mightyiam
build-musl:
runs-on: ubuntu-latest
steps:
Expand All @@ -20,19 +20,19 @@ jobs:
- uses: dtolnay/rust-toolchain@stable
with:
targets: x86_64-unknown-linux-musl
- run: TARGET_CC=clang-11 TARGET_AR=llvm-ar-11 cargo build --all-targets --target x86_64-unknown-linux-musl
- run: TARGET_CC=clang-11 TARGET_AR=llvm-ar-11 cargo build --all-targets --all-features --target x86_64-unknown-linux-musl

This comment has been minimized.

Copy link
@warren2k

warren2k Dec 10, 2023

Seems like this can be already committed to main, regardless.

This comment has been minimized.

Copy link
@mightyiam
build-macos:
runs-on: macos-latest
steps:
- uses: actions/checkout@v3
- uses: dtolnay/rust-toolchain@stable
- run: cargo build --all-targets
- run: cargo build --all-targets --all-features

This comment has been minimized.

Copy link
@mightyiam
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: dtolnay/rust-toolchain@stable
- run: cargo test
- run: cargo test --all-targets --all-features

This comment has been minimized.

Copy link
@mightyiam

mightyiam Dec 10, 2023

Regarding --all-features, #9 .

--all-targets seems unnecessary, according to the book.

test-musl:
runs-on: ubuntu-latest
steps:
Expand All @@ -41,13 +41,13 @@ jobs:
- uses: dtolnay/rust-toolchain@stable
with:
targets: x86_64-unknown-linux-musl
- run: TARGET_CC=clang-11 TARGET_AR=llvm-ar-11 cargo test --target x86_64-unknown-linux-musl
- run: TARGET_CC=clang-11 TARGET_AR=llvm-ar-11 cargo test --target x86_64-unknown-linux-musl --all-targets --all-features

This comment has been minimized.

Copy link
@mightyiam

mightyiam Dec 10, 2023

Regarding --all-features, #9 .

--all-targets seems unnecessary, according to the book.

test-macos:
runs-on: macos-latest
steps:
- uses: actions/checkout@v3
- uses: dtolnay/rust-toolchain@stable
- run: cargo test
- run: cargo test --all-targets --all-features

This comment has been minimized.

Copy link
@mightyiam

mightyiam Dec 10, 2023

Regarding --all-features, #9 .

--all-targets seems unnecessary, according to the book.

lint:
runs-on: ubuntu-latest
steps:
Expand Down
5 changes: 3 additions & 2 deletions src/pty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ impl Pty {
/// unable to put it into non-blocking mode.
pub fn new() -> crate::Result<Self> {
let pty = crate::sys::Pty::open()?;
pty.set_nonblocking()?;
Ok(Self(tokio::io::unix::AsyncFd::new(pty)?))
}

Expand All @@ -35,7 +34,9 @@ impl Pty {
/// Returns an error if the device node to open could not be determined,
/// or if the device node could not be opened.
pub fn pts(&self) -> crate::Result<Pts> {
Ok(Pts(self.0.get_ref().pts()?))
let pts = Pts(self.0.get_ref().pts()?);
self.0.get_ref().set_nonblocking()?;
Ok(pts)
}

/// Splits a `Pty` into a read half and a write half, which can be used to
Expand Down
27 changes: 17 additions & 10 deletions tests/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ fn test_cat_blocking() {
use std::io::Write as _;

let mut pty = pty_process::blocking::Pty::new().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
let mut child = pty_process::blocking::Command::new("cat")
.spawn(&pty.pts().unwrap())
.unwrap();
let mut child = {
let pts = pty.pts().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
pty_process::blocking::Command::new("cat")
.spawn(&pts)
.unwrap()
};

pty.write_all(b"foo\n").unwrap();

Expand All @@ -28,9 +31,11 @@ async fn test_cat_async() {
use tokio::io::AsyncWriteExt as _;

let mut pty = pty_process::Pty::new().unwrap();
let pts = pty.pts().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
let mut child = pty_process::Command::new("cat").spawn(&pts).unwrap();
let mut child = {
let pts = pty.pts().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
pty_process::Command::new("cat").spawn(&pts).unwrap()
};

let (pty_r, mut pty_w) = pty.split();

Expand All @@ -51,9 +56,11 @@ async fn test_yes_async() {
use tokio::io::AsyncReadExt as _;

let mut pty = pty_process::Pty::new().unwrap();
let pts = pty.pts().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
let mut child = pty_process::Command::new("yes").spawn(&pts).unwrap();
let mut child = {
let pts = pty.pts().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
pty_process::Command::new("yes").spawn(&pts).unwrap()
};

let mut buf = [0u8; 3];

Expand Down
33 changes: 24 additions & 9 deletions tests/behavior.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
mod helpers;

// macos doesn't appear to support spawning a second process onto a pty which
// already had a process spawned onto it
#[cfg(not(target_os = "macos"))]
#[test]
fn test_multiple() {
let pty = pty_process::blocking::Pty::new().unwrap();
Expand Down Expand Up @@ -28,6 +31,9 @@ fn test_multiple() {
assert_eq!(status.code().unwrap(), 0);
}

// macos doesn't appear to support spawning a second process onto a pty which
// already had a process spawned onto it
#[cfg(not(target_os = "macos"))]
#[cfg(feature = "async")]
#[tokio::test]
async fn test_multiple_async() {
Expand Down Expand Up @@ -60,6 +66,9 @@ async fn test_multiple_async() {
assert_eq!(status.code().unwrap(), 0);
}

// macos doesn't appear to support spawning a second process onto a pty which
// already had a process spawned onto it
#[cfg(not(target_os = "macos"))]
#[test]
fn test_multiple_configured() {
use std::io::BufRead as _;
Expand Down Expand Up @@ -129,6 +138,9 @@ fn test_multiple_configured() {
assert_eq!(status.code().unwrap(), 0);
}

// macos doesn't appear to support spawning a second process onto a pty which
// already had a process spawned onto it
#[cfg(not(target_os = "macos"))]
#[cfg(feature = "async")]
#[tokio::test]
async fn test_multiple_configured_async() {
Expand Down Expand Up @@ -246,16 +258,18 @@ async fn test_controlling_terminal_async() {
use futures::stream::StreamExt as _;

let mut pty = pty_process::Pty::new().unwrap();
let pts = pty.pts().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
let mut child = {
let pts = pty.pts().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
pty_process::Command::new("perl")
.arg(
"-Eopen my $fh, '<', '/dev/tty' or die; \
if (-t $fh) { say 'true' } else { say 'false' }",
)
.spawn(&pts)
.unwrap()
};
let (pty_r, _) = pty.split();
let mut child = pty_process::Command::new("perl")
.arg(
"-Eopen my $fh, '<', '/dev/tty' or die; \
if (-t $fh) { say 'true' } else { say 'false' }",
)
.spawn(&pts)
.unwrap();

let mut output = helpers::output_async(pty_r);
assert_eq!(output.next().await.unwrap(), "true\r\n");
Expand Down Expand Up @@ -303,6 +317,7 @@ async fn test_session_leader_async() {
assert_eq!(status.code().unwrap(), 0);
}

#[cfg(not(target_os = "macos"))]
fn pipe() -> (std::os::fd::OwnedFd, std::os::fd::OwnedFd) {
use std::os::fd::FromRawFd as _;

Expand Down
11 changes: 7 additions & 4 deletions tests/fds_async.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ mod helpers;
fn test_fds_async() {
use futures::stream::StreamExt as _;

check_open_fds(&[0, 1, 2]);
let mut expected = String::new();
for fd in get_open_fds() {
expected.push_str(&format!("{}", fd));
}

let rt = tokio::runtime::Builder::new_multi_thread()
.enable_all()
Expand All @@ -31,7 +34,7 @@ fn test_fds_async() {

let (pty_r, _) = pty.split();
let mut output = helpers::output_async(pty_r);
assert_eq!(output.next().await.unwrap(), "012\r\n");
assert_eq!(output.next().await.unwrap(), format!("{expected}\r\n"));

let status = child.wait().await.unwrap();
assert_eq!(status.code().unwrap(), 0);
Expand All @@ -56,7 +59,7 @@ fn test_fds_async() {

let (pty_r, _) = pty.split();
let mut output = helpers::output_async(pty_r);
assert_eq!(output.next().await.unwrap(), "012\r\n");
assert_eq!(output.next().await.unwrap(), format!("{expected}\r\n"));

let status = child.wait().await.unwrap();
assert_eq!(status.code().unwrap(), 0);
Expand All @@ -81,7 +84,7 @@ fn test_fds_async() {

let (pty_r, _) = pty.split();
let mut output = helpers::output_async(pty_r);
assert_eq!(output.next().await.unwrap(), "012\r\n");
assert_eq!(output.next().await.unwrap(), format!("{expected}\r\n"));

let status = child.wait().await.unwrap();
assert_eq!(status.code().unwrap(), 0);
Expand Down
21 changes: 15 additions & 6 deletions tests/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ fn test_pipe_basic() {
child_from.args(["1", "10"]);
child_from.stdout(std::process::Stdio::from(write_fd));

let mut child_to = std::process::Command::new("tac");
let mut child_to = std::process::Command::new("perl");
child_to
.arg("-nle")
.arg("unshift @a, $_; END { print for @a }");
child_to.stdin(std::process::Stdio::from(read_fd));
child_to.stdout(std::process::Stdio::piped());

Expand All @@ -33,15 +36,18 @@ fn test_pipe_blocking() {

let mut pty_to = pty_process::blocking::Pty::new().unwrap();
let pts_to = pty_to.pts().unwrap();
let mut cmd_to = pty_process::blocking::Command::new("tac");
let mut cmd_to = pty_process::blocking::Command::new("perl");
cmd_to
.arg("-nle")
.arg("unshift @a, $_; END { print for @a }");
cmd_to.stdin(std::process::Stdio::from(read_fd));
let mut child_to = cmd_to.spawn(&pts_to).unwrap();

assert!(child_from.wait().unwrap().success());
drop(cmd_from);

// wait for the `tac` process to finish generating output (we don't really
// have a good way to detect when that happens)
// wait for the `perl` process to finish generating output (we don't
// really have a good way to detect when that happens)
std::thread::sleep(std::time::Duration::from_millis(100));

let mut buf = [0u8; 1024];
Expand Down Expand Up @@ -71,14 +77,17 @@ async fn test_pipe_async() {

let mut pty_to = pty_process::Pty::new().unwrap();
let pts_to = pty_to.pts().unwrap();
let mut cmd_to = pty_process::Command::new("tac");
let mut cmd_to = pty_process::Command::new("perl");
cmd_to
.arg("-nle")
.arg("unshift @a, $_; END { print for @a }");
cmd_to.stdin(std::process::Stdio::from(read_fd));
let mut child_to = cmd_to.spawn(&pts_to).unwrap();

assert!(child_from.wait().await.unwrap().success());
drop(cmd_from);

// wait for the `tac` process to finish generating output (we
// wait for the `perl` process to finish generating output (we
// don't really have a good way to detect when that happens)
tokio::time::sleep(std::time::Duration::from_millis(100)).await;

Expand Down
24 changes: 14 additions & 10 deletions tests/split.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ async fn test_split() {
use tokio::io::AsyncWriteExt as _;

let mut pty = pty_process::Pty::new().unwrap();
let pts = pty.pts().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
let mut cmd = pty_process::Command::new("perl");
cmd.args(["-plE", "BEGIN { $SIG{WINCH} = sub { say 'WINCH' } }"]);
let mut child = cmd.spawn(&pts).unwrap();
let mut child = {
let pts = pty.pts().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
let mut cmd = pty_process::Command::new("perl");
cmd.args(["-plE", "BEGIN { $SIG{WINCH} = sub { say 'WINCH' } }"]);
cmd.spawn(&pts).unwrap()
};

{
pty.write_all(b"foo\n").await.unwrap();
Expand Down Expand Up @@ -46,11 +48,13 @@ async fn test_into_split() {
use tokio::io::{AsyncBufReadExt as _, AsyncWriteExt as _};

let mut pty = pty_process::Pty::new().unwrap();
let pts = pty.pts().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
let mut cmd = pty_process::Command::new("perl");
cmd.args(["-plE", "BEGIN { $SIG{WINCH} = sub { say 'WINCH' } }"]);
let mut child = cmd.spawn(&pts).unwrap();
let mut child = {
let pts = pty.pts().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
let mut cmd = pty_process::Command::new("perl");
cmd.args(["-plE", "BEGIN { $SIG{WINCH} = sub { say 'WINCH' } }"]);
cmd.spawn(&pts).unwrap()
};

{
pty.write_all(b"foo\n").await.unwrap();
Expand Down
40 changes: 22 additions & 18 deletions tests/winch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,17 @@ fn test_winch_std() {
use std::io::Write as _;

let mut pty = pty_process::blocking::Pty::new().unwrap();
let pts = pty.pts().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
let mut child = pty_process::blocking::Command::new("perl")
.args([
"-E",
"$|++; $SIG{WINCH} = sub { say 'WINCH' }; say 'started'; <>",
])
.spawn(&pts)
.unwrap();
let mut child = {
let pts = pty.pts().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
pty_process::blocking::Command::new("perl")
.args([
"-E",
"$|++; $SIG{WINCH} = sub { say 'WINCH' }; say 'started'; <>",
])
.spawn(&pts)
.unwrap()
};

let mut output = helpers::output(&pty);
assert_eq!(output.next().unwrap(), "started\r\n");
Expand All @@ -33,15 +35,17 @@ async fn test_winch_async() {
use tokio::io::AsyncWriteExt as _;

let mut pty = pty_process::Pty::new().unwrap();
let pts = pty.pts().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
let mut child = pty_process::Command::new("perl")
.args(&[
"-E",
"$|++; $SIG{WINCH} = sub { say 'WINCH' }; say 'started'; <>",
])
.spawn(&pts)
.unwrap();
let mut child = {
let pts = pty.pts().unwrap();
pty.resize(pty_process::Size::new(24, 80)).unwrap();
pty_process::Command::new("perl")
.args([
"-E",
"$|++; $SIG{WINCH} = sub { say 'WINCH' }; say 'started'; <>",
])
.spawn(&pts)
.unwrap()
};

let (pty_r, mut pty_w) = pty.split();
let mut output = helpers::output_async(pty_r);
Expand Down

1 comment on commit 1895e6c

@mightyiam
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#9 is all we got done this weekend. We hope that is merged by next weekend, so that we could proceed in this direction. We love contributing upstream.

Please sign in to comment.