Builder: use OutputPaths for root names

src/build_loop.rs

@@ -4,9 +4,9 @@
 use crate::builder;
 use crate::notify;
 use crate::pathreduction::reduce_paths;
+use crate::project::roots;
+use crate::project::roots::Roots;
 use crate::project::Project;
-use crate::roots;
-use crate::roots::Roots;
 use crate::watch::Watch;
 use std::sync::mpsc::Sender;
 
@@ -104,14 +104,10 @@ impl<'a> BuildLoop<'a> {
 
         debug!("named drvs: {:#?}", build.output_paths);
 
-        // create root for every field in OutputPaths
-        // TODO: remove build-0 dependency (see direnv.rs)
-        let output_paths = ::builder::OutputPaths {
-            shell_gc_root: roots.add("build-0", &build.output_paths.shell_gc_root)?,
+        let event = BuildResults {
+            output_paths: roots.create_roots(build.output_paths)?,
         };
 
-        let event = BuildResults { output_paths };
-
         // add all new (reduced) nix sources to the input source watchlist
         self.watch.extend(&paths.into_iter().collect::<Vec<_>>())?;
 

src/builder.rs

@@ -19,6 +19,7 @@ use std::process::{Command, Stdio};
 use std::thread;
 use NixFile;
 
+// TODO: when moving to CallOpts, you have to change the names of the roots CallOpts generates!
 fn instrumented_build(
     root_nix_file: &NixFile,
     cas: &ContentAddressable,

src/lib.rs

@@ -43,7 +43,6 @@ pub mod ops;
 pub mod osstrlines;
 pub mod pathreduction;
 pub mod project;
-pub mod roots;
 pub mod socket;
 pub mod watch;
 

src/ops/direnv/mod.rs

@@ -4,6 +4,7 @@ mod version;
 
 use self::version::{DirenvVersion, MIN_DIRENV_VERSION};
 use crate::ops::{ok, ok_msg, ExitError, OpResult};
+use crate::project::roots::Roots;
 use crate::project::Project;
 use crate::socket::communicate::client;
 use crate::socket::communicate::{Ping, DEFAULT_READ_TIMEOUT};
@@ -14,9 +15,6 @@ use std::process::Command;
 pub fn main(project: Project) -> OpResult {
     check_direnv_version()?;
 
-    let mut shell_root = project.gc_root_path.to_owned();
-    shell_root.push("build-0"); // !!!
-
     // TODO: don’t start build/evaluation automatically, let the user decide
     if let Ok(client) = client::ping(DEFAULT_READ_TIMEOUT).connect(
         &::socket::path::SocketPath::from(::ops::get_paths()?.daemon_socket_file()),
@@ -30,7 +28,9 @@ pub fn main(project: Project) -> OpResult {
         eprintln!("Uh oh, your lorri daemon is not running.");
     }
 
-    if !shell_root.exists() {
+    let root_paths = Roots::from_project(&project).paths();
+
+    if !root_paths.all_exist() {
         return Err(ExitError::errmsg(
             "Please start `lorri daemon` or run `lorri watch` before using direnv integration.",
         ));
@@ -50,7 +50,7 @@ watch_file "$EVALUATION_ROOT"
 
 {}
 "#,
-        shell_root.display(),
+        root_paths.shell_gc_root,
         include_str!("envrc.bash")
     ))
 }

src/project.rs

@@ -1,5 +1,7 @@
 //! Wrap a nix file and manage corresponding state.
 
+pub mod roots;
+
 use cas::ContentAddressable;
 use std::os::unix::ffi::OsStrExt;
 use std::path::{Path, PathBuf};
@@ -14,7 +16,7 @@ pub struct Project {
 
     /// Directory in which this project’s
     /// garbage collection roots are stored.
-    pub gc_root_path: PathBuf,
+    gc_root_path: PathBuf,
 
     /// Hash of the nix file’s absolute path.
     hash: String,

src/roots.rs → src/project/roots.rs

@@ -1,13 +1,17 @@
-//! TODO
+//! Handling of nix GC roots
+//!
+//! TODO: inline this module into `::project`
 use crate::project::Project;
+use builder::OutputPaths;
+use nix::StorePath;
 use std::env;
-use std::os::unix::fs::symlink;
 use std::path::{Path, PathBuf};
 
 /// Roots manipulation
 #[derive(Clone)]
 pub struct Roots {
-    root_dir: PathBuf,
+    /// The GC root directory in the lorri user cache dir
+    gc_root_path: PathBuf,
     id: String,
 }
 
@@ -22,6 +26,24 @@ impl RootPath {
     }
 }
 
+impl OutputPaths<RootPath> {
+    /// Check whether all all GC roots exist.
+    pub fn all_exist(&self) -> bool {
+        match self {
+            // Match here to ensure we cover every field
+            ::builder::OutputPaths { shell_gc_root } => shell_gc_root.0.exists(),
+        }
+    }
+
+    /// Check that the shell_gc_root is a directory.
+    pub fn shell_gc_root_is_dir(&self) -> bool {
+        match self.shell_gc_root.0.metadata() {
+            Err(_) => false,
+            Ok(m) => m.is_dir(),
+        }
+    }
+}
+
 /// Proxy through the `Display` class for `PathBuf`.
 impl std::fmt::Display for RootPath {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
@@ -35,25 +57,48 @@ impl Roots {
     /// and ID.
     pub fn from_project(project: &Project) -> Roots {
         Roots {
-            root_dir: project.gc_root_path.to_path_buf(),
+            gc_root_path: project.gc_root_path.to_path_buf(),
             id: project.hash().to_string(),
         }
     }
 
-    // TODO: rename to create_root()
+    /// Return the filesystem paths for these roots.
+    pub fn paths(&self) -> OutputPaths<RootPath> {
+        OutputPaths {
+            shell_gc_root: RootPath(self.gc_root_path.join("shell_gc_root")),
+        }
+    }
+
+    /// Create roots to store paths.
+    pub fn create_roots(
+        &self,
+        // Important: this intentionally only allows creating
+        // roots to `StorePath`, not to `DrvFile`, because we have
+        // no use case for creating GC roots for drv files.
+        paths: OutputPaths<StorePath>,
+    ) -> Result<OutputPaths<RootPath>, AddRootError>
+where {
+        Ok(OutputPaths {
+            shell_gc_root: self.add("shell_gc_root", &paths.shell_gc_root)?,
+        })
+    }
+
     /// Store a new root under name
-    pub fn add(&self, name: &str, store_path: &::nix::StorePath) -> Result<RootPath, AddRootError> {
-        let mut path = self.root_dir.clone();
+    fn add(&self, name: &str, store_path: &StorePath) -> Result<RootPath, AddRootError> {
+        // final path in the `self.gc_root_path` directory
+        let mut path = self.gc_root_path.clone();
         path.push(name);
 
-        debug!("Adding root from {:?} to {:?}", store_path, path,);
+        debug!("Adding root from {:?} to {:?}", store_path.as_path(), path,);
         std::fs::remove_file(&path).or_else(|e| AddRootError::remove(e, &path))?;
 
         std::fs::remove_file(&path).or_else(|e| AddRootError::remove(e, &path))?;
 
-        symlink(store_path.as_path(), &path)
+        // the forward GC root that points from the store path to our cache gc_roots dir
+        std::os::unix::fs::symlink(store_path.as_path(), &path)
             .map_err(|e| AddRootError::symlink(e, store_path.as_path(), &path))?;
 
+        // the reverse GC root that points from nix to our cache gc_roots dir
         let mut root = if let Ok(path) = env::var("NIX_STATE_DIR") {
             PathBuf::from(path)
         } else {
@@ -62,7 +107,7 @@ impl Roots {
         root.push("gcroots");
         root.push("per-user");
 
-        // TODO: check on start
+        // TODO: check on start of lorri
         root.push(env::var("USER").expect("env var 'USER' must be set"));
 
         // The user directory sometimes doesn’t exist,
@@ -76,8 +121,10 @@ impl Roots {
         debug!("Connecting root from {:?} to {:?}", path, root,);
         std::fs::remove_file(&root).or_else(|e| AddRootError::remove(e, &root))?;
 
-        symlink(&path, &root).map_err(|e| AddRootError::symlink(e, &path, &root))?;
+        std::os::unix::fs::symlink(&path, &root)
+            .map_err(|e| AddRootError::symlink(e, &path, &root))?;
 
+        // TODO: don’t return the RootPath here
         Ok(RootPath(path))
     }
 }

tests/integration/direnvtestcase.rs

@@ -20,7 +20,7 @@ pub struct DirenvTestCase {
     projectdir: TempDir,
     // only kept around to not delete tempdir
     #[allow(dead_code)]
-    cachedir: TempDir,
+    pub cachedir: TempDir,
     project: Project,
 }
 

tests/integration/trivial.rs

@@ -2,10 +2,26 @@ use direnv::DirenvValue;
 use direnvtestcase::DirenvTestCase;
 
 #[test]
-fn trivial() {
+fn trivial() -> std::io::Result<()> {
     let mut testcase = DirenvTestCase::new("basic");
-    testcase.evaluate().expect("Failed to build the first time");
+    let res = testcase.evaluate().expect("Failed to build the first time");
+
+    assert!(
+        res.output_paths.all_exist(),
+        "no build output (build-0) in {}.\nContents of {}\n{}",
+        res.output_paths.shell_gc_root,
+        testcase.cachedir.path().display(),
+        std::str::from_utf8(
+            &std::process::Command::new("ls")
+                .args(&["-la", "--recursive"])
+                .args(&[testcase.cachedir.path().as_os_str()])
+                .output()?
+                .stdout
+        )
+        .unwrap()
+    );
 
     let env = testcase.get_direnv_variables();
     assert_eq!(env.get_env("MARKER"), DirenvValue::Value("present"));
+    Ok(())
 }