Patchwork D7577: rust-configparser: implement Mercurial's config file discovery logic

login
register
mail settings
Submitter phabricator
Date Dec. 7, 2019, 8:19 p.m.
Message ID <differential-rev-PHID-DREV-a5lcy4x2ys2wohyzll5h-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/43635/
State New
Headers show

Comments

phabricator - Dec. 7, 2019, 8:19 p.m.
indygreg created this revision.
Herald added subscribers: mercurial-devel, kevincox, durin42.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The imported Facebook code for locating Mercurial config files
  differed from what Mercurial itself does. This commit modifies the
  logic to match what Mercurial currently does.
  
  As part of this, we added some crate dependencies. We could probably
  avoid these dependencies if we want. But these crates are quite popular
  and useful and I imagine we'll end up using them eventually.
  
  We should probably add some test coverage of this code...

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  rust/Cargo.lock
  rust/hg-core/Cargo.toml
  rust/hg-core/src/configparser/config.rs
  rust/hg-core/src/configparser/hg.rs

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: durin42, kevincox, mercurial-devel
phabricator - Dec. 9, 2019, 12:23 p.m.
kevincox added inline comments.
kevincox accepted this revision.

INLINE COMMENTS

> config.rs:145
> +            let paths = glob_res
> +                .filter_map(|x| if let Ok(x) = x { Some(x) } else { None })
> +                .sorted();

I believe this can just be `.flatten()`.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7577/new/

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

To: indygreg, #hg-reviewers, kevincox
Cc: durin42, kevincox, mercurial-devel
phabricator - April 22, 2020, 3:53 p.m.
Herald added a subscriber: mercurial-patches.
This revision now requires changes to proceed.
baymax added a comment.
baymax requested changes to this revision.


  There seems to have been no activities on this Diff for the past 3 Months.
  
  By policy, we are automatically moving it out of the `need-review` state.
  
  Please, move it back to `need-review` without hesitation if this diff should still be discussed.
  
  :baymax:need-review-idle:

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7577/new/

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

To: indygreg, #hg-reviewers, kevincox, Alphare, baymax
Cc: mercurial-patches, durin42, kevincox, mercurial-devel

Patch

diff --git a/rust/hg-core/src/configparser/hg.rs b/rust/hg-core/src/configparser/hg.rs
--- a/rust/hg-core/src/configparser/hg.rs
+++ b/rust/hg-core/src/configparser/hg.rs
@@ -15,6 +15,8 @@ 
 
 use anyhow::Result;
 use bytes::Bytes;
+#[cfg(windows)]
+use winreg::RegKey;
 
 use crate::utils::path::expand_path;
 use super::config::{ConfigSet, Options};
