summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMatt Mastracci <matthew@mastracci.com>2023-08-03 14:04:37 -0600
committerGitHub <noreply@github.com>2023-08-03 14:04:37 -0600
commit0f07dc95f130b9ace00ad98f1b2a3f5c34662e4a (patch)
tree13a2bd036e3f671bf6a0676d4fd16c075b7581f5
parent6ba245fe2570b29e35a4fd296a196a58870b1e3c (diff)
chore: fix pty support on Macs (#20037)
Many of the CI tests have been failing on my M2 Pro mac (Ventura 13.4) when running inside of a vscode terminal (a strange `ENOTTY` error). This modifies the pty-handling code to use libc directly rather than the older pty library that appears mostly unmaintained (outside of @littledivy's fork). As a bonus, this should allow us to run pty tests on the mac CI runner. After this PR, the tests now complete with 100% success on my local machine. Before this PR, I needed to pass `CI=true` to get my local test suite to pass.
-rw-r--r--Cargo.lock12
-rw-r--r--test_util/Cargo.toml4
-rw-r--r--test_util/src/pty.rs92
3 files changed, 64 insertions, 44 deletions
diff --git a/Cargo.lock b/Cargo.lock
index 7b8f5064c..9e8366880 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -3806,16 +3806,6 @@ dependencies = [
]
[[package]]
-name = "pty2"
-version = "0.1.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "4461e7f96399674b9112e620e511089bc7c4c0d76545b3cc3e0b46bab72a15d5"
-dependencies = [
- "errno",
- "libc",
-]
-
-[[package]]
name = "pulldown-cmark"
version = "0.9.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -5274,13 +5264,13 @@ dependencies = [
"glob",
"hyper 0.14.26",
"lazy-regex",
+ "libc",
"lsp-types",
"nix",
"once_cell",
"os_pipe",
"parking_lot 0.12.1",
"pretty_assertions",
- "pty2",
"regex",
"reqwest",
"ring",
diff --git a/test_util/Cargo.toml b/test_util/Cargo.toml
index 73df1fab9..2f35473e8 100644
--- a/test_util/Cargo.toml
+++ b/test_util/Cargo.toml
@@ -24,6 +24,7 @@ futures.workspace = true
glob.workspace = true
hyper = { workspace = true, features = ["server", "http1", "http2", "runtime"] }
lazy-regex.workspace = true
+libc.workspace = true
lsp-types.workspace = true
nix.workspace = true
once_cell.workspace = true
@@ -43,8 +44,5 @@ tokio.workspace = true
tokio-rustls.workspace = true
url.workspace = true
-[target.'cfg(unix)'.dependencies]
-pty2 = "0.1.0"
-
[target.'cfg(windows)'.dependencies]
winapi = { workspace = true, features = ["consoleapi", "synchapi", "handleapi", "namedpipeapi", "winbase", "winerror"] }
diff --git a/test_util/src/pty.rs b/test_util/src/pty.rs
index 2f89e481e..0d9151715 100644
--- a/test_util/src/pty.rs
+++ b/test_util/src/pty.rs
@@ -44,10 +44,10 @@ impl Pty {
}
pub fn is_supported() -> bool {
- let is_mac_or_windows = cfg!(target_os = "macos") || cfg!(windows);
- if is_mac_or_windows && std::env::var("CI").is_ok() {
- // the pty tests give a ENOTTY error for Mac and don't really start up
- // on the windows CI for some reason so ignore them for now
+ let is_windows = cfg!(windows);
+ if is_windows && std::env::var("CI").is_ok() {
+ // the pty tests don't really start up on the windows CI for some reason
+ // so ignore them for now
eprintln!("Ignoring windows CI.");
false
} else {
@@ -216,8 +216,10 @@ impl Pty {
trait SystemPty: Read + Write {}
+impl SystemPty for std::fs::File {}
+
#[cfg(unix)]
-fn setup_pty(master: &pty2::fork::Master) {
+fn setup_pty(fd: i32) {
use nix::fcntl::fcntl;
use nix::fcntl::FcntlArg;
use nix::fcntl::OFlag;
@@ -225,9 +227,7 @@ fn setup_pty(master: &pty2::fork::Master) {
use nix::sys::termios::tcgetattr;
use nix::sys::termios::tcsetattr;
use nix::sys::termios::SetArg;
- use std::os::fd::AsRawFd;
- let fd = master.as_raw_fd();
let mut term = tcgetattr(fd).unwrap();
// disable cooked mode
term.local_flags.remove(termios::LocalFlags::ICANON);
@@ -246,21 +246,60 @@ fn create_pty(
cwd: &Path,
env_vars: Option<HashMap<String, String>>,
) -> Box<dyn SystemPty> {
- let fork = pty2::fork::Fork::from_ptmx().unwrap();
- if fork.is_parent().is_ok() {
- let master = fork.is_parent().unwrap();
- setup_pty(&master);
- Box::new(unix::UnixPty { fork })
- } else {
- std::process::Command::new(program)
+ use crate::pty::unix::UnixPty;
+ use std::os::unix::process::CommandExt;
+
+ // Manually open pty main/secondary sides in the test process. Since we're not actually
+ // changing uid/gid here, this is the easiest way to do it.
+
+ // SAFETY: Posix APIs
+ let (fdm, fds) = unsafe {
+ let fdm = libc::posix_openpt(libc::O_RDWR);
+ if fdm < 0 {
+ panic!("posix_openpt failed");
+ }
+ let res = libc::grantpt(fdm);
+ if res != 0 {
+ panic!("grantpt failed");
+ }
+ let res = libc::unlockpt(fdm);
+ if res != 0 {
+ panic!("unlockpt failed");
+ }
+ let fds = libc::open(libc::ptsname(fdm), libc::O_RDWR);
+ if fdm < 0 {
+ panic!("open(ptsname) failed");
+ }
+ (fdm, fds)
+ };
+
+ // SAFETY: Posix APIs
+ unsafe {
+ let cmd = std::process::Command::new(program)
.current_dir(cwd)
.args(args)
.envs(env_vars.unwrap_or_default())
+ .pre_exec(move || {
+ // Close parent's main handle
+ libc::close(fdm);
+ libc::dup2(fds, 0);
+ libc::dup2(fds, 1);
+ libc::dup2(fds, 2);
+ // Note that we could close `fds` here as well, but this is a short-lived process and
+ // we're just not going to worry about "leaking" it
+ Ok(())
+ })
.spawn()
- .unwrap()
- .wait()
.unwrap();
- std::process::exit(0);
+
+ // Close child's secondary handle
+ libc::close(fds);
+ setup_pty(fdm);
+
+ use std::os::fd::FromRawFd;
+ let pid = nix::unistd::Pid::from_raw(cmd.id() as _);
+ let file = std::fs::File::from_raw_fd(fdm);
+ Box::new(UnixPty { pid, file })
}
}
@@ -272,19 +311,15 @@ mod unix {
use super::SystemPty;
pub struct UnixPty {
- pub fork: pty2::fork::Fork,
+ pub pid: nix::unistd::Pid,
+ pub file: std::fs::File,
}
impl Drop for UnixPty {
fn drop(&mut self) {
use nix::sys::signal::kill;
use nix::sys::signal::Signal;
- use nix::unistd::Pid;
-
- if let pty2::fork::Fork::Parent(child_pid, _) = self.fork {
- let pid = Pid::from_raw(child_pid);
- kill(pid, Signal::SIGTERM).unwrap()
- }
+ kill(self.pid, Signal::SIGTERM).unwrap()
}
}
@@ -292,20 +327,17 @@ mod unix {
impl Read for UnixPty {
fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
- let mut master = self.fork.is_parent().unwrap();
- master.read(buf)
+ self.file.read(buf)
}
}
impl Write for UnixPty {
fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
- let mut master = self.fork.is_parent().unwrap();
- master.write(buf)
+ self.file.write(buf)
}
fn flush(&mut self) -> std::io::Result<()> {
- let mut master = self.fork.is_parent().unwrap();
- master.flush()
+ self.file.flush()
}
}
}