Patchwork D9964: rhg: Parse per-repository configuration

login
register
mail settings
Submitter phabricator
Date Feb. 5, 2021, 9:27 a.m.
Message ID <differential-rev-PHID-DREV-knx45eahyhr4lie54myo-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48277/
State Superseded
Headers show

Comments

phabricator - Feb. 5, 2021, 9:27 a.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/D9964

AFFECTED FILES
  rust/hg-core/src/config/config.rs
  rust/hg-core/src/repo.rs
  rust/rhg/src/error.rs

CHANGE DETAILS




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

Patch

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
@@ -3,7 +3,7 @@ 
 use format_bytes::format_bytes;
 use hg::config::{ConfigError, ConfigParseError};
 use hg::errors::HgError;
-use hg::repo::RepoFindError;
+use hg::repo::RepoError;
 use hg::revlog::revlog::RevlogError;
 use hg::utils::files::get_bytes_from_path;
 use std::convert::From;
@@ -51,18 +51,17 @@ 
     }
 }
 
-impl From<RepoFindError> for CommandError {
-    fn from(error: RepoFindError) -> Self {
+impl From<RepoError> for CommandError {
+    fn from(error: RepoError) -> Self {
         match error {
-            RepoFindError::NotFoundInCurrentDirectoryOrAncestors {
-                current_directory,
-            } => CommandError::Abort {
+            RepoError::NotFound { current_directory } => CommandError::Abort {
                 message: format_bytes!(
                     b"no repository found in '{}' (.hg not found)!",
                     get_bytes_from_path(current_directory)
                 ),
             },
-            RepoFindError::Other(error) => error.into(),
+            RepoError::ConfigParseError(error) => error.into(),
+            RepoError::Other(error) => error.into(),
         }
     }
 }
@@ -70,33 +69,35 @@ 
 impl From<ConfigError> for CommandError {
     fn from(error: ConfigError) -> Self {
         match error {
-            ConfigError::Parse(ConfigParseError {
-                origin,
-                line,
-                bytes,
-            }) => {
-                let line_message = if let Some(line_number) = line {
-                    format_bytes!(
-                        b" at line {}",
-                        line_number.to_string().into_bytes()
-                    )
-                } else {
-                    Vec::new()
-                };
-                CommandError::Abort {
-                    message: format_bytes!(
-                        b"config parse error in {}{}: '{}'",
-                        origin.to_bytes(),
-                        line_message,
-                        bytes
-                    ),
-                }
-            }
+            ConfigError::Parse(error) => error.into(),
             ConfigError::Other(error) => error.into(),
         }
     }
 }
 
+impl From<ConfigParseError> for CommandError {
+    fn from(error: ConfigParseError) -> Self {
+        let ConfigParseError {
+            origin,
+            line,
+            bytes,
+        } = error;
+        let line_message = if let Some(line_number) = line {
+            format_bytes!(b" at line {}", line_number.to_string().into_bytes())
+        } else {
+            Vec::new()
+        };
+        CommandError::Abort {
+            message: format_bytes!(
+                b"config parse error in {}{}: '{}'",
+                origin.to_bytes(),
+                line_message,
+                bytes
+            ),
+        }
+    }
+}
+
 impl From<(RevlogError, &str)> for CommandError {
     fn from((err, rev): (RevlogError, &str)) -> CommandError {
         match err {
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,4 +1,4 @@ 
-use crate::config::Config;
+use crate::config::{Config, ConfigError, ConfigParseError};
 use crate::errors::{HgError, IoResultExt};
 use crate::requirements;
 use crate::utils::files::get_path_from_bytes;
@@ -12,17 +12,29 @@ 
     dot_hg: PathBuf,
     store: PathBuf,
     requirements: HashSet<String>,
+    config: Config,
 }
 
 #[derive(Debug, derive_more::From)]
-pub enum RepoFindError {
-    NotFoundInCurrentDirectoryOrAncestors {
+pub enum RepoError {
+    NotFound {
         current_directory: PathBuf,
     },
     #[from]
+    ConfigParseError(ConfigParseError),
+    #[from]
     Other(HgError),
 }
 
+impl From<ConfigError> for RepoError {
+    fn from(error: ConfigError) -> Self {
+        match error {
+            ConfigError::Parse(error) => error.into(),
+            ConfigError::Other(error) => error.into(),
+        }
+    }
+}
+
 /// Filesystem access abstraction for the contents of a given "base" diretory
 #[derive(Clone, Copy)]
 pub(crate) struct Vfs<'a> {
@@ -32,7 +44,7 @@ 
 impl Repo {
     /// Search the current directory and its ancestores for a repository:
     /// a working directory that contains a `.hg` sub-directory.
-    pub fn find(config: &Config) -> Result<Self, RepoFindError> {
+    pub fn find(config: &Config) -> Result<Self, RepoError> {
         let current_directory = crate::utils::current_dir()?;
         // ancestors() is inclusive: it first yields `current_directory` as-is.
         for ancestor in current_directory.ancestors() {
@@ -40,18 +52,20 @@ 
                 return Ok(Self::new_at_path(ancestor.to_owned(), config)?);
             }
         }
-        Err(RepoFindError::NotFoundInCurrentDirectoryOrAncestors {
-            current_directory,
-        })
+        Err(RepoError::NotFound { current_directory })
     }
 
     /// To be called after checking that `.hg` is a sub-directory
     fn new_at_path(
         working_directory: PathBuf,
         config: &Config,
-    ) -> Result<Self, HgError> {
+    ) -> Result<Self, RepoError> {
         let dot_hg = working_directory.join(".hg");
 
+        let mut repo_config_files = Vec::new();
+        repo_config_files.push(dot_hg.join("hgrc"));
+        repo_config_files.push(dot_hg.join("hgrc-not-shared"));
+
         let hg_vfs = Vfs { base: &dot_hg };
         let mut reqs = requirements::load_if_exists(hg_vfs)?;
         let relative =
@@ -89,7 +103,8 @@ 
                 return Err(HgError::corrupted(format!(
                     ".hg/sharedpath points to nonexistent directory {}",
                     shared_path.display()
-                )));
+                ))
+                .into());
             }
 
             store_path = shared_path.join("store");
@@ -99,12 +114,15 @@ 
                     .contains(requirements::SHARESAFE_REQUIREMENT);
 
             if share_safe && !source_is_share_safe {
-                return Err(match config.get(b"safe-mismatch", b"source-not-safe") {
+                return Err(match config
+                    .get(b"safe-mismatch", b"source-not-safe")
+                {
                     Some(b"abort") | None => HgError::abort(
-                        "share source does not support share-safe requirement"
+                        "share source does not support share-safe requirement",
                     ),
-                    _ => HgError::unsupported("share-safe downgrade")
-                });
+                    _ => HgError::unsupported("share-safe downgrade"),
+                }
+                .into());
             } else if source_is_share_safe && !share_safe {
                 return Err(
                     match config.get(b"safe-mismatch", b"source-safe") {
@@ -113,16 +131,24 @@ 
                             functionality while the current share does not",
                         ),
                         _ => HgError::unsupported("share-safe upgrade"),
-                    },
+                    }
+                    .into(),
                 );
             }
+
+            if share_safe {
+                repo_config_files.insert(0, shared_path.join("hgrc"))
+            }
         }
 
+        let repo_config = config.combine_with_repo(&repo_config_files)?;
+
         let repo = Self {
             requirements: reqs,
             working_directory,
             store: store_path,
             dot_hg,
+            config: repo_config,
         };
 
         requirements::check(&repo)?;
@@ -138,6 +164,10 @@ 
         &self.requirements
     }
 
+    pub fn config(&self) -> &Config {
+        &self.config
+    }
+
     /// For accessing repository files (in `.hg`), except for the store
     /// (`.hg/store`).
     pub(crate) fn hg_vfs(&self) -> Vfs<'_> {
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
@@ -13,15 +13,20 @@ 
 };
 use crate::utils::files::get_bytes_from_path;
 use std::env;