@@ -228,25 +230,55 @@ 
         let mut errors = Vec::new();
 
         if env::var(HGRCPATH).is_err() {
+            let current_exe = env::current_exe();
+            let exe_dir = if let Ok(exe_path) = &current_exe {
+                exe_path.parent()
+            } else {
+                None
+            };
+
             #[cfg(unix)]
             {
-                errors.append(&mut self.load_path("/etc/mercurial/system.rc", &opts));
-                // TODO(T40519286): Remove this after the tupperware overrides move out of hgrc.d
-                errors.append(
-                    &mut self.load_path("/etc/mercurial/hgrc.d/tupperware_overrides.rc", &opts),
-                );
-                // TODO(quark): Remove this after packages using system.rc are rolled out
-                errors.append(&mut self.load_path("/etc/mercurial/hgrc.d/include.rc", &opts));
+                // See mercurial.scmposix.systemrcpath() for reference implementation.
+                // Processing order:
+                // 1. /etc/mercurial/hgrc
+                // 2. /etc/mercurial/hgrc.d/*.rc
+                // 3. <install>/etc/mercurial/hgrc
+                // 4. <install>/etc/mercurial/hgrc.d/*.rc
+
+                errors.append(&mut self.load_path("/etc/mercurial/hgrc", &opts));
+                errors.extend(self.load_rc_files_in_dir(Path::new("/etc/mercurial/hgrc.d"), &opts));
+
+                if let Some(exe_dir) = exe_dir {
+                    errors.append(&mut self.load_path(&exe_dir.join("etc/mercurial/hgrc"), &opts));
+                    errors.extend(self.load_rc_files_in_dir(&exe_dir.join("etc/mercurial/hgrc.d"), &opts));
+                }
             }
 
             #[cfg(windows)]
             {
-                if let Ok(program_data_path) = env::var("PROGRAMDATA") {
-                    use std::path::Path;
-                    let hgrc_dir = Path::new(&program_data_path).join("Facebook\\Mercurial");
-                    errors.append(&mut self.load_path(hgrc_dir.join("system.rc"), &opts));
-                    // TODO(quark): Remove this after packages using system.rc are rolled out
-                    errors.append(&mut self.load_path(hgrc_dir.join("hgrc"), &opts));
+                // See mercurial.scmwindows.systemrcpath() for reference implementation.
+                // Processing order:
+                // 1. <install-dir>/mercurial.ini
+                // 2. <install-dir>/hgrc.d/*.rc
+                // 3. Paths from registry entry
+                if let Some(exe_dir) = exe_dir {
+                    errors.append(&mut self.load_path(&exe_dir.join("mercurial.ini"), &opts));
+                    errors.extend(self.load_rc_files_in_dir(&exe_dir.join("hgrc.d"), &opts));
+                }
+
+                // Then try to load files defined from a registry key.
+                let hklm = RegKey::predef(winreg::enums::HKEY_LOCAL_MACHINE);
+                if let Ok(key) = hklm.open_subkey("SOFTWARE\\Mercurial") {
+                    for key_path in key.split(';') {
+                        let p = Path::new(&key_path);
+
+                        if key_path.lower().ends_with("mercurial.ini") {
+                            errors.append(&mut self.load_path(p, &opts));
+                        } else if p.is_dir() {
+                            errors.extend(self.load_rc_files_in_dir(p, &opts));
+                        }
+                    }
                 }
             }
         }
diff --git a/rust/hg-core/src/configparser/config.rs b/rust/hg-core/src/configparser/config.rs
--- a/rust/hg-core/src/configparser/config.rs
+++ b/rust/hg-core/src/configparser/config.rs
@@ -16,6 +16,7 @@ 
 
 use bytes::Bytes;
 use indexmap::IndexMap;
+use itertools::Itertools;
 use pest::{self, Parser, Span};
 
 use super::error::Error;
@@ -104,6 +105,38 @@ 
         errors
     }
 
+    /// Load *.rc files in a directory.
+    ///
+    /// Matching paths are sorted and loaded in that order.
+    pub fn load_rc_files_in_dir(&mut self, path: &Path, opts: &Options) -> Vec<Error> {
+        let mut errors = Vec::new();
+
+        if !path.is_dir() {
+            return errors;
+        }
+
+        let mut options = glob::MatchOptions::default();
+        options.case_sensitive = false;
+
+        let mut visited = HashSet::new();
+
+        let glob_res = glob::glob(&format!("{}/*.rc", path.display()));
+        if let Ok(glob_res) = glob_res {
+            let paths = glob_res.filter_map(|x| {
+                if let Ok(x) = x {
+                    Some(x)
+                } else {
+                    None
+                }}).sorted();
+
+            for path in paths {
+                self.load_file(path.as_ref(), opts, &mut visited, &mut errors);
+            }
+        }
+
+        errors
+    }
+
     /// Load content of an unnamed config file. The `ValueLocation`s of loaded config items will
     /// have an empty `path`.
     ///
diff --git a/rust/hg-core/Cargo.toml b/rust/hg-core/Cargo.toml
--- a/rust/hg-core/Cargo.toml
+++ b/rust/hg-core/Cargo.toml
@@ -13,7 +13,9 @@ 
 byteorder = "1.3.1"
 bytes = "0.4.10"
 dirs = "2.0.2"
