This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8959

hg-core: define a `DebugData` `Operation`
ClosedPublic

Authored by acezar on Aug 26 2020, 9:23 AM.

Details

Reviewers
Alphare
indygreg
Group Reviewers
hg-reviewers
Commits
rHGb56df13a0450: hg-core: define a `DebugData` `Operation`
Summary

Read the content of a revlog at a given revision.

Diff Detail

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

Event Timeline

acezar created this revision.Aug 26 2020, 9:23 AM
acezar edited the summary of this revision. (Show Details)Aug 26 2020, 9:45 AM
Alphare requested changes to this revision.Aug 28 2020, 8:46 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
rust/hg-core/src/operations/debugdata.rs
36

Let's not implement Error, it requires a UTF8 description and has other issues.

38

Same for Display

This revision now requires changes to proceed.Aug 28 2020, 8:46 AM
acezar updated this revision to Diff 22483.Aug 28 2020, 12:52 PM
acezar marked 2 inline comments as done.Aug 31 2020, 10:54 AM
acezar updated this revision to Diff 22529.Sep 3 2020, 4:50 AM
acezar updated this revision to Diff 22533.Sep 3 2020, 8:50 AM
Alphare accepted this revision.Sep 6 2020, 3:59 AM
acezar updated this revision to Diff 22590.Sep 9 2020, 9:52 AM
indygreg accepted this revision.Sep 22 2020, 10:02 PM
indygreg added a subscriber: indygreg.
indygreg added inline comments.
rust/hg-core/src/operations/debugdata.rs
24

This enum duplicates a lot of states from RevlogError. This kinda feels like an anti-pattern. Should it store a RevlogError(RevlogError) instead?

(I legitimately do not know what the best practice is here. Completely duplicating an enum just doesn't sit right with me!)

102

We would ideally have a method on the repo resolve this path, since the logic is influenced by .hg/requires. But this is probably fine for now.

This revision is now accepted and ready to land.Sep 22 2020, 10:02 PM
acezar updated this revision to Diff 22780.Sep 23 2020, 8:07 AM
acezar added a commit: rHGb56df13a0450: hg-core: define a `DebugData` `Operation`.Sep 23 2020, 10:55 AM
Closed by commit rHGb56df13a0450: hg-core: define a `DebugData` `Operation` (authored by acezar). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 22590

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

  • This file was added.
// debugdata.rs
//
// Copyright 2020 Antoine Cezar <antoine.cezar@octobus.net>
//
// 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 crate::revlog::revlog::{Revlog, RevlogError};
use crate::revlog::Revision;
/// Kind of data to debug
#[derive(Debug, Copy, Clone)]
pub enum DebugDataKind {
Changelog,
Manifest,
}
/// 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),
indygregUnsubmitted
Not Done

This enum duplicates a lot of states from RevlogError. This kinda feels like an anti-pattern. Should it store a RevlogError(RevlogError) instead?

(I legitimately do not know what the best practice is here. Completely duplicating an enum just doesn't sit right with me!)

indygreg: This enum duplicates a lot of states from `RevlogError`. This kinda feels like an anti-pattern.
/// The revision has not been found.
InvalidRevision,
/// A `revlog` file is corrupted.
CorruptedRevlog,
/// The `revlog` format version is not supported.
UnsuportedRevlogVersion(u16),
/// The `revlog` data format is not supported.
UnknowRevlogDataFormat(u8),
}
/// A DebugData error
#[derive(Debug)]
AlphareUnsubmitted
Done

Let's not implement Error, it requires a UTF8 description and has other issues.

Alphare: Let's not implement `Error`, it requires a UTF8 description and has other issues.
pub struct DebugDataError {
/// Kind of error encountered by DebugData
AlphareUnsubmitted
Done

Same for Display

Alphare: Same for `Display`
pub kind: DebugDataErrorKind,
}
impl From<DebugDataErrorKind> for DebugDataError {
fn from(kind: DebugDataErrorKind) -> Self {
DebugDataError { kind }
}
}
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);
DebugDataError { kind }
}
}
impl From<RevlogError> for DebugDataError {
fn from(err: RevlogError) -> Self {
match err {
RevlogError::IoError(err) => DebugDataErrorKind::IoError(err),
RevlogError::UnsuportedVersion(version) => {
DebugDataErrorKind::UnsuportedRevlogVersion(version)
}
RevlogError::InvalidRevision => {
DebugDataErrorKind::InvalidRevision
}
RevlogError::Corrupted => DebugDataErrorKind::CorruptedRevlog,
RevlogError::UnknowDataFormat(format) => {
DebugDataErrorKind::UnknowRevlogDataFormat(format)
}
}
.into()
}
}
/// 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.
kind: DebugDataKind,
}
impl<'a> DebugData<'a> {
pub fn new(rev: &'a str, kind: DebugDataKind) -> Self {
DebugData { rev, kind }
}
pub fn run(&mut self) -> Result<Vec<u8>, DebugDataError> {
let rev = self
.rev
.parse::<Revision>()
.or(Err(DebugDataErrorKind::InvalidRevision))?;
let root = find_root::FindRoot::new().run()?;
let index_file = match self.kind {
DebugDataKind::Changelog => root.join(".hg/store/00changelog.i"),
indygregUnsubmitted
Not Done

We would ideally have a method on the repo resolve this path, since the logic is influenced by .hg/requires. But this is probably fine for now.

indygreg: We would ideally have a method on the repo resolve this path, since the logic is influenced by…
DebugDataKind::Manifest => root.join(".hg/store/00manifest.i"),
};
let revlog = Revlog::open(&index_file)?;
let data = revlog.get_rev_data(rev)?;
Ok(data)
}
}

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

//! A distinction is made between operations and commands. //! A distinction is made between operations and commands.
//! An operation is what can be done whereas a command is what is exposed by //! An operation is what can be done whereas a command is what is exposed by
//! the cli. A single command can use several operations to achieve its goal. //! the cli. A single command can use several operations to achieve its goal.
mod debugdata;
mod dirstate_status; mod dirstate_status;
mod find_root; mod find_root;
mod list_tracked_files; mod list_tracked_files;
pub use find_root::{FindRoot, FindRootError, FindRootErrorKind}; pub use find_root::{FindRoot, FindRootError, FindRootErrorKind};
pub use list_tracked_files::{ pub use list_tracked_files::{
ListTrackedFiles, ListTrackedFilesError, ListTrackedFilesErrorKind, ListTrackedFiles, ListTrackedFilesError, ListTrackedFilesErrorKind,
}; };
// TODO add an `Operation` trait when GAT have landed (rust #44265): // TODO add an `Operation` trait when GAT have landed (rust #44265):
// there is no way to currently define a trait which can both return // 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. // 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 // Generic Associated Types may fix this and allow us to have a unified
// interface. // interface.