Repository: radicle-dev/heartwood (rust)

Issue:
Canonicalize `RAD_HOME`
Currently if we set `RAD_HOME` to some relative path, eg. `../tmp`, and then call `rad self`, the paths will show up as relative. This is because we store them that way in Home/Profile.

We should canonicalize them so that they are stored as absolute paths.

https://doc.rust-lang.org/std/path/struct.Path.html#method.canonicalize

Pull request:
radicle: canonicalize home path
If a path was passed to `Home` in non-canonical form it would not use the right path for any initialisation or use.

The aim is to use the `canonicalize` function for paths. However, this may return an error if none of the paths exist. To ensure that the path exists the `new` constructor for `Home` creates the directory and canonicalizes the path.

Since `new` now initialises the home directory, it would be useful if it also initialised all the necessary subdirectories -- which removes the need for the `init` method.

Fixes #313 

Patch:
diff --git a/radicle/src/profile.rs b/radicle/src/profile.rs
index 8c1b4b2d..c5d8b4c3 100644
--- a/radicle/src/profile.rs
+++ b/radicle/src/profile.rs
@@ -67,7 +67,6 @@ pub struct Profile {
 
 impl Profile {
     pub fn init(home: Home, passphrase: impl Into<Passphrase>) -> Result<Self, Error> {
-        let home = home.init()?;
         let storage = Storage::open(home.storage())?;
         let keystore = Keystore::new(&home.keys());
         let public_key = keystore.init("radicle", passphrase)?;
@@ -142,9 +141,9 @@ impl Profile {
 /// Get the path to the radicle home folder.
 pub fn home() -> Result<Home, io::Error> {
     if let Some(home) = env::var_os(env::RAD_HOME) {
-        Ok(Home::new(PathBuf::from(home)))
+        Ok(Home::new(PathBuf::from(home))?)
     } else if let Some(home) = env::var_os("HOME") {
-        Ok(Home::new(PathBuf::from(home).join(".radicle")))
+        Ok(Home::new(PathBuf::from(home).join(".radicle"))?)
     } else {
         Err(io::Error::new(
             io::ErrorKind::NotFound,
@@ -166,14 +165,33 @@ impl From<PathBuf> for Home {
 }
 
 impl Home {
-    pub fn init(self) -> Result<Self, io::Error> {
-        fs::create_dir_all(self.node()).ok();
+    /// Creates the Radicle Home directories.
+    ///
+    /// The `home` path is used as the base directory for all
+    /// necessary subdirectories.
+    ///
+    /// If `home` does not already exist then it and any
+    /// subdirectories are created using [`fs::create_dir_all`].
+    ///
+    /// The `home` path is also canonicalized using [`fs::canonicalize`].
+    ///
+    /// All necessary subdirectories are also created.
+    pub fn new(home: impl Into<PathBuf>) -> Result<Self, io::Error> {
+        let path = home.into();
+        if !path.exists() {
+            fs::create_dir_all(path.clone())?;
+        }
+        let home = Self {
+            path: path.canonicalize()?,
+        };
 
-        Ok(self)
-    }
+        for dir in &[home.storage(), home.keys(), home.node()] {
+            if !dir.exists() {
+                fs::create_dir_all(dir)?;
+            }
+        }
 
-    pub fn new(home: impl Into<PathBuf>) -> Self {
-        Self { path: home.into() }
+        Ok(home)
     }
 
     pub fn path(&self) -> &Path {
@@ -198,3 +216,33 @@ impl Home {
             .unwrap_or_else(|| self.node().join(node::DEFAULT_SOCKET_NAME))
     }
 }
+
+#[cfg(test)]
+mod test {
+    use std::fs;
+
+    use super::Home;
+
+    // Checks that if we have:
+    // '/run/user/1000/.tmpqfK6ih/../.tmpqfK6ih/Radicle/Home'
+    //
+    // that it gets normalized to:
+    // '/run/user/1000/.tmpqfK6ih/Radicle/Home'
+    #[test]
+    fn canonicalize_home() {
+        let tmp = tempfile::tempdir().unwrap();
+        let path = tmp.path().join("Home").join("Radicle");
+        fs::create_dir_all(path.clone()).unwrap();
+
+        let last = tmp.path().components().last().unwrap();
+        let home = Home::new(
+            tmp.path()
+                .join("..")
+                .join(last)
+                .join("Home")
+                .join("Radicle"),
+        )
+        .unwrap();
+        assert_eq!(home.path, path);
+    }
+}


Test patch:
diff --git a/radicle-node/src/test/environment.rs b/radicle-node/src/test/environment.rs
index 48037e91..cca8f149 100644
--- a/radicle-node/src/test/environment.rs
+++ b/radicle-node/src/test/environment.rs
@@ -71,9 +71,7 @@ impl Environment {
     /// Create a new profile in this environment.
     /// This should be used when a running node is not required.
     pub fn profile(&mut self, name: &str) -> Profile {
-        let home = Home::new(self.tmp().join("home").join(name))
-            .init()
-            .unwrap();
+        let home = Home::new(self.tmp().join("home").join(name)).unwrap();
         let storage = Storage::open(home.storage()).unwrap();
         let keystore = Keystore::new(&home.keys());
         let keypair = KeyPair::from_seed(Seed::from([!(self.users as u8); 32]));
@@ -232,7 +230,7 @@ impl Node<MockSigner> {
                 .take(8)
                 .collect::<String>(),
         );
-        let home = Home::new(home).init().unwrap();
+        let home = Home::new(home).unwrap();
         let signer = MockSigner::default();
         let storage = Storage::open(home.storage()).unwrap();
 
diff --git a/radicle/src/test.rs b/radicle/src/test.rs
index 47cc6762..5e96a722 100644
--- a/radicle/src/test.rs
+++ b/radicle/src/test.rs
@@ -19,7 +19,7 @@ pub mod setup {
         let mut rng = fastrand::Rng::new();
         let signer = MockSigner::new(&mut rng);
         let home = tmp.path().join("home");
-        let paths = Home::new(home.as_path());
+        let paths = Home::new(home.as_path()).unwrap();
         let storage = Storage::open(paths.storage()).unwrap();
         let (id, _, _, _) = fixtures::project(tmp.path().join("copy"), &storage, &signer).unwrap();
         let project = storage.repository(id).unwrap();