diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml new file mode 100644 index 00000000..58cc43fe --- /dev/null +++ b/.github/workflows/nightly.yml @@ -0,0 +1,18 @@ +name: nightly +on: + schedule: + - cron: '04 05 * * *' + +jobs: + deny: + name: cargo deny --all-features check + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + - uses: moonrepo/setup-rust@b8edcc56aab474d90c7cf0bb8beeaf8334c15e9f + with: + bins: cargo-deny + - run: cargo deny --all-features check + + postsubmit: + uses: ./.github/workflows/presubmit.yml diff --git a/.github/workflows/presubmit.yml b/.github/workflows/presubmit.yml new file mode 100644 index 00000000..54851b61 --- /dev/null +++ b/.github/workflows/presubmit.yml @@ -0,0 +1,66 @@ +name: presubmit +on: [push, pull_request, workflow_call, workflow_dispatch] + +env: + # Without this, tracy-client won't build because the github runners don't have + # TSCs. We don't care since we're not actually testing tracy functionality but + # we still want to test with --all-features. + TRACY_NO_INVARIANT_CHECK: 1 + +jobs: + test: + name: cargo test --all-features + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + - uses: moonrepo/setup-rust@b8edcc56aab474d90c7cf0bb8beeaf8334c15e9f + - run: sudo apt-get install zsh fish + - run: cargo test --all-features + + # miri does not handle all the IO we do, disabled for now. + # + # miri: + # name: cargo +nightly miri test + # runs-on: ubuntu-22.04 + # steps: + # - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + # - uses: moonrepo/setup-rust@b8edcc56aab474d90c7cf0bb8beeaf8334c15e9f + # with: + # components: miri + # channel: nightly + # - run: sudo apt-get install zsh fish + # - run: MIRIFLAGS="-Zmiri-disable-isolation" cargo +nightly miri test + + rustfmt: + name: cargo +nightly fmt -- --check + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + - uses: moonrepo/setup-rust@b8edcc56aab474d90c7cf0bb8beeaf8334c15e9f + with: + components: rustfmt + channel: nightly + - run: cargo +nightly fmt -- --check + + cranky: + name: cargo +nightly cranky --all-targets -- -D warnings + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + - uses: moonrepo/setup-rust@b8edcc56aab474d90c7cf0bb8beeaf8334c15e9f + with: + components: clippy + bins: cargo-cranky@0.3.0 + channel: nightly + - run: sudo apt-get install zsh fish + - run: cargo +nightly cranky --all-targets -- -D warnings + + deny: + name: cargo deny --all-features check licenses + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + - uses: moonrepo/setup-rust@b8edcc56aab474d90c7cf0bb8beeaf8334c15e9f + with: + bins: cargo-deny + - run: cargo deny --all-features check licenses diff --git a/libshpool/src/attach.rs b/libshpool/src/attach.rs index 5422c745..cd7be6bf 100644 --- a/libshpool/src/attach.rs +++ b/libshpool/src/attach.rs @@ -67,7 +67,7 @@ pub fn run( .context("writing detach request header")?; let detach_reply: protocol::DetachReply = client.read_reply().context("reading reply")?; - if detach_reply.not_found_sessions.len() > 0 { + if !detach_reply.not_found_sessions.is_empty() { warn!("could not find session '{}' to detach it", name); } @@ -107,7 +107,7 @@ fn do_attach( cmd: &Option, socket: &PathBuf, ) -> anyhow::Result<()> { - let mut client = dial_client(&socket)?; + let mut client = dial_client(socket)?; let tty_size = match tty::Size::from_fd(0) { Ok(s) => s, @@ -209,7 +209,7 @@ impl SignalHandler { use signal_hook::{consts::*, iterator::*}; let sigs = vec![SIGWINCH]; - let mut signals = Signals::new(&sigs).context("creating signal iterator")?; + let mut signals = Signals::new(sigs).context("creating signal iterator")?; thread::spawn(move || { for signal in &mut signals { diff --git a/libshpool/src/common.rs b/libshpool/src/common.rs index 05b074fe..f08dfbad 100644 --- a/libshpool/src/common.rs +++ b/libshpool/src/common.rs @@ -19,13 +19,13 @@ use std::env; use anyhow::anyhow; pub fn resolve_sessions(sessions: &mut Vec, action: &str) -> anyhow::Result<()> { - if sessions.len() == 0 { + if sessions.is_empty() { if let Ok(current_session) = env::var("SHPOOL_SESSION_NAME") { sessions.push(current_session); } } - if sessions.len() == 0 { + if sessions.is_empty() { eprintln!("no session to {}", action); return Err(anyhow!("no session to {}", action)); } diff --git a/libshpool/src/daemon/etc_environment.rs b/libshpool/src/daemon/etc_environment.rs index e8222130..6905dd28 100644 --- a/libshpool/src/daemon/etc_environment.rs +++ b/libshpool/src/daemon/etc_environment.rs @@ -36,7 +36,7 @@ pub fn parse_compat(file: R) -> anyhow::Result> { // us! let line: String = line.chars().take_while(|c| *c != '#').collect(); - let parts: Vec<_> = line.splitn(2, "=").collect(); + let parts: Vec<_> = line.splitn(2, '=').collect(); if parts.len() != 2 { warn!("parsing /etc/environment: split failed (should be impossible)"); continue; @@ -55,11 +55,11 @@ pub fn parse_compat(file: R) -> anyhow::Result> { // single quotes with double quotes and strip unmatched leading // quotes while doing nothing for unmatched trailing quotes. let has_leading_quote = val.starts_with('\'') || val.starts_with('"'); - val = val.strip_prefix("'").unwrap_or(val); - val = val.strip_prefix("\"").unwrap_or(val); + val = val.strip_prefix('\'').unwrap_or(val); + val = val.strip_prefix('"').unwrap_or(val); if has_leading_quote { - val = val.strip_suffix("'").unwrap_or(val); - val = val.strip_suffix("\"").unwrap_or(val); + val = val.strip_suffix('\'').unwrap_or(val); + val = val.strip_suffix('"').unwrap_or(val); } pairs.push((String::from(key), String::from(val))); } diff --git a/libshpool/src/daemon/keybindings.rs b/libshpool/src/daemon/keybindings.rs index 6fcaf1e1..ee96ffb0 100644 --- a/libshpool/src/daemon/keybindings.rs +++ b/libshpool/src/daemon/keybindings.rs @@ -104,7 +104,7 @@ impl Bindings { let mut chords = Trie::new(); let mut sequences = Trie::new(); - let mut chord_atom_counter = 0; + let mut chord_atom_counter: usize = 0; let mut chord_atom_tab = HashMap::new(); let tokenizer = Lexer::new(); @@ -117,11 +117,11 @@ impl Bindings { let code = chord.key_code()?; let chord_atom = chord_atom_tab.entry(chord.clone()).or_insert_with(|| { - let atom = ChordAtom(chord_atom_counter); + let atom = ChordAtom(chord_atom_counter as u8); chord_atom_counter += 1; atom }); - if chord_atom_counter >= u8::MAX { + if chord_atom_counter >= u8::MAX as usize { return Err(anyhow!( "shpool only supports up to {} unique chords at a time", u8::MAX @@ -319,7 +319,7 @@ fn parse>(tokens: T) -> anyhow::Result { } } - if keys.len() > 0 { + if !keys.is_empty() { chords.push(Chord(keys)); } @@ -481,7 +481,7 @@ where } } - fn get<'trie>(&'trie self, cursor: TrieCursor) -> Option<&'trie V> { + fn get(&self, cursor: TrieCursor) -> Option<&V> { if let TrieCursor::Match { idx, .. } = cursor { self.nodes[idx].value.as_ref() } else { @@ -532,7 +532,7 @@ impl TrieTab for Vec> { } fn get(&self, index: u8) -> Option<&usize> { - (&self[index as usize]).as_ref() + self[index as usize].as_ref() } fn set(&mut self, index: u8, elem: usize) { @@ -546,7 +546,7 @@ impl TrieTab for Vec> { } fn get(&self, index: ChordAtom) -> Option<&usize> { - (&self[index.0 as usize]).as_ref() + self[index.0 as usize].as_ref() } fn set(&mut self, index: ChordAtom, elem: usize) { @@ -687,15 +687,13 @@ mod test { let seq = parse(tokens)?; let chord = seq.0[0].clone(); - if errstr == "" { + if errstr.is_empty() { chord.check_valid()?; + } else if let Err(e) = chord.check_valid() { + let got = format!("{:?}", e); + assert!(got.contains(errstr)); } else { - if let Err(e) = chord.check_valid() { - let got = format!("{:?}", e); - assert!(got.contains(errstr)); - } else { - panic!("bad success, want err with: {}", errstr); - } + panic!("bad success, want err with: {}", errstr); } } @@ -778,7 +776,7 @@ mod test { let errstr = format!("{:?}", err); assert!(errstr.contains(errsubstr)); } else { - assert!(false, "expected an error") + panic!("expected an error") } } diff --git a/libshpool/src/daemon/server.rs b/libshpool/src/daemon/server.rs index 2fc05249..af621b2a 100644 --- a/libshpool/src/daemon/server.rs +++ b/libshpool/src/daemon/server.rs @@ -249,10 +249,8 @@ impl Server { shells.insert(header.name.clone(), Box::new(session)); // fallthrough to bidi streaming - } else { - if let Err(err) = self.hooks.on_reattach(&header.name) { - warn!("reattach hook: {:?}", err); - } + } else if let Err(err) = self.hooks.on_reattach(&header.name) { + warn!("reattach hook: {:?}", err); } // return a reference to the inner session so that @@ -316,10 +314,8 @@ impl Server { .map_err(|e| anyhow!("joining reader after child exit: {:?}", e))? .context("within reader thread after child exit")?; } - } else { - if let Err(err) = self.hooks.on_client_disconnect(&header.name) { - warn!("client_disconnect hook: {:?}", err); - } + } else if let Err(err) = self.hooks.on_client_disconnect(&header.name) { + warn!("client_disconnect hook: {:?}", err); } info!("finished attach streaming section"); @@ -393,7 +389,7 @@ impl Server { not_attached_sessions.push(session); } } else { - not_found_sessions.push(String::from(session)); + not_found_sessions.push(session); } } } @@ -433,7 +429,7 @@ impl Server { for session in to_remove.iter() { shells.remove(session); } - if to_remove.len() > 0 { + if !to_remove.is_empty() { test_hooks::emit("daemon-handle-kill-removed-shells"); } } @@ -541,9 +537,9 @@ impl Server { // stdout/stderr/stdin. The pty crate automatically `dup2`s the file // descriptors for us. let mut cmd = if let Some(cmd_str) = &header.cmd { - let cmd_parts = shell_words::split(&cmd_str).context("parsing cmd")?; + let cmd_parts = shell_words::split(cmd_str).context("parsing cmd")?; info!("running cmd: {:?}", cmd_parts); - if cmd_parts.len() < 1 { + if cmd_parts.is_empty() { return Err(anyhow!("no command to run")); } let mut cmd = process::Command::new(&cmd_parts[0]); @@ -632,7 +628,7 @@ impl Server { // inject the prompt prefix, if any let prompt_prefix = self.config.prompt_prefix.clone().unwrap_or(String::from("")); if let Some(shell_basename) = shell_basename { - if prompt_prefix.len() > 0 { + if !prompt_prefix.is_empty() { if let Err(err) = prompt::inject_prefix(&mut fork, shell_basename, &prompt_prefix, &header.name) { @@ -661,25 +657,23 @@ impl Server { reader_join_h: None, }; let child_pid = session_inner.pty_master.child_pid().ok_or(anyhow!("no child pid"))?; - session_inner.reader_join_h = Some( - session_inner.spawn_reader( - conn_id, - header.local_tty_size.clone(), - match (self.config.output_spool_lines, &self.config.session_restore_mode) { - (Some(l), _) => l, - (None, Some(config::SessionRestoreMode::Lines(l))) => *l as usize, - (None, _) => DEFAULT_OUTPUT_SPOOL_LINES, - }, - self.config - .session_restore_mode - .clone() - .unwrap_or(config::SessionRestoreMode::default()), - client_connection_rx, - client_connection_ack_tx, - tty_size_change_rx, - tty_size_change_ack_tx, - )?, - ); + session_inner.reader_join_h = Some(session_inner.spawn_reader(shell::ReaderArgs { + conn_id, + tty_size: header.local_tty_size.clone(), + scrollback_lines: match ( + self.config.output_spool_lines, + &self.config.session_restore_mode, + ) { + (Some(l), _) => l, + (None, Some(config::SessionRestoreMode::Lines(l))) => *l as usize, + (None, _) => DEFAULT_OUTPUT_SPOOL_LINES, + }, + session_restore_mode: self.config.session_restore_mode.clone().unwrap_or_default(), + client_connection: client_connection_rx, + client_connection_ack: client_connection_ack_tx, + tty_size_change: tty_size_change_rx, + tty_size_change_ack: tty_size_change_ack_tx, + })?); if let Some(ttl_secs) = header.ttl_secs { info!("registering session with ttl with the reaper"); @@ -754,7 +748,7 @@ impl Server { env }; - if env.len() > 0 { + if !env.is_empty() { cmd.envs(env); } } diff --git a/libshpool/src/daemon/shell.rs b/libshpool/src/daemon/shell.rs index ec995931..83642c0c 100644 --- a/libshpool/src/daemon/shell.rs +++ b/libshpool/src/daemon/shell.rs @@ -167,6 +167,17 @@ pub enum ClientConnectionMsg { Disconnect, } +pub struct ReaderArgs { + pub conn_id: usize, + pub tty_size: tty::Size, + pub scrollback_lines: usize, + pub session_restore_mode: config::SessionRestoreMode, + pub client_connection: crossbeam_channel::Receiver, + pub client_connection_ack: crossbeam_channel::Sender, + pub tty_size_change: crossbeam_channel::Receiver, + pub tty_size_change_ack: crossbeam_channel::Sender<()>, +} + impl SessionInner { /// Spawn the reader thread which continually reads from the pty /// and sends data both to the output spool and to the client, @@ -174,27 +185,24 @@ impl SessionInner { #[instrument(skip_all, fields(s = self.name))] pub fn spawn_reader( &self, - conn_id: usize, - tty_size: tty::Size, - scrollback_lines: usize, - session_restore_mode: config::SessionRestoreMode, - client_connection: crossbeam_channel::Receiver, - client_connection_ack: crossbeam_channel::Sender, - tty_size_change: crossbeam_channel::Receiver, - tty_size_change_ack: crossbeam_channel::Sender<()>, + args: ReaderArgs, ) -> anyhow::Result>> { use nix::poll; let mut pty_master = self.pty_master.is_parent()?.clone(); let name = self.name.clone(); let mut closure = move || { - let _s = span!(Level::INFO, "reader", s = name, cid = conn_id).entered(); + let _s = span!(Level::INFO, "reader", s = name, cid = args.conn_id).entered(); let mut output_spool = - if matches!(session_restore_mode, config::SessionRestoreMode::Simple) { + if matches!(args.session_restore_mode, config::SessionRestoreMode::Simple) { None } else { - Some(shpool_vt100::Parser::new(tty_size.rows, VTERM_WIDTH, scrollback_lines)) + Some(shpool_vt100::Parser::new( + args.tty_size.rows, + VTERM_WIDTH, + args.scrollback_lines, + )) }; let mut buf: Vec = vec![0; consts::BUF_SIZE]; let mut poll_fds = [poll::PollFd::new( @@ -206,8 +214,8 @@ impl SessionInner { // the initial prompt on the floor info!("waiting for initial client connection"); let mut client_conn: ClientConnectionMsg = - client_connection.recv().context("waiting for initial client connection")?; - client_connection_ack + args.client_connection.recv().context("waiting for initial client connection")?; + args.client_connection_ack .send(ClientConnectionStatus::New) .context("sending initial client connection ack")?; info!("got initial client connection"); @@ -221,7 +229,7 @@ impl SessionInner { loop { let mut do_reattach = false; crossbeam_channel::select! { - recv(client_connection) -> new_connection => { + recv(args.client_connection) -> new_connection => { match new_connection { Ok(ClientConnectionMsg::New(conn)) => { info!("got new connection (rows={}, cols={})", conn.size.rows, conn.size.cols); @@ -252,7 +260,7 @@ impl SessionInner { }); client_conn = ClientConnectionMsg::New(conn); - client_connection_ack.send(ack) + args.client_connection_ack.send(ack) .context("sending client connection ack")?; } Ok(ClientConnectionMsg::Disconnect) => { @@ -266,7 +274,7 @@ impl SessionInner { }; client_conn = ClientConnectionMsg::Disconnect; - client_connection_ack.send(ack) + args.client_connection_ack.send(ack) .context("sending client connection ack")?; } Ok(ClientConnectionMsg::DisconnectExit(exit_status)) => { @@ -302,7 +310,7 @@ impl SessionInner { exit_status); ClientConnectionStatus::DetachNone }; - client_connection_ack.send(ack) + args.client_connection_ack.send(ack) .context("sending client connection ack")?; return Ok(()); @@ -315,19 +323,19 @@ impl SessionInner { }, } } - recv(tty_size_change) -> new_size => { + recv(args.tty_size_change) -> new_size => { match new_size { Ok(size) => { info!("resize size={:?}", size); output_spool.as_mut().map(|s| s.screen_mut() .set_size(size.rows, std::u16::MAX)); resize_cmd = Some(ResizeCmd { - size: size, + size, // No delay needed for ordinary resizes, just // for reconnects. when: time::Instant::now(), }); - tty_size_change_ack.send(()) + args.tty_size_change_ack.send(()) .context("sending size change ack")?; } Err(err) => { @@ -364,8 +372,8 @@ impl SessionInner { if do_reattach { use config::SessionRestoreMode::*; - info!("executing reattach protocol (mode={:?})", session_restore_mode); - let restore_buf = match (output_spool.as_mut(), &session_restore_mode) { + info!("executing reattach protocol (mode={:?})", args.session_restore_mode); + let restore_buf = match (output_spool.as_mut(), &args.session_restore_mode) { (Some(spool), Screen) => { let (rows, cols) = spool.screen().size(); info!( @@ -435,7 +443,7 @@ impl SessionInner { } trace!("read pty master len={} '{}'", len, String::from_utf8_lossy(&buf[..len])); - if !matches!(session_restore_mode, config::SessionRestoreMode::Simple) { + if !matches!(args.session_restore_mode, config::SessionRestoreMode::Simple) { output_spool.as_mut().map(|s| s.process(&buf[..len])); } diff --git a/libshpool/src/protocol.rs b/libshpool/src/protocol.rs index 27c80259..dd2ff63b 100644 --- a/libshpool/src/protocol.rs +++ b/libshpool/src/protocol.rs @@ -13,7 +13,6 @@ // limitations under the License. use std::{ - convert::TryFrom, fmt, io::{self, Read, Write}, os::unix::net::UnixStream, @@ -512,7 +511,6 @@ impl Client { #[cfg(test)] mod test { use super::*; - use std::io; #[test] fn chunk_round_trip() { diff --git a/libshpool/src/test_hooks.rs b/libshpool/src/test_hooks.rs index 3896fcd7..271c6dfe 100644 --- a/libshpool/src/test_hooks.rs +++ b/libshpool/src/test_hooks.rs @@ -49,9 +49,7 @@ pub fn scoped(event: &str) -> ScopedEvent { } #[cfg(not(feature = "test_hooks"))] -pub fn scoped(_event: &str) -> () { - () -} +pub fn scoped(_event: &str) {} /// ScopedEvent emits an event when it goes out of scope pub struct ScopedEvent<'a> { diff --git a/shpool/src/main.rs b/shpool/src/main.rs index 81c68fe6..d2852d97 100644 --- a/shpool/src/main.rs +++ b/shpool/src/main.rs @@ -15,7 +15,6 @@ /// aims to provide a simpler user experience. See [the /// README](https://github.com/shell-pool/shpool) for more /// info. - use clap::Parser; const VERSION: &str = env!("CARGO_PKG_VERSION");