rust: Introduce a get_bytes_from_os_str utility function

It does the same as get_bytes_from_path but takes an `OsStr`
instead of a `Path`. The implementation is the same so using either
ends up correct but the function name suggests it’s not.

Differential Revision: https://phab.mercurial-scm.org/D10007

rust/hg-core/src/config/config.rs

@@ -1,344 +1,343 b''
 // config.rs
 //
 // Copyright 2020
 //      Valentin Gatien-Baron,
 //      Raphaël Gomès <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 use super::layer;
 use crate::config::layer::{
     ConfigError, ConfigLayer, ConfigParseError, ConfigValue,
 };
-use crate::utils::files::get_bytes_from_path;
+use crate::utils::files::get_bytes_from_os_str;
 use format_bytes::{write_bytes, DisplayBytes};
 use std::env;
 use std::path::{Path, PathBuf};
 
 use crate::errors::{HgResultExt, IoResultExt};
 
 /// Holds the config values for the current repository
 /// TODO update this docstring once we support more sources
 pub struct Config {
     layers: Vec<layer::ConfigLayer>,
 }
 
 impl DisplayBytes for Config {
     fn display_bytes(
         &self,
         out: &mut dyn std::io::Write,
     ) -> std::io::Result<()> {
         for (index, layer) in self.layers.iter().rev().enumerate() {
             write_bytes!(
                 out,
                 b"==== Layer {} (trusted: {}) ====\n{}",
                 index,
                 if layer.trusted {
                     &b"yes"[..]
                 } else {
                     &b"no"[..]
                 },
                 layer
             )?;
         }
         Ok(())
     }
 }
 
 pub enum ConfigSource {
     /// Absolute path to a config file
     AbsPath(PathBuf),
     /// Already parsed (from the CLI, env, Python resources, etc.)
     Parsed(layer::ConfigLayer),
 }
 
 pub fn parse_bool(v: &[u8]) -> Option<bool> {
     match v.to_ascii_lowercase().as_slice() {
         b"1" | b"yes" | b"true" | b"on" | b"always" => Some(true),
         b"0" | b"no" | b"false" | b"off" | b"never" => Some(false),
         _ => None,
     }
 }
 
 impl Config {
     /// Load system and user configuration from various files.
     ///
     /// This is also affected by some environment variables.
     pub fn load(
         cli_config_args: impl IntoIterator<Item = impl AsRef<[u8]>>,
     ) -> Result<Self, ConfigError> {
         let mut config = Self { layers: Vec::new() };
         let opt_rc_path = env::var_os("HGRCPATH");
         // HGRCPATH replaces system config
         if opt_rc_path.is_none() {
             config.add_system_config()?
         }
         config.add_for_environment_variable("EDITOR", b"ui", b"editor");
         config.add_for_environment_variable("VISUAL", b"ui", b"editor");
         config.add_for_environment_variable("PAGER", b"pager", b"pager");
         // HGRCPATH replaces user config
         if opt_rc_path.is_none() {
             config.add_user_config()?
         }
         if let Some(rc_path) = &opt_rc_path {
             for path in env::split_paths(rc_path) {
                 if !path.as_os_str().is_empty() {
                     if path.is_dir() {
                         config.add_trusted_dir(&path)?
                     } else {
                         config.add_trusted_file(&path)?
                     }
                 }
             }
         }
         if let Some(layer) = ConfigLayer::parse_cli_args(cli_config_args)? {
             config.layers.push(layer)
         }
         Ok(config)
     }
 
     fn add_trusted_dir(&mut self, path: &Path) -> Result<(), ConfigError> {
         if let Some(entries) = std::fs::read_dir(path)
             .for_file(path)
             .io_not_found_as_none()?
         {
             for entry in entries {
                 let file_path = entry.for_file(path)?.path();
                 if file_path.extension() == Some(std::ffi::OsStr::new("rc")) {
                     self.add_trusted_file(&file_path)?
                 }
             }
         }
         Ok(())
     }
 
     fn add_trusted_file(&mut self, path: &Path) -> Result<(), ConfigError> {
         if let Some(data) =
             std::fs::read(path).for_file(path).io_not_found_as_none()?
         {
             self.layers.extend(ConfigLayer::parse(path, &data)?)
         }
         Ok(())
     }
 
     fn add_for_environment_variable(
         &mut self,
         var: &str,
         section: &[u8],
         key: &[u8],
     ) {
         if let Some(value) = env::var_os(var) {
             let origin = layer::ConfigOrigin::Environment(var.into());
             let mut layer = ConfigLayer::new(origin);
             layer.add(
                 section.to_owned(),
                 key.to_owned(),
-                // `value` is not a path but this works for any `OsStr`:
-                get_bytes_from_path(value),
+                get_bytes_from_os_str(value),
                 None,
             );
             self.layers.push(layer)
         }
     }
 
     #[cfg(unix)] // TODO: other platforms
     fn add_system_config(&mut self) -> Result<(), ConfigError> {
         let mut add_for_prefix = |prefix: &Path| -> Result<(), ConfigError> {
             let etc = prefix.join("etc").join("mercurial");
             self.add_trusted_file(&etc.join("hgrc"))?;
             self.add_trusted_dir(&etc.join("hgrc.d"))
         };
         let root = Path::new("/");
         // TODO: use `std::env::args_os().next().unwrap()` a.k.a. argv[0]
         // instead? TODO: can this be a relative path?
         let hg = crate::utils::current_exe()?;
         // TODO: this order (per-installation then per-system) matches
         // `systemrcpath()` in `mercurial/scmposix.py`, but
         // `mercurial/helptext/config.txt` suggests it should be reversed
         if let Some(installation_prefix) = hg.parent().and_then(Path::parent) {
             if installation_prefix != root {
                 add_for_prefix(&installation_prefix)?
             }
         }
         add_for_prefix(root)?;
         Ok(())
     }
 
     #[cfg(unix)] // TODO: other plateforms
     fn add_user_config(&mut self) -> Result<(), ConfigError> {
         let opt_home = home::home_dir();
         if let Some(home) = &opt_home {
             self.add_trusted_file(&home.join(".hgrc"))?
         }
         let darwin = cfg!(any(target_os = "macos", target_os = "ios"));
         if !darwin {
             if let Some(config_home) = env::var_os("XDG_CONFIG_HOME")
                 .map(PathBuf::from)
                 .or_else(|| opt_home.map(|home| home.join(".config")))
             {
                 self.add_trusted_file(&config_home.join("hg").join("hgrc"))?
             }
         }
         Ok(())
     }
 
     /// Loads in order, which means that the precedence is the same
     /// as the order of `sources`.
     pub fn load_from_explicit_sources(
         sources: Vec<ConfigSource>,
     ) -> Result<Self, ConfigError> {
         let mut layers = vec![];
 
         for source in sources.into_iter() {
             match source {
                 ConfigSource::Parsed(c) => layers.push(c),
                 ConfigSource::AbsPath(c) => {
                     // TODO check if it should be trusted
                     // mercurial/ui.py:427
                     let data = match std::fs::read(&c) {
                         Err(_) => continue, // same as the python code
                         Ok(data) => data,
                     };
                     layers.extend(ConfigLayer::parse(&c, &data)?)
                 }
             }
         }
 
         Ok(Config { layers })
     }
 
     /// 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, other_layers) = self
             .layers
             .iter()
             .cloned()
             .partition(ConfigLayer::is_from_command_line);
 
         let mut repo_config = Self {
             layers: other_layers,
         };
         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.layers.extend(cli_layers);
         Ok(repo_config)
     }
 
     /// Returns an `Err` if the first value found is not a valid boolean.
     /// Otherwise, returns an `Ok(option)`, where `option` is the boolean if
     /// found, or `None`.
     pub fn get_option(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Result<Option<bool>, ConfigParseError> {
         match self.get_inner(&section, &item) {
             Some((layer, v)) => match parse_bool(&v.bytes) {
                 Some(b) => Ok(Some(b)),
                 None => Err(ConfigParseError {
                     origin: layer.origin.to_owned(),
                     line: v.line,
                     bytes: v.bytes.to_owned(),
                 }),
             },
             None => Ok(None),
         }
     }
 
     /// Returns the corresponding boolean in the config. Returns `Ok(false)`
     /// if the value is not found, an `Err` if it's not a valid boolean.
     pub fn get_bool(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Result<bool, ConfigError> {
         Ok(self.get_option(section, item)?.unwrap_or(false))
     }
 
     /// Returns the raw value bytes of the first one found, or `None`.
     pub fn get(&self, section: &[u8], item: &[u8]) -> Option<&[u8]> {
         self.get_inner(section, item)
             .map(|(_, value)| value.bytes.as_ref())
     }
 
     /// Returns the layer and the value of the first one found, or `None`.
     fn get_inner(
         &self,
         section: &[u8],
         item: &[u8],
     ) -> Option<(&ConfigLayer, &ConfigValue)> {
         for layer in self.layers.iter().rev() {
             if !layer.trusted {
                 continue;
             }
             if let Some(v) = layer.get(&section, &item) {
                 return Some((&layer, v));
             }
         }
         None
     }
 
     /// Get raw values bytes from all layers (even untrusted ones) in order
     /// of precedence.
     #[cfg(test)]
     fn get_all(&self, section: &[u8], item: &[u8]) -> Vec<&[u8]> {
         let mut res = vec![];
         for layer in self.layers.iter().rev() {
             if let Some(v) = layer.get(&section, &item) {
                 res.push(v.bytes.as_ref());
             }
         }
         res
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
     use pretty_assertions::assert_eq;
     use std::fs::File;
     use std::io::Write;
 
     #[test]
     fn test_include_layer_ordering() {
         let tmpdir = tempfile::tempdir().unwrap();
         let tmpdir_path = tmpdir.path();
         let mut included_file =
             File::create(&tmpdir_path.join("included.rc")).unwrap();
 
         included_file.write_all(b"[section]\nitem=value1").unwrap();
         let base_config_path = tmpdir_path.join("base.rc");
         let mut config_file = File::create(&base_config_path).unwrap();
         let data =
             b"[section]\nitem=value0\n%include included.rc\nitem=value2";
         config_file.write_all(data).unwrap();
 
         let sources = vec![ConfigSource::AbsPath(base_config_path)];
         let config = Config::load_from_explicit_sources(sources)
             .expect("expected valid config");
 
         let (_, value) = config.get_inner(b"section", b"item").unwrap();
         assert_eq!(
             value,
             &ConfigValue {
                 bytes: b"value2".to_vec(),
                 line: Some(4)
             }
         );
 
         let value = config.get(b"section", b"item").unwrap();
         assert_eq!(value, b"value2",);
         assert_eq!(
             config.get_all(b"section", b"item"),
             [b"value2", b"value1", b"value0"]
         );
     }
 }

rust/hg-core/src/utils/files.rs

@@ -1,442 +1,448 b''
 // files.rs
 //
 // Copyright 2019
 // Raphaël Gomès <[e-mail]>,
 // Yuya Nishihara <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 //! Functions for fiddling with files.
 
 use crate::utils::{
     hg_path::{path_to_hg_path_buf, HgPath, HgPathBuf, HgPathError},
     path_auditor::PathAuditor,
     replace_slice,
 };
 use lazy_static::lazy_static;
 use same_file::is_same_file;
 use std::borrow::{Cow, ToOwned};
+use std::ffi::OsStr;
 use std::fs::Metadata;
 use std::iter::FusedIterator;
 use std::ops::Deref;
 use std::path::{Path, PathBuf};
 
 pub fn get_path_from_bytes(bytes: &[u8]) -> &Path {
     let os_str;
     #[cfg(unix)]
     {
         use std::os::unix::ffi::OsStrExt;
         os_str = std::ffi::OsStr::from_bytes(bytes);
     }
     // TODO Handle other platforms
     // TODO: convert from WTF8 to Windows MBCS (ANSI encoding).
     // Perhaps, the return type would have to be Result<PathBuf>.
 
     Path::new(os_str)
 }
 
 // TODO: need to convert from WTF8 to MBCS bytes on Windows.
 // that's why Vec<u8> is returned.
 #[cfg(unix)]
 pub fn get_bytes_from_path(path: impl AsRef<Path>) -> Vec<u8> {
+    get_bytes_from_os_str(path.as_ref())
+}
+
+#[cfg(unix)]
+pub fn get_bytes_from_os_str(str: impl AsRef<OsStr>) -> Vec<u8> {
     use std::os::unix::ffi::OsStrExt;
-    path.as_ref().as_os_str().as_bytes().to_vec()
+    str.as_ref().as_bytes().to_vec()
 }
 
 /// An iterator over repository path yielding itself and its ancestors.
 #[derive(Copy, Clone, Debug)]
 pub struct Ancestors<'a> {
     next: Option<&'a HgPath>,
 }
 
 impl<'a> Iterator for Ancestors<'a> {
     type Item = &'a HgPath;
 
     fn next(&mut self) -> Option<Self::Item> {
         let next = self.next;
         self.next = match self.next {
             Some(s) if s.is_empty() => None,
             Some(s) => {
                 let p = s.bytes().rposition(|c| *c == b'/').unwrap_or(0);
                 Some(HgPath::new(&s.as_bytes()[..p]))
             }
             None => None,
         };
         next
     }
 }
 
 impl<'a> FusedIterator for Ancestors<'a> {}
 
 /// An iterator over repository path yielding itself and its ancestors.
 #[derive(Copy, Clone, Debug)]
 pub(crate) struct AncestorsWithBase<'a> {
     next: Option<(&'a HgPath, &'a HgPath)>,
 }
 
 impl<'a> Iterator for AncestorsWithBase<'a> {
     type Item = (&'a HgPath, &'a HgPath);
 
     fn next(&mut self) -> Option<Self::Item> {
         let next = self.next;
         self.next = match self.next {
             Some((s, _)) if s.is_empty() => None,
             Some((s, _)) => Some(s.split_filename()),
             None => None,
         };
         next
     }
 }
 
 impl<'a> FusedIterator for AncestorsWithBase<'a> {}
 
 /// Returns an iterator yielding ancestor directories of the given repository
 /// path.
 ///
 /// The path is separated by '/', and must not start with '/'.
 ///
 /// The path itself isn't included unless it is b"" (meaning the root
 /// directory.)
 pub fn find_dirs(path: &HgPath) -> Ancestors {
     let mut dirs = Ancestors { next: Some(path) };
     if !path.is_empty() {
         dirs.next(); // skip itself
     }
     dirs
 }
 
 /// Returns an iterator yielding ancestor directories of the given repository
 /// path.
 ///
 /// The path is separated by '/', and must not start with '/'.
 ///
 /// The path itself isn't included unless it is b"" (meaning the root
 /// directory.)
 pub(crate) fn find_dirs_with_base(path: &HgPath) -> AncestorsWithBase {
     let mut dirs = AncestorsWithBase {
         next: Some((path, HgPath::new(b""))),
     };
     if !path.is_empty() {
         dirs.next(); // skip itself
     }
     dirs
 }
 
 /// TODO more than ASCII?
 pub fn normalize_case(path: &HgPath) -> HgPathBuf {
     #[cfg(windows)] // NTFS compares via upper()
     return path.to_ascii_uppercase();
     #[cfg(unix)]
     path.to_ascii_lowercase()
 }
 
 lazy_static! {
     static ref IGNORED_CHARS: Vec<Vec<u8>> = {
         [
             0x200c, 0x200d, 0x200e, 0x200f, 0x202a, 0x202b, 0x202c, 0x202d,
             0x202e, 0x206a, 0x206b, 0x206c, 0x206d, 0x206e, 0x206f, 0xfeff,
         ]
         .iter()
         .map(|code| {
             std::char::from_u32(*code)
                 .unwrap()
                 .encode_utf8(&mut [0; 3])
                 .bytes()
                 .collect()
         })
         .collect()
     };
 }
 
 fn hfs_ignore_clean(bytes: &[u8]) -> Vec<u8> {
     let mut buf = bytes.to_owned();
     let needs_escaping = bytes.iter().any(|b| *b == b'\xe2' || *b == b'\xef');
     if needs_escaping {
         for forbidden in IGNORED_CHARS.iter() {
             replace_slice(&mut buf, forbidden, &[])
         }
         buf
     } else {
         buf
     }
 }
 
 pub fn lower_clean(bytes: &[u8]) -> Vec<u8> {
     hfs_ignore_clean(&bytes.to_ascii_lowercase())
 }
 
 #[derive(Eq, PartialEq, Ord, PartialOrd, Copy, Clone)]
 pub struct HgMetadata {
     pub st_dev: u64,
     pub st_mode: u32,
     pub st_nlink: u64,
     pub st_size: u64,
     pub st_mtime: i64,
     pub st_ctime: i64,
 }
 
 // TODO support other plaforms
 #[cfg(unix)]
 impl HgMetadata {
     pub fn from_metadata(metadata: Metadata) -> Self {
         use std::os::unix::fs::MetadataExt;
         Self {
             st_dev: metadata.dev(),
             st_mode: metadata.mode(),
             st_nlink: metadata.nlink(),
             st_size: metadata.size(),
             st_mtime: metadata.mtime(),
             st_ctime: metadata.ctime(),
         }
     }
 }
 
 /// Returns the canonical path of `name`, given `cwd` and `root`
 pub fn canonical_path(
     root: impl AsRef<Path>,
     cwd: impl AsRef<Path>,
     name: impl AsRef<Path>,
 ) -> Result<PathBuf, HgPathError> {
     // TODO add missing normalization for other platforms
     let root = root.as_ref();
     let cwd = cwd.as_ref();
     let name = name.as_ref();
 
     let name = if !name.is_absolute() {
         root.join(&cwd).join(&name)
     } else {
         name.to_owned()
     };
     let auditor = PathAuditor::new(&root);
     if name != root && name.starts_with(&root) {
         let name = name.strip_prefix(&root).unwrap();
         auditor.audit_path(path_to_hg_path_buf(name)?)?;
         Ok(name.to_owned())
     } else if name == root {
         Ok("".into())
     } else {
         // Determine whether `name' is in the hierarchy at or beneath `root',
         // by iterating name=name.parent() until it returns `None` (can't
         // check name == '/', because that doesn't work on windows).
         let mut name = name.deref();
         let original_name = name.to_owned();
         loop {
             let same = is_same_file(&name, &root).unwrap_or(false);
             if same {
                 if name == original_name {
                     // `name` was actually the same as root (maybe a symlink)
                     return Ok("".into());
                 }
                 // `name` is a symlink to root, so `original_name` is under
                 // root
                 let rel_path = original_name.strip_prefix(&name).unwrap();
                 auditor.audit_path(path_to_hg_path_buf(&rel_path)?)?;
                 return Ok(rel_path.to_owned());
             }
             name = match name.parent() {
                 None => break,
                 Some(p) => p,
             };
         }
         // TODO hint to the user about using --cwd
         // Bubble up the responsibility to Python for now
         Err(HgPathError::NotUnderRoot {
             path: original_name.to_owned(),
             root: root.to_owned(),
         })
     }
 }
 
 /// Returns the representation of the path relative to the current working
 /// directory for display purposes.
 ///
 /// `cwd` is a `HgPath`, so it is considered relative to the root directory
 /// of the repository.
 ///
 /// # Examples
 ///
 /// ```
 /// use hg::utils::hg_path::HgPath;
 /// use hg::utils::files::relativize_path;
 /// use std::borrow::Cow;
 ///
 /// let file = HgPath::new(b"nested/file");
 /// let cwd = HgPath::new(b"");
 /// assert_eq!(relativize_path(file, cwd), Cow::Borrowed(b"nested/file"));
 ///
 /// let cwd = HgPath::new(b"nested");
 /// assert_eq!(relativize_path(file, cwd), Cow::Borrowed(b"file"));
 ///
 /// let cwd = HgPath::new(b"other");
 /// assert_eq!(relativize_path(file, cwd), Cow::Borrowed(b"../nested/file"));
 /// ```
 pub fn relativize_path(path: &HgPath, cwd: impl AsRef<HgPath>) -> Cow<[u8]> {
     if cwd.as_ref().is_empty() {
         Cow::Borrowed(path.as_bytes())
     } else {
         let mut res: Vec<u8> = Vec::new();
         let mut path_iter = path.as_bytes().split(|b| *b == b'/').peekable();
         let mut cwd_iter =
             cwd.as_ref().as_bytes().split(|b| *b == b'/').peekable();
         loop {
             match (path_iter.peek(), cwd_iter.peek()) {
                 (Some(a), Some(b)) if a == b => (),
                 _ => break,
             }
             path_iter.next();
             cwd_iter.next();
         }
         let mut need_sep = false;
         for _ in cwd_iter {
             if need_sep {
                 res.extend(b"/")
             } else {
                 need_sep = true
             };
             res.extend(b"..");
         }
         for c in path_iter {
             if need_sep {
                 res.extend(b"/")
             } else {
                 need_sep = true
             };
             res.extend(c);
         }
         Cow::Owned(res)
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
     use pretty_assertions::assert_eq;
 
     #[test]
     fn find_dirs_some() {
         let mut dirs = super::find_dirs(HgPath::new(b"foo/bar/baz"));
         assert_eq!(dirs.next(), Some(HgPath::new(b"foo/bar")));
         assert_eq!(dirs.next(), Some(HgPath::new(b"foo")));
         assert_eq!(dirs.next(), Some(HgPath::new(b"")));
         assert_eq!(dirs.next(), None);
         assert_eq!(dirs.next(), None);
     }
 
     #[test]
     fn find_dirs_empty() {
         // looks weird, but mercurial.pathutil.finddirs(b"") yields b""
         let mut dirs = super::find_dirs(HgPath::new(b""));
         assert_eq!(dirs.next(), Some(HgPath::new(b"")));
         assert_eq!(dirs.next(), None);
         assert_eq!(dirs.next(), None);
     }
 
     #[test]
     fn test_find_dirs_with_base_some() {
         let mut dirs = super::find_dirs_with_base(HgPath::new(b"foo/bar/baz"));
         assert_eq!(
             dirs.next(),
             Some((HgPath::new(b"foo/bar"), HgPath::new(b"baz")))
         );
         assert_eq!(
             dirs.next(),
             Some((HgPath::new(b"foo"), HgPath::new(b"bar")))
         );
         assert_eq!(dirs.next(), Some((HgPath::new(b""), HgPath::new(b"foo"))));
         assert_eq!(dirs.next(), None);
         assert_eq!(dirs.next(), None);
     }
 
     #[test]
     fn test_find_dirs_with_base_empty() {
         let mut dirs = super::find_dirs_with_base(HgPath::new(b""));
         assert_eq!(dirs.next(), Some((HgPath::new(b""), HgPath::new(b""))));
         assert_eq!(dirs.next(), None);
         assert_eq!(dirs.next(), None);
     }
 
     #[test]
     fn test_canonical_path() {
         let root = Path::new("/repo");
         let cwd = Path::new("/dir");
         let name = Path::new("filename");
         assert_eq!(
             canonical_path(root, cwd, name),
             Err(HgPathError::NotUnderRoot {
                 path: PathBuf::from("/dir/filename"),
                 root: root.to_path_buf()
             })
         );
 
         let root = Path::new("/repo");
         let cwd = Path::new("/");
         let name = Path::new("filename");
         assert_eq!(
             canonical_path(root, cwd, name),
             Err(HgPathError::NotUnderRoot {
                 path: PathBuf::from("/filename"),
                 root: root.to_path_buf()
             })
         );
 
         let root = Path::new("/repo");
         let cwd = Path::new("/");
         let name = Path::new("repo/filename");
         assert_eq!(
             canonical_path(root, cwd, name),
             Ok(PathBuf::from("filename"))
         );
 
         let root = Path::new("/repo");
         let cwd = Path::new("/repo");
         let name = Path::new("filename");
         assert_eq!(
             canonical_path(root, cwd, name),
             Ok(PathBuf::from("filename"))
         );
 
         let root = Path::new("/repo");
         let cwd = Path::new("/repo/subdir");
         let name = Path::new("filename");
         assert_eq!(
             canonical_path(root, cwd, name),
             Ok(PathBuf::from("subdir/filename"))
         );
     }
 
     #[test]
     fn test_canonical_path_not_rooted() {
         use std::fs::create_dir;
         use tempfile::tempdir;
 
         let base_dir = tempdir().unwrap();
         let base_dir_path = base_dir.path();
         let beneath_repo = base_dir_path.join("a");
         let root = base_dir_path.join("a/b");
         let out_of_repo = base_dir_path.join("c");
         let under_repo_symlink = out_of_repo.join("d");
 
         create_dir(&beneath_repo).unwrap();
         create_dir(&root).unwrap();
 
         // TODO make portable
         std::os::unix::fs::symlink(&root, &out_of_repo).unwrap();
 
         assert_eq!(
             canonical_path(&root, Path::new(""), out_of_repo),
             Ok(PathBuf::from(""))
         );
         assert_eq!(
             canonical_path(&root, Path::new(""), &beneath_repo),
             Err(HgPathError::NotUnderRoot {
                 path: beneath_repo.to_owned(),
                 root: root.to_owned()
             })
         );
         assert_eq!(
             canonical_path(&root, Path::new(""), &under_repo_symlink),
             Ok(PathBuf::from("d"))
         );
     }
 }

rust/rhg/src/main.rs

@@ -1,176 +1,175 b''
 extern crate log;
 use crate::ui::Ui;
 use clap::App;
 use clap::AppSettings;
 use clap::Arg;
 use clap::ArgMatches;
 use format_bytes::format_bytes;
 use hg::config::Config;
 use hg::repo::{Repo, RepoError};
 use std::path::{Path, PathBuf};
 
 mod error;
 mod exitcode;
 mod ui;
 use error::CommandError;
 
 fn add_global_args<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b> {
     app.arg(
         Arg::with_name("repository")
             .help("repository root directory")
             .short("-R")
             .long("--repository")
             .value_name("REPO")
             .takes_value(true),
     )
     .arg(
         Arg::with_name("config")
             .help("set/override config option (use 'section.name=value')")
             .long("--config")
             .value_name("CONFIG")
             .takes_value(true)
             // Ok: `--config section.key1=val --config section.key2=val2`
             .multiple(true)
             // Not ok: `--config section.key1=val section.key2=val2`
             .number_of_values(1),
     )
 }
 
 fn main_with_result(ui: &ui::Ui) -> Result<(), CommandError> {
     env_logger::init();
     let app = App::new("rhg")
         .setting(AppSettings::AllowInvalidUtf8)
         .setting(AppSettings::SubcommandRequired)
         .setting(AppSettings::VersionlessSubcommands)
         .version("0.0.1");
     let app = add_global_args(app);
     let app = add_subcommand_args(app);
 
     let matches = app.clone().get_matches_safe()?;
 
     let (subcommand_name, subcommand_matches) = matches.subcommand();
     let run = subcommand_run_fn(subcommand_name)
         .expect("unknown subcommand name from clap despite AppSettings::SubcommandRequired");
     let subcommand_args = subcommand_matches
         .expect("no subcommand arguments from clap despite AppSettings::SubcommandRequired");
 
     // Global arguments can be in either based on e.g. `hg -R ./foo log` v.s.
     // `hg log -R ./foo`
     let value_of_global_arg = |name| {
         subcommand_args
             .value_of_os(name)
             .or_else(|| matches.value_of_os(name))
     };
     // For arguments where multiple occurences are allowed, return a
     // possibly-iterator of all values.
     let values_of_global_arg = |name: &str| {
         let a = matches.values_of_os(name).into_iter().flatten();
         let b = subcommand_args.values_of_os(name).into_iter().flatten();
         a.chain(b)
     };
 
     let config_args = values_of_global_arg("config")
-        // `get_bytes_from_path` works for OsStr the same as for Path
-        .map(hg::utils::files::get_bytes_from_path);
+        .map(hg::utils::files::get_bytes_from_os_str);
     let non_repo_config = &hg::config::Config::load(config_args)?;
 
     let repo_path = value_of_global_arg("repository").map(Path::new);
     let repo = match Repo::find(non_repo_config, repo_path) {
         Ok(repo) => Ok(repo),
         Err(RepoError::NotFound { at }) if repo_path.is_none() => {
             // Not finding a repo is not fatal yet, if `-R` was not given
             Err(NoRepoInCwdError { cwd: at })
         }
         Err(error) => return Err(error.into()),
     };
 
     run(&CliInvocation {
         ui,
         subcommand_args,
         non_repo_config,
         repo: repo.as_ref(),
     })
 }
 
 fn main() {
     let ui = ui::Ui::new();
 
     let exit_code = match main_with_result(&ui) {
         Ok(()) => exitcode::OK,
 
         // Exit with a specific code and no error message to let a potential
         // wrapper script fallback to Python-based Mercurial.
         Err(CommandError::Unimplemented) => exitcode::UNIMPLEMENTED,
 
         Err(CommandError::Abort { message }) => {
             if !message.is_empty() {
                 // Ignore errors when writing to stderr, we’re already exiting
                 // with failure code so there’s not much more we can do.
                 let _ =
                     ui.write_stderr(&format_bytes!(b"abort: {}\n", message));
             }
             exitcode::ABORT
         }
     };
     std::process::exit(exit_code)
 }
 
 macro_rules! subcommands {
     ($( $command: ident )+) => {
         mod commands {
             $(
                 pub mod $command;
             )+
         }
 
         fn add_subcommand_args<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b> {
             app
             $(
                 .subcommand(add_global_args(commands::$command::args()))
             )+
         }
 
         pub type RunFn = fn(&CliInvocation) -> Result<(), CommandError>;
 
         fn subcommand_run_fn(name: &str) -> Option<RunFn> {
             match name {
                 $(
                     stringify!($command) => Some(commands::$command::run),
                 )+
                 _ => None,
             }
         }
     };
 }
 
 subcommands! {
     cat
     debugdata
     debugrequirements
     files
     root
     config
 }
 pub struct CliInvocation<'a> {
     ui: &'a Ui,
     subcommand_args: &'a ArgMatches<'a>,
     non_repo_config: &'a Config,
     /// References inside `Result` is a bit peculiar but allow
     /// `invocation.repo?` to work out with `&CliInvocation` since this
     /// `Result` type is `Copy`.
     repo: Result<&'a Repo, &'a NoRepoInCwdError>,
 }
 
 struct NoRepoInCwdError {
     cwd: PathBuf,
 }
 
 impl CliInvocation<'_> {
     fn config(&self) -> &Config {
         if let Ok(repo) = self.repo {
             repo.config()
         } else {
             self.non_repo_config
         }
     }
 }