Merge pull request #502 from Oxen-AI/bug/oxen-add

Bug/oxen add

Author: gschoeni

src/cli/src/cmd/add.rs

@@ -4,6 +4,7 @@ use async_trait::async_trait;
 use clap::{Arg, Command};
 use liboxen::error::OxenError;
 
+use crate::util;
 use liboxen::model::LocalRepository;
 use liboxen::opts::AddOpts;
 use liboxen::repositories;
@@ -48,7 +49,7 @@ impl RunCmd for AddCmd {
                     OxenError::basic_str(format!("Failed to get current directory: {}", e))
                 })?;
                 let joined_path = current_dir.join(p);
-                joined_path.canonicalize().or_else(|_| Ok(joined_path))
+                util::fs::canonicalize(&joined_path).or_else(|_| Ok(joined_path))
             })
             .collect::<Result<Vec<PathBuf>, OxenError>>()?;
 

src/cli/src/cmd/init.rs

@@ -7,6 +7,7 @@ use liboxen::error::OxenError;
 
 use crate::cmd::RunCmd;
 use crate::helpers::{check_remote_version, get_host_or_default};
+use crate::util;
 use liboxen::repositories;
 
 pub const INIT: &str = "init";
@@ -48,7 +49,7 @@ impl RunCmd for InitCmd {
         check_remote_version(host).await?;
 
         // Initialize the repository
-        let directory = dunce::canonicalize(PathBuf::from(&path))?;
+        let directory = util::fs::canonicalize(PathBuf::from(&path))?;
         repositories::init::init_with_version(&directory, oxen_version)?;
         println!("🐂 repository initialized at: {directory:?}");
         Ok(())

src/cli/src/cmd/rm.rs

@@ -3,6 +3,7 @@ use clap::{Arg, ArgMatches, Command};
 
 use crate::helpers::check_repo_migration_needed;
 
+use crate::util;
 use liboxen::error::OxenError;
 use liboxen::model::LocalRepository;
 use liboxen::opts::RmOpts;
@@ -52,7 +53,7 @@ impl RunCmd for RmCmd {
                     OxenError::basic_str(format!("Failed to get current directory: {}", e))
                 })?;
                 let joined_path = current_dir.join(p);
-                joined_path.canonicalize().or_else(|_| Ok(joined_path))
+                util::fs::canonicalize(&joined_path).or_else(|_| Ok(joined_path))
             })
             .collect::<Result<Vec<PathBuf>, OxenError>>()?;
 

src/lib/src/api/client/workspaces/files.rs

