add github ci config (#1)

This patch configures shpool to start using github
actions for CI. It is cribbed from
[wprs](https://github.com/wayland-transpositor/wprs/tree/master/.github/workflows).

I found that miri can't handle all the stuff shpool does
just yet (in particular the pipe2 syscall is not yet
supported). We should periodically check to see if sufficient
support has landed to test shpool under miri.

I had to fix a bunch of lints since this adds a new linter that
has not been run before over the codebase.

.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

.github/workflows/presubmit.yml

@@ -0,0 +1,60 @@
+name: presubmit
+on: [push, pull_request, workflow_call, workflow_dispatch]
+
+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: [email protected]
+          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

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<String>,
     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 {

libshpool/src/common.rs

@@ -19,13 +19,13 @@ use std::env;
 use anyhow::anyhow;
 
 pub fn resolve_sessions(sessions: &mut Vec<String>, 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));
     }

libshpool/src/daemon/etc_environment.rs

@@ -36,7 +36,7 @@ pub fn parse_compat<R: Read>(file: R) -> anyhow::Result<Vec<(String, String)>> {
                 // 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<R: Read>(file: R) -> anyhow::Result<Vec<(String, String)>> {
                 // 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)));
             }

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<T: IntoIterator<Item = Token>>(tokens: T) -> anyhow::Result<Sequence> {
         }
     }
 
-    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<u8> for Vec<Option<usize>> {
     }
 
     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<ChordAtom> for Vec<Option<usize>> {
     }
 
     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) {
@@ -622,24 +622,24 @@ mod test {
                 // the bindings mapping
                 vec![("a", Action::Detach)],
                 // the keypresses to scan over
-                vec!['a'].iter().map(|c| *c as u32 as u8).collect::<Vec<_>>(),
+                ['a'].iter().map(|c| *c as u32 as u8).collect::<Vec<_>>(),
                 BindingResult::Match(Action::Detach), // the final output from the engine
             ),
             (
                 // the bindings mapping
                 vec![("a", Action::Detach)],
                 // the keypresses to scan over
-                vec!['b', 'x', 'y', 'a'].iter().map(|c| *c as u32 as u8).collect::<Vec<_>>(),
+                ['b', 'x', 'y', 'a'].iter().map(|c| *c as u32 as u8).collect::<Vec<_>>(),
                 BindingResult::Match(Action::Detach), // the final output from the engine
             ),
             (
                 vec![("a", Action::Detach)],
-                vec!['b'].iter().map(|c| *c as u32 as u8).collect::<Vec<_>>(),
+                ['b'].iter().map(|c| *c as u32 as u8).collect::<Vec<_>>(),
                 BindingResult::NoMatch,
             ),
             (
                 vec![("a", Action::Detach)],
-                vec!['a', 'a', 'x', 'a', 'b'].iter().map(|c| *c as u32 as u8).collect::<Vec<_>>(),
+                ['a', 'a', 'x', 'a', 'b'].iter().map(|c| *c as u32 as u8).collect::<Vec<_>>(),
                 BindingResult::NoMatch,
             ),
             (vec![("Ctrl-a", Action::Detach)], vec![1], BindingResult::Match(Action::Detach)),
@@ -653,7 +653,7 @@ mod test {
             (vec![("Ctrl-Space Ctrl-d", Action::Detach)], vec![0, 4, 20], BindingResult::NoMatch),
             (
                 vec![("a b c", Action::Detach)],
-                vec!['a', 'b'].iter().map(|c| *c as u32 as u8).collect::<Vec<_>>(),
+                ['a', 'b'].iter().map(|c| *c as u32 as u8).collect::<Vec<_>>(),
                 BindingResult::Partial,
             ),
         ];
@@ -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")
             }
         }
 

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);
             }
         }