+glob = "0.3.0"
 indexmap = "1.0.1"
+itertools = "0.8.2"
 lazy_static = "1.3.0"
 memchr = "2.2.0"
 parking_lot = "0.9"
@@ -27,6 +29,9 @@ 
 tempfile = "3.0.4"
 thiserror = "1.0.5"
 
+[target.'cfg(windows)'.dependencies]
+winreg = "0.6.2"
+
 [dev-dependencies]
 lazy_static = "1.3"
 tempdir = "0.3"
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -280,6 +280,11 @@ 
 ]
 
 [[package]]
+name = "glob"
+version = "0.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
+[[package]]
 name = "hg-core"
 version = "0.1.0"
 dependencies = [
@@ -287,7 +292,9 @@ 
  "byteorder 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "bytes 0.4.12 (registry+https://github.com/rust-lang/crates.io-index)",
  "dirs 2.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "glob 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "indexmap 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "itertools 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "memchr 2.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "parking_lot 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -301,6 +308,7 @@ 
  "tempdir 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "tempfile 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "thiserror 1.0.9 (registry+https://github.com/rust-lang/crates.io-index)",
+ "winreg 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -337,6 +345,14 @@ 
 ]
 
 [[package]]
+name = "itertools"
+version = "0.8.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "either 1.5.3 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
+[[package]]
 name = "lazy_static"
 version = "1.4.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -893,6 +909,14 @@ 
 version = "0.4.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
+[[package]]
+name = "winreg"
+version = "0.6.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "winapi 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [metadata]
 "checksum aho-corasick 0.7.6 (registry+https://github.com/rust-lang/crates.io-index)" = "58fb5e95d83b38284460a5fda7d6470aa0b8844d283a0b614b8535e880800d2d"
 "checksum anyhow 1.0.25 (registry+https://github.com/rust-lang/crates.io-index)" = "9267dff192e68f3399525901e709a48c1d3982c9c072fa32f2127a0cb0babf14"
@@ -930,8 +954,10 @@ 
 "checksum fuchsia-cprng 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "a06f77d526c1a601b7c4cdd98f54b5eaabffc14d5f2f0296febdc7f357c6d3ba"
 "checksum generic-array 0.12.3 (registry+https://github.com/rust-lang/crates.io-index)" = "c68f0274ae0e023facc3c97b2e00f076be70e254bc851d972503b328db79b2ec"
 "checksum getrandom 0.1.13 (registry+https://github.com/rust-lang/crates.io-index)" = "e7db7ca94ed4cd01190ceee0d8a8052f08a247aa1b469a7f68c6a3b71afcf407"
+"checksum glob 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9b919933a397b79c37e33b77bb2aa3dc8eb6e165ad809e58ff75bc7db2e34574"
 "checksum indexmap 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "712d7b3ea5827fcb9d4fda14bf4da5f136f0db2ae9c8f4bd4e2d1c6fde4e6db2"
 "checksum iovec 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "b2b3ea6ff95e175473f8ffe6a7eb7c00d054240321b84c57051175fe3c1e075e"
+"checksum itertools 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)" = "f56a2d0bc861f9165be4eb3442afd3c236d8a98afd426f65d92324ae1091a484"
 "checksum lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646"
 "checksum libc 0.2.64 (registry+https://github.com/rust-lang/crates.io-index)" = "74dfca3d9957906e8d1e6a0b641dc9a59848e793f1da2165889fd4f62d10d79c"
 "checksum lock_api 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "e57b3997725d2b60dbec1297f6c2e2957cc383db1cebd6be812163f969c7d586"
@@ -1000,3 +1026,4 @@ 
 "checksum winapi 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)" = "8093091eeb260906a183e6ae1abdba2ef5ef2257a21801128899c3fc699229c6"
 "checksum winapi-i686-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6"
 "checksum winapi-x86_64-pc-windows-gnu 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f"
+"checksum winreg 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "b2986deb581c4fe11b621998a5e53361efe6b48a151178d0cd9eeffa4dc6acc9"