@@ -62,7 +62,7 @@ fn resolve_remote_add_file_path(
     opts: &AddOpts,
 ) -> Result<(PathBuf, PathBuf), OxenError> {
     let path = path.as_ref();
-    match dunce::canonicalize(path) {
+    match util::fs::canonicalize(path) {
         Ok(path) => {
             if util::fs::file_exists_in_directory(&repo.path, &path) {
                 // Path is in the repo, so we get the remote directory from the repo path

src/lib/src/core/v0_19_0/add.rs

@@ -190,7 +190,7 @@ pub fn process_add_dir(
     let path = path.clone();
     let repo = repo.clone();
     let maybe_head_commit = maybe_head_commit.clone();
-    let repo_path = repo.path.clone();
+    let repo_path = &repo.path.clone();
 
     use std::sync::atomic::{AtomicU64, Ordering};
     use std::sync::Arc;
@@ -219,7 +219,9 @@ pub fn process_add_dir(
             let added_file_counter_clone = Arc::clone(&added_file_counter);
             let unchanged_file_counter_clone = Arc::clone(&unchanged_file_counter);
 
-            let dir_path = util::fs::path_relative_to_dir(dir, &repo_path).unwrap();
+            let dir_path = util::fs::path_relative_to_dir(dir, repo_path).unwrap();
+            log::debug!("path now: {dir_path:?}");
+
             let dir_node = maybe_load_directory(&repo, &maybe_head_commit, &dir_path).unwrap();
             let seen_dirs = Arc::new(Mutex::new(HashSet::new()));
 
@@ -246,7 +248,7 @@ pub fn process_add_dir(
                 let seen_dirs_clone = Arc::clone(&seen_dirs);
                 match process_add_file(
                     &repo,
-                    &repo_path,
+                    repo_path,
                     versions_path,
                     staged_db,
                     &dir_node,
@@ -314,21 +316,21 @@ fn add_file_inner(
     path: &Path,
     staged_db: &DBWithThreadMode<MultiThreaded>,
 ) -> Result<Option<StagedMerkleTreeNode>, OxenError> {
-    let repo_path = repo.path.clone();
+    let repo_path = &repo.path.clone();
     let versions_path = util::fs::oxen_hidden_dir(&repo.path)
         .join(VERSIONS_DIR)
         .join(FILES_DIR);
     let mut maybe_dir_node = None;
     if let Some(head_commit) = maybe_head_commit {
-        let path = util::fs::path_relative_to_dir(path, &repo_path)?;
+        let path = util::fs::path_relative_to_dir(path, repo_path)?;
         let parent_path = path.parent().unwrap_or(Path::new(""));
         maybe_dir_node = CommitMerkleTree::dir_with_children(repo, head_commit, parent_path)?;
     }
 
     let seen_dirs = Arc::new(Mutex::new(HashSet::new()));
     process_add_file(
         repo,
-        &repo_path,
+        repo_path,
         &versions_path,
         staged_db,
         &maybe_dir_node,

src/lib/src/core/v0_19_0/rm.rs

@@ -383,6 +383,7 @@ fn remove_inner(
     for path in paths {
         // Get parent node
         let path = util::fs::path_relative_to_dir(path, &repo.path)?;
+
         let parent_path = path.parent().unwrap_or(Path::new(""));
         let parent_node: MerkleTreeNode = if let Some(dir_node) =
             CommitMerkleTree::dir_with_children(repo, &head_commit, parent_path)?

src/lib/src/model/data_frame/schema.rs

@@ -284,7 +284,7 @@ impl Schema {
     }
 
     pub fn num_bytes(&self) -> u64 {
-        let bytes = serde_json::to_string(&self).unwrap().as_bytes().len();
+        let bytes = serde_json::to_string(&self).unwrap().len();
         bytes as u64
     }
 

src/lib/src/repositories/diffs.rs

@@ -130,13 +130,12 @@ pub fn diff(
             return Err(OxenError::basic_str(error));
         };
 
-        let (file_node,) = (
-            repositories::entries::get_file(&repository, &commit, &path_1)?.ok_or_else(|| {
+        let file_node = repositories::entries::get_file(&repository, &commit, &path_1)?
+            .ok_or_else(|| {
                 OxenError::ResourceNotFound(
                     format!("Error: file {} not committed", path_1.as_ref().display()).into(),
                 )
-            })?,
-        );
+            })?;
 
         let hash = file_node.hash.to_string();
 

src/lib/src/util/fs.rs

@@ -1341,17 +1341,64 @@ pub fn is_in_oxen_hidden_dir(path: &Path) -> bool {
     false
 }
 
+// Return canonicalized path if possible, otherwise return original
+pub fn canonicalize(path: impl AsRef<Path>) -> Result<PathBuf, OxenError> {
+    let path = path.as_ref();
+    //log::debug!("Path to canonicalize: {path:?}");
+    match dunce::canonicalize(path) {
+        Ok(canon_path) => Ok(canon_path),
+        Err(err) => Err(OxenError::basic_str(format!(
+            "path {path:?} cannot be canonicalized due to err {err:?}"
+        ))),
+    }
+}
+
 pub fn path_relative_to_dir(
     path: impl AsRef<Path>,
     dir: impl AsRef<Path>,
 ) -> Result<PathBuf, OxenError> {
-    let path = path.as_ref();
-    let dir = dir.as_ref();
+    // Embedded canonicalize calls to catch every case:
+    // -- if both paths can be canonicalized, they both will be
+    // -- if only the dir can be canonicalized, only the dir will be
+    // -- if the dir cannot be canonicalized, neither will be
+    let (path, dir) = match canonicalize(&dir) {
+        Ok(canon_dir) => {
+            /*log::debug!(
+              "dir {:?} canonicalized. Checking path {:?}",
+                dir.as_ref(),
+                path.as_ref()
+            );*/
+            match canonicalize(&path) {
+                Ok(canon_path) => (canon_path, canon_dir),
+                // '_' because the debug statement is commented out
+                Err(_err) => {
+                    /*log::debug!(
+                        "Err with canonicalization: {err:?}. Returning path {:?} immediately",
+                        path.as_ref()
+                    );*/
+                    return Ok(path.as_ref().to_path_buf());
+                }
+            }
+        }
+        // '_' because the debug statement is commented out
+        Err(_err) => {
+            /*log::debug!(
+                "Err with canonicalization: {err:?}. Skipping canonicalization of path {:?}",
+                path.as_ref()
+            );*/
+            (path.as_ref().to_path_buf(), dir.as_ref().to_path_buf())
+        }
+    };
 
-    let mut mut_path = path.to_path_buf();
+    let mut mut_path = path.clone();
     let mut components: Vec<PathBuf> = vec![];
     while mut_path.parent().is_some() {
-        // println!("Comparing {:?} => {:?} => {:?}", path, mut_path.parent(), dir);
+        /*log::debug!(
+            "Comparing {:?} => {:?} => {:?}",
+            path,
+            mut_path.parent(),
+            dir
+        );*/
         if let Some(filename) = mut_path.file_name() {
             if mut_path != dir {
                 components.push(PathBuf::from(filename));