Patchwork D9906: rust: Fold find_root and check_requirements into Repo::find

login
register
mail settings
Submitter phabricator
Date Jan. 28, 2021, 7:40 p.m.
Message ID <differential-rev-PHID-DREV-fkfnbpldywuqmmesteyo-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48217/
State Superseded
Headers show

Comments

phabricator - Jan. 28, 2021, 7:40 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D9906

AFFECTED FILES
  rust/hg-core/src/config/config.rs
  rust/hg-core/src/operations/find_root.rs
  rust/hg-core/src/operations/mod.rs
  rust/hg-core/src/repo.rs
  rust/rhg/src/commands/cat.rs
  rust/rhg/src/commands/files.rs
  rust/rhg/src/error.rs
  tests/test-rhg.t

CHANGE DETAILS




To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/tests/test-rhg.t b/tests/test-rhg.t
--- a/tests/test-rhg.t
+++ b/tests/test-rhg.t
@@ -22,7 +22,7 @@ 
 
 Finding root
   $ rhg root
-  abort: no repository found in '$TESTTMP' (.hg not found)!
+  abort: no repository found in the current directory or ancestors! (.hg not found)
   [255]
 
   $ hg init repository
@@ -153,13 +153,7 @@ 
   [252]
 
   $ rhg debugrequirements
-  dotencode
-  fncache
-  generaldelta
-  revlogv1
-  sparserevlog
-  store
-  indoor-pool
+  [252]
 
   $ echo -e '\xFF' >> .hg/requires
   $ rhg debugrequirements
diff --git a/rust/rhg/src/error.rs b/rust/rhg/src/error.rs
--- a/rust/rhg/src/error.rs
+++ b/rust/rhg/src/error.rs
@@ -1,7 +1,7 @@ 
 use crate::ui::utf8_to_local;
 use crate::ui::UiError;
-use hg::errors::{HgError, IoErrorContext};
-use hg::operations::FindRootError;
+use hg::errors::HgError;
+use hg::repo::RepoFindError;
 use hg::revlog::revlog::RevlogError;
 use std::convert::From;
 
@@ -48,18 +48,16 @@ 
     }
 }
 