+use std::ops::RangeFrom;
 use std::path::{Path, PathBuf};
 
 use crate::errors::{HgResultExt, IoResultExt};
-use crate::repo::Repo;
 
 /// Holds the config values for the current repository
 /// TODO update this docstring once we support more sources
 pub struct Config {
     layers: Vec<layer::ConfigLayer>,
+
+    /// Which layers if any are for `--config` command-line arguments.
+    /// This matters for pre-repository config which has less precedence than
+    /// CLI arguments but more than everything else.
+    cli_layers_indices: Option<RangeFrom<usize>>,
 }
 
 impl std::fmt::Debug for Config {
@@ -59,7 +64,10 @@ 
     ///
     /// TODO: add a parameter for `--config` CLI arguments
     pub fn load() -> Result<Self, ConfigError> {
-        let mut config = Self { layers: Vec::new() };
+        let mut config = Self {
+            layers: Vec::new(),
+            cli_layers_indices: None,
+        };
         let opt_rc_path = env::var_os("HGRCPATH");
         // HGRCPATH replaces system config
         if opt_rc_path.is_none() {
@@ -193,15 +201,36 @@ 
             }
         }
 
-        Ok(Config { layers })
+        Ok(Config {
+            layers,
+            cli_layers_indices: None,
+        })
     }
 
-    /// 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_for_repo(repo: &Repo) -> Result<Self, ConfigError> {
-        Ok(Self::load_from_explicit_sources(vec![
-            ConfigSource::AbsPath(repo.hg_vfs().join("hgrc")),
-        ])?)
+    /// Loads the per-repository config into a new `Config` which is combined
+    /// with `self`.
+    pub(crate) fn combine_with_repo(
+        &self,
+        repo_config_files: &[PathBuf],
+    ) -> Result<Self, ConfigError> {
+        let cli_layers_start = match self.cli_layers_indices {
+            Some(RangeFrom { start }) => start,
+            None => self.layers.len(),
+        };
+        let (non_cli_layers, cli_layers) =
+            self.layers.split_at(cli_layers_start);
+        let mut repo_config = Self {
+            layers: non_cli_layers.iter().cloned().collect::<Vec<_>>(),
+            cli_layers_indices: None,
+        };
+        for path in repo_config_files {
+            // TODO: check if this file should be trusted:
+            // `mercurial/ui.py:427`
+            repo_config.add_trusted_file(path)?;
+        }
+        repo_config.cli_layers_indices = Some(repo_config.layers.len()..);
+        repo_config.layers.extend(cli_layers.iter().cloned());
+        Ok(repo_config)
     }
 
     /// Returns an `Err` if the first value found is not a valid boolean.