( )⚙ D10200 rhg: Make `files` work on repo-relative paths when possible

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10200

rhg: Make `files` work on repo-relative paths when possible
ClosedPublic

Authored by SimonSapin on Mar 13 2021, 3:54 AM.

Details

Reviewers
Alphare
pulkit
Group Reviewers
hg-reviewers
Commits
rHGb5e8bf10436e: rhg: Make `files` work on repo-relative paths when possible
Summary

When the current directory is outside of the repository we need to turn
everything into absolute filesystem paths in order to compute correct
relative paths.

This was previously done unconditionally, but is not necessary when
the current directory is inside the repository.

With this change rhg files > /dev/null at the root of a mozilla-central
snapshot goes from ~150 ms to ~70 ms. My repository is located at a
somewhat long path though (93 bytes). The effect may not be as pronounced
at a shorter path.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

SimonSapin created this revision.Mar 13 2021, 3:54 AM
Alphare added a subscriber: Alphare.Mar 15 2021, 7:30 AM
Alphare added inline comments.
rust/rhg/src/commands/files.rs
73

Looks to me that the if/else logic could be smaller, and the actual loop written only once after.

pulkit added a subscriber: pulkit.Apr 1 2021, 4:51 AM

Hm, what's the status of this?

Alphare accepted this revision.Apr 1 2021, 8:54 AM
Alphare added inline comments.
rust/rhg/src/commands/files.rs
73

Thinking about it some more, this looks fine as it is.

pulkit accepted this revision.Apr 11 2021, 5:24 PM
This revision is now accepted and ready to land.Apr 11 2021, 5:24 PM
pulkit added a comment.Apr 11 2021, 7:30 PM

This needs to be rebased.

baymax updated this revision to Diff 26814.Apr 12 2021, 7:46 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

pulkit accepted this revision.Apr 13 2021, 4:54 AM

Revision Contents
Changeset List

PathPackages
Mrust/rhg/src/commands/files.rs (36 lines)

Diff 26927

rust/rhg/src/commands/files.rs

} }
} }
fn display_files<'a>( fn display_files<'a>(
ui: &Ui, ui: &Ui,
repo: &Repo, repo: &Repo,
files: impl IntoIterator<Item = &'a HgPath>, files: impl IntoIterator<Item = &'a HgPath>,
) -> Result<(), CommandError> { ) -> 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(working_directory));
let mut stdout = ui.stdout_buffer(); let mut stdout = ui.stdout_buffer();
let cwd = current_dir()?;
let working_directory = repo.working_directory_path();
let working_directory = cwd.join(working_directory); // Make it absolute
let mut any = false; let mut any = false;
if let Ok(cwd_relative_to_repo) = cwd.strip_prefix(&working_directory) {
// The current directory is inside the repo, so we can work with
// relative paths
let cwd = HgPathBuf::from(get_bytes_from_path(cwd_relative_to_repo));
for file in files {
any = true;
stdout.write_all(relativize_path(&file, &cwd).as_ref())?;
stdout.write_all(b"\n")?;
}
} else {
let working_directory =
AlphareUnsubmitted
Not Done

Looks to me that the if/else logic could be smaller, and the actual loop written only once after.

Alphare: Looks to me that the `if/else` logic could be smaller, and the actual loop written only once…
AlphareUnsubmitted
Not Done

Thinking about it some more, this looks fine as it is.

Alphare: Thinking about it some more, this looks fine as it is.
HgPathBuf::from(get_bytes_from_path(working_directory));
let cwd = HgPathBuf::from(get_bytes_from_path(cwd));
for file in files { for file in files {
any = true; any = true;
// Absolute path in the filesystem
let file = working_directory.join(file); let file = working_directory.join(file);
stdout.write_all(relativize_path(&file, &cwd).as_ref())?; stdout.write_all(relativize_path(&file, &cwd).as_ref())?;
stdout.write_all(b"\n")?; stdout.write_all(b"\n")?;
} }
}
stdout.flush()?; stdout.flush()?;
if any { if any {
Ok(()) Ok(())
} else { } else {
Err(CommandError::Unsuccessful) Err(CommandError::Unsuccessful)
} }
} }