-impl From<FindRootError> for CommandError {
-    fn from(err: FindRootError) -> Self {
-        match err {
-            FindRootError::RootNotFound(path) => CommandError::abort(format!(
-                "no repository found in '{}' (.hg not found)!",
-                path.display()
-            )),
-            FindRootError::GetCurrentDirError(error) => HgError::IoError {
-                error,
-                context: IoErrorContext::CurrentDir,
+impl From<RepoFindError> for CommandError {
+    fn from(error: RepoFindError) -> Self {
+        match error {
+            RepoFindError::NotFoundInCurrentDirectoryAncestors => {
+                CommandError::abort(
+                "no repository found in the current directory or ancestors! \
+                        (.hg not found)"
+                )
             }
-            .into(),
+            RepoFindError::Other(error) => error.into()
         }
     }
 }
diff --git a/rust/rhg/src/commands/files.rs b/rust/rhg/src/commands/files.rs
--- a/rust/rhg/src/commands/files.rs
+++ b/rust/rhg/src/commands/files.rs
@@ -48,7 +48,6 @@ 
 impl<'a> Command for FilesCommand<'a> {
     fn run(&self, ui: &Ui) -> Result<(), CommandError> {
         let repo = Repo::find()?;
-        repo.check_requirements()?;
         if let Some(rev) = self.rev {
             let files =
                 list_rev_tracked_files(&repo, rev).map_err(|e| (e, rev))?;
diff --git a/rust/rhg/src/commands/cat.rs b/rust/rhg/src/commands/cat.rs
--- a/rust/rhg/src/commands/cat.rs
+++ b/rust/rhg/src/commands/cat.rs
@@ -31,7 +31,6 @@ 
     #[timed]
     fn run(&self, ui: &Ui) -> Result<(), CommandError> {
         let repo = Repo::find()?;
-        repo.check_requirements()?;
         let cwd = hg::utils::current_dir()?;
 
         let mut files = vec![];
diff --git a/rust/hg-core/src/repo.rs b/rust/hg-core/src/repo.rs
--- a/rust/hg-core/src/repo.rs
+++ b/rust/hg-core/src/repo.rs
@@ -1,5 +1,4 @@ 
 use crate::errors::{HgError, IoResultExt};
-use crate::operations::{find_root, FindRootError};
 use crate::requirements;
 use memmap::{Mmap, MmapOptions};
 use std::path::{Path, PathBuf};
@@ -11,6 +10,13 @@ 
     store: PathBuf,
 }
 
+#[derive(Debug, derive_more::From)]
+pub enum RepoFindError {
+    NotFoundInCurrentDirectoryAncestors,
+    #[from]
+    Other(HgError),
+}
+
 /// Filesystem access abstraction for the contents of a given "base" diretory
 #[derive(Clone, Copy)]
 pub(crate) struct Vfs<'a> {
@@ -18,24 +24,24 @@ 
 }
 
 impl Repo {
-    /// Returns `None` if the given path doesn’t look like a repository
-    /// (doesn’t contain a `.hg` sub-directory).
-    pub fn for_path(root: impl Into<PathBuf>) -> Self {
-        let working_directory = root.into();
-        let dot_hg = working_directory.join(".hg");
-        Self {
-            store: dot_hg.join("store"),
-            dot_hg,
-            working_directory,
+    /// Search the current directory and its ancestores for a repository:
+    /// a working directory that contains a `.hg` sub-directory.
+    pub fn find() -> Result<Self, RepoFindError> {
+        // ancestors() is inclusive: it first yields the current directory
+        // as-is.
+        for ancestor in crate::utils::current_dir()?.ancestors() {
+            let dot_hg = ancestor.join(".hg");
+            if dot_hg.is_dir() {
+                let repo = Self {
+                    store: dot_hg.join("store"),
+                    dot_hg,
+                    working_directory: ancestor.to_owned(),
+                };
+                requirements::check(&repo)?;
+                return Ok(repo);
+            }
         }
-    }
-
-    pub fn find() -> Result<Self, FindRootError> {
-        find_root().map(Self::for_path)
-    }
-
-    pub fn check_requirements(&self) -> Result<(), HgError> {
-        requirements::check(self)
+        Err(RepoFindError::NotFoundInCurrentDirectoryAncestors)
     }
 
     pub fn working_directory_path(&self) -> &Path {
@@ -65,11 +71,15 @@ 
 }
 
 impl Vfs<'_> {
+    pub(crate) fn join(&self, relative_path: impl AsRef<Path>) -> PathBuf {
+        self.base.join(relative_path)
+    }
+
     pub(crate) fn read(
         &self,
         relative_path: impl AsRef<Path>,
     ) -> Result<Vec<u8>, HgError> {
-        let path = self.base.join(relative_path);
+        let path = self.join(relative_path);
         std::fs::read(&path).for_file(&path)
     }
 
diff --git a/rust/hg-core/src/operations/mod.rs b/rust/hg-core/src/operations/mod.rs
--- a/rust/hg-core/src/operations/mod.rs
+++ b/rust/hg-core/src/operations/mod.rs
@@ -5,10 +5,8 @@ 
 mod cat;
 mod debugdata;
 mod dirstate_status;
-mod find_root;
 mod list_tracked_files;
 pub use cat::cat;
 pub use debugdata::{debug_data, DebugDataKind};
-pub use find_root::{find_root, find_root_from_path, FindRootError};
 pub use list_tracked_files::Dirstate;
 pub use list_tracked_files::{list_rev_tracked_files, FilesForRev};
diff --git a/rust/hg-core/src/operations/find_root.rs b/rust/hg-core/src/operations/find_root.rs
deleted file mode 100644
--- a/rust/hg-core/src/operations/find_root.rs
+++ /dev/null
@@ -1,79 +0,0 @@ 
-use std::path::{Path, PathBuf};
-
-/// Error type for `find_root`
-#[derive(Debug)]
-pub enum FindRootError {
-    /// Root of the repository has not been found
-    /// Contains the current directory used by FindRoot
-    RootNotFound(PathBuf),
-    /// The current directory does not exists or permissions are insufficient
-    /// to get access to it
-    GetCurrentDirError(std::io::Error),
-}
-
-/// Find the root of the repository
-/// by searching for a .hg directory in the process’ current directory and its
-/// ancestors
-pub fn find_root() -> Result<PathBuf, FindRootError> {
-    let current_dir = std::env::current_dir()
-        .map_err(|e| FindRootError::GetCurrentDirError(e))?;
-    Ok(find_root_from_path(&current_dir)?.into())
-}
-
-/// Find the root of the repository
-/// by searching for a .hg directory in the given directory and its ancestors
-pub fn find_root_from_path(start: &Path) -> Result<&Path, FindRootError> {
-    if start.join(".hg").exists() {
-        return Ok(start);
-    }
-    for ancestor in start.ancestors() {
-        if ancestor.join(".hg").exists() {
-            return Ok(ancestor);
-        }
-    }
-    Err(FindRootError::RootNotFound(start.into()))
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use std::fs;
-    use tempfile;
-
-    #[test]
-    fn dot_hg_not_found() {
-        let tmp_dir = tempfile::tempdir().unwrap();
-        let path = tmp_dir.path();
-
-        let err = find_root_from_path(&path).unwrap_err();
-
-        // TODO do something better
-        assert!(match err {
-            FindRootError::RootNotFound(p) => p == path.to_path_buf(),
-            _ => false,
-        })
-    }
-
-    #[test]
-    fn dot_hg_in_current_path() {
-        let tmp_dir = tempfile::tempdir().unwrap();
-        let root = tmp_dir.path();
-        fs::create_dir_all(root.join(".hg")).unwrap();
-
-        let result = find_root_from_path(&root).unwrap();
-
-        assert_eq!(result, root)
-    }
-
-    #[test]
-    fn dot_hg_in_parent() {
-        let tmp_dir = tempfile::tempdir().unwrap();
-        let root = tmp_dir.path();
-        fs::create_dir_all(root.join(".hg")).unwrap();
-
-        let directory = root.join("some/nested/directory");
-        let result = find_root_from_path(&directory).unwrap();
-
-        assert_eq!(result, root)
-    }
-} /* tests */
diff --git a/rust/hg-core/src/config/config.rs b/rust/hg-core/src/config/config.rs
--- a/rust/hg-core/src/config/config.rs
+++ b/rust/hg-core/src/config/config.rs
@@ -11,7 +11,7 @@ 
 use crate::config::layer::{ConfigError, ConfigLayer, ConfigValue};
 use std::path::PathBuf;
 
-use crate::operations::find_root;
+use crate::repo::Repo;
 use crate::utils::files::read_whole_file;
 
 /// Holds the config values for the current repository
@@ -76,10 +76,9 @@ 
 
     /// Loads the local config. In a future version, this will also load the
     /// `$HOME/.hgrc` and more to mirror the Python implementation.
-    pub fn load() -> Result<Self, ConfigError> {
-        let root = find_root().unwrap();
+    pub fn load_for_repo(repo: &Repo) -> Result<Self, ConfigError> {
         Ok(Self::load_from_explicit_sources(vec![
-            ConfigSource::AbsPath(root.join(".hg/hgrc")),
+            ConfigSource::AbsPath(repo.hg_vfs().join("hgrc")),
         ])?)
     }