From 3db14abafa37d08c0a286a8a7a40a748a090260c Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Sat, 20 Apr 2024 17:49:44 +0200 Subject: [PATCH] refact(project): add connection to struct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was a lot more complicated than I expected, because you cannot share a rusqlite `Connection` object across unwrap boundaries (you can share it across threads, but our `Async` does automatic unwrapping …). So what I do here is that whenever we want to clone a `Project`, I create something I call “skeleton”, which is a clone of the project without the connection object (just the sqlite file path), then on the other side of the boundary I turn the skeleton back into a Project by connecting to the database again. I think this is quite an okay solution. --- src/build_loop.rs | 10 ++-- src/daemon.rs | 33 ++++++----- src/main.rs | 10 ++-- src/ops.rs | 15 ++--- src/ops/error.rs | 22 ++++++++ src/project.rs | 88 ++++++++++++++++++++++------- src/sqlite.rs | 33 ++++++++--- tests/integration/direnvtestcase.rs | 15 ++--- tests/shell/main.rs | 4 +- 9 files changed, 160 insertions(+), 70 deletions(-) diff --git a/src/build_loop.rs b/src/build_loop.rs index fde3e590..9e9e76fd 100644 --- a/src/build_loop.rs +++ b/src/build_loop.rs @@ -60,9 +60,9 @@ pub enum Reason { /// If a build is ongoing, it will finish the build first. /// If there was intermediate requests for new builds, it will schedule a build to be run right after. /// Additionally, we create GC roots for the build results. -pub struct BuildLoop<'a> { +pub struct BuildLoop { /// Project to be built. - project: &'a Project, + project: Project, /// Extra options to pass to each nix invocation extra_nix_options: NixOptions, /// Watches all input files for changes. @@ -102,19 +102,19 @@ impl BuildState { } } -impl<'a> BuildLoop<'a> { +impl BuildLoop { /// Instatiate a new BuildLoop. Uses an internal filesystem /// watching implementation. /// /// Will start by only watching the project’s nix file, /// and then add new files after each nix run. pub fn new( - project: &'a Project, + project: Project, extra_nix_options: NixOptions, nix_gc_root_user_dir: project::NixGcRootUserDir, cas: ContentAddressable, logger: slog::Logger, - ) -> anyhow::Result> { + ) -> anyhow::Result { let mut watch = Watch::try_new(logger.clone()).map_err(|err| anyhow!(err))?; watch .extend(vec![WatchPathBuf::Normal( diff --git a/src/daemon.rs b/src/daemon.rs index 59e480ca..4c86ebb8 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -9,6 +9,7 @@ use crate::ops::error::ExitError; use crate::socket::communicate; use crate::socket::path::SocketPath; use crate::sqlite::Sqlite; +use crate::thread::Pool; use crate::{project, AbsPathBuf, NixFile}; use crossbeam_channel as chan; use slog::debug; @@ -83,7 +84,7 @@ impl Daemon { chan::Receiver, ) = chan::unbounded(); - let mut pool = crate::thread::Pool::new(logger.clone()); + let mut pool: Pool = crate::thread::Pool::new(logger.clone()); let tx_build_events = self.tx_build_events.clone(); let server = server::Server::new(tx_activity, tx_build_events); @@ -94,7 +95,7 @@ impl Daemon { let logger3 = logger.clone(); pool.spawn("accept-loop", move || { - server.listen(&socket_path, &logger).map(|n| n.never()) + server.listen(&socket_path, &logger).map(|n| n.never())? })?; let rx_build_events = self.rx_build_events.clone(); @@ -109,17 +110,17 @@ impl Daemon { let gc_root_dir = gc_root_dir.clone(); let sqlite_path = sqlite_path.clone(); pool.spawn("build-instruction-handler", move || { - let mut conn = Sqlite::new_connection(&sqlite_path); + let conn = Sqlite::new_connection(&sqlite_path)?; Self::build_instruction_handler( tx_build_events, extra_nix_options, rx_activity, &gc_root_dir, - &mut conn, + conn, cas, nix_gc_root_user_dir, &logger3, - ); + )?; Ok(()) })?; @@ -182,21 +183,23 @@ impl Daemon { extra_nix_options: NixOptions, rx_activity: chan::Receiver, gc_root_dir: &AbsPathBuf, - conn: &mut Sqlite, + conn: Sqlite, cas: crate::cas::ContentAddressable, nix_gc_root_user_dir: project::NixGcRootUserDir, logger: &slog::Logger, - ) { + ) -> Result<(), ExitError> { // A thread for each `BuildLoop`, keyed by the nix files listened on. let mut handler_threads: HashMap> = HashMap::new(); // For each build instruction, add the corresponding file // to the watch list. for IndicateActivity { nix_file, rebuild } in rx_activity { - let project = - crate::project::Project::new_and_gc_nix_files(conn, nix_file, gc_root_dir) - // TODO: the project needs to create its gc root dir - .unwrap(); + let project = crate::project::Project::new_and_gc_nix_files( + conn.clone()?, + nix_file.clone(), + gc_root_dir, + ) + .map_err(|err| ExitError::panic(err))?; let key = project.nix_file.clone(); let project_is_watched = handler_threads.get(&key); @@ -223,6 +226,7 @@ impl Daemon { let logger = logger.clone(); let logger2 = logger.clone(); let cas2 = cas.clone(); + let nix_file2 = nix_file.clone(); // TODO: how to use the pool here? // We cannot just spawn new threads once messages come in, // because then then pool objects is stuck in this loop @@ -233,7 +237,7 @@ impl Daemon { // pool.spawn(format!("build_loop for {}", nix_file.display()), let _ = std::thread::spawn(move || { match BuildLoop::new( - &project, + project, extra_nix_options, nix_gc_root_user_dir, cas2, @@ -247,12 +251,12 @@ impl Daemon { { tx_build_events .send(LoopHandlerEvent::BuildEvent(Event::Failure { - nix_file: project.nix_file.clone(), + nix_file: nix_file2, failure: crate::builder::BuildError::Io { msg: err .context(format!( "could not start the watcher for {}", - &project.nix_file.display() + nix_file.display() )) .to_string(), }, @@ -274,5 +278,6 @@ impl Daemon { } } } + Ok(()) } } diff --git a/src/main.rs b/src/main.rs index b7535fa8..dc4261ad 100644 --- a/src/main.rs +++ b/src/main.rs @@ -90,16 +90,16 @@ fn find_nix_file(shellfile: &Path) -> Result { /// Run the main function of the relevant command. fn run_command(orig_logger: &slog::Logger, opts: Arguments) -> Result<(), ExitError> { let paths = lorri::ops::get_paths()?; - let mut conn = Sqlite::new_connection(&paths.sqlite_db); + let conn = Sqlite::new_connection(&paths.sqlite_db)?; // TODO: TMP conn.migrate_gc_roots(&orig_logger, &paths).unwrap(); - let mut with_project_resolved = - |nix_file| -> std::result::Result<(Project, slog::Logger), ExitError> { + let with_project_resolved = + |nix_file: NixFile| -> std::result::Result<(Project, slog::Logger), ExitError> { let project = { let paths = &lorri::ops::get_paths()?; - Project::new_and_gc_nix_files(&mut conn, nix_file, paths.gc_root_dir()).map_err( + Project::new_and_gc_nix_files(conn, nix_file.clone(), paths.gc_root_dir()).map_err( |err| { ExitError::temporary( anyhow::anyhow!(err).context("Could not set up project paths"), @@ -110,7 +110,7 @@ fn run_command(orig_logger: &slog::Logger, opts: Arguments) -> Result<(), ExitEr let logger = orig_logger.new(o!("nix_file" => project.nix_file.clone())); Ok((project, logger)) }; - let mut with_project = |nix_file| -> std::result::Result<(Project, slog::Logger), ExitError> { + let with_project = |nix_file| -> std::result::Result<(Project, slog::Logger), ExitError> { with_project_resolved(find_nix_file(nix_file)?) }; diff --git a/src/ops.rs b/src/ops.rs index 350fd824..b7589788 100644 --- a/src/ops.rs +++ b/src/ops.rs @@ -423,14 +423,14 @@ fn build_root( logger: &slog::Logger, ) -> Result { let logger2 = logger.clone(); - let project2 = project.clone(); + let nix_file = project.nix_file.clone(); let cas2 = cas.clone(); let run_result = crate::thread::race( logger, move |_ignored_stop| { Ok(builder::instantiate_and_build( - &project2.nix_file, + &nix_file, &cas2, &crate::nix::options::NixOptions::empty(), &logger2, @@ -938,7 +938,7 @@ fn main_run_once( ) -> Result<(), ExitError> { // TODO: add the ability to pass extra_nix_options to watch let mut build_loop = BuildLoop::new( - &project, + project, NixOptions::empty(), nix_gc_root_user_dir, cas.clone(), @@ -968,8 +968,8 @@ pub fn op_gc( paths: &Paths, opts: crate::cli::GcOptions, ) -> Result<(), ExitError> { - let infos = Project::list_roots(logger, paths)?; - let mut conn = Sqlite::new_connection(&paths.sqlite_db); + let conn = Sqlite::new_connection(&paths.sqlite_db)?; + let infos = Project::list_roots(logger, paths, conn)?; match opts.action { cli::GcSubcommand::Info => { if opts.json { @@ -1027,7 +1027,7 @@ pub fn op_gc( } } else { for (info, project) in to_remove { - match project.remove_project(&mut conn) { + match project.remove_project() { Ok(()) => result.push(Ok(info)), Err(e) => result.push(Err((info, e.to_string()))), } @@ -1095,11 +1095,12 @@ fn main_run_forever( let (tx_ping, rx_ping) = chan::unbounded(); let logger2 = logger.clone(); let cas2 = cas.clone(); + let project2 = project.into_skeleton(); // TODO: add the ability to pass extra_nix_options to watch let build_thread = { Async::run(logger, move || { match BuildLoop::new( - &project, + project2.into_project()?, NixOptions::empty(), nix_gc_root_user_dir, cas2, diff --git a/src/ops/error.rs b/src/ops/error.rs index d5488ca3..248a24a5 100644 --- a/src/ops/error.rs +++ b/src/ops/error.rs @@ -153,3 +153,25 @@ where } } } + +/// Spit up sqlite errors into programming errors, setup/environment issues and temporary things like full disk +impl ExitAs for rusqlite::Error { + fn exit_as(&self) -> ExitErrorType { + match self { + rusqlite::Error::SqliteFailure(err, _msg) => match err.code { + rusqlite::ErrorCode::CannotOpen + | rusqlite::ErrorCode::PermissionDenied + | rusqlite::ErrorCode::ReadOnly + | rusqlite::ErrorCode::NoLargeFileSupport + | rusqlite::ErrorCode::NotADatabase => ExitErrorType::EnvironmentProblem, + rusqlite::ErrorCode::OutOfMemory + | rusqlite::ErrorCode::SystemIoFailure + | rusqlite::ErrorCode::DatabaseCorrupt + | rusqlite::ErrorCode::DiskFull => ExitErrorType::Temporary, + + _ => ExitErrorType::Panic, + }, + _ => ExitErrorType::Panic, + } + } +} diff --git a/src/project.rs b/src/project.rs index f08daaff..274b0c38 100644 --- a/src/project.rs +++ b/src/project.rs @@ -17,7 +17,6 @@ use std::time::SystemTime; /// A “project” knows how to handle the lorri state /// for a given nix file. -#[derive(Clone)] pub struct Project { /// Absolute path to this project’s nix file. pub nix_file: NixFile, @@ -27,6 +26,8 @@ pub struct Project { /// Hash of the nix file’s absolute path. hash: String, + + conn: Sqlite, } impl Project { @@ -34,11 +35,11 @@ impl Project { /// and the base GC root directory /// (as returned by `Paths.gc_root_dir()`), pub fn new_and_gc_nix_files( - conn: &mut Sqlite, + mut conn: Sqlite, nix_file: NixFile, gc_root_dir: &AbsPathBuf, ) -> anyhow::Result { - let p = Self::new_internal(nix_file.clone(), gc_root_dir)?; + let p = Self::new_internal(nix_file.clone(), gc_root_dir, conn.clone()?)?; // Adjust the nix_file symlink to point to this project’s nix file conn.in_transaction(|t| { @@ -78,7 +79,11 @@ impl Project { })? } - fn new_internal(nix_file: NixFile, gc_root_dir: &AbsPathBuf) -> std::io::Result { + fn new_internal( + nix_file: NixFile, + gc_root_dir: &AbsPathBuf, + conn: Sqlite, + ) -> std::io::Result { let hash = format!( "{:x}", md5::compute(nix_file.as_absolute_path().as_os_str().as_bytes()) @@ -91,6 +96,7 @@ impl Project { project_root_dir, nix_file, hash, + conn, }) } @@ -98,6 +104,7 @@ impl Project { fn new_internal_from_existing_gc_dir( hash: String, gc_root_dir: &AbsPathBuf, + conn: Sqlite, ) -> Result { let project_root_dir = gc_root_dir.join(&hash); @@ -111,6 +118,7 @@ impl Project { project_root_dir, nix_file, hash, + conn, }) } @@ -153,6 +161,21 @@ impl Project { } } + /// Convert the project to a less juicy representation + pub fn into_skeleton(&self) -> ProjectSkeleton { + ProjectSkeleton { + nix_file: self.nix_file.clone(), + project_root_dir: self.project_root_dir.clone(), + hash: self.hash.clone(), + sqlite_path: self.conn.sqlite_path(), + } + } + + /// Clone this project via its skeleton + pub fn clone(&self) -> Result { + self.into_skeleton().into_project() + } + /// Create roots to store paths. pub fn create_roots( &self, @@ -201,8 +224,8 @@ where { } /// Removes this project from lorri. Removes the GC root and consumes the project. - pub fn remove_project(self, conn: &mut Sqlite) -> anyhow::Result<()> { - conn.in_transaction(|t| { + pub fn remove_project(mut self) -> anyhow::Result<()> { + self.conn.in_transaction(|t| { std::fs::remove_dir_all(&self.project_root_dir).context(format!( "Unable to remove the project directory from {}", self.project_root_dir.display() @@ -220,6 +243,7 @@ where { pub fn list_roots( logger: &slog::Logger, paths: &Paths, + conn: Sqlite, ) -> Result, ExitError> { let mut res = Vec::new(); let gc_root_dir_iter = std::fs::read_dir(paths.gc_root_dir()).map_err(|e| { @@ -256,21 +280,23 @@ where { .file_name() .to_string_lossy() .into_owned(); - let project = - match Project::new_internal_from_existing_gc_dir(hash.clone(), paths.gc_root_dir()) - { - Err(e) => { - warn!( - logger, - "Could not create project for hash {} in root dir {}, skipping: {}", - &hash, - paths.gc_root_dir().display(), - e - ); - continue; - } - Ok(p) => p, - }; + let project = match Project::new_internal_from_existing_gc_dir( + hash.clone(), + paths.gc_root_dir(), + conn.clone()?, + ) { + Err(e) => { + warn!( + logger, + "Could not create project for hash {} in root dir {}, skipping: {}", + &hash, + paths.gc_root_dir().display(), + e + ); + continue; + } + Ok(p) => p, + }; let timestamp = project.last_built_timestamp(); let alive = project.nix_file.as_absolute_path().is_file(); @@ -288,6 +314,26 @@ where { } } +/// A version of a project that can be re-initialized to a Project but transferred across thread boundaries. +pub struct ProjectSkeleton { + nix_file: NixFile, + project_root_dir: AbsPathBuf, + hash: String, + sqlite_path: AbsPathBuf, +} + +impl ProjectSkeleton { + /// Restores the skeleton into a Project again + pub fn into_project(self) -> rusqlite::Result { + Ok(Project { + nix_file: self.nix_file, + project_root_dir: self.project_root_dir, + hash: self.hash, + conn: Sqlite::new_connection(&self.sqlite_path)?, + }) + } +} + /// Represents a gc root along with some metadata, used for json output of lorri gc info pub struct GcRootInfo { /// directory where root is stored diff --git a/src/sqlite.rs b/src/sqlite.rs index e1241d27..64abc188 100644 --- a/src/sqlite.rs +++ b/src/sqlite.rs @@ -10,28 +10,47 @@ use crate::{constants::Paths, ops::error::ExitError, project::Project, AbsPathBu /// TODO pub struct Sqlite { conn: sqlite::Connection, + sqlite_path: AbsPathBuf, } impl Sqlite { /// Connect to sqlite - pub fn new_connection(sqlite_path: &AbsPathBuf) -> Self { - let conn = sqlite::Connection::open(sqlite_path.as_path()).expect("cannot open sqlite db"); - conn.execute_batch( + pub fn new_connection(sqlite_path: &AbsPathBuf) -> sqlite::Result { + let new = Self::new_connection_internal(sqlite_path)?; + new.conn.execute_batch( r#"CREATE TABLE IF NOT EXISTS gc_roots ( id INTEGER PRIMARY KEY, nix_file PATH UNIQUE, last_updated EPOCH_TIME ); "#, - ) - .unwrap(); + )?; - Self { conn } + Ok(new) + } + + fn new_connection_internal(sqlite_path: &AbsPathBuf) -> sqlite::Result { + let conn = sqlite::Connection::open(sqlite_path.as_path())?; + + Ok(Self { + conn, + sqlite_path: sqlite_path.clone(), + }) + } + + /// Clone this by creating a new connection to the database + pub fn clone(&self) -> sqlite::Result { + Self::new_connection_internal(&self.sqlite_path) + } + + /// Return the path to the sqlite file + pub fn sqlite_path(&self) -> AbsPathBuf { + self.sqlite_path.clone() } /// Migrate the GC roots into our sqlite pub fn migrate_gc_roots(&self, logger: &slog::Logger, paths: &Paths) -> Result<(), ExitError> { - let infos = Project::list_roots(logger, paths)?; + let infos = Project::list_roots(logger, paths, self.clone()?)?; self.conn.execute("DELETE FROM gc_roots", ()).unwrap(); diff --git a/tests/integration/direnvtestcase.rs b/tests/integration/direnvtestcase.rs index a948a00f..6228c5aa 100644 --- a/tests/integration/direnvtestcase.rs +++ b/tests/integration/direnvtestcase.rs @@ -42,13 +42,10 @@ impl DirenvTestCase { let shell_file = NixFile::from(AbsPathBuf::new(test_root.join("shell.nix")).unwrap()); let cas = ContentAddressable::new(cachedir.join("cas").to_owned()).unwrap(); - let mut conn = Sqlite::new_connection(&cachedir.join("sqlite")); - let project = Project::new_and_gc_nix_files( - &mut conn, - shell_file.clone(), - &cachedir.join("gc_roots"), - ) - .unwrap(); + let conn = Sqlite::new_connection(&cachedir.join("sqlite")).unwrap(); + let project = + Project::new_and_gc_nix_files(conn, shell_file.clone(), &cachedir.join("gc_roots")) + .unwrap(); DirenvTestCase { projectdir, @@ -63,7 +60,7 @@ impl DirenvTestCase { pub fn evaluate(&mut self) -> Result { let username = project::Username::from_env_var().unwrap(); BuildLoop::new( - &self.project, + self.project.clone().unwrap(), NixOptions::empty(), project::NixGcRootUserDir::get_or_create(&username).unwrap(), self.cas.clone(), @@ -78,7 +75,7 @@ impl DirenvTestCase { pub fn get_direnv_variables(&self) -> DirenvEnv { let envrc = File::create(self.projectdir.path().join(".envrc")).unwrap(); let paths = lorri::ops::get_paths().unwrap(); - ops::op_direnv(self.project.clone(), &paths, envrc, &self.logger).unwrap(); + ops::op_direnv(self.project.clone().unwrap(), &paths, envrc, &self.logger).unwrap(); { let mut allow = self.direnv_cmd(); diff --git a/tests/shell/main.rs b/tests/shell/main.rs index 8b9f74ef..e83fa2fc 100644 --- a/tests/shell/main.rs +++ b/tests/shell/main.rs @@ -28,7 +28,7 @@ fn cargo_bin(name: &str) -> PathBuf { fn loads_env() { let tempdir = tempfile::tempdir().expect("tempfile::tempdir() failed us!"); let cache_dir = &lorri::AbsPathBuf::new(tempdir.path().to_owned()).unwrap(); - let mut conn = Sqlite::new_connection(&cache_dir.join("sqlite")); + let conn = Sqlite::new_connection(&cache_dir.join("sqlite")).unwrap(); let project = { let test_root = AbsPathBuf::new(PathBuf::from_iter(&[ env!("CARGO_MANIFEST_DIR"), @@ -38,7 +38,7 @@ fn loads_env() { ])) .expect("CARGO_MANIFEST_DIR was not absolute"); Project::new_and_gc_nix_files( - &mut conn, + conn, NixFile::from(test_root.join("shell.nix")), &cache_dir.join("gc_roots"), )