Assure basic 'organize' operation is working as expected

Author: Byron

git-features/src/fs.rs

@@ -10,7 +10,7 @@
 ///
 pub mod walkdir {
     pub use jwalk::{DirEntry as DirEntryGeneric, Error, WalkDir};
-    use std::path::PathBuf;
+    use std::path::{Path, PathBuf};
 
     /// An alias for an uncustomized directory entry to match the one of the non-parallel version offered by `walkdir`.
     pub type DirEntry = DirEntryGeneric<((), ())>;
@@ -24,12 +24,17 @@ pub mod walkdir {
     pub fn direntry_path(entry: &DirEntry) -> PathBuf {
         entry.path()
     }
+
+    /// Instantiate a new directory iterator which will not skip hidden files.
+    pub fn walkdir_new(root: impl AsRef<Path>) -> WalkDir {
+        WalkDir::new(root).skip_hidden(false)
+    }
 }
 
 #[cfg(not(feature = "parallel"))]
 ///
 pub mod walkdir {
-    use std::path::PathBuf;
+    use std::path::{Path, PathBuf};
     pub use walkdir::{DirEntry, Error, WalkDir};
 
     /// Enable sorting by filename
@@ -41,6 +46,11 @@ pub mod walkdir {
     pub fn direntry_path(entry: &DirEntry) -> PathBuf {
         entry.path().to_owned()
     }
+
+    /// Instantiate a new directory iterator which will not skip hidden files.
+    pub fn walkdir_new(root: impl AsRef<Path>) -> WalkDir {
+        WalkDir::new(root)
+    }
 }
 
-pub use self::walkdir::{direntry_path, sorted, DirEntry, WalkDir};
+pub use self::walkdir::{direntry_path, sorted, walkdir_new, DirEntry, WalkDir};

gitoxide-core/src/organize.rs

@@ -34,14 +34,15 @@ fn find_git_repository_workdirs(root: impl AsRef<Path>, mut progress: impl Progr
         }
     }
 
-    let walk = fs::sorted(fs::WalkDir::new(root).follow_links(false));
+    let walk = fs::sorted(fs::walkdir_new(root).follow_links(false));
     walk.into_iter()
         .filter_map(move |entry| {
             progress.step();
             match entry {
                 Ok(entry) => Some(entry),
-                Err(err) => {
-                    progress.fail(format!("Ignored: {}", err.to_string()));
+                Err(_err) => {
+                    // TODO: remove this line once we properly ignore git repository - they get moved
+                    // progress.fail(format!("Ignored: {}", _err.to_string()));
                     None
                 }
             }
@@ -69,10 +70,15 @@ fn find_origin_remote(repo: &Path) -> anyhow::Result<Option<git_url::Url>> {
     }
 }
 
-fn handle(mode: Mode, git_workdir: &Path, destination: &Path, progress: &mut impl Progress) -> anyhow::Result<()> {
+fn handle(
+    mode: Mode,
+    git_workdir: &Path,
+    canonicalized_destination: &Path,
+    progress: &mut impl Progress,
+) -> anyhow::Result<()> {
     fn to_relative(path: PathBuf) -> PathBuf {
-        std::iter::once(std::path::Component::CurDir)
-            .chain(path.components())
+        path.components()
+            .skip_while(|c| c == &std::path::Component::RootDir)
             .collect()
     }
 
@@ -95,16 +101,30 @@ fn handle(mode: Mode, git_workdir: &Path, destination: &Path, progress: &mut imp
         return Ok(());
     }
 
-    let destination = destination.join(to_relative(git_url::expand_path(None, url.path.as_bstr())?));
+    let destination = canonicalized_destination
+        .join(
+            url.host
+                .as_ref()
+                .ok_or_else(|| anyhow::Error::msg(format!("Remote URLs must have host names: {}", url)))?,
+        )
+        .join(to_relative(git_url::expand_path(None, url.path.as_bstr())?));
     match mode {
         Mode::Simulate => progress.info(format!(
             "WOULD move {} to {}",
             git_workdir.display(),
             destination.display()
         )),
         Mode::Execute => {
-            std::fs::rename(git_workdir, &destination)?;
-            progress.info(format!("Moved {} to {}", git_workdir.display(), destination.display()))
+            if git_workdir.canonicalize()? == destination {
+                progress.info(format!(
+                    "Skipping {:?} as it is in the correct spot",
+                    git_workdir.display()
+                ));
+            } else {
+                std::fs::create_dir_all(destination.parent().expect("repo destination is not the root"))?;
+                progress.info(format!("Moving {} to {}", git_workdir.display(), destination.display()));
+                std::fs::rename(git_workdir, &destination)?;
+            }
         }
     }
     Ok(())
@@ -114,6 +134,7 @@ pub fn run(mode: Mode, source_dir: PathBuf, destination: PathBuf, mut progress:
     let search_progress = progress.add_child("Searching repositories");
 
     let mut num_errors = 0usize;
+    let destination = destination.canonicalize()?;
     for path_to_move in find_git_repository_workdirs(source_dir, search_progress) {
         if let Err(err) = handle(mode, &path_to_move, &destination, &mut progress) {
             progress.fail(format!(

src/porcelain/lean.rs

@@ -73,8 +73,8 @@ pub fn main() -> Result<()> {
                 } else {
                     organize::Mode::Simulate
                 },
-                repository_source.unwrap_or_else(|| std::env::current_dir().expect("CWD as default source")),
-                destination_directory.unwrap_or_else(|| std::env::current_dir().expect("CWD as default destination")),
+                repository_source.unwrap_or_else(|| [std::path::Component::CurDir].iter().collect()),
+                destination_directory.unwrap_or_else(|| [std::path::Component::CurDir].iter().collect()),
                 DoOrDiscard::from(progress),
             )
         }

src/porcelain/pretty.rs

@@ -78,9 +78,8 @@ pub fn main() -> Result<()> {
                         } else {
                             organize::Mode::Simulate
                         },
-                        repository_source.unwrap_or_else(|| std::env::current_dir().expect("CWD as default source")),
-                        destination_directory
-                            .unwrap_or_else(|| std::env::current_dir().expect("CWD as default destination")),
+                        repository_source.unwrap_or_else(|| [std::path::Component::CurDir].iter().collect()),
+                        destination_directory.unwrap_or_else(|| [std::path::Component::CurDir].iter().collect()),
                         DoOrDiscard::from(progress),
                     )
                 },

tests/snapshots/organize/directory-structure-after-organize

@@ -0,0 +1,11 @@
+.
+├── dir
+│   └── one-origin
+├── example.com
+│   ├── one-origin
+│   └── origin-and-fork
+├── no-origin
+├── origin-and-fork
+└── special-origin
+
+8 directories, 0 files

tests/snapshots/organize/execute-success

@@ -62,6 +62,30 @@ title "Porcelain ${kind}"
           expect_run $SUCCESSFULLY tree -L 2
         }
       )
+
+      (when "running with --execute"
+        it "succeeds" && {
+          WITH_SNAPSHOT="$snapshot/execute-success" \
+          expect_run_sh $SUCCESSFULLY "$exe organize --execute 2>/dev/null"
+        }
+
+        it "changes the directory structure" && {
+          WITH_SNAPSHOT="$snapshot/directory-structure-after-organize" \
+          expect_run $SUCCESSFULLY tree -L 2
+        }
+      )
+
+      (when "running with --execute again"
+        it "succeeds" && {
+          WITH_SNAPSHOT="$snapshot/execute-success" \
+          expect_run_sh $SUCCESSFULLY "$exe organize --execute 2>/dev/null"
+        }
+
+        it "does not alter the directory structure as these are already in place" && {
+          WITH_SNAPSHOT="$snapshot/directory-structure-after-organize" \
+          expect_run $SUCCESSFULLY tree -L 2
+        }
+      )
     )
   )
 )