Mercurial > hg-stable
changeset 46136:dca9cb99971c
rust: replace most "operation" structs with functions
The hg-core crate has a partially-formed concept of "operation",
represented as structs with constructors and a `run` method.
Each struct’s contructor takes different parameters,
and each `run` has a different return type.
Constructors typically don’t do much more than store parameters
for `run` to access them.
There was a comment about adding an `Operation` trait
when the language supports expressing something so general,
but it’s hard to imagine how operations with such different APIs
could be used in a generic context.
This commit starts removing the concept of "operation",
since those are pretty much just functions.
Differential Revision: https://phab.mercurial-scm.org/D9595
author | Simon Sapin <simon.sapin@octobus.net> |
---|---|
date | Mon, 14 Dec 2020 14:59:23 +0100 |
parents | cc6faec62cb7 |
children | 3158522085f0 |
files | rust/hg-core/src/operations/cat.rs rust/hg-core/src/operations/debugdata.rs rust/hg-core/src/operations/find_root.rs rust/hg-core/src/operations/list_tracked_files.rs rust/hg-core/src/operations/mod.rs rust/rhg/src/commands/cat.rs rust/rhg/src/commands/debugdata.rs rust/rhg/src/commands/debugrequirements.rs rust/rhg/src/commands/files.rs rust/rhg/src/commands/root.rs |
diffstat | 10 files changed, 150 insertions(+), 260 deletions(-) [+] |
line wrap: on
line diff
--- a/rust/hg-core/src/operations/cat.rs Mon Dec 14 13:47:44 2020 +0100 +++ b/rust/hg-core/src/operations/cat.rs Mon Dec 14 14:59:23 2020 +0100 @@ -9,7 +9,7 @@ use std::path::{Path, PathBuf}; use crate::revlog::changelog::Changelog; -use crate::revlog::manifest::{Manifest, ManifestEntry}; +use crate::revlog::manifest::Manifest; use crate::revlog::path_encode::path_encode; use crate::revlog::revlog::Revlog; use crate::revlog::revlog::RevlogError; @@ -70,97 +70,60 @@ } /// List files under Mercurial control at a given revision. -pub struct CatRev<'a> { - root: &'a Path, - /// The revision to cat the files from. - rev: &'a str, - /// The files to output. - files: &'a [HgPathBuf], - /// The changelog file - changelog: Changelog, - /// The manifest file - manifest: Manifest, - /// The manifest entry corresponding to the revision. - /// - /// Used to hold the owner of the returned references. - manifest_entry: Option<ManifestEntry>, -} +/// +/// * `root`: Repository root +/// * `rev`: The revision to cat the files from. +/// * `files`: The files to output. +pub fn cat( + root: &Path, + rev: &str, + files: &[HgPathBuf], +) -> Result<Vec<u8>, CatRevError> { + let changelog = Changelog::open(&root)?; + let manifest = Manifest::open(&root)?; + + let changelog_entry = match rev.parse::<Revision>() { + Ok(rev) => changelog.get_rev(rev)?, + _ => { + let changelog_node = NodePrefix::from_hex(&rev) + .map_err(|_| CatRevErrorKind::InvalidRevision)?; + changelog.get_node(changelog_node.borrow())? + } + }; + let manifest_node = Node::from_hex(&changelog_entry.manifest_node()?) + .map_err(|_| CatRevErrorKind::CorruptedRevlog)?; + + let manifest_entry = manifest.get_node((&manifest_node).into())?; + let mut bytes = vec![]; -impl<'a> CatRev<'a> { - pub fn new( - root: &'a Path, - rev: &'a str, - files: &'a [HgPathBuf], - ) -> Result<Self, CatRevError> { - let changelog = Changelog::open(&root)?; - let manifest = Manifest::open(&root)?; - let manifest_entry = None; + for (manifest_file, node_bytes) in manifest_entry.files_with_nodes() { + for cat_file in files.iter() { + if cat_file.as_bytes() == manifest_file.as_bytes() { + let index_path = store_path(root, manifest_file, b".i"); + let data_path = store_path(root, manifest_file, b".d"); - Ok(Self { - root, - rev, - files, - changelog, - manifest, - manifest_entry, - }) + let file_log = Revlog::open(&index_path, Some(&data_path))?; + let file_node = Node::from_hex(node_bytes) + .map_err(|_| CatRevErrorKind::CorruptedRevlog)?; + let file_rev = file_log.get_node_rev((&file_node).into())?; + let data = file_log.get_rev_data(file_rev)?; + if data.starts_with(&METADATA_DELIMITER) { + let end_delimiter_position = data + [METADATA_DELIMITER.len()..] + .windows(METADATA_DELIMITER.len()) + .position(|bytes| bytes == METADATA_DELIMITER); + if let Some(position) = end_delimiter_position { + let offset = METADATA_DELIMITER.len() * 2; + bytes.extend(data[position + offset..].iter()); + } + } else { + bytes.extend(data); + } + } + } } - pub fn run(&mut self) -> Result<Vec<u8>, CatRevError> { - let changelog_entry = match self.rev.parse::<Revision>() { - Ok(rev) => self.changelog.get_rev(rev)?, - _ => { - let changelog_node = NodePrefix::from_hex(&self.rev) - .map_err(|_| CatRevErrorKind::InvalidRevision)?; - self.changelog.get_node(changelog_node.borrow())? - } - }; - let manifest_node = Node::from_hex(&changelog_entry.manifest_node()?) - .map_err(|_| CatRevErrorKind::CorruptedRevlog)?; - - self.manifest_entry = - Some(self.manifest.get_node((&manifest_node).into())?); - if let Some(ref manifest_entry) = self.manifest_entry { - let mut bytes = vec![]; - - for (manifest_file, node_bytes) in - manifest_entry.files_with_nodes() - { - for cat_file in self.files.iter() { - if cat_file.as_bytes() == manifest_file.as_bytes() { - let index_path = - store_path(self.root, manifest_file, b".i"); - let data_path = - store_path(self.root, manifest_file, b".d"); - - let file_log = - Revlog::open(&index_path, Some(&data_path))?; - let file_node = Node::from_hex(node_bytes) - .map_err(|_| CatRevErrorKind::CorruptedRevlog)?; - let file_rev = - file_log.get_node_rev((&file_node).into())?; - let data = file_log.get_rev_data(file_rev)?; - if data.starts_with(&METADATA_DELIMITER) { - let end_delimiter_position = data - [METADATA_DELIMITER.len()..] - .windows(METADATA_DELIMITER.len()) - .position(|bytes| bytes == METADATA_DELIMITER); - if let Some(position) = end_delimiter_position { - let offset = METADATA_DELIMITER.len() * 2; - bytes.extend(data[position + offset..].iter()); - } - } else { - bytes.extend(data); - } - } - } - } - - Ok(bytes) - } else { - unreachable!("manifest_entry should have been stored"); - } - } + Ok(bytes) } fn store_path(root: &Path, hg_path: &HgPath, suffix: &[u8]) -> PathBuf {
--- a/rust/hg-core/src/operations/debugdata.rs Mon Dec 14 13:47:44 2020 +0100 +++ b/rust/hg-core/src/operations/debugdata.rs Mon Dec 14 14:59:23 2020 +0100 @@ -5,7 +5,8 @@ // 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::find_root; +use std::path::Path; + use crate::revlog::revlog::{Revlog, RevlogError}; use crate::revlog::NodePrefix; use crate::revlog::Revision; @@ -20,7 +21,6 @@ /// Kind of error encountered by DebugData #[derive(Debug)] pub enum DebugDataErrorKind { - FindRootError(find_root::FindRootError), /// Error when reading a `revlog` file. IoError(std::io::Error), /// The revision has not been found. @@ -48,13 +48,6 @@ } } -impl From<find_root::FindRootError> for DebugDataError { - fn from(err: find_root::FindRootError) -> Self { - let kind = DebugDataErrorKind::FindRootError(err); - DebugDataError { kind } - } -} - impl From<std::io::Error> for DebugDataError { fn from(err: std::io::Error) -> Self { let kind = DebugDataErrorKind::IoError(err); @@ -85,36 +78,26 @@ } /// Dump the contents data of a revision. -pub struct DebugData<'a> { - /// Revision or hash of the revision. - rev: &'a str, - /// Kind of data to debug. +pub fn debug_data( + root: &Path, + rev: &str, kind: DebugDataKind, -} - -impl<'a> DebugData<'a> { - pub fn new(rev: &'a str, kind: DebugDataKind) -> Self { - DebugData { rev, kind } - } +) -> Result<Vec<u8>, DebugDataError> { + let index_file = match kind { + DebugDataKind::Changelog => root.join(".hg/store/00changelog.i"), + DebugDataKind::Manifest => root.join(".hg/store/00manifest.i"), + }; + let revlog = Revlog::open(&index_file, None)?; - pub fn run(&mut self) -> Result<Vec<u8>, DebugDataError> { - let root = find_root::FindRoot::new().run()?; - let index_file = match self.kind { - DebugDataKind::Changelog => root.join(".hg/store/00changelog.i"), - DebugDataKind::Manifest => root.join(".hg/store/00manifest.i"), - }; - let revlog = Revlog::open(&index_file, None)?; + let data = match rev.parse::<Revision>() { + Ok(rev) => revlog.get_rev_data(rev)?, + _ => { + let node = NodePrefix::from_hex(&rev) + .map_err(|_| DebugDataErrorKind::InvalidRevision)?; + let rev = revlog.get_node_rev(node.borrow())?; + revlog.get_rev_data(rev)? + } + }; - let data = match self.rev.parse::<Revision>() { - Ok(rev) => revlog.get_rev_data(rev)?, - _ => { - let node = NodePrefix::from_hex(&self.rev) - .map_err(|_| DebugDataErrorKind::InvalidRevision)?; - let rev = revlog.get_node_rev(node.borrow())?; - revlog.get_rev_data(rev)? - } - }; - - Ok(data) - } + Ok(data) }
--- a/rust/hg-core/src/operations/find_root.rs Mon Dec 14 13:47:44 2020 +0100 +++ b/rust/hg-core/src/operations/find_root.rs Mon Dec 14 14:59:23 2020 +0100 @@ -28,46 +28,29 @@ } /// Find the root of the repository -/// by searching for a .hg directory in the current directory and its +/// by searching for a .hg directory in the process’ current directory and its /// ancestors -pub struct FindRoot<'a> { - current_dir: Option<&'a Path>, +pub fn find_root() -> Result<PathBuf, FindRootError> { + let current_dir = std::env::current_dir().map_err(|e| FindRootError { + kind: FindRootErrorKind::GetCurrentDirError(e), + })?; + Ok(find_root_from_path(¤t_dir)?.into()) } -impl<'a> FindRoot<'a> { - pub fn new() -> Self { - Self { current_dir: None } +/// Find the root of the repository +/// by searching for a .hg directory in the given directory and its ancestors +pub fn find_root_from_path(start: &Path) -> Result<&Path, FindRootError> { + if start.join(".hg").exists() { + return Ok(start); } - - pub fn new_from_path(current_dir: &'a Path) -> Self { - Self { - current_dir: Some(current_dir), + for ancestor in start.ancestors() { + if ancestor.join(".hg").exists() { + return Ok(ancestor); } } - - pub fn run(&self) -> Result<PathBuf, FindRootError> { - let current_dir = match self.current_dir { - None => std::env::current_dir().or_else(|e| { - Err(FindRootError { - kind: FindRootErrorKind::GetCurrentDirError(e), - }) - })?, - Some(path) => path.into(), - }; - - if current_dir.join(".hg").exists() { - return Ok(current_dir); - } - let ancestors = current_dir.ancestors(); - for parent in ancestors { - if parent.join(".hg").exists() { - return Ok(parent.into()); - } - } - Err(FindRootError { - kind: FindRootErrorKind::RootNotFound(current_dir.to_path_buf()), - }) - } + Err(FindRootError { + kind: FindRootErrorKind::RootNotFound(start.into()), + }) } #[cfg(test)] @@ -81,7 +64,7 @@ let tmp_dir = tempfile::tempdir().unwrap(); let path = tmp_dir.path(); - let err = FindRoot::new_from_path(&path).run().unwrap_err(); + let err = find_root_from_path(&path).unwrap_err(); // TODO do something better assert!(match err { @@ -98,7 +81,7 @@ let root = tmp_dir.path(); fs::create_dir_all(root.join(".hg")).unwrap(); - let result = FindRoot::new_from_path(&root).run().unwrap(); + let result = find_root_from_path(&root).unwrap(); assert_eq!(result, root) } @@ -109,10 +92,8 @@ let root = tmp_dir.path(); fs::create_dir_all(root.join(".hg")).unwrap(); - let result = - FindRoot::new_from_path(&root.join("some/nested/directory")) - .run() - .unwrap(); + let directory = root.join("some/nested/directory"); + let result = find_root_from_path(&directory).unwrap(); assert_eq!(result, root) }
--- a/rust/hg-core/src/operations/list_tracked_files.rs Mon Dec 14 13:47:44 2020 +0100 +++ b/rust/hg-core/src/operations/list_tracked_files.rs Mon Dec 14 14:59:23 2020 +0100 @@ -51,20 +51,20 @@ /// List files under Mercurial control in the working directory /// by reading the dirstate -pub struct ListDirstateTrackedFiles { +pub struct Dirstate { /// The `dirstate` content. content: Vec<u8>, } -impl ListDirstateTrackedFiles { +impl Dirstate { pub fn new(root: &Path) -> Result<Self, ListDirstateTrackedFilesError> { let dirstate = root.join(".hg/dirstate"); let content = fs::read(&dirstate)?; Ok(Self { content }) } - pub fn run( - &mut self, + pub fn tracked_files( + &self, ) -> Result<Vec<&HgPath>, ListDirstateTrackedFilesError> { let (_, entries, _) = parse_dirstate(&self.content) .map_err(ListDirstateTrackedFilesErrorKind::ParseError)?; @@ -137,58 +137,31 @@ } /// List files under Mercurial control at a given revision. -pub struct ListRevTrackedFiles<'a> { - /// The revision to list the files from. - rev: &'a str, - /// The changelog file - changelog: Changelog, - /// The manifest file - manifest: Manifest, - /// The manifest entry corresponding to the revision. - /// - /// Used to hold the owner of the returned references. - manifest_entry: Option<ManifestEntry>, +pub fn list_rev_tracked_files( + root: &Path, + rev: &str, +) -> Result<FilesForRev, ListRevTrackedFilesError> { + let changelog = Changelog::open(root)?; + let manifest = Manifest::open(root)?; + + let changelog_entry = match rev.parse::<Revision>() { + Ok(rev) => changelog.get_rev(rev)?, + _ => { + let changelog_node = NodePrefix::from_hex(&rev) + .or(Err(ListRevTrackedFilesErrorKind::InvalidRevision))?; + changelog.get_node(changelog_node.borrow())? + } + }; + let manifest_node = Node::from_hex(&changelog_entry.manifest_node()?) + .or(Err(ListRevTrackedFilesErrorKind::CorruptedRevlog))?; + let manifest_entry = manifest.get_node((&manifest_node).into())?; + Ok(FilesForRev(manifest_entry)) } -impl<'a> ListRevTrackedFiles<'a> { - pub fn new( - root: &Path, - rev: &'a str, - ) -> Result<Self, ListRevTrackedFilesError> { - let changelog = Changelog::open(root)?; - let manifest = Manifest::open(root)?; - - Ok(Self { - rev, - changelog, - manifest, - manifest_entry: None, - }) - } +pub struct FilesForRev(ManifestEntry); - pub fn run( - &mut self, - ) -> Result<impl Iterator<Item = &HgPath>, ListRevTrackedFilesError> { - let changelog_entry = match self.rev.parse::<Revision>() { - Ok(rev) => self.changelog.get_rev(rev)?, - _ => { - let changelog_node = NodePrefix::from_hex(&self.rev) - .or(Err(ListRevTrackedFilesErrorKind::InvalidRevision))?; - self.changelog.get_node(changelog_node.borrow())? - } - }; - let manifest_node = Node::from_hex(&changelog_entry.manifest_node()?) - .or(Err(ListRevTrackedFilesErrorKind::CorruptedRevlog))?; - - self.manifest_entry = - Some(self.manifest.get_node((&manifest_node).into())?); - - if let Some(ref manifest_entry) = self.manifest_entry { - Ok(manifest_entry.files()) - } else { - panic!( - "manifest entry should have been stored in self.manifest_node to ensure its lifetime since references are returned from it" - ) - } +impl FilesForRev { + pub fn iter(&self) -> impl Iterator<Item = &HgPath> { + self.0.files() } }
--- a/rust/hg-core/src/operations/mod.rs Mon Dec 14 13:47:44 2020 +0100 +++ b/rust/hg-core/src/operations/mod.rs Mon Dec 14 14:59:23 2020 +0100 @@ -7,22 +7,17 @@ mod dirstate_status; mod find_root; mod list_tracked_files; -pub use cat::{CatRev, CatRevError, CatRevErrorKind}; +pub use cat::{cat, CatRevError, CatRevErrorKind}; pub use debugdata::{ - DebugData, DebugDataError, DebugDataErrorKind, DebugDataKind, + debug_data, DebugDataError, DebugDataErrorKind, DebugDataKind, }; -pub use find_root::{FindRoot, FindRootError, FindRootErrorKind}; -pub use list_tracked_files::{ - ListDirstateTrackedFiles, ListDirstateTrackedFilesError, - ListDirstateTrackedFilesErrorKind, +pub use find_root::{ + find_root, find_root_from_path, FindRootError, FindRootErrorKind, }; pub use list_tracked_files::{ - ListRevTrackedFiles, ListRevTrackedFilesError, + list_rev_tracked_files, FilesForRev, ListRevTrackedFilesError, ListRevTrackedFilesErrorKind, }; - -// TODO add an `Operation` trait when GAT have landed (rust #44265): -// there is no way to currently define a trait which can both return -// references to `self` and to passed data, which is what we would need. -// Generic Associated Types may fix this and allow us to have a unified -// interface. +pub use list_tracked_files::{ + Dirstate, ListDirstateTrackedFilesError, ListDirstateTrackedFilesErrorKind, +};
--- a/rust/rhg/src/commands/cat.rs Mon Dec 14 13:47:44 2020 +0100 +++ b/rust/rhg/src/commands/cat.rs Mon Dec 14 14:59:23 2020 +0100 @@ -2,8 +2,8 @@ use crate::error::{CommandError, CommandErrorKind}; use crate::ui::utf8_to_local; use crate::ui::Ui; -use hg::operations::FindRoot; -use hg::operations::{CatRev, CatRevError, CatRevErrorKind}; +use hg::operations::find_root; +use hg::operations::{cat, CatRevError, CatRevErrorKind}; use hg::requirements; use hg::utils::hg_path::HgPathBuf; use micro_timer::timed; @@ -32,7 +32,7 @@ impl<'a> Command for CatCommand<'a> { #[timed] fn run(&self, ui: &Ui) -> Result<(), CommandError> { - let root = FindRoot::new().run()?; + let root = find_root()?; requirements::check(&root)?; let cwd = std::env::current_dir() .or_else(|e| Err(CommandErrorKind::CurrentDirNotFound(e)))?; @@ -50,10 +50,8 @@ match self.rev { Some(rev) => { - let mut operation = CatRev::new(&root, rev, &files) + let data = cat(&root, rev, &files) .map_err(|e| map_rev_error(rev, e))?; - let data = - operation.run().map_err(|e| map_rev_error(rev, e))?; self.display(ui, &data) } None => Err(CommandErrorKind::Unimplemented.into()),
--- a/rust/rhg/src/commands/debugdata.rs Mon Dec 14 13:47:44 2020 +0100 +++ b/rust/rhg/src/commands/debugdata.rs Mon Dec 14 14:59:23 2020 +0100 @@ -2,8 +2,9 @@ use crate::error::{CommandError, CommandErrorKind}; use crate::ui::utf8_to_local; use crate::ui::Ui; +use hg::operations::find_root; use hg::operations::{ - DebugData, DebugDataError, DebugDataErrorKind, DebugDataKind, + debug_data, DebugDataError, DebugDataErrorKind, DebugDataKind, }; use micro_timer::timed; @@ -25,9 +26,9 @@ impl<'a> Command for DebugDataCommand<'a> { #[timed] fn run(&self, ui: &Ui) -> Result<(), CommandError> { - let mut operation = DebugData::new(self.rev, self.kind); - let data = - operation.run().map_err(|e| to_command_error(self.rev, e))?; + let root = find_root()?; + let data = debug_data(&root, self.rev, self.kind) + .map_err(|e| to_command_error(self.rev, e))?; let mut stdout = ui.stdout_buffer(); stdout.write_all(&data)?; @@ -40,7 +41,6 @@ /// Convert operation errors to command errors fn to_command_error(rev: &str, err: DebugDataError) -> CommandError { match err.kind { - DebugDataErrorKind::FindRootError(err) => CommandError::from(err), DebugDataErrorKind::IoError(err) => CommandError { kind: CommandErrorKind::Abort(Some( utf8_to_local(&format!("abort: {}\n", err)).into(),
--- a/rust/rhg/src/commands/debugrequirements.rs Mon Dec 14 13:47:44 2020 +0100 +++ b/rust/rhg/src/commands/debugrequirements.rs Mon Dec 14 14:59:23 2020 +0100 @@ -1,7 +1,7 @@ use crate::commands::Command; use crate::error::CommandError; use crate::ui::Ui; -use hg::operations::FindRoot; +use hg::operations::find_root; use hg::requirements; pub const HELP_TEXT: &str = " @@ -18,7 +18,7 @@ impl Command for DebugRequirementsCommand { fn run(&self, ui: &Ui) -> Result<(), CommandError> { - let root = FindRoot::new().run()?; + let root = find_root()?; let mut output = String::new(); for req in requirements::load(&root)? { output.push_str(&req);
--- a/rust/rhg/src/commands/files.rs Mon Dec 14 13:47:44 2020 +0100 +++ b/rust/rhg/src/commands/files.rs Mon Dec 14 14:59:23 2020 +0100 @@ -2,14 +2,13 @@ use crate::error::{CommandError, CommandErrorKind}; use crate::ui::utf8_to_local; use crate::ui::Ui; -use hg::operations::FindRoot; +use hg::operations::find_root; use hg::operations::{ - ListDirstateTrackedFiles, ListDirstateTrackedFilesError, - ListDirstateTrackedFilesErrorKind, + list_rev_tracked_files, ListRevTrackedFilesError, + ListRevTrackedFilesErrorKind, }; use hg::operations::{ - ListRevTrackedFiles, ListRevTrackedFilesError, - ListRevTrackedFilesErrorKind, + Dirstate, ListDirstateTrackedFilesError, ListDirstateTrackedFilesErrorKind, }; use hg::requirements; use hg::utils::files::{get_bytes_from_path, relativize_path}; @@ -57,17 +56,15 @@ impl<'a> Command for FilesCommand<'a> { fn run(&self, ui: &Ui) -> Result<(), CommandError> { - let root = FindRoot::new().run()?; + let root = find_root()?; requirements::check(&root)?; if let Some(rev) = self.rev { - let mut operation = ListRevTrackedFiles::new(&root, rev) + let files = list_rev_tracked_files(&root, rev) .map_err(|e| map_rev_error(rev, e))?; - let files = operation.run().map_err(|e| map_rev_error(rev, e))?; - self.display_files(ui, &root, files) + self.display_files(ui, &root, files.iter()) } else { - let mut operation = ListDirstateTrackedFiles::new(&root) - .map_err(map_dirstate_error)?; - let files = operation.run().map_err(map_dirstate_error)?; + let distate = Dirstate::new(&root).map_err(map_dirstate_error)?; + let files = distate.tracked_files().map_err(map_dirstate_error)?; self.display_files(ui, &root, files) } }
--- a/rust/rhg/src/commands/root.rs Mon Dec 14 13:47:44 2020 +0100 +++ b/rust/rhg/src/commands/root.rs Mon Dec 14 14:59:23 2020 +0100 @@ -2,7 +2,7 @@ use crate::error::CommandError; use crate::ui::Ui; use format_bytes::format_bytes; -use hg::operations::FindRoot; +use hg::operations::find_root; use hg::utils::files::get_bytes_from_path; pub const HELP_TEXT: &str = " @@ -21,7 +21,7 @@ impl Command for RootCommand { fn run(&self, ui: &Ui) -> Result<(), CommandError> { - let path_buf = FindRoot::new().run()?; + let path_buf = find_root()?; let bytes = get_bytes_from_path(path_buf);