( )⚙ D11771 rhg: Propogate manifest parse errors instead of panicking

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11771

rhg: Propogate manifest parse errors instead of panicking
ClosedPublic

Authored by SimonSapin on Nov 23 2021, 2:10 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG10c32e1b892a: rhg: Propogate manifest parse errors instead of panicking
Summary

The Rust parser for the manifest file format is an iterator. Now every item
from that iterator is a Result, which makes error handling required
in multiple new places.

This makes better recovery on errors possible, compare to a run time panic.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

SimonSapin created this revision.Nov 23 2021, 2:10 PM
Alphare accepted this revision.Nov 24 2021, 6:28 AM
This revision is now accepted and ready to land.Nov 24 2021, 6:28 AM
baymax updated this revision to Diff 31162.Nov 26 2021, 10:33 AM

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

Revision Contents
Changeset List

Diff 31093

rust/hg-core/src/operations/cat.rs

// list_tracked_files.rs // list_tracked_files.rs
// //
// Copyright 2020 Antoine Cezar <antoine.cezar@octobus.net> // Copyright 2020 Antoine Cezar <antoine.cezar@octobus.net>
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
use crate::repo::Repo; use crate::repo::Repo;
use crate::revlog::revlog::RevlogError; use crate::revlog::revlog::RevlogError;
use crate::revlog::Node; use crate::revlog::Node;
use crate::utils::hg_path::HgPath; use crate::utils::hg_path::HgPath;
use crate::errors::HgError;
use itertools::put_back; use itertools::put_back;
use itertools::PutBack; use itertools::PutBack;
use std::cmp::Ordering; use std::cmp::Ordering;
pub struct CatOutput<'a> { pub struct CatOutput<'a> {
/// Whether any file in the manifest matched the paths given as CLI /// Whether any file in the manifest matched the paths given as CLI
/// arguments /// arguments
pub found_any: bool, pub found_any: bool,
/// The contents of matching files, in manifest order /// The contents of matching files, in manifest order
pub results: Vec<(&'a HgPath, Vec<u8>)>, pub results: Vec<(&'a HgPath, Vec<u8>)>,
/// Which of the CLI arguments did not match any manifest file /// Which of the CLI arguments did not match any manifest file
pub missing: Vec<&'a HgPath>, pub missing: Vec<&'a HgPath>,
/// The node ID that the given revset was resolved to /// The node ID that the given revset was resolved to
pub node: Node, pub node: Node,
} }
// Find an item in an iterator over a sorted collection. // Find an item in an iterator over a sorted collection.
fn find_item<'a, 'b, 'c, D, I: Iterator<Item = (&'a HgPath, D)>>( fn find_item<'a, D, I: Iterator<Item = Result<(&'a HgPath, D), HgError>>>(
i: &mut PutBack<I>, i: &mut PutBack<I>,
needle: &'b HgPath, needle: &HgPath,
) -> Option<D> { ) -> Result<Option<D>, HgError> {
loop { loop {
match i.next() { match i.next() {
None => return None, None => return Ok(None),
Some(val) => match needle.as_bytes().cmp(val.0.as_bytes()) { Some(result) => {
let (path, value) = result?;
match needle.as_bytes().cmp(path.as_bytes()) {
Ordering::Less => { Ordering::Less => {
i.put_back(val); i.put_back(Ok((path, value)));
return None; return Ok(None);
} }
Ordering::Greater => continue, Ordering::Greater => continue,
Ordering::Equal => return Some(val.1), Ordering::Equal => return Ok(Some(value)),
}, }
}
} }
} }
} }
fn find_files_in_manifest< fn find_files_in_manifest<
'manifest, 'manifest,
'query, 'query,
Data, Data,
Manifest: Iterator<Item = (&'manifest HgPath, Data)>, Manifest: Iterator<Item = Result<(&'manifest HgPath, Data), HgError>>,
Query: Iterator<Item = &'query HgPath>, Query: Iterator<Item = &'query HgPath>,
>( >(
manifest: Manifest, manifest: Manifest,
query: Query, query: Query,
) -> (Vec<(&'query HgPath, Data)>, Vec<&'query HgPath>) { ) -> Result<(Vec<(&'query HgPath, Data)>, Vec<&'query HgPath>), HgError> {
let mut manifest = put_back(manifest); let mut manifest = put_back(manifest);
let mut res = vec![]; let mut res = vec![];
let mut missing = vec![]; let mut missing = vec![];
for file in query { for file in query {
match find_item(&mut manifest, file) { match find_item(&mut manifest, file)? {
None => missing.push(file), None => missing.push(file),
Some(item) => res.push((file, item)), Some(item) => res.push((file, item)),
} }
} }
return (res, missing); return Ok((res, missing));
} }
/// Output the given revision of files /// Output the given revision of files
/// ///
/// * `root`: Repository root /// * `root`: Repository root
/// * `rev`: The revision to cat the files from. /// * `rev`: The revision to cat the files from.
/// * `files`: The files to output. /// * `files`: The files to output.
pub fn cat<'a>( pub fn cat<'a>(
let mut results: Vec<(&'a HgPath, Vec<u8>)> = vec![]; let mut results: Vec<(&'a HgPath, Vec<u8>)> = vec![];
let mut found_any = false; let mut found_any = false;
files.sort_unstable(); files.sort_unstable();
let (found, missing) = find_files_in_manifest( let (found, missing) = find_files_in_manifest(
manifest.files_with_nodes(), manifest.files_with_nodes(),
files.into_iter().map(|f| f.as_ref()), files.into_iter().map(|f| f.as_ref()),
); )?;
for (file_path, node_bytes) in found { for (file_path, node_bytes) in found {
found_any = true; found_any = true;
let file_log = repo.filelog(file_path)?; let file_log = repo.filelog(file_path)?;
let file_node = Node::from_hex_for_repo(node_bytes)?; let file_node = Node::from_hex_for_repo(node_bytes)?;
results.push(( results.push((
file_path, file_path,
file_log.data_for_node(file_node)?.into_data()?, file_log.data_for_node(file_node)?.into_data()?,

rust/hg-core/src/operations/list_tracked_files.rs

) -> Result<FilesForRev, RevlogError> { ) -> Result<FilesForRev, RevlogError> {
let rev = crate::revset::resolve_single(revset, repo)?; let rev = crate::revset::resolve_single(revset, repo)?;
Ok(FilesForRev(repo.manifest_for_rev(rev)?)) Ok(FilesForRev(repo.manifest_for_rev(rev)?))
} }
pub struct FilesForRev(Manifest); pub struct FilesForRev(Manifest);
impl FilesForRev { impl FilesForRev {
pub fn iter(&self) -> impl Iterator<Item = &HgPath> { pub fn iter(&self) -> impl Iterator<Item = Result<&HgPath, HgError>> {
self.0.files() self.0.files()
} }
} }

rust/hg-core/src/revlog/manifest.rs

/// Return an iterator over the lines of the entry. /// Return an iterator over the lines of the entry.
pub fn lines(&self) -> impl Iterator<Item = &[u8]> { pub fn lines(&self) -> impl Iterator<Item = &[u8]> {
self.bytes self.bytes
.split(|b| b == &b'\n') .split(|b| b == &b'\n')
.filter(|line| !line.is_empty()) .filter(|line| !line.is_empty())
} }
/// Return an iterator over the files of the entry. /// Return an iterator over the files of the entry.
pub fn files(&self) -> impl Iterator<Item = &HgPath> { pub fn files(&self) -> impl Iterator<Item = Result<&HgPath, HgError>> {
self.lines().filter(|line| !line.is_empty()).map(|line| { self.lines().filter(|line| !line.is_empty()).map(|line| {
let pos = line let pos =
.iter() line.iter().position(|x| x == &b'\0').ok_or_else(|| {
.position(|x| x == &b'\0') HgError::corrupted("manifest line should contain \\0")
.expect("manifest line should contain \\0"); })?;
HgPath::new(&line[..pos]) Ok(HgPath::new(&line[..pos]))
}) })
} }
/// Return an iterator over the files of the entry. /// Return an iterator over the files of the entry.
pub fn files_with_nodes(&self) -> impl Iterator<Item = (&HgPath, &[u8])> { pub fn files_with_nodes(
&self,
) -> impl Iterator<Item = Result<(&HgPath, &[u8]), HgError>> {
self.lines().filter(|line| !line.is_empty()).map(|line| { self.lines().filter(|line| !line.is_empty()).map(|line| {
let pos = line let pos =
.iter() line.iter().position(|x| x == &b'\0').ok_or_else(|| {
.position(|x| x == &b'\0') HgError::corrupted("manifest line should contain \\0")
.expect("manifest line should contain \\0"); })?;
let hash_start = pos + 1; let hash_start = pos + 1;
let hash_end = hash_start + 40; let hash_end = hash_start + 40;
(HgPath::new(&line[..pos]), &line[hash_start..hash_end]) Ok((HgPath::new(&line[..pos]), &line[hash_start..hash_end]))
}) })
} }
/// If the given path is in this manifest, return its filelog node ID /// If the given path is in this manifest, return its filelog node ID
pub fn find_file(&self, path: &HgPath) -> Result<Option<Node>, HgError> { pub fn find_file(&self, path: &HgPath) -> Result<Option<Node>, HgError> {
// TODO: use binary search instead of linear scan. This may involve // TODO: use binary search instead of linear scan. This may involve
// building (and caching) an index of the byte indicex of each manifest // building (and caching) an index of the byte indicex of each manifest
// line. // line.
for (manifest_path, node) in self.files_with_nodes() { for entry in self.files_with_nodes() {
let (manifest_path, node) = entry?;
if manifest_path == path { if manifest_path == path {
return Ok(Some(Node::from_hex_for_repo(node)?)); return Ok(Some(Node::from_hex_for_repo(node)?));
} }
} }
Ok(None) Ok(None)
} }
} }

rust/rhg/src/commands/files.rs

use crate::error::CommandError; use crate::error::CommandError;
use crate::ui::Ui; use crate::ui::Ui;
use crate::ui::UiError; use crate::ui::UiError;
use crate::utils::path_utils::relativize_paths; use crate::utils::path_utils::relativize_paths;
use clap::Arg; use clap::Arg;
use hg::errors::HgError;
use hg::operations::list_rev_tracked_files; use hg::operations::list_rev_tracked_files;
use hg::operations::Dirstate; use hg::operations::Dirstate;
use hg::repo::Repo; use hg::repo::Repo;
use hg::utils::hg_path::HgPath; use hg::utils::hg_path::HgPath;
use std::borrow::Cow; use std::borrow::Cow;
pub const HELP_TEXT: &str = " pub const HELP_TEXT: &str = "
List tracked files. List tracked files.
let repo = invocation.repo?; let repo = invocation.repo?;
if let Some(rev) = rev { if let Some(rev) = rev {
let files = list_rev_tracked_files(repo, rev).map_err(|e| (e, rev))?; let files = list_rev_tracked_files(repo, rev).map_err(|e| (e, rev))?;
display_files(invocation.ui, repo, files.iter()) display_files(invocation.ui, repo, files.iter())
} else { } else {
let distate = Dirstate::new(repo)?; let distate = Dirstate::new(repo)?;
let files = distate.tracked_files()?; let files = distate.tracked_files()?;
display_files(invocation.ui, repo, files) display_files(invocation.ui, repo, files.into_iter().map(Ok))
} }
} }
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 = Result<&'a HgPath, HgError>>,
) -> Result<(), CommandError> { ) -> Result<(), CommandError> {
let mut stdout = ui.stdout_buffer(); let mut stdout = ui.stdout_buffer();
let mut any = false; let mut any = false;
relativize_paths(repo, files, |path: Cow<[u8]>| -> Result<(), UiError> { relativize_paths(repo, files, |path: Cow<[u8]>| -> Result<(), UiError> {
any = true; any = true;
stdout.write_all(path.as_ref())?; stdout.write_all(path.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)
} }
} }

rust/rhg/src/commands/status.rs

paths.sort_unstable(); paths.sort_unstable();
let mut relative: bool = config.get_bool(b"ui", b"relative-paths")?; let mut relative: bool = config.get_bool(b"ui", b"relative-paths")?;
relative = config relative = config
.get_option(b"commands", b"status.relative")? .get_option(b"commands", b"status.relative")?
.unwrap_or(relative); .unwrap_or(relative);
if relative && !ui.plain() { if relative && !ui.plain() {
relativize_paths( relativize_paths(
repo, repo,
paths, paths.iter().map(Ok),
|path: Cow<[u8]>| -> Result<(), UiError> { |path: Cow<[u8]>| -> Result<(), UiError> {
ui.write_stdout( ui.write_stdout(
&[status_prefix, b" ", path.as_ref(), b"\n"].concat(), &[status_prefix, b" ", path.as_ref(), b"\n"].concat(),
) )
}, },
)?; )?;
} else { } else {
for path in paths { for path in paths {

rust/rhg/src/utils/path_utils.rs

// path utils module // path utils module
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
use crate::error::CommandError; use crate::error::CommandError;
use crate::ui::UiError; use crate::ui::UiError;
use hg::errors::HgError;
use hg::repo::Repo; use hg::repo::Repo;
use hg::utils::current_dir; use hg::utils::current_dir;
use hg::utils::files::{get_bytes_from_path, relativize_path}; use hg::utils::files::{get_bytes_from_path, relativize_path};
use hg::utils::hg_path::HgPath; use hg::utils::hg_path::HgPath;
use hg::utils::hg_path::HgPathBuf; use hg::utils::hg_path::HgPathBuf;
use std::borrow::Cow; use std::borrow::Cow;
pub fn relativize_paths( pub fn relativize_paths(
repo: &Repo, repo: &Repo,
paths: impl IntoIterator<Item = impl AsRef<HgPath>>, paths: impl IntoIterator<Item = Result<impl AsRef<HgPath>, HgError>>,
mut callback: impl FnMut(Cow<[u8]>) -> Result<(), UiError>, mut callback: impl FnMut(Cow<[u8]>) -> Result<(), UiError>,
) -> Result<(), CommandError> { ) -> Result<(), CommandError> {
let cwd = current_dir()?; let cwd = current_dir()?;
let repo_root = repo.working_directory_path(); let repo_root = repo.working_directory_path();
let repo_root = cwd.join(repo_root); // Make it absolute let repo_root = cwd.join(repo_root); // Make it absolute
let repo_root_hgpath = let repo_root_hgpath =
HgPathBuf::from(get_bytes_from_path(repo_root.to_owned())); HgPathBuf::from(get_bytes_from_path(repo_root.to_owned()));
let outside_repo: bool; let outside_repo: bool;
let cwd_hgpath: HgPathBuf; let cwd_hgpath: HgPathBuf;
if let Ok(cwd_relative_to_repo) = cwd.strip_prefix(&repo_root) { if let Ok(cwd_relative_to_repo) = cwd.strip_prefix(&repo_root) {
// The current directory is inside the repo, so we can work with // The current directory is inside the repo, so we can work with
// relative paths // relative paths
outside_repo = false; outside_repo = false;
cwd_hgpath = cwd_hgpath =
HgPathBuf::from(get_bytes_from_path(cwd_relative_to_repo)); HgPathBuf::from(get_bytes_from_path(cwd_relative_to_repo));
} else { } else {
outside_repo = true; outside_repo = true;
cwd_hgpath = HgPathBuf::from(get_bytes_from_path(cwd)); cwd_hgpath = HgPathBuf::from(get_bytes_from_path(cwd));
} }
for file in paths { for file in paths {
if outside_repo { if outside_repo {
let file = repo_root_hgpath.join(file.as_ref()); let file = repo_root_hgpath.join(file?.as_ref());
callback(relativize_path(&file, &cwd_hgpath))?; callback(relativize_path(&file, &cwd_hgpath))?;
} else { } else {
callback(relativize_path(file.as_ref(), &cwd_hgpath))?; callback(relativize_path(file?.as_ref(), &cwd_hgpath))?;
} }
} }
Ok(()) Ok(())
} }