This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12546

rust-revlog: move check for nodemap requirement to caller
Needs ReviewPublic

Authored by martinvonz on Wed, Apr 13, 9:10 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Summary

It's good for both making Revlog testable and reusable to have it
not depend on the higher-level Repo type. This patch is one step in
towards that. Additionally, this change in particular gives the
callers more control over when to use a nodemap.

Diff Detail

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

Event Timeline

martinvonz created this revision.Wed, Apr 13, 9:10 AM
Alphare requested changes to this revision.Thu, Apr 14, 5:03 AM
Alphare added a subscriber: Alphare.

While I think you're right about separating the revlogs from the repo, the repeated logic should probably be factored into a function somewhere. Why not in nodemap.rs?

This revision now requires changes to proceed.Thu, Apr 14, 5:03 AM
martinvonz added a comment.Thu, Apr 14, 12:10 PM
In D12546#192085, @Alphare wrote:

While I think you're right about separating the revlogs from the repo, the repeated logic should probably be factored into a function somewhere. Why not in nodemap.rs?

Two things:

  • Do we want to use persistent nodemaps for all types of revlogs? This was what I meant by "Additionally, this change in particular gives the callers more control over when to use a nodemap.". Looking at the Python code, we don't seem to use it for filelogs. I'll add a patch to this series to stop using persistent nodemap for filelogs.
  • I was thinking of moving the conditions further up the stack, to the repo level. If we do that, then the repo might be a natural place to extract a function. Or maybe I'll put it in nodemap.rs as you said. I'll update this patch a bit later.
Alphare added a comment.Fri, Apr 15, 5:05 AM
In D12546#192112, @martinvonz wrote:

Two things:

  • Do we want to use persistent nodemaps for all types of revlogs? This was what I meant by "Additionally, this change in particular gives the callers more control over when to use a nodemap.". Looking at the Python code, we don't seem to use it for filelogs. I'll add a patch to this series to stop using persistent nodemap for filelogs.

We will probably only want it for changelogs and manifestlogs. Thanks for sending the patch for the filelogs. Maybe that'll change in the future (with unified revlog?), but that can always be added.

  • I was thinking of moving the conditions further up the stack, to the repo level. If we do that, then the repo might be a natural place to extract a function. Or maybe I'll put it in nodemap.rs as you said. I'll update this patch a bit later.

Either seem fine to me.

martinvonz added a comment.Fri, Apr 15, 12:53 PM
In D12546#192136, @Alphare wrote:
In D12546#192112, @martinvonz wrote:
  • I was thinking of moving the conditions further up the stack, to the repo level. If we do that, then the repo might be a natural place to extract a function. Or maybe I'll put it in nodemap.rs as you said. I'll update this patch a bit later.

Either seem fine to me.

I moved it to Repo in D12562.

Revision Contents
Changeset List

Diff 33176

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

// debugdata.rs // debugdata.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::requirements;
use crate::revlog::revlog::{Revlog, RevlogError}; use crate::revlog::revlog::{Revlog, RevlogError};
/// Kind of data to debug /// Kind of data to debug
#[derive(Debug, Copy, Clone)] #[derive(Debug, Copy, Clone)]
pub enum DebugDataKind { pub enum DebugDataKind {
Changelog, Changelog,
Manifest, Manifest,
} }
/// Dump the contents data of a revision. /// Dump the contents data of a revision.
pub fn debug_data( pub fn debug_data(
repo: &Repo, repo: &Repo,
revset: &str, revset: &str,
kind: DebugDataKind, kind: DebugDataKind,
) -> Result<Vec<u8>, RevlogError> { ) -> Result<Vec<u8>, RevlogError> {
let index_file = match kind { let index_file = match kind {
DebugDataKind::Changelog => "00changelog.i", DebugDataKind::Changelog => "00changelog.i",
DebugDataKind::Manifest => "00manifest.i", DebugDataKind::Manifest => "00manifest.i",
}; };
let revlog = Revlog::open(repo, index_file, None)?; let use_nodemap = repo
.requirements()
.contains(requirements::NODEMAP_REQUIREMENT);
let revlog = Revlog::open(repo, index_file, None, use_nodemap)?;
let rev = let rev =
crate::revset::resolve_rev_number_or_hex_prefix(revset, &revlog)?; crate::revset::resolve_rev_number_or_hex_prefix(revset, &revlog)?;
let data = revlog.get_rev_data(rev)?; let data = revlog.get_rev_data(rev)?;
Ok(data.into_owned()) Ok(data.into_owned())
} }

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

