Skip to content

Commit

Permalink
fix disable echoctl (#7109)
Browse files Browse the repository at this point in the history
### Description

Disable ECHOCTL for the pseudo terminal that we start:

> ECHOCTL
              (not in POSIX) If ECHO is also set, terminal special
              characters other than TAB, NL, START, and STOP are echoed
              as ^X, where X is the character with ASCII code 0x40
              greater than the special character.  For example,
              character 0x08 (BS) is echoed as ^H.

This will remove the `^D` that gets generated from the EOT when using a
PTY. Using PTY still does generate a leading `\n` as EOT is only
respected if it immediately follows a newline. `portable_pty` tries to
prevent failed EOT by emitting a `\nEOT` on drop in case end users
didn't end the last write with a newline.

### Testing Instructions

Made existing child process tests check for exact output modulo
whitespace/non-printing chars.

Closes TURBO-2160
  • Loading branch information
chris-olszewski committed Jan 26, 2024
1 parent 528cd15 commit 466258a
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 8 deletions.
1 change: 1 addition & 0 deletions crates/turborepo-lib/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![feature(assert_matches)]
#![feature(box_patterns)]
#![feature(byte_slice_trim_ascii)]
#![feature(error_generic_member_access)]
#![feature(hash_extract_if)]
#![feature(option_get_or_insert_default)]
Expand Down
41 changes: 33 additions & 8 deletions crates/turborepo-lib/src/process/child.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,26 @@ impl ChildHandle {
let controller = pair.master;
let receiver = pair.slave;

#[cfg(unix)]
{
use nix::sys::termios;
if let Some((file_desc, mut termios)) = controller
.as_raw_fd()
.and_then(|fd| Some(fd).zip(termios::tcgetattr(fd).ok()))
{
// We unset ECHOCTL to disable rendering of the closing of stdin
// as ^D
termios.local_flags &= !nix::sys::termios::LocalFlags::ECHOCTL;
if let Err(e) = nix::sys::termios::tcsetattr(
file_desc,
nix::sys::termios::SetArg::TCSANOW,
&termios,
) {
debug!("unable to unset ECHOCTL: {e}");
}
}
}

let child = receiver
.spawn_command(command)
.map_err(|err| match err.downcast() {
Expand Down Expand Up @@ -762,6 +782,7 @@ mod test {
const STARTUP_DELAY: Duration = Duration::from_millis(500);
// We skip testing PTY usage on Windows
const TEST_PTY: bool = !cfg!(windows);
const EOT: char = '\u{4}';

fn find_script_dir() -> AbsoluteSystemPathBuf {
let cwd = AbsoluteSystemPathBuf::cwd().unwrap();
Expand Down Expand Up @@ -845,8 +866,10 @@ mod test {
};

let output_str = String::from_utf8(output).expect("Failed to parse stdout");
let trimmed_output = output_str.trim();
let trimmed_output = trimmed_output.strip_prefix(EOT).unwrap_or(trimmed_output);

assert!(output_str.contains("hello world"), "got: {}", output_str);
assert_eq!(trimmed_output, "hello world");
}

child.wait().await;
Expand Down Expand Up @@ -884,8 +907,10 @@ mod test {
};

let output_str = String::from_utf8(output).expect("Failed to parse stdout");
let trimmed_out = output_str.trim();
let trimmed_out = trimmed_out.strip_prefix(EOT).unwrap_or(trimmed_out);

assert!(output_str.contains(input), "got: {}", output_str);
assert!(trimmed_out.contains(input), "got: {}", trimmed_out);

child.wait().await;

Expand Down Expand Up @@ -1031,8 +1056,10 @@ mod test {
let exit = child.wait_with_piped_outputs(&mut out).await.unwrap();

let out = String::from_utf8(out).unwrap();
let trimmed_out = out.trim();
let trimmed_out = trimmed_out.strip_prefix(EOT).unwrap_or(trimmed_out);

assert!(out.contains("hello world"), "got: {}", out);
assert_eq!(trimmed_out, "hello world");
assert_matches!(exit, Some(ChildExit::Finished(Some(0))));
}

Expand Down Expand Up @@ -1073,11 +1100,9 @@ mod test {
let exit = child.wait_with_piped_outputs(&mut out).await.unwrap();

let expected = &[0, 159, 146, 150];
assert!(
out.windows(4).any(|actual| actual == expected),
"got: {:?}",
out
);
let trimmed_out = out.trim_ascii();
let trimmed_out = trimmed_out.strip_prefix(&[4]).unwrap_or(trimmed_out);
assert_eq!(trimmed_out, expected);
assert_matches!(exit, Some(ChildExit::Finished(Some(0))));
}

Expand Down

1 comment on commit 466258a

@vercel
Copy link

@vercel vercel bot commented on 466258a Jan 26, 2024

Choose a reason for hiding this comment

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

Please sign in to comment.