D10138: rhg: Don’t make repository path absolute too early

[SimonSapin]

SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Some error messages want to include a relative path

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  rust/hg-core/src/errors.rs
  rust/hg-core/src/repo.rs
  rust/rhg/src/commands/cat.rs
  rust/rhg/src/commands/files.rs
  rust/rhg/src/commands/root.rs

CHANGE DETAILS

diff --git a/rust/rhg/src/commands/root.rs b/rust/rhg/src/commands/root.rs
--- a/rust/rhg/src/commands/root.rs
+++ b/rust/rhg/src/commands/root.rs
@@ -1,5 +1,6 @@
 use crate::error::CommandError;
 use format_bytes::format_bytes;
+use hg::errors::{IoErrorContext, IoResultExt};
 use hg::utils::files::get_bytes_from_path;
 
 pub const HELP_TEXT: &str = "
@@ -14,7 +15,12 @@
 
 pub fn run(invocation: &crate::CliInvocation) -> Result<(), CommandError> {
     let repo = invocation.repo?;
-    let bytes = get_bytes_from_path(repo.working_directory_path());
+    let working_directory = repo.working_directory_path();
+    let working_directory = std::fs::canonicalize(working_directory)
+        .with_context(|| {
+            IoErrorContext::CanonicalizingPath(working_directory.to_owned())
+        })?;
+    let bytes = get_bytes_from_path(&working_directory);
     invocation
         .ui
         .write_stdout(&format_bytes!(b"{}\n", bytes.as_slice()))?;
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
@@ -4,6 +4,7 @@
 use hg::operations::list_rev_tracked_files;
 use hg::operations::Dirstate;
 use hg::repo::Repo;
+use hg::utils::current_dir;
 use hg::utils::files::{get_bytes_from_path, relativize_path};
 use hg::utils::hg_path::{HgPath, HgPathBuf};
 
@@ -51,8 +52,10 @@
     files: impl IntoIterator<Item = &'a HgPath>,
 ) -> Result<(), CommandError> {
     let cwd = HgPathBuf::from(get_bytes_from_path(hg::utils::current_dir()?));
+    let working_directory = repo.working_directory_path();
+    let working_directory = current_dir()?.join(working_directory); // Make it absolute
     let working_directory =
-        HgPathBuf::from(get_bytes_from_path(repo.working_directory_path()));
+        HgPathBuf::from(get_bytes_from_path(working_directory));
 
     let mut stdout = ui.stdout_buffer();
 
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
@@ -40,13 +40,15 @@
 
     let repo = invocation.repo?;
     let cwd = hg::utils::current_dir()?;
+    let working_directory = repo.working_directory_path();
+    let working_directory = cwd.join(working_directory); // Make it absolute
 
     let mut files = vec![];
     for file in file_args.iter() {
         // TODO: actually normalize `..` path segments etc?
         let normalized = cwd.join(&file);
         let stripped = normalized
-            .strip_prefix(&repo.working_directory_path())
+            .strip_prefix(&working_directory)
             // TODO: error message for path arguments outside of the repo
             .map_err(|_| CommandError::abort(""))?;
         let hg_file = HgPathBuf::try_from(stripped.to_path_buf())
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
@@ -2,7 +2,7 @@
 use crate::errors::{HgError, IoErrorContext, IoResultExt};
 use crate::requirements;
 use crate::utils::files::get_path_from_bytes;
-use crate::utils::{current_dir, SliceExt};
+use crate::utils::SliceExt;
 use memmap::{Mmap, MmapOptions};
 use std::collections::HashSet;
 use std::path::{Path, PathBuf};
@@ -56,12 +56,9 @@
         explicit_path: Option<&Path>,
     ) -> Result<Self, RepoError> {
         if let Some(root) = explicit_path {
-            // Having an absolute path isn’t necessary here but can help code
-            // elsewhere
-            let absolute_root = current_dir()?.join(root);
-            if absolute_root.join(".hg").is_dir() {
-                Self::new_at_path(absolute_root, config)
-            } else if absolute_root.is_file() {
+            if root.join(".hg").is_dir() {
+                Self::new_at_path(root.to_owned(), config)
+            } else if root.is_file() {
                 Err(HgError::unsupported("bundle repository").into())
             } else {
                 Err(RepoError::NotFound {
diff --git a/rust/hg-core/src/errors.rs b/rust/hg-core/src/errors.rs
--- a/rust/hg-core/src/errors.rs
+++ b/rust/hg-core/src/errors.rs
@@ -50,6 +50,8 @@
         from: std::path::PathBuf,
         to: std::path::PathBuf,
     },
+    /// `std::fs::canonicalize`
+    CanonicalizingPath(std::path::PathBuf),
     /// `std::env::current_dir`
     CurrentDir,
     /// `std::env::current_exe`
@@ -128,6 +130,9 @@
                 from.display(),
                 to.display()
             ),
+            IoErrorContext::CanonicalizingPath(path) => {
+                write!(f, "when canonicalizing {}", path.display())
+            }
             IoErrorContext::CurrentDir => {
                 write!(f, "error getting current working directory")
             }



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