use crate::errors::HgError; use crate::errors::HgError;
use crate::repo::Repo; use crate::repo::Repo;
use crate::requirements;
use crate::revlog::revlog::{Revlog, RevlogEntry, RevlogError}; use crate::revlog::revlog::{Revlog, RevlogEntry, RevlogError};
use crate::revlog::Revision; use crate::revlog::Revision;
use crate::revlog::{Node, NodePrefix}; use crate::revlog::{Node, NodePrefix};
use crate::utils::hg_path::HgPath; use crate::utils::hg_path::HgPath;
use itertools::Itertools; use itertools::Itertools;
use std::ascii::escape_default; use std::ascii::escape_default;
use std::fmt::{Debug, Formatter}; use std::fmt::{Debug, Formatter};
/// A specialized `Revlog` to work with `changelog` data format. /// A specialized `Revlog` to work with `changelog` data format.
pub struct Changelog { pub struct Changelog {
/// The generic `revlog` format. /// The generic `revlog` format.
pub(crate) revlog: Revlog, pub(crate) revlog: Revlog,
} }
impl Changelog { impl Changelog {
/// Open the `changelog` of a repository given by its root. /// Open the `changelog` of a repository given by its root.
pub fn open(repo: &Repo) -> Result<Self, HgError> { pub fn open(repo: &Repo) -> Result<Self, HgError> {
let revlog = Revlog::open(repo, "00changelog.i", None)?; let use_nodemap = repo
.requirements()
.contains(requirements::NODEMAP_REQUIREMENT);
let revlog = Revlog::open(repo, "00changelog.i", None, use_nodemap)?;
Ok(Self { revlog }) Ok(Self { revlog })
} }
/// Return the `ChangelogEntry` for the given node ID. /// Return the `ChangelogEntry` for the given node ID.
pub fn data_for_node( pub fn data_for_node(
&self, &self,
node: NodePrefix, node: NodePrefix,
) -> Result<ChangelogRevisionData, RevlogError> { ) -> Result<ChangelogRevisionData, RevlogError> {
&bytes.iter().flat_map(|b| escape_default(*b)).collect_vec(), &bytes.iter().flat_map(|b| escape_default(*b)).collect_vec(),
) )
.to_string() .to_string()
} }
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use itertools::Itertools;
use pretty_assertions::assert_eq; use pretty_assertions::assert_eq;
#[test] #[test]
fn test_create_changelogrevisiondata_invalid() { fn test_create_changelogrevisiondata_invalid() {
// Completely empty // Completely empty
assert!(ChangelogRevisionData::new(b"abcd".to_vec()).is_err()); assert!(ChangelogRevisionData::new(b"abcd".to_vec()).is_err());
// No newline after manifest // No newline after manifest
assert!(ChangelogRevisionData::new(b"abcd".to_vec()).is_err()); assert!(ChangelogRevisionData::new(b"abcd".to_vec()).is_err());

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

use crate::errors::HgError; use crate::errors::HgError;
use crate::repo::Repo; use crate::repo::Repo;
use crate::requirements;
use crate::revlog::path_encode::path_encode; use crate::revlog::path_encode::path_encode;
use crate::revlog::revlog::RevlogEntry; use crate::revlog::revlog::RevlogEntry;
use crate::revlog::revlog::{Revlog, RevlogError}; use crate::revlog::revlog::{Revlog, RevlogError};
use crate::revlog::NodePrefix; use crate::revlog::NodePrefix;
use crate::revlog::Revision; use crate::revlog::Revision;
use crate::utils::files::get_path_from_bytes; use crate::utils::files::get_path_from_bytes;
use crate::utils::hg_path::HgPath; use crate::utils::hg_path::HgPath;
use crate::utils::SliceExt; use crate::utils::SliceExt;
use std::path::PathBuf; use std::path::PathBuf;
/// A specialized `Revlog` to work with file data logs. /// A specialized `Revlog` to work with file data logs.
pub struct Filelog { pub struct Filelog {
/// The generic `revlog` format. /// The generic `revlog` format.
revlog: Revlog, revlog: Revlog,
} }
impl Filelog { impl Filelog {
pub fn open(repo: &Repo, file_path: &HgPath) -> Result<Self, HgError> { pub fn open(repo: &Repo, file_path: &HgPath) -> Result<Self, HgError> {
let index_path = store_path(file_path, b".i"); let index_path = store_path(file_path, b".i");
let data_path = store_path(file_path, b".d"); let data_path = store_path(file_path, b".d");
let revlog = Revlog::open(repo, index_path, Some(&data_path))?; let use_nodemap = repo
.requirements()
.contains(requirements::NODEMAP_REQUIREMENT);
let revlog =
Revlog::open(repo, index_path, Some(&data_path), use_nodemap)?;
Ok(Self { revlog }) Ok(Self { revlog })
} }
/// The given node ID is that of the file as found in a filelog, not of a /// The given node ID is that of the file as found in a filelog, not of a
/// changeset. /// changeset.
pub fn data_for_node( pub fn data_for_node(
&self, &self,
file_node: impl Into<NodePrefix>, file_node: impl Into<NodePrefix>,

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

use crate::errors::HgError; use crate::errors::HgError;
use crate::repo::Repo; use crate::repo::Repo;
use crate::requirements;
use crate::revlog::revlog::{Revlog, RevlogError}; use crate::revlog::revlog::{Revlog, RevlogError};
use crate::revlog::Revision; use crate::revlog::Revision;
use crate::revlog::{Node, NodePrefix}; use crate::revlog::{Node, NodePrefix};
use crate::utils::hg_path::HgPath; use crate::utils::hg_path::HgPath;
use crate::utils::SliceExt; use crate::utils::SliceExt;
/// A specialized `Revlog` to work with `manifest` data format. /// A specialized `Revlog` to work with `manifest` data format.
pub struct Manifestlog { pub struct Manifestlog {
/// The generic `revlog` format. /// The generic `revlog` format.
revlog: Revlog, revlog: Revlog,
} }
impl Manifestlog { impl Manifestlog {
/// Open the `manifest` of a repository given by its root. /// Open the `manifest` of a repository given by its root.
pub fn open(repo: &Repo) -> Result<Self, HgError> { pub fn open(repo: &Repo) -> Result<Self, HgError> {
let revlog = Revlog::open(repo, "00manifest.i", None)?; let use_nodemap = repo
.requirements()
.contains(requirements::NODEMAP_REQUIREMENT);
let revlog = Revlog::open(repo, "00manifest.i", None, use_nodemap)?;
Ok(Self { revlog }) Ok(Self { revlog })
} }
/// Return the `Manifest` for the given node ID. /// Return the `Manifest` for the given node ID.
/// ///
/// Note: this is a node ID in the manifestlog, typically found through /// Note: this is a node ID in the manifestlog, typically found through
/// `ChangelogEntry::manifest_node`. It is *not* the node ID of any /// `ChangelogEntry::manifest_node`. It is *not* the node ID of any
/// changeset. /// changeset.

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

use super::node::{NodePrefix, NODE_BYTES_LENGTH, NULL_NODE}; use super::node::{NodePrefix, NODE_BYTES_LENGTH, NULL_NODE};
use super::nodemap; use super::nodemap;
use super::nodemap::{NodeMap, NodeMapError}; use super::nodemap::{NodeMap, NodeMapError};
use super::nodemap_docket::NodeMapDocket; use super::nodemap_docket::NodeMapDocket;
use super::patch; use super::patch;
use crate::errors::HgError; use crate::errors::HgError;
use crate::repo::Repo; use crate::repo::Repo;
use crate::revlog::Revision; use crate::revlog::Revision;
use crate::{requirements, Node, NULL_REVISION}; use crate::{Node, NULL_REVISION};
const REVISION_FLAG_CENSORED: u16 = 1 << 15; const REVISION_FLAG_CENSORED: u16 = 1 << 15;
const REVISION_FLAG_ELLIPSIS: u16 = 1 << 14; const REVISION_FLAG_ELLIPSIS: u16 = 1 << 14;
const REVISION_FLAG_EXTSTORED: u16 = 1 << 13; const REVISION_FLAG_EXTSTORED: u16 = 1 << 13;
const REVISION_FLAG_HASCOPIESINFO: u16 = 1 << 12; const REVISION_FLAG_HASCOPIESINFO: u16 = 1 << 12;
// Keep this in sync with REVIDX_KNOWN_FLAGS in // Keep this in sync with REVIDX_KNOWN_FLAGS in
// mercurial/revlogutils/flagutil.py // mercurial/revlogutils/flagutil.py
/// ///
/// It will also open the associated data file if index and data are not /// It will also open the associated data file if index and data are not
/// interleaved. /// interleaved.
#[timed] #[timed]
pub fn open( pub fn open(
repo: &Repo, repo: &Repo,
index_path: impl AsRef<Path>, index_path: impl AsRef<Path>,
data_path: Option<&Path>, data_path: Option<&Path>,
use_nodemap: bool,
) -> Result<Self, HgError> { ) -> Result<Self, HgError> {
let index_path = index_path.as_ref(); let index_path = index_path.as_ref();
let index = { let index = {
match repo.store_vfs().mmap_open_opt(&index_path)? { match repo.store_vfs().mmap_open_opt(&index_path)? {
None => Index::new(Box::new(vec![])), None => Index::new(Box::new(vec![])),
Some(index_mmap) => { Some(index_mmap) => {
let index = Index::new(Box::new(index_mmap))?; let index = Index::new(Box::new(index_mmap))?;
Ok(index) Ok(index)
} else { } else {
let data_path = data_path.unwrap_or(&default_data_path); let data_path = data_path.unwrap_or(&default_data_path);
let data_mmap = repo.store_vfs().mmap_open(data_path)?; let data_mmap = repo.store_vfs().mmap_open(data_path)?;
Some(Box::new(data_mmap)) Some(Box::new(data_mmap))
}; };
let nodemap = if index.is_inline() { let nodemap = if index.is_inline() {
None None
} else if !repo } else if !use_nodemap {
.requirements()
.contains(requirements::NODEMAP_REQUIREMENT)
{
// If .hg/requires does not opt it, don’t try to open a nodemap
None None
} else { } else {
NodeMapDocket::read_from_file(&repo.store_vfs(), index_path)?.map( NodeMapDocket::read_from_file(&repo.store_vfs(), index_path)?.map(
|(docket, data)| { |(docket, data)| {
nodemap::NodeTree::load_bytes( nodemap::NodeTree::load_bytes(
Box::new(data), Box::new(data),
docket.data_length, docket.data_length,
) )