switch from docopt to structopt

A couple reasons for this:

* the docopt crate is "unlikely to see significant future evolution",
  and the wider docopt project is "mostly unmaintained at this point".
  clap/structopt is more full-featured, has more natural subcommand
  support, etc.

* it may allow me to shrink the binary (#70). This change alone seems
  to be a slight regression, but it's a step toward getting rid of
  regex, which is pretty large. And I feel less ridiculous now that I
  don't have two parsing crates anyway; prettydiff was pulling in
  structopt.

There are some behavior changes here:

* misc --help output changes and such as you'd expect from switching
  argument-parsing libraries

* I properly used PathBuf and OsString for stuff that theoretically
  could be non-UTF-8. I haven't tested that it actually made any
  difference. I'm also still storing the sample file dirname as "text"
  in the database to avoid causing a diff when not doing a schema
  change.
This commit is contained in:
Scott Lamb
2020-04-17 22:41:55 -07:00
parent 066c086050
commit e8eb764b90
17 changed files with 383 additions and 417 deletions

View File

@@ -42,6 +42,7 @@ use rusqlite::{Connection, Transaction, params};
use std::collections::BTreeMap;
use std::fmt;
use std::net::IpAddr;
use std::str::FromStr;
use std::sync::Arc;
lazy_static! {
@@ -57,7 +58,7 @@ pub(crate) fn set_test_config() {
Arc::new(libpasta::Config::with_primitive(libpasta::primitives::Bcrypt::new(2)));
}
enum UserFlags {
enum UserFlag {
Disabled = 1,
}
@@ -91,7 +92,7 @@ impl User {
}
pub fn has_password(&self) -> bool { self.password_hash.is_some() }
fn disabled(&self) -> bool { (self.flags & UserFlags::Disabled as i32) != 0 }
fn disabled(&self) -> bool { (self.flags & UserFlag::Disabled as i32) != 0 }
}
/// A change to a user.
@@ -132,7 +133,7 @@ impl UserChange {
}
pub fn disable(&mut self) {
self.flags |= UserFlags::Disabled as i32;
self.flags |= UserFlag::Disabled as i32;
}
}
@@ -194,13 +195,29 @@ impl rusqlite::types::FromSql for FromSqlIpAddr {
}
}
pub enum SessionFlags {
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
#[repr(i32)]
pub enum SessionFlag {
HttpOnly = 1,
Secure = 2,
SameSite = 4,
SameSiteStrict = 8,
}
impl FromStr for SessionFlag {
type Err = Error;
fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"http-only" => Ok(Self::HttpOnly),
"secure" => Ok(Self::Secure),
"same-site" => Ok(Self::SameSite),
"same-site-strict" => Ok(Self::SameSiteStrict),
_ => bail!("No such session flag {:?}", s),
}
}
}
#[derive(Copy, Clone)]
pub enum RevocationReason {
LoggedOut = 1,
@@ -209,7 +226,7 @@ pub enum RevocationReason {
#[derive(Debug, Default)]
pub struct Session {
user_id: i32,
flags: i32, // bitmask of SessionFlags enum values
flags: i32, // bitmask of SessionFlag enum values
domain: Option<Vec<u8>>,
description: Option<String>,
seed: Seed,

View File

@@ -41,7 +41,7 @@ use log::warn;
use protobuf::Message;
use nix::{NixPath, fcntl::{FlockArg, OFlag}, sys::stat::Mode};
use nix::sys::statvfs::Statvfs;
use std::ffi::{CStr, CString};
use std::ffi::CStr;
use std::fs;
use std::io::{Read, Write};
use std::os::unix::io::{AsRawFd, RawFd};
@@ -104,16 +104,14 @@ impl Drop for Fd {
impl Fd {
/// Opens the given path as a directory.
pub fn open(path: &str, mkdir: bool) -> Result<Fd, nix::Error> {
let cstring = CString::new(path).map_err(|_| nix::Error::InvalidPath)?;
pub fn open<P: ?Sized + NixPath>(path: &P, mkdir: bool) -> Result<Fd, nix::Error> {
if mkdir {
match nix::unistd::mkdir(cstring.as_c_str(), nix::sys::stat::Mode::S_IRWXU) {
match nix::unistd::mkdir(path, nix::sys::stat::Mode::S_IRWXU) {
Ok(()) | Err(nix::Error::Sys(nix::errno::Errno::EEXIST)) => {},
Err(e) => return Err(e),
}
}
let fd = nix::fcntl::open(cstring.as_c_str(), OFlag::O_DIRECTORY | OFlag::O_RDONLY,
Mode::empty())?;
let fd = nix::fcntl::open(path, OFlag::O_DIRECTORY | OFlag::O_RDONLY, Mode::empty())?;
Ok(Fd(fd))
}

View File

@@ -52,9 +52,9 @@ const UPGRADE_NOTES: &'static str =
#[derive(Debug)]
pub struct Args<'a> {
pub flag_sample_file_dir: Option<&'a str>,
pub flag_preset_journal: &'a str,
pub flag_no_vacuum: bool,
pub sample_file_dir: Option<&'a std::path::Path>,
pub preset_journal: &'a str,
pub no_vacuum: bool,
}
fn set_journal_mode(conn: &rusqlite::Connection, requested: &str) -> Result<(), Error> {
@@ -86,7 +86,7 @@ fn upgrade(args: &Args, target_ver: i32, conn: &mut rusqlite::Connection) -> Res
bail!("Database is at negative version {}!", old_ver);
}
info!("Upgrading database from version {} to version {}...", old_ver, target_ver);
set_journal_mode(&conn, args.flag_preset_journal)?;
set_journal_mode(&conn, args.preset_journal)?;
for ver in old_ver .. target_ver {
info!("...from version {} to version {}", ver, ver + 1);
let tx = conn.transaction()?;
@@ -118,7 +118,7 @@ pub fn run(args: &Args, conn: &mut rusqlite::Connection) -> Result<(), Error> {
// WAL is the preferred journal mode for normal operation; it reduces the number of syncs
// without compromising safety.
set_journal_mode(&conn, "wal")?;
if !args.flag_no_vacuum {
if !args.no_vacuum {
info!("...vacuuming database after upgrade.");
conn.execute_batch(r#"
pragma page_size = 16384;
@@ -157,7 +157,7 @@ impl NixPath for UuidPath {
mod tests {
use crate::compare;
use crate::testutil;
use failure::{ResultExt, format_err};
use failure::ResultExt;
use super::*;
fn new_conn() -> Result<rusqlite::Connection, Error> {
@@ -183,7 +183,7 @@ mod tests {
fn upgrade_and_compare() -> Result<(), Error> {
testutil::init();
let tmpdir = tempdir::TempDir::new("moonfire-nvr-test")?;
let path = tmpdir.path().to_str().ok_or_else(|| format_err!("invalid UTF-8"))?.to_owned();
//let path = tmpdir.path().to_str().ok_or_else(|| format_err!("invalid UTF-8"))?.to_owned();
let mut upgraded = new_conn()?;
upgraded.execute_batch(include_str!("v0.sql"))?;
upgraded.execute_batch(r#"
@@ -218,9 +218,9 @@ mod tests {
(4, None), // transitional; don't compare schemas.
(5, Some(include_str!("../schema.sql")))] {
upgrade(&Args {
flag_sample_file_dir: Some(&path),
flag_preset_journal: "delete",
flag_no_vacuum: false,
sample_file_dir: Some(&tmpdir.path()),
preset_journal: "delete",
no_vacuum: false,
}, *ver, &mut upgraded).context(format!("upgrading to version {}", ver))?;
if let Some(f) = fresh_sql {
compare(&upgraded, *ver, f)?;

View File

@@ -42,7 +42,7 @@ use uuid::Uuid;
pub fn run(args: &super::Args, tx: &rusqlite::Transaction) -> Result<(), Error> {
let sample_file_path =
args.flag_sample_file_dir
args.sample_file_dir
.ok_or_else(|| format_err!("--sample-file-dir required when upgrading from \
schema version 1 to 2."))?;
@@ -122,6 +122,9 @@ pub fn run(args: &super::Args, tx: &rusqlite::Transaction) -> Result<(), Error>
}
dir::write_meta(d.as_raw_fd(), &meta)?;
let sample_file_path = sample_file_path.to_str()
.ok_or_else(|| format_err!("sample file dir {} is not a valid string",
sample_file_path.display()))?;
tx.execute(r#"
insert into sample_file_dir (path, uuid, last_complete_open_id)
values (?, ?, ?)
@@ -293,7 +296,7 @@ pub fn run(args: &super::Args, tx: &rusqlite::Transaction) -> Result<(), Error>
/// * optional: reserved sample file uuids.
/// * optional: meta and meta-tmp from half-completed update attempts.
/// * forbidden: anything else.
fn verify_dir_contents(sample_file_path: &str, dir: &mut nix::dir::Dir,
fn verify_dir_contents(sample_file_path: &std::path::Path, dir: &mut nix::dir::Dir,
tx: &rusqlite::Transaction) -> Result<(), Error> {
// Build a hash of the uuids found in the directory.
let n: i64 = tx.query_row(r#"
@@ -337,7 +340,7 @@ fn verify_dir_contents(sample_file_path: &str, dir: &mut nix::dir::Dir,
while let Some(row) = rows.next()? {
let uuid: crate::db::FromSqlUuid = row.get(0)?;
if !files.remove(&uuid.0) {
bail!("{} is missing from dir {}!", uuid.0, sample_file_path);
bail!("{} is missing from dir {}!", uuid.0, sample_file_path.display());
}
}
}
@@ -360,7 +363,7 @@ fn verify_dir_contents(sample_file_path: &str, dir: &mut nix::dir::Dir,
if !files.is_empty() {
bail!("{} unexpected sample file uuids in dir {}: {:?}!",
files.len(), sample_file_path, files);
files.len(), sample_file_path.display(), files);
}
Ok(())
}