rust: use the new `UncheckedRevision` everywhere applicable
This step converts all revisions that shouldn't be considered "valid" in any
context to `UncheckedRevison`, allowing `Revision` to be changed for a
stronger type in a later changeset.
Note that the conversion from unchecked to checked is manual and requires
at least some thought from the programmer, although directly using `Revision`
is still possible. A later changeset will make this mistake harder to make.
--- a/rust/hg-core/src/operations/cat.rs Mon Sep 11 11:52:33 2023 +0200
+++ b/rust/hg-core/src/operations/cat.rs Thu Aug 10 11:00:34 2023 +0200
@@ -84,10 +84,10 @@
mut files: Vec<&'a HgPath>,
) -> Result<CatOutput<'a>, RevlogError> {
let rev = crate::revset::resolve_single(revset, repo)?;
- let manifest = repo.manifest_for_rev(rev)?;
+ let manifest = repo.manifest_for_rev(rev.into())?;
let node = *repo
.changelog()?
- .node_from_rev(rev)
+ .node_from_rev(rev.into())
.expect("should succeed when repo.manifest did");
let mut results: Vec<(&'a HgPath, Vec<u8>)> = vec![];
let mut found_any = false;
--- a/rust/hg-core/src/operations/debugdata.rs Mon Sep 11 11:52:33 2023 +0200
+++ b/rust/hg-core/src/operations/debugdata.rs Thu Aug 10 11:00:34 2023 +0200
@@ -33,6 +33,6 @@
Revlog::open(&repo.store_vfs(), index_file, None, use_nodemap)?;
let rev =
crate::revset::resolve_rev_number_or_hex_prefix(revset, &revlog)?;
- let data = revlog.get_rev_data(rev)?;
+ let data = revlog.get_rev_data_for_checked_rev(rev)?;
Ok(data.into_owned())
}
--- a/rust/hg-core/src/operations/list_tracked_files.rs Mon Sep 11 11:52:33 2023 +0200
+++ b/rust/hg-core/src/operations/list_tracked_files.rs Thu Aug 10 11:00:34 2023 +0200
@@ -21,7 +21,7 @@
) -> Result<FilesForRev, RevlogError> {
let rev = crate::revset::resolve_single(revset, repo)?;
Ok(FilesForRev {
- manifest: repo.manifest_for_rev(rev)?,
+ manifest: repo.manifest_for_rev(rev.into())?,
narrow_matcher,
})
}
--- a/rust/hg-core/src/repo.rs Mon Sep 11 11:52:33 2023 +0200
+++ b/rust/hg-core/src/repo.rs Thu Aug 10 11:00:34 2023 +0200
@@ -15,8 +15,8 @@
use crate::utils::hg_path::HgPath;
use crate::utils::SliceExt;
use crate::vfs::{is_dir, is_file, Vfs};
-use crate::{requirements, NodePrefix};
-use crate::{DirstateError, Revision};
+use crate::DirstateError;
+use crate::{requirements, NodePrefix, UncheckedRevision};
use std::cell::{Ref, RefCell, RefMut};
use std::collections::HashSet;
use std::io::Seek;
@@ -562,7 +562,7 @@
/// Returns the manifest of the *changeset* with the given revision number
pub fn manifest_for_rev(
&self,
- revision: Revision,
+ revision: UncheckedRevision,
) -> Result<Manifest, RevlogError> {
self.manifestlog()?.data_for_node(
self.changelog()?
--- a/rust/hg-core/src/revlog/changelog.rs Mon Sep 11 11:52:33 2023 +0200
+++ b/rust/hg-core/src/revlog/changelog.rs Thu Aug 10 11:00:34 2023 +0200
@@ -4,6 +4,7 @@
use crate::revlog::{Revlog, RevlogEntry, RevlogError};
use crate::utils::hg_path::HgPath;
use crate::vfs::Vfs;
+use crate::UncheckedRevision;
use itertools::Itertools;
use std::ascii::escape_default;
use std::borrow::Cow;
@@ -29,15 +30,24 @@
node: NodePrefix,
) -> Result<ChangelogRevisionData, RevlogError> {
let rev = self.revlog.rev_from_node(node)?;
- self.data_for_rev(rev)
+ self.entry_for_checked_rev(rev)?.data()
}
/// Return the [`ChangelogEntry`] for the given revision number.
pub fn entry_for_rev(
&self,
+ rev: UncheckedRevision,
+ ) -> Result<ChangelogEntry, RevlogError> {
+ let revlog_entry = self.revlog.get_entry(rev)?;
+ Ok(ChangelogEntry { revlog_entry })
+ }
+
+ /// Same as [`Self::entry_for_rev`] for checked revisions.
+ fn entry_for_checked_rev(
+ &self,
rev: Revision,
) -> Result<ChangelogEntry, RevlogError> {
- let revlog_entry = self.revlog.get_entry(rev)?;
+ let revlog_entry = self.revlog.get_entry_for_checked_rev(rev)?;
Ok(ChangelogEntry { revlog_entry })
}
@@ -49,12 +59,12 @@
/// [entry_for_rev](`Self::entry_for_rev`) and doing everything from there.
pub fn data_for_rev(
&self,
- rev: Revision,
+ rev: UncheckedRevision,
) -> Result<ChangelogRevisionData, RevlogError> {
self.entry_for_rev(rev)?.data()
}
- pub fn node_from_rev(&self, rev: Revision) -> Option<&Node> {
+ pub fn node_from_rev(&self, rev: UncheckedRevision) -> Option<&Node> {
self.revlog.node_from_rev(rev)
}
@@ -330,12 +340,12 @@
let changelog = Changelog { revlog };
assert_eq!(
- changelog.data_for_rev(NULL_REVISION)?,
+ changelog.data_for_rev(NULL_REVISION.into())?,
ChangelogRevisionData::null()
);
// same with the intermediate entry object
assert_eq!(
- changelog.entry_for_rev(NULL_REVISION)?.data()?,
+ changelog.entry_for_rev(NULL_REVISION.into())?.data()?,
ChangelogRevisionData::null()
);
Ok(())
--- a/rust/hg-core/src/revlog/filelog.rs Mon Sep 11 11:52:33 2023 +0200
+++ b/rust/hg-core/src/revlog/filelog.rs Thu Aug 10 11:00:34 2023 +0200
@@ -1,4 +1,5 @@
use crate::errors::HgError;
+use crate::exit_codes;
use crate::repo::Repo;
use crate::revlog::path_encode::path_encode;
use crate::revlog::NodePrefix;
@@ -8,6 +9,7 @@
use crate::utils::files::get_path_from_bytes;
use crate::utils::hg_path::HgPath;
use crate::utils::SliceExt;
+use crate::UncheckedRevision;
use std::path::PathBuf;
/// A specialized `Revlog` to work with file data logs.
@@ -39,14 +41,14 @@
file_node: impl Into<NodePrefix>,
) -> Result<FilelogRevisionData, RevlogError> {
let file_rev = self.revlog.rev_from_node(file_node.into())?;
- self.data_for_rev(file_rev)
+ self.data_for_rev(file_rev.into())
}
/// The given revision is that of the file as found in a filelog, not of a
/// changeset.
pub fn data_for_rev(
&self,
- file_rev: Revision,
+ file_rev: UncheckedRevision,
) -> Result<FilelogRevisionData, RevlogError> {
let data: Vec<u8> = self.revlog.get_rev_data(file_rev)?.into_owned();
Ok(FilelogRevisionData(data))
@@ -59,16 +61,25 @@
file_node: impl Into<NodePrefix>,
) -> Result<FilelogEntry, RevlogError> {
let file_rev = self.revlog.rev_from_node(file_node.into())?;
- self.entry_for_rev(file_rev)
+ self.entry_for_checked_rev(file_rev)
}
/// The given revision is that of the file as found in a filelog, not of a
/// changeset.
pub fn entry_for_rev(
&self,
+ file_rev: UncheckedRevision,
+ ) -> Result<FilelogEntry, RevlogError> {
+ Ok(FilelogEntry(self.revlog.get_entry(file_rev)?))
+ }
+
+ fn entry_for_checked_rev(
+ &self,
file_rev: Revision,
) -> Result<FilelogEntry, RevlogError> {
- Ok(FilelogEntry(self.revlog.get_entry(file_rev)?))
+ Ok(FilelogEntry(
+ self.revlog.get_entry_for_checked_rev(file_rev)?,
+ ))
}
}
@@ -165,7 +176,19 @@
}
pub fn data(&self) -> Result<FilelogRevisionData, HgError> {
- Ok(FilelogRevisionData(self.0.data()?.into_owned()))
+ let data = self.0.data();
+ match data {
+ Ok(data) => Ok(FilelogRevisionData(data.into_owned())),
+ // Errors other than `HgError` should not happen at this point
+ Err(e) => match e {
+ RevlogError::Other(hg_error) => Err(hg_error),
+ revlog_error => Err(HgError::abort(
+ revlog_error.to_string(),
+ exit_codes::ABORT,
+ None,
+ )),
+ },
+ }
}
}
--- a/rust/hg-core/src/revlog/index.rs Mon Sep 11 11:52:33 2023 +0200
+++ b/rust/hg-core/src/revlog/index.rs Thu Aug 10 11:00:34 2023 +0200
@@ -1,3 +1,4 @@
+use std::fmt::Debug;
use std::ops::Deref;
use byteorder::{BigEndian, ByteOrder};
@@ -5,6 +6,7 @@
use crate::errors::HgError;
use crate::revlog::node::Node;
use crate::revlog::{Revision, NULL_REVISION};
+use crate::UncheckedRevision;
pub const INDEX_ENTRY_SIZE: usize = 64;
@@ -86,6 +88,15 @@
uses_generaldelta: bool,
}
+impl Debug for Index {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ f.debug_struct("Index")
+ .field("offsets", &self.offsets)
+ .field("uses_generaldelta", &self.uses_generaldelta)
+ .finish()
+ }
+}
+
impl Index {
/// Create an index from bytes.
/// Calculate the start of each entry when is_inline is true.
@@ -175,36 +186,32 @@
if rev == NULL_REVISION {
return None;
}
- if let Some(offsets) = &self.offsets {
+ Some(if let Some(offsets) = &self.offsets {
self.get_entry_inline(rev, offsets)
} else {
self.get_entry_separated(rev)
- }
+ })
}
fn get_entry_inline(
&self,
rev: Revision,
offsets: &[usize],
- ) -> Option<IndexEntry> {
- let start = *offsets.get(rev as usize)?;
- let end = start.checked_add(INDEX_ENTRY_SIZE)?;
+ ) -> IndexEntry {
+ let start = offsets[rev as usize];
+ let end = start + INDEX_ENTRY_SIZE;
let bytes = &self.bytes[start..end];
// See IndexEntry for an explanation of this override.
let offset_override = Some(end);
- Some(IndexEntry {
+ IndexEntry {
bytes,
offset_override,
- })
+ }
}
- fn get_entry_separated(&self, rev: Revision) -> Option<IndexEntry> {
- let max_rev = self.bytes.len() / INDEX_ENTRY_SIZE;
- if rev as usize >= max_rev {
- return None;
- }
+ fn get_entry_separated(&self, rev: Revision) -> IndexEntry {
let start = rev as usize * INDEX_ENTRY_SIZE;
let end = start + INDEX_ENTRY_SIZE;
let bytes = &self.bytes[start..end];
@@ -213,10 +220,10 @@
// for the index's metadata (saving space because it is always 0)
let offset_override = if rev == 0 { Some(0) } else { None };
- Some(IndexEntry {
+ IndexEntry {
bytes,
offset_override,
- })
+ }
}
}
@@ -273,23 +280,23 @@
}
/// Return the revision upon which the data has been derived.
- pub fn base_revision_or_base_of_delta_chain(&self) -> Revision {
+ pub fn base_revision_or_base_of_delta_chain(&self) -> UncheckedRevision {
// TODO Maybe return an Option when base_revision == rev?
// Requires to add rev to IndexEntry
- BigEndian::read_i32(&self.bytes[16..])
+ BigEndian::read_i32(&self.bytes[16..]).into()
}
- pub fn link_revision(&self) -> Revision {
- BigEndian::read_i32(&self.bytes[20..])
+ pub fn link_revision(&self) -> UncheckedRevision {
+ BigEndian::read_i32(&self.bytes[20..]).into()
}
- pub fn p1(&self) -> Revision {
- BigEndian::read_i32(&self.bytes[24..])
+ pub fn p1(&self) -> UncheckedRevision {
+ BigEndian::read_i32(&self.bytes[24..]).into()
}
- pub fn p2(&self) -> Revision {
- BigEndian::read_i32(&self.bytes[28..])
+ pub fn p2(&self) -> UncheckedRevision {
+ BigEndian::read_i32(&self.bytes[28..]).into()
}
/// Return the hash of revision's full text.
@@ -547,7 +554,7 @@
offset_override: None,
};
- assert_eq!(entry.base_revision_or_base_of_delta_chain(), 1)
+ assert_eq!(entry.base_revision_or_base_of_delta_chain(), 1.into())
}
#[test]
@@ -559,7 +566,7 @@
offset_override: None,
};
- assert_eq!(entry.link_revision(), 123);
+ assert_eq!(entry.link_revision(), 123.into());
}
#[test]
@@ -571,7 +578,7 @@
offset_override: None,
};
- assert_eq!(entry.p1(), 123);
+ assert_eq!(entry.p1(), 123.into());
}
#[test]
@@ -583,7 +590,7 @@
offset_override: None,
};
- assert_eq!(entry.p2(), 123);
+ assert_eq!(entry.p2(), 123.into());
}
#[test]
--- a/rust/hg-core/src/revlog/manifest.rs Mon Sep 11 11:52:33 2023 +0200
+++ b/rust/hg-core/src/revlog/manifest.rs Thu Aug 10 11:00:34 2023 +0200
@@ -1,10 +1,10 @@
use crate::errors::HgError;
-use crate::revlog::Revision;
use crate::revlog::{Node, NodePrefix};
use crate::revlog::{Revlog, RevlogError};
use crate::utils::hg_path::HgPath;
use crate::utils::SliceExt;
use crate::vfs::Vfs;
+use crate::{Revision, UncheckedRevision};
/// A specialized `Revlog` to work with `manifest` data format.
pub struct Manifestlog {
@@ -32,7 +32,7 @@
node: NodePrefix,
) -> Result<Manifest, RevlogError> {
let rev = self.revlog.rev_from_node(node)?;
- self.data_for_rev(rev)
+ self.data_for_checked_rev(rev)
}
/// Return the `Manifest` of a given revision number.
@@ -43,9 +43,18 @@
/// See also `Repo::manifest_for_rev`
pub fn data_for_rev(
&self,
+ rev: UncheckedRevision,
+ ) -> Result<Manifest, RevlogError> {
+ let bytes = self.revlog.get_rev_data(rev)?.into_owned();
+ Ok(Manifest { bytes })
+ }
+
+ pub fn data_for_checked_rev(
+ &self,
rev: Revision,
) -> Result<Manifest, RevlogError> {
- let bytes = self.revlog.get_rev_data(rev)?.into_owned();
+ let bytes =
+ self.revlog.get_rev_data_for_checked_rev(rev)?.into_owned();
Ok(Manifest { bytes })
}
}
--- a/rust/hg-core/src/revlog/mod.rs Mon Sep 11 11:52:33 2023 +0200
+++ b/rust/hg-core/src/revlog/mod.rs Thu Aug 10 11:00:34 2023 +0200
@@ -47,7 +47,24 @@
///
/// As noted in revlog.c, revision numbers are actually encoded in
/// 4 bytes, and are liberally converted to ints, whence the i32
-pub type UncheckedRevision = i32;
+#[derive(
+ Debug,
+ derive_more::Display,
+ Clone,
+ Copy,
+ Hash,
+ PartialEq,
+ Eq,
+ PartialOrd,
+ Ord,
+)]
+pub struct UncheckedRevision(i32);
+
+impl From<Revision> for UncheckedRevision {
+ fn from(value: Revision) -> Self {
+ Self(value)
+ }
+}
/// Marker expressing the absence of a parent
///
@@ -60,7 +77,8 @@
/// This is also equal to `i32::max_value()`, but it's better to spell
/// it out explicitely, same as in `mercurial.node`
#[allow(clippy::unreadable_literal)]
-pub const WORKING_DIRECTORY_REVISION: Revision = 0x7fffffff;
+pub const WORKING_DIRECTORY_REVISION: UncheckedRevision =
+ UncheckedRevision(0x7fffffff);
pub const WORKING_DIRECTORY_HEX: &str =
"ffffffffffffffffffffffffffffffffffffffff";
@@ -90,14 +108,14 @@
self.len() == 0
}
- /// Return a reference to the Node or `None` if rev is out of bounds
- ///
- /// `NULL_REVISION` is not considered to be out of bounds.
+ /// Return a reference to the Node or `None` for `NULL_REVISION`
fn node(&self, rev: Revision) -> Option<&Node>;
/// Return a [`Revision`] if `rev` is a valid revision number for this
/// index
fn check_revision(&self, rev: UncheckedRevision) -> Option<Revision> {
+ let rev = rev.0;
+
if rev == NULL_REVISION || (rev >= 0 && (rev as usize) < self.len()) {
Some(rev)
} else {
@@ -120,7 +138,7 @@
const NULL_REVLOG_ENTRY_FLAGS: u16 = 0;
-#[derive(Debug, derive_more::From)]
+#[derive(Debug, derive_more::From, derive_more::Display)]
pub enum RevlogError {
InvalidRevision,
/// Working directory is not supported
@@ -231,10 +249,11 @@
/// Returns the node ID for the given revision number, if it exists in this
/// revlog
- pub fn node_from_rev(&self, rev: Revision) -> Option<&Node> {
- if rev == NULL_REVISION {
+ pub fn node_from_rev(&self, rev: UncheckedRevision) -> Option<&Node> {
+ if rev == NULL_REVISION.into() {
return Some(&NULL_NODE);
}
+ let rev = self.index.check_revision(rev)?;
Some(self.index.get_entry(rev)?.hash())
}
@@ -296,8 +315,8 @@
}
/// Returns whether the given revision exists in this revlog.
- pub fn has_rev(&self, rev: Revision) -> bool {
- self.index.get_entry(rev).is_some()
+ pub fn has_rev(&self, rev: UncheckedRevision) -> bool {
+ self.index.check_revision(rev).is_some()
}
/// Return the full data associated to a revision.
@@ -307,12 +326,23 @@
/// snapshot to rebuild the final data.
pub fn get_rev_data(
&self,
+ rev: UncheckedRevision,
+ ) -> Result<Cow<[u8]>, RevlogError> {
+ if rev == NULL_REVISION.into() {
+ return Ok(Cow::Borrowed(&[]));
+ };
+ self.get_entry(rev)?.data()
+ }
+
+ /// [`Self::get_rev_data`] for checked revisions.
+ pub fn get_rev_data_for_checked_rev(
+ &self,
rev: Revision,
) -> Result<Cow<[u8]>, RevlogError> {
if rev == NULL_REVISION {
return Ok(Cow::Borrowed(&[]));
};
- Ok(self.get_entry(rev)?.data()?)
+ self.get_entry_for_checked_rev(rev)?.data()
}
/// Check the hash of some given data against the recorded hash.
@@ -380,8 +410,7 @@
}
}
- /// Get an entry of the revlog.
- pub fn get_entry(
+ fn get_entry_for_checked_rev(
&self,
rev: Revision,
) -> Result<RevlogEntry, RevlogError> {
@@ -399,36 +428,60 @@
} else {
&self.data()[start..end]
};
+ let base_rev = self
+ .index
+ .check_revision(index_entry.base_revision_or_base_of_delta_chain())
+ .ok_or_else(|| {
+ RevlogError::corrupted(format!(
+ "base revision for rev {} is invalid",
+ rev
+ ))
+ })?;
+ let p1 =
+ self.index.check_revision(index_entry.p1()).ok_or_else(|| {
+ RevlogError::corrupted(format!(
+ "p1 for rev {} is invalid",
+ rev
+ ))
+ })?;
+ let p2 =
+ self.index.check_revision(index_entry.p2()).ok_or_else(|| {
+ RevlogError::corrupted(format!(
+ "p2 for rev {} is invalid",
+ rev
+ ))
+ })?;
let entry = RevlogEntry {
revlog: self,
rev,
bytes: data,
compressed_len: index_entry.compressed_len(),
uncompressed_len: index_entry.uncompressed_len(),
- base_rev_or_base_of_delta_chain: if index_entry
- .base_revision_or_base_of_delta_chain()
- == rev
- {
+ base_rev_or_base_of_delta_chain: if base_rev == rev {
None
} else {
- Some(index_entry.base_revision_or_base_of_delta_chain())
+ Some(base_rev)
},
- p1: index_entry.p1(),
- p2: index_entry.p2(),
+ p1,
+ p2,
flags: index_entry.flags(),
hash: *index_entry.hash(),
};
Ok(entry)
}
- /// when resolving internal references within revlog, any errors
- /// should be reported as corruption, instead of e.g. "invalid revision"
- fn get_entry_internal(
+ /// Get an entry of the revlog.
+ pub fn get_entry(
&self,
- rev: Revision,
- ) -> Result<RevlogEntry, HgError> {
- self.get_entry(rev)
- .map_err(|_| corrupted(format!("revision {} out of range", rev)))
+ rev: UncheckedRevision,
+ ) -> Result<RevlogEntry, RevlogError> {
+ if rev == NULL_REVISION.into() {
+ return Ok(self.make_null_entry());
+ }
+ let rev = self.index.check_revision(rev).ok_or_else(|| {
+ RevlogError::corrupted(format!("rev {} is invalid", rev))
+ })?;
+ self.get_entry_for_checked_rev(rev)
}
}
@@ -486,7 +539,7 @@
if self.p1 == NULL_REVISION {
Ok(None)
} else {
- Ok(Some(self.revlog.get_entry(self.p1)?))
+ Ok(Some(self.revlog.get_entry_for_checked_rev(self.p1)?))
}
}
@@ -496,7 +549,7 @@
if self.p2 == NULL_REVISION {
Ok(None)
} else {
- Ok(Some(self.revlog.get_entry(self.p2)?))
+ Ok(Some(self.revlog.get_entry_for_checked_rev(self.p2)?))
}
}
@@ -527,7 +580,7 @@
}
/// The data for this entry, after resolving deltas if any.
- pub fn rawdata(&self) -> Result<Cow<'revlog, [u8]>, HgError> {
+ pub fn rawdata(&self) -> Result<Cow<'revlog, [u8]>, RevlogError> {
let mut entry = self.clone();
let mut delta_chain = vec![];
@@ -539,11 +592,11 @@
while let Some(base_rev) = entry.base_rev_or_base_of_delta_chain {
entry = if uses_generaldelta {
delta_chain.push(entry);
- self.revlog.get_entry_internal(base_rev)?
+ self.revlog.get_entry_for_checked_rev(base_rev)?
} else {
- let base_rev = entry.rev - 1;
+ let base_rev = UncheckedRevision(entry.rev - 1);
delta_chain.push(entry);
- self.revlog.get_entry_internal(base_rev)?
+ self.revlog.get_entry(base_rev)?
};
}
@@ -559,7 +612,7 @@
fn check_data(
&self,
data: Cow<'revlog, [u8]>,
- ) -> Result<Cow<'revlog, [u8]>, HgError> {
+ ) -> Result<Cow<'revlog, [u8]>, RevlogError> {
if self.revlog.check_hash(
self.p1,
self.p2,
@@ -571,22 +624,24 @@
if (self.flags & REVISION_FLAG_ELLIPSIS) != 0 {
return Err(HgError::unsupported(
"ellipsis revisions are not supported by rhg",
- ));
+ )
+ .into());
}
Err(corrupted(format!(
"hash check failed for revision {}",
self.rev
- )))
+ ))
+ .into())
}
}
- pub fn data(&self) -> Result<Cow<'revlog, [u8]>, HgError> {
+ pub fn data(&self) -> Result<Cow<'revlog, [u8]>, RevlogError> {
let data = self.rawdata()?;
if self.rev == NULL_REVISION {
return Ok(data);
}
if self.is_censored() {
- return Err(HgError::CensoredNodeError);
+ return Err(HgError::CensoredNodeError.into());
}
self.check_data(data)
}
@@ -705,13 +760,13 @@
let revlog = Revlog::open(&vfs, "foo.i", None, false).unwrap();
assert!(revlog.is_empty());
assert_eq!(revlog.len(), 0);
- assert!(revlog.get_entry(0).is_err());
- assert!(!revlog.has_rev(0));
+ assert!(revlog.get_entry(0.into()).is_err());
+ assert!(!revlog.has_rev(0.into()));
assert_eq!(
revlog.rev_from_node(NULL_NODE.into()).unwrap(),
NULL_REVISION
);
- let null_entry = revlog.get_entry(NULL_REVISION).ok().unwrap();
+ let null_entry = revlog.get_entry(NULL_REVISION.into()).ok().unwrap();
assert_eq!(null_entry.revision(), NULL_REVISION);
assert!(null_entry.data().unwrap().is_empty());
}
@@ -750,7 +805,7 @@
std::fs::write(temp.path().join("foo.i"), contents).unwrap();
let revlog = Revlog::open(&vfs, "foo.i", None, false).unwrap();
- let entry0 = revlog.get_entry(0).ok().unwrap();
+ let entry0 = revlog.get_entry(0.into()).ok().unwrap();
assert_eq!(entry0.revision(), 0);
assert_eq!(*entry0.node(), node0);
assert!(!entry0.has_p1());
@@ -761,7 +816,7 @@
let p2_entry = entry0.p2_entry().unwrap();
assert!(p2_entry.is_none());
- let entry1 = revlog.get_entry(1).ok().unwrap();
+ let entry1 = revlog.get_entry(1.into()).ok().unwrap();
assert_eq!(entry1.revision(), 1);
assert_eq!(*entry1.node(), node1);
assert!(!entry1.has_p1());
@@ -772,7 +827,7 @@
let p2_entry = entry1.p2_entry().unwrap();
assert!(p2_entry.is_none());
- let entry2 = revlog.get_entry(2).ok().unwrap();
+ let entry2 = revlog.get_entry(2.into()).ok().unwrap();
assert_eq!(entry2.revision(), 2);
assert_eq!(*entry2.node(), node2);
assert!(entry2.has_p1());
@@ -817,7 +872,7 @@
let revlog = Revlog::open(&vfs, "foo.i", None, false).unwrap();
// accessing the data shows the corruption
- revlog.get_entry(0).unwrap().data().unwrap_err();
+ revlog.get_entry(0.into()).unwrap().data().unwrap_err();
assert_eq!(revlog.rev_from_node(NULL_NODE.into()).unwrap(), -1);
assert_eq!(revlog.rev_from_node(node0.into()).unwrap(), 0);
--- a/rust/hg-core/src/revlog/nodemap.rs Mon Sep 11 11:52:33 2023 +0200
+++ b/rust/hg-core/src/revlog/nodemap.rs Thu Aug 10 11:00:34 2023 +0200
@@ -12,6 +12,8 @@
//! Following existing implicit conventions, the "nodemap" terminology
//! is used in a more abstract context.
+use crate::UncheckedRevision;
+
use super::{
node::NULL_NODE, Node, NodePrefix, Revision, RevlogIndex, NULL_REVISION,
};
@@ -30,7 +32,7 @@
/// This can be returned by methods meant for (at most) one match.
MultipleResults,
/// A `Revision` stored in the nodemap could not be found in the index
- RevisionNotInIndex(Revision),
+ RevisionNotInIndex(UncheckedRevision),
}
/// Mapping system from Mercurial nodes to revision numbers.
@@ -125,7 +127,9 @@
/// use.
#[derive(Clone, Debug, Eq, PartialEq)]
enum Element {
- Rev(Revision),
+ // This is not a Mercurial revision. It's a `i32` because this is the
+ // right type for this structure.
+ Rev(i32),
Block(usize),
None,
}
@@ -245,17 +249,21 @@
fn has_prefix_or_none(
idx: &impl RevlogIndex,
prefix: NodePrefix,
- rev: Revision,
+ rev: UncheckedRevision,
) -> Result<Option<Revision>, NodeMapError> {
- idx.node(rev)
- .ok_or(NodeMapError::RevisionNotInIndex(rev))
- .map(|node| {
- if prefix.is_prefix_of(node) {
- Some(rev)
- } else {
- None
- }
- })
+ match idx.check_revision(rev) {
+ Some(checked) => idx
+ .node(checked)
+ .ok_or(NodeMapError::RevisionNotInIndex(rev))
+ .map(|node| {
+ if prefix.is_prefix_of(node) {
+ Some(checked)
+ } else {
+ None
+ }
+ }),
+ None => Err(NodeMapError::RevisionNotInIndex(rev)),
+ }
}
/// validate that the candidate's node starts indeed with given prefix,
@@ -266,7 +274,7 @@
fn validate_candidate(
idx: &impl RevlogIndex,
prefix: NodePrefix,
- candidate: (Option<Revision>, usize),
+ candidate: (Option<UncheckedRevision>, usize),
) -> Result<(Option<Revision>, usize), NodeMapError> {
let (rev, steps) = candidate;
if let Some(nz_nybble) = prefix.first_different_nybble(&NULL_NODE) {
@@ -384,6 +392,8 @@
/// be inferred from
/// the `NodeTree` data is that `rev` is the revision with the longest
/// common node prefix with the given prefix.
+ /// We return an [`UncheckedRevision`] because we have no guarantee that
+ /// the revision we found is valid for the index.
///
/// The second returned value is the size of the smallest subprefix
/// of `prefix` that would give the same result, i.e. not the
@@ -392,7 +402,7 @@
fn lookup(
&self,
prefix: NodePrefix,
- ) -> Result<(Option<Revision>, usize), NodeMapError> {
+ ) -> Result<(Option<UncheckedRevision>, usize), NodeMapError> {
for (i, visit_item) in self.visit(prefix).enumerate() {
if let Some(opt) = visit_item.final_revision() {
return Ok((opt, i + 1));
@@ -464,9 +474,9 @@
self.mutable_block(deepest.block_idx);
if let Element::Rev(old_rev) = deepest.element {
- let old_node = index
- .node(old_rev)
- .ok_or(NodeMapError::RevisionNotInIndex(old_rev))?;
+ let old_node = index.node(old_rev).ok_or_else(|| {
+ NodeMapError::RevisionNotInIndex(old_rev.into())
+ })?;
if old_node == node {
return Ok(()); // avoid creating lots of useless blocks
}
@@ -623,13 +633,13 @@
impl NodeTreeVisitItem {
// Return `Some(opt)` if this item is final, with `opt` being the
- // `Revision` that it may represent.
+ // `UncheckedRevision` that it may represent.
//
// If the item is not terminal, return `None`
- fn final_revision(&self) -> Option<Option<Revision>> {
+ fn final_revision(&self) -> Option<Option<UncheckedRevision>> {
match self.element {
Element::Block(_) => None,
- Element::Rev(r) => Some(Some(r)),
+ Element::Rev(r) => Some(Some(r.into())),
Element::None => Some(None),
}
}
@@ -733,16 +743,20 @@
assert_eq!(block.get(4), Element::Rev(1));
}
- type TestIndex = HashMap<Revision, Node>;
+ type TestIndex = HashMap<UncheckedRevision, Node>;
impl RevlogIndex for TestIndex {
fn node(&self, rev: Revision) -> Option<&Node> {
- self.get(&rev)
+ self.get(&rev.into())
}
fn len(&self) -> usize {
self.len()
}
+
+ fn check_revision(&self, rev: UncheckedRevision) -> Option<Revision> {
+ self.get(&rev).map(|_| rev.0)
+ }
}
/// Pad hexadecimal Node prefix with zeros on the right
@@ -756,7 +770,7 @@
/// Pad hexadecimal Node prefix with zeros on the right, then insert
fn pad_insert(idx: &mut TestIndex, rev: Revision, hex: &str) {
- idx.insert(rev, pad_node(hex));
+ idx.insert(rev.into(), pad_node(hex));
}
fn sample_nodetree() -> NodeTree {
@@ -796,7 +810,7 @@
assert_eq!(nt.find_bin(&idx, hex("ab"))?, None);
// and with full binary Nodes
- assert_eq!(nt.find_node(&idx, idx.get(&1).unwrap())?, Some(1));
+ assert_eq!(nt.find_node(&idx, idx.get(&1.into()).unwrap())?, Some(1));
let unknown = Node::from_hex(&hex_pad_right("3d")).unwrap();
assert_eq!(nt.find_node(&idx, &unknown)?, None);
Ok(())
@@ -857,14 +871,15 @@
}
}
- fn insert(
- &mut self,
- rev: Revision,
- hex: &str,
- ) -> Result<(), NodeMapError> {
+ fn insert(&mut self, rev: i32, hex: &str) -> Result<(), NodeMapError> {
let node = pad_node(hex);
+ let rev: UncheckedRevision = rev.into();
self.index.insert(rev, node);
- self.nt.insert(&self.index, &node, rev)?;
+ self.nt.insert(
+ &self.index,
+ &node,
+ self.index.check_revision(rev).unwrap(),
+ )?;
Ok(())
}
@@ -971,9 +986,9 @@
let node0 = Node::from_hex(&node0_hex).unwrap();
let node1 = Node::from_hex(&node1_hex).unwrap();
- idx.insert(0, node0);
+ idx.insert(0.into(), node0);
nt.insert(idx, &node0, 0)?;
- idx.insert(1, node1);
+ idx.insert(1.into(), node1);
nt.insert(idx, &node1, 1)?;
assert_eq!(nt.find_bin(idx, (&node0).into())?, Some(0));
--- a/rust/hg-core/src/revset.rs Mon Sep 11 11:52:33 2023 +0200
+++ b/rust/hg-core/src/revset.rs Thu Aug 10 11:00:34 2023 +0200
@@ -53,7 +53,7 @@
if let Ok(integer) = input.parse::<i32>() {
if integer.to_string() == input
&& integer >= 0
- && revlog.has_rev(integer)
+ && revlog.has_rev(integer.into())
{
return Ok(integer);
}
--- a/rust/hg-cpython/src/revlog.rs Mon Sep 11 11:52:33 2023 +0200
+++ b/rust/hg-cpython/src/revlog.rs Thu Aug 10 11:00:34 2023 +0200
@@ -18,7 +18,7 @@
use hg::{
nodemap::{Block, NodeMapError, NodeTree},
revlog::{nodemap::NodeMap, NodePrefix, RevlogIndex},
- Revision,
+ Revision, UncheckedRevision,
};
use std::cell::RefCell;
@@ -252,7 +252,7 @@
// Note that we don't seem to have a direct way to call
// PySequence_GetItem (does the job), which would possibly be better
// for performance
- let key = match key.extract::<Revision>(py) {
+ let key = match key.extract::<i32>(py) {
Ok(rev) => rev.to_py_object(py).into_object(),
Err(_) => key,
};
@@ -268,7 +268,7 @@
// this is an equivalent implementation of the index_contains()
// defined in revlog.c
let cindex = self.cindex(py).borrow();
- match item.extract::<Revision>(py) {
+ match item.extract::<i32>(py) {
Ok(rev) => {
Ok(rev >= -1 && rev < cindex.inner().len(py)? as Revision)
}
@@ -448,9 +448,12 @@
let mut nt = NodeTree::load_bytes(Box::new(bytes), len);
let data_tip =
- docket.getattr(py, "tip_rev")?.extract::<Revision>(py)?;
+ docket.getattr(py, "tip_rev")?.extract::<i32>(py)?.into();
self.docket(py).borrow_mut().replace(docket.clone_ref(py));
let idx = self.cindex(py).borrow();
+ let data_tip = idx.check_revision(data_tip).ok_or_else(|| {
+ nodemap_error(py, NodeMapError::RevisionNotInIndex(data_tip))
+ })?;
let current_tip = idx.len();
for r in (data_tip + 1)..current_tip as Revision {
@@ -479,7 +482,7 @@
}
}
-fn rev_not_in_index(py: Python, rev: Revision) -> PyErr {
+fn rev_not_in_index(py: Python, rev: UncheckedRevision) -> PyErr {
PyErr::new::<ValueError, _>(
py,